Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
test_runner: add enqueue and dequeue events#48428
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
nodejs-github-bot commented Jun 11, 2023
Review requested:
|
MoLow commented Jun 11, 2023
@sankalp1999 FWIW, if you want to complete the work on #47797 I will close this PR |
Uh oh!
There was an error while loading. Please reload this page.
sankalp1999 commented Jun 11, 2023 • 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.
@MoLow sorry, I couldn't find the time. I think you can continue with this one. I will close my PR. Thank you for reviewing. |
| *`file`{string|undefined} The path of the test file, | ||
| undefined if test is not ran through a file. | ||
| *`name`{string} The test name. | ||
| *`nesting`{number} The nesting level of the test. |
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.
Nit
| *`nesting`{number} The nesting level of the test. | |
| *`level`{number} The nesting level of the test. |
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.
nesting is already used on all other events on this class
nodejs-github-bot commented Jun 12, 2023
nodejs-github-bot commented Jun 12, 2023
| *`data`{Object} | ||
| *`file`{string|undefined} The path of the test file, | ||
| undefined if test is not ran through a file. |
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.
| undefined if test is not ran through a file. | |
| undefined if test was not run through a file. |
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.
this exact terminology is used in 6 other places in this file, so I will fix it in a follow-up PR
| *`data`{Object} | ||
| *`file`{string|undefined} The path of the test file, | ||
| undefined if test is not ran through a file. |
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.
What are you trying to say here?
"will not run" in this (and above) perhaps?
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 running within REPL is the main example of this. Il refine this in a follow-up
| constfilename=`custom.${ext}`; | ||
| constchild=spawnSync(process.execPath, | ||
| ['--test','--test-reporter',fixtures.fileURL('test-runner/custom_reporters/',filename), | ||
| testFile]); |
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.
Would be useful to also test the arguments
nodejs-github-bot commented Jun 13, 2023
Landed in 8bc6e19 |
PR-URL: #48428 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#48428 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: #48428 Backport-PR-URL: #48684 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#48428 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#48428 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes#46727