Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up tests in jscomp/test #6969

Closed
cknitt opened this issue Aug 23, 2024 · 5 comments
Closed

Clean up tests in jscomp/test #6969

cknitt opened this issue Aug 23, 2024 · 5 comments
Assignees
Labels
Milestone

Comments

@cknitt
Copy link
Member

cknitt commented Aug 23, 2024

As part of the effort to get rid of ninja.js for building the stdlibs and tests, the files in jscomp/test need to be cleaned up.

Note that due to ninja.js weirdness, there are currently some restrictions:

  1. Files in subdirectories are not compiled, only files directly in jscomp/test.
  2. File names must start with a lowercase letter, otherwise dependencies on such files are not handled correctly.

Tests are in the process of being converted from mocha (Mt.*) to the node test runner (Node_test.*/Node_assert.*).
Currently, mocha test files need to have the suffix _test and node test files need to have _ntest.

Proposed cleanup plan:

  1. Categorize test files and mark them with a prefix:
    • belt_: tests for the Belt namespace
    • js_: tests for the Js namespace
    • runtime_: tests for the compiler runtime (Curry etc.)
    • ocaml_: tests for the OCaml standard library
    • testutil_ (?): files containing only utility functions used by tests
    • output_: test files that do not call any actual test functions and are just here to be able to manually verify changes in the compiler output
  2. For files that do not contain any tests, remove the _test suffix.
  3. Finish converting tests from Mocha to the node test runner, with the exception of the tests for the OCaml standard library.
  4. When the OCaml standard library is removed from the compiler repo, also remove the corresponding tests.
  5. Once we have moved away from ninja.js to bsb or rewatch for building the stdlibs and tests, reorganize into subdirectories.

/cc @cometkim @fhammerschmidt

@cknitt cknitt added this to the v12 milestone Aug 23, 2024
@fhammerschmidt fhammerschmidt self-assigned this Aug 23, 2024
@cometkim
Copy link
Member

Can we wait for #6899 so that the changes don't conflict?

I honestly want to completely restructure the tests, not just change the format.

There are many tests there that don't actually have a runtime, so it's like build_tests with default config.

@cknitt
Copy link
Member Author

cknitt commented Aug 23, 2024

What is your timeframe for #6899?

In #6899, you are saying

I won't refactor tests here yet.

So there shouldn't be too many conflicts?

@cknitt
Copy link
Member Author

cknitt commented Aug 23, 2024

I would like to get at least the Belt tests converted to the node test runner soon so that I can add them more easily to https://github.com/rescript-lang/experimental-rescript-stdlib-build. I had problems with the weird mt.res there.

@cometkim
Copy link
Member

Nevermind. Some module resolutions might change in #6899. But that's build_tests/ and has nothing to do with test/.

@cknitt cknitt added the cleanup label Aug 27, 2024
@cknitt
Copy link
Member Author

cknitt commented Oct 11, 2024

While I still think that the tests could need some cleanup, what I described here is completely outdated and largely obsolete after the recent changes on master. Removal of the OCaml standard library is done, and runtime and tests are already built with bsb.

So I'll close this issue.

@cknitt cknitt closed this as completed Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants