test_runner: support watch mode by MoLow · Pull Request #45214 · nodejs/node

MoLow · 2022-10-27T21:00:33Z

TODO:

tests
documentation?
call clearFileFilters for specific child process
if it doesn't take too long - wait for TAP parser to land to avoid conflicts

cjihrig · 2022-11-09T04:39:46Z

@MoLow is this ready for review? At a minimum, it looks like there are linter issues.

MoLow · 2022-11-09T10:42:48Z

is this ready for review? At a minimum, it looks like there are linter issues.

not yet, will work on it today/tomorrow

MoLow · 2022-11-10T22:02:08Z

@nodejs/test_runner ready for review

aduh95

We should probably update the doc of --watch-path and --test to clarify they can't be used together

node/doc/api/cli.md

Lines 1214 to 1216 in f1e9382

    
           Starts the Node.js command line test runner. This flag cannot be combined with 
        
           `--check`, `--eval`, `--interactive`, or the inspector. See the documentation 
        
           on [running tests from the command line][] for more details.

node/doc/api/cli.md

Lines 1610 to 1611 in f1e9382

    
           This flag cannot be combined with 
        
           `--check`, `--eval`, `--interactive`, or the REPL.

We could add an entry to the history of --watch and --test to say that now they can be used together.

Could we add a test to ensure watching ESM works? And maybe even ESM+CJS?

MoLow · 2022-11-11T13:03:18Z

@ErickWendel you were asking about this

aduh95

I don't know if you missed it in my previous review, I was asking if it would be possible to add tests for ESM (i.e. does dependencies loaded though import() from a CJS files are picked up)

MoLow · 2022-11-13T15:03:28Z

Going to rebase on top of #45348 to avoid conflicts

nodejs-github-bot · 2022-11-13T19:17:04Z

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

nodejs-github-bot · 2022-11-13T21:40:28Z

Landed in 97355c7

PR-URL: #45214 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: nodejs#45214 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #45214 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

filterFile() reads #ownerDependencies with the dependency path, but writes the set back keyed by the owner, so each call starts a fresh set and overwrites the previous one. An owner keeps only the last dependency registered for it. unfilterFilesOwnedBy() walks that set to release an owner's dependencies, so every other one stays in #filteredFiles and #dependencyOwners. Under `node --test --watch` a restart re-reports the modules the test still loads, which hides it for those, but a dependency it stopped requiring is never reported again and keeps re-running the test. Read #ownerDependencies with the key it is written with, and collapse the repeated #dependencyOwners lookup in the loop, which now runs once per dependency rather than once per owner. Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com> PR-URL: #66139 Refs: #45214 Reviewed-By: Moshe Atlow <moshe@atlow.co.il>

nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 27, 2022

MoLow added test_runner Issues and PRs related to the test runner subsystem. watch-mode Issues and PRs related to watch mode. labels Oct 27, 2022

VoltrexKeyva reviewed Oct 28, 2022

View reviewed changes

Comment thread lib/internal/test_runner/runner.js Resolved

MoLow force-pushed the test_runner_watch_mode branch from 91df6e5 to 49ad862 Compare November 6, 2022 21:41

MoLow force-pushed the test_runner_watch_mode branch from 49ad862 to 9be722d Compare November 10, 2022 18:01

aduh95 reviewed Nov 11, 2022

View reviewed changes

Comment thread lib/internal/test_runner/runner.js Resolved

aduh95 reviewed Nov 11, 2022

View reviewed changes

Comment thread test/fixtures/test-runner/dependency.js Resolved

Comment thread doc/api/cli.md Resolved

aduh95 reviewed Nov 11, 2022

View reviewed changes

Comment thread lib/internal/test_runner/runner.js Resolved

aduh95 reviewed Nov 11, 2022

View reviewed changes

Comment thread lib/internal/test_runner/runner.js Resolved

Comment thread lib/internal/watch_mode/files_watcher.js Resolved

cjihrig reviewed Nov 11, 2022

View reviewed changes

Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare November 12, 2022 01:49

MoLow requested review from aduh95, benjamingr and cjihrig November 12, 2022 16:45

MoLow added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Nov 12, 2022

aduh95 reviewed Nov 12, 2022

View reviewed changes

Comment thread lib/internal/test_runner/runner.js Resolved

Comment thread test/fixtures/test-runner/dependent.js Resolved

benjamingr approved these changes Nov 13, 2022

View reviewed changes

cjihrig reviewed Nov 13, 2022

View reviewed changes

Comment thread test/fixtures/test-runner/dependency.mjs Resolved

Comment thread test/parallel/test-runner-watch-mode.mjs Resolved

Comment thread test/parallel/test-runner-watch-mode.mjs Resolved

test_runner: support watch mode

28f8da3

MoLow force-pushed the test_runner_watch_mode branch from bf37536 to 28f8da3 Compare November 13, 2022 15:21

MoLow removed the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Nov 13, 2022

MoLow requested a review from cjihrig November 13, 2022 15:23

richardlau reviewed Nov 13, 2022

View reviewed changes

Comment thread test/parallel/test-runner-watch-mode.mjs Resolved

MoLow added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Nov 13, 2022

CR

f6f9ebf

MoLow requested a review from aduh95 November 13, 2022 17:32

MoLow mentioned this pull request Nov 13, 2022

watch mode: use recursive fs.watch #45271

Closed

aduh95 approved these changes Nov 13, 2022

View reviewed changes

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 13, 2022

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 13, 2022

MoLow added the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 13, 2022

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 13, 2022

nodejs-github-bot merged commit 97355c7 into nodejs:main Nov 13, 2022

MoLow deleted the test_runner_watch_mode branch November 13, 2022 21:41

ruyadorno pushed a commit that referenced this pull request Nov 21, 2022

test_runner: support watch mode

e9760b4

PR-URL: #45214 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

MoLow mentioned this pull request Nov 24, 2022

[v16.x backport] backport multiple test runner features #45602

Closed

ruyadorno mentioned this pull request Nov 24, 2022

v19.2.0 proposal #45615

Merged

danielleadams mentioned this pull request Dec 30, 2022

v18.13.0 release proposal #46025

Merged

danielleadams pushed a commit that referenced this pull request Dec 30, 2022

test_runner: support watch mode

da7042a

PR-URL: #45214 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

danielleadams pushed a commit that referenced this pull request Jan 3, 2023

test_runner: support watch mode

55b64e0

PR-URL: #45214 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

MoLow mentioned this pull request Apr 23, 2023

Test Runner is slow #47663

Closed

christianaurichzm mentioned this pull request Sep 19, 2026

watch: track all dependencies of an owner #66139

Merged

	Starts the Node.js command line test runner. This flag cannot be combined with
	`--check`, `--eval`, `--interactive`, or the inspector. See the documentation
	on [running tests from the command line][] for more details.

	This flag cannot be combined with
	`--check`, `--eval`, `--interactive`, or the REPL.

Conversation

MoLow commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cjihrig commented Nov 9, 2022

Uh oh!

MoLow commented Nov 9, 2022

Uh oh!

MoLow commented Nov 10, 2022

Uh oh!

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MoLow commented Nov 11, 2022

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MoLow commented Nov 13, 2022

Uh oh!

Uh oh!

nodejs-github-bot commented Nov 13, 2022

Uh oh!

nodejs-github-bot commented Nov 13, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

MoLow commented Oct 27, 2022 •

edited

Loading