Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.4k
test: refactor test-abortcontroller to use node:test#54574
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Starting the long process of refactoring our own tests to use the node:test module and mocks.
| @@ -1,7 +1,7 @@ | |||
| // Flags: --no-warnings --expose-gc --expose-internals | |||
| // Flags: --expose-gc | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add an eslint rule to avoid re-adding expose-internals to files that we avoid.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
cjihrig commented Aug 27, 2024
Not that I mind, but I'm curious what the motivation is for this? I'm assuming something related to Workers. |
jasnell commented Aug 27, 2024
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. |
lpinca commented Aug 27, 2024
To be honest, I think it goes in the opposite direction. Using BDD specific syntax does not help. |
jasnell commented Aug 27, 2024
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. |
This comment was marked as outdated.
This comment was marked as outdated.
cjihrig commented Aug 27, 2024
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. |
lpinca commented Aug 27, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Using |
jasnell commented Aug 27, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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 |
lpinca commented Aug 27, 2024
Yes, I disagree. The |
This comment was marked as outdated.
This comment was marked as outdated.
nodejs-github-bot commented Aug 28, 2024 • edited by jasnell
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by jasnell
Uh oh!
There was an error while loading. Please reload this page.
Starting the long process of refactoring our own tests to use the node:test module and mocks. PR-URL: #54574 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
jasnell commented Aug 29, 2024
Landed in 39215e1 |
Starting the long process of refactoring our own tests to use the node:test module and mocks. PR-URL: #54574 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Starting the long process of refactoring our own tests to use the node:test module and mocks. PR-URL: #54574 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Starting the long process of refactoring our own tests to use the node:test module and mocks. PR-URL: #54574 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Starting the long process of refactoring our own tests to use the node:test module and mocks. PR-URL: nodejs#54574 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
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.