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?

doc/api/test.md

lib/internal/test_runner/runner.js

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

lib/internal/test_runner/runner.js

      triggerUncaughtException(error, true /* fromPromise */);
    }));
  });
+  signal?.addEventListener('abort', () => {


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 run does 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>

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

doc/api/test.md Outdated Show resolved Hide resolved

lib/internal/test_runner/runner.js Outdated Show resolved Hide resolved

MoLow force-pushed the test-runner-watch-mode-additions branch from 39bc086 to a52fb04 Compare 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-actions bot 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 d55ee1e Compare 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-actions bot 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-actions bot 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-actions bot 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-actions bot 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

franciszek-koltuniuk-red pushed a commit to franciszek-koltuniuk-red/node that referenced this pull request Jun 7, 2023

test_runner: emit test:watch:drained event

53af71e

PR-URL: nodejs#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: 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!

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!

benjamingr May 31, 2023

Choose a reason for hiding this comment

Uh oh!

MoLow May 31, 2023

Choose a reason for hiding this comment

Uh oh!

benjamingr May 31, 2023

Choose a reason for hiding this comment

Uh oh!

MoLow May 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