Use Node's built-in test runner for tests that don't need to be run in a browser #1306
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We can use node:test to run unit tests for situations where we don't need the tests to run in a browser with karma, which we will be replacing.
I started looking into using node:test for our unit tests and learned that node requires this. Other environments like Typescript, or bundlers like rollup/esbuild/webpack/ or CJS imports, in the past have been more forgiving of skipping the file extension, and they will automatically add the '.js' or look in 'index.js' Node import with ES modules will not do this, for reasons. (it actually mimics the behavior of imports in a browser environment) So going forward, we should just always include the full path and extension for all local file imports. see also https://stackoverflow.com/questions/72491392/when-do-we-need-to-add-file-extension-when-importing-javascript-modules
Also created "headless.js" for code that can run without a browser. We may need a esbuild step for this eventually, but for now this is all code that should be able to run directly in vanilla JS / node with no complication step.
Introducing async in these tests was unnecessary and adding a bit of overhead. We found that just running the tests synchronously makes them a bit faster.
Also remove a few more of the unnecessary async/await
Also in intersection.js, add explicit `true`/`false` for `only` and `no` Before, it would just leave them `undefined` if they were unmatched.
|
This is going pretty well and we were able to convert over the tests from
I'm going to merge what we have so far to avoid the branch from going on too long and conflicting other things. For next steps I'd like to attack:
This is work that we can spread out and do over time. Improved testing of the validator would be the most impactful one. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been working with @tannerwuster for the past week on an experiment to convert some of our tests from executing under karma/mocha to using Node's test runner, available in Node v18 and newer. The experiment has been going pretty well, so I'm opening a PR for visibility.
In order to make this work, I needed to create a separate file that only exports the parts of code that can be run "headless" (i.e. no browser). When running in node, there wont be things like
window,document,navigator, etc. But what we can do is run plain Javascript code significantly faster, and use the c8 tool to gather detailed code coverage numbers:There are a few reasons why it makes sense to do this work now:
actionSyncCrossingTags#1285).