node
node copied to clipboard
nodejs
test: refactor test-abortcontroller to use node:test
Starting the long process of refactoring our own tests to use the node:test module and mocks.
Also, splits the test that depends on internal API from the rest of the tests to make it easier to differentiate.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 87.29%. Comparing base (
4f1c27a) to head (fb44af9). Report is 317 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #54574 +/- ##
==========================================
- Coverage 87.33% 87.29% -0.04%
==========================================
Files 649 649
Lines 182620 182620
Branches 35042 35035 -7
==========================================
- Hits 159490 159424 -66
- Misses 16394 16457 +63
- Partials 6736 6739 +3
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Starting the long process of refactoring our own tests to use the node:test module
Not that I mind, but I'm curious what the motivation is for this? I'm assuming something related to Workers.
Not that I mind, but I'm curious what the motivation is for this? I'm assuming something related to Workers.
Key motivation is make the tests more structured and easier to decompose... and yes, it's largely for other runtimes that want to be able to run/port the tests more easily in order to better verify Node.js compatibility. But it's also just good to have improved structure in our own tests just for us.
Key motivation is make the tests more structured and easier to decompose... and yes, it's largely for other runtimes that want to be able to run/port the tests more easily in order to better verify Node.js compatibility
To be honest, I think it goes in the opposite direction. Using BDD specific syntax does not help.
... Using BDD specific syntax does not help.
As someone who has been working to port many of these tests to another runtime adding this kind of structure does help significantly. The current unstructured tests mix a number of public API, internal API, and test harness code in ways that make it difficult and time consuming to decompose or, often, to even know what exactly is being tested. Sure, the organization can be further improved but introducing some structure and organization is objectively better than having none, which is what we have now.
The current unstructured tests mix a number of public API, internal API, and test harness code in ways that make it difficult and time consuming to decompose or, often, to even know what exactly is being tested.
As someone who has also ported many Node tests to another runtime, I can confirm that was a pain point for me at the time.
The current unstructured tests mix a number of public API, internal API, and test harness code in ways that make it difficult and time consuming to decompose or, often, to even know what exactly is being tested
Using node:test does not change that, it only add an additional module to the mix and a specific non portable test syntax. I think there is nothing better than a single file per test with no dependencies other than the strictly necessary ones. I said it elsewhere but I'll say it again. I have always loved the way tests were made and run in Node.js: a single script with no bullshit just like the minimal test cases we maintainers ask for bug reports.
... Using node:test does not change that,
Not sure we'll agree on this point. Decomposing the tests into more structured units; going through the tests and relying, as much as possible, on public API surface (for instance, replacing common.mustCall(...) with mock.fn() checks); and having cleaner separation between things that are testing public API vs internal API helps significantly in porting these tests to other environments to ensure compatibility. Sure, the syntax isn't perfect but this absolutely helps the process along.
replacing
common.mustCall(...)withmock.fn()checks
Yes, I disagree. The common module is still required, so we actually end up duplicating/adding more code.