search

rescript-lang / rescript-compiler

The compiler for ReScript.
https://rescript-lang.org
Other
6.65k stars 442 forks source link

Clean up tests in `jscomp/test` #6969

Open cknitt opened 3 weeks ago

cknitt commented 3 weeks ago

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

cometkim commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago

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