Skip to content

test,tools: move Flags: comment processing from Python to JavaScript

Rodrigo Muino Tomonari requested to merge github/fork/Trott/flags into master

This is a big change set but one that can be broken down into many smaller pull requests for easier review if necessary.

This adds common.relaunchWithFlags() along with two useful-but-not-strictly-necessary helper functions, common.exposeInternals() and common.experimentalWorker().

This allows tests to relaunch themselves with the appropriate command line flags.

Advantages:

  • Tests work right from the command line without having to specify the flags or use the Python test runner.
  • Less magic. Having what is going on be presented explicitly in the executable JavaScript is preferable to comments affecting code but only affecting it when run a certain way.
  • Related to the previous two, but this is more friendly to new contributors and sporadic/infrequent contributors.
  • This removes a small bit of dependency on Python that isn't necessary. Arguably, this aligns with https://github.com/nodejs/TSC/issues/642.

Downside:

  • It takes slightly longer for these tests to run. Not much, though. On my laptop, make test with this changeset took 5 minutes and 23 seconds. On master, it took 5 minutes and 16 seconds.

Upside to the downside:

  • If this encourages us to go through and figure out places where tests can be appropriately and correctly refactored to not use CLI flags, that's a win. In many cases, the use of a flag (especially --expose-internals) could be a sign the test is testing the wrong thing.

@nodejs/testing

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Merge request reports