Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test_runner: add enqueue and dequeue events #48428

Merged
merged 1 commit into from
Jun 13, 2023
Merged

test_runner: add enqueue and dequeue events #48428

merged 1 commit into from
Jun 13, 2023

Conversation

MoLow
Copy link
Member

@MoLow MoLow commented Jun 11, 2023

Fixes #46727

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jun 11, 2023
@MoLow
Copy link
Member Author

MoLow commented Jun 11, 2023

@sankalp1999 FWIW, if you want to complete the work on #47797 I will close this PR

@MoLow MoLow mentioned this pull request Jun 11, 2023
Closed
VoltrexKeyva
VoltrexKeyva reviewed Jun 11, 2023
View reviewed changes
doc/api/test.md Outdated Show resolved Hide resolved
@sankalp1999
Copy link
Contributor

sankalp1999 commented Jun 11, 2023 •
edited
Loading

@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.

anonrig
anonrig reviewed Jun 11, 2023
View reviewed changes
doc/api/test.md
* `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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

Suggested change
* `nesting` {number} The nesting level of the test.
* `level` {number} The nesting level of the test.

Copy link
Member Author

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

@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 12, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 12, 2023
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52218/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/52223/

@MoLow MoLow added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 12, 2023
benjamingr
benjamingr reviewed Jun 12, 2023
View reviewed changes
doc/api/test.md

* `data` {Object}
* `file` {string|undefined} The path of the test file,
undefined if test is not ran through a file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
undefined if test is not ran through a file.
undefined if test was not run through a file.

Copy link
Member Author

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

benjamingr
benjamingr reviewed Jun 12, 2023
View reviewed changes
doc/api/test.md

* `data` {Object}
* `file` {string|undefined} The path of the test file,
undefined if test is not ran through a file.
Copy link
Member

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?

Copy link
Member Author

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

benjamingr
benjamingr reviewed Jun 12, 2023
View reviewed changes
test/parallel/test-runner-reporters.js
@@ -97,7 +97,7 @@ describe('node:test reporters', { concurrency: true }, () => {
testFile]);
Copy link
Member

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

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 13, 2023
@nodejs-github-bot nodejs-github-bot merged commit 8bc6e19 into nodejs:main Jun 13, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 8bc6e19

@MoLow MoLow deleted the enque-deque branch June 13, 2023 18:39
@MoLow MoLow mentioned this pull request Jun 13, 2023
Merged
@github-actions github-actions bot mentioned this pull request Jun 21, 2023
Open
29 tasks
RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
@MoLow @RafaelGSS
test_runner: add enqueue and dequeue events
de4f14c 
PR-URL: #48428
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jul 3, 2023
Merged
@MoLow MoLow mentioned this pull request Jul 9, 2023
Closed
@MoLow MoLow added the backport-open-v18.x Indicate that the PR has an open backport. label Jul 9, 2023
MoLow added a commit to MoLow/node that referenced this pull request Jul 9, 2023
@MoLow
test_runner: add enqueue and dequeue events
c8cd582 
PR-URL: nodejs#48428
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit that referenced this pull request Jul 12, 2023
@MoLow @danielleadams
test_runner: add enqueue and dequeue events
5d910ca 
PR-URL: #48428
Backport-PR-URL: #48684
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@danielleadams danielleadams added backported-to-v18.x PRs backported to the v18.x-staging branch. and removed backport-open-v18.x Indicate that the PR has an open backport. labels Jul 12, 2023
@danielleadams danielleadams mentioned this pull request Jul 12, 2023
Merged
@philnash philnash mentioned this pull request Aug 2, 2023
Merged
7 tasks
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@MoLow @Ceres6
test_runner: add enqueue and dequeue events
7d6dd95 
PR-URL: nodejs#48428
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
@MoLow @Ceres6
test_runner: add enqueue and dequeue events
7d6edcf 
PR-URL: nodejs#48428
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VoltrexKeyva VoltrexKeyva VoltrexKeyva left review comments

@anonrig anonrig anonrig left review comments

@benjamingr benjamingr benjamingr approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backported-to-v18.x PRs backported to the v18.x-staging branch. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Indicate when a test is started in test_runner
8 participants
@MoLow @nodejs-github-bot @sankalp1999 @benjamingr @anonrig @cjihrig @VoltrexKeyva @danielleadams