test_runner: watch mode improvments by MoLow · Pull Request #48259 · nodejs/node

MoLow · 2023-05-30T23:58:09Z

some fixes for using run with watch: true:

emit an event when no more queued tests
the queued tests were in a global state preventing the event ^ from working correctly when using multiple run's
respect AbortSignal

nodejs-github-bot · 2023-05-30T23:58:14Z

Review requested:

@nodejs/test_runner

cjihrig

Code LGTM with a few minor comments. I do have two questions:

What was the motivation for this new event?
Why do we only care about this event in the context of watch mode? Is there value in having a generic test:drain event?

MoLow · 2023-05-31T08:19:41Z

Code LGTM with a few minor comments. I do have two questions:
What was the motivation for this new event?
Why do we only care about this event in the context of watch mode? Is there value in having a generic test:drain event?

I have started experimenting with an npm package that runs tests in interactive mode. just a feedback/feature request from friends.
I don't think a generic test:drain is needed - since when not in watch mode you can simply listen to the testsStream end event

Screen.Recording.2023-05-31.at.11.13.02.mov

benjamingr · 2023-05-31T09:34:13Z

need to remove this listener if abort never happens but the watcher is already cleared or is this always before the process dies and "hanging" this if we never abort is fine?

it is ok - if you don't pass a signal the watcher will never clear/close

The concern was about leaking the listener on the signal after run finished.

indeed - in watch mode rundoes not finish besides this new flow

nodejs-github-bot · 2023-05-31T09:45:27Z

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

MoLow · 2023-05-31T21:04:48Z

@benjamingr I pushed a fix for a race condition (the test finishes before the watch mode ipc message is processed) - so the abortSignal is now passed into fs.watch
PTAL (and re-approve for convenience of the commit queue)

nodejs-github-bot · 2023-05-31T21:11:56Z

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

nodejs-github-bot · 2023-06-01T02:56:08Z

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

nodejs-github-bot · 2023-06-02T00:28:11Z

Landed in 0cf8bbe...5d685e4

PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: nodejs#48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

MoLow requested review from benjamingr and cjihrig May 30, 2023 23:58

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 May 30, 2023

MoLow added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label May 31, 2023

cjihrig approved these changes May 31, 2023
View reviewed changes

Comment threaddoc/api/test.md Outdated
Comment threadlib/internal/test_runner/runner.js Outdated

MoLow force-pushed the test-runner-watch-mode-additions branch from 39bc086 to a52fb04Compare May 31, 2023 09:08

benjamingr reviewed May 31, 2023
View reviewed changes

benjamingr approved these changes May 31, 2023
View reviewed changes

MoLow added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 31, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 31, 2023

MoLow added 2 commits May 31, 2023 23:56

test_runner: stop watch mode when abortSignal aborted
078a25f

test_runner: emit test:watch:drained event
d55ee1e

MoLow force-pushed the test-runner-watch-mode-additions branch from a52fb04 to d55ee1eCompare May 31, 2023 21:00

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label May 31, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 31, 2023

benjamingr approved these changes Jun 1, 2023
View reviewed changes

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 1, 2023

github-actionsBot mentioned this pull request Jun 2, 2023
CI Reliability 2023-06-02 nodejs/reliability#583
Open
34 tasks

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 2, 2023

nodejs-github-bot closed this Jun 2, 2023

nodejs-github-bot pushed a commit that referenced this pull request Jun 2, 2023
test_runner: stop watch mode when abortSignal aborted
ca71ae6
PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

nodejs-github-bot pushed a commit that referenced this pull request Jun 2, 2023
test_runner: emit test:watch:drained event
5d685e4
PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

github-actionsBot mentioned this pull request Jun 3, 2023
CI Reliability 2023-06-03 nodejs/reliability#584
Open
33 tasks

MoLow deleted the test-runner-watch-mode-additions branch June 3, 2023 19:25

github-actionsBot mentioned this pull request Jun 4, 2023
CI Reliability 2023-06-04 nodejs/reliability#585
Open
30 tasks

targos pushed a commit that referenced this pull request Jun 4, 2023
test_runner: stop watch mode when abortSignal aborted
c9f8e8c
PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

targos pushed a commit that referenced this pull request Jun 4, 2023
test_runner: emit test:watch:drained event
3f259b7
PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

targos mentioned this pull request Jun 4, 2023
v20.3.0 release proposal #48332
Merged

This was referenced Jun 5, 2023
CI Reliability 2023-06-05 nodejs/reliability#586
Open
CI Reliability 2023-06-06 nodejs/reliability#587
Open

danielleadams pushed a commit that referenced this pull request Jul 6, 2023
test_runner: stop watch mode when abortSignal aborted
97aad16
PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

danielleadams pushed a commit that referenced this pull request Jul 6, 2023
test_runner: emit test:watch:drained event
68f2c49
PR-URL: #48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

MoLow added a commit to MoLow/node that referenced this pull request Jul 6, 2023
test_runner: stop watch mode when abortSignal aborted
618a9e1
PR-URL: nodejs#48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

MoLow added a commit to MoLow/node that referenced this pull request Jul 6, 2023
test_runner: emit test:watch:drained event
033d0bb
PR-URL: nodejs#48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

danielleadams mentioned this pull request Jul 10, 2023
v18.17.0 release proposal #48694
Merged

philnash mentioned this pull request Aug 2, 2023
[node] Adds test:enqueue, test:dequeue and test:watch:drained to test reporter events DefinitelyTyped/DefinitelyTyped#66257
Merged
7 tasks

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
test_runner: stop watch mode when abortSignal aborted
40a500d
PR-URL: nodejs#48259 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
test_runner: emit test:watch:drained event
6ba2949
PR-URL: nodejs#48259 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
test_runner: stop watch mode when abortSignal aborted
7082c47
PR-URL: nodejs#48259 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
test_runner: emit test:watch:drained event
a27c30f
PR-URL: nodejs#48259 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Uh oh!

Uh oh!

Conversation

MoLow commented May 30, 2023

Uh oh!

nodejs-github-bot commented May 30, 2023

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MoLow commented May 31, 2023

Uh oh!

benjamingrMay 31, 2023

Choose a reason for hiding this comment

Uh oh!

MoLowMay 31, 2023

Choose a reason for hiding this comment

Uh oh!

benjamingrMay 31, 2023

Choose a reason for hiding this comment

Uh oh!

MoLowMay 31, 2023

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 31, 2023

Uh oh!

MoLow commented May 31, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented May 31, 2023

Uh oh!

nodejs-github-bot commented Jun 1, 2023

Uh oh!

nodejs-github-bot commented Jun 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MoLow commented May 31, 2023•
edited
Loading