From e9760b4ae82246833003e5fad07f4b5e6c5901d8 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sun, 13 Nov 2022 23:40:26 +0200 Subject: [PATCH] test_runner: support watch mode PR-URL: https://github.com/nodejs/node/pull/45214 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Antoine du Hamel --- doc/api/cli.md | 15 ++++- doc/api/test.md | 19 +++++++ lib/internal/main/test_runner.js | 3 +- lib/internal/test_runner/runner.js | 72 +++++++++++++++++++++--- lib/internal/watch_mode/files_watcher.js | 34 +++++++++-- src/node_options.cc | 6 +- test/fixtures/test-runner/dependency.js | 1 + test/fixtures/test-runner/dependency.mjs | 1 + test/fixtures/test-runner/dependent.js | 3 + test/parallel/test-runner-watch-mode.mjs | 46 +++++++++++++++ 10 files changed, 180 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/test-runner/dependency.js create mode 100644 test/fixtures/test-runner/dependency.mjs create mode 100644 test/fixtures/test-runner/dependent.js create mode 100644 test/parallel/test-runner-watch-mode.mjs diff --git a/doc/api/cli.md b/doc/api/cli.md index aeebb55933d622..9fa3463eda1d36 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1208,11 +1208,16 @@ status code 1. added: - v18.1.0 - v16.17.0 +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/45214 + description: Test runner now supports running in watch mode. --> 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. +`--watch-path`, `--check`, `--eval`, `--interactive`, or the inspector. +See the documentation on [running tests from the command line][] +for more details. ### `--test-name-pattern` @@ -1574,6 +1579,10 @@ will be chosen. > Stability: 1 - Experimental @@ -1607,7 +1616,7 @@ This will turn off watching of required or imported modules, even when used in combination with `--watch`. This flag cannot be combined with -`--check`, `--eval`, `--interactive`, or the REPL. +`--check`, `--eval`, `--interactive`, `--test`, or the REPL. ```console $ node --watch-path=./src --watch-path=./tests index.js diff --git a/doc/api/test.md b/doc/api/test.md index cc55eb6943028b..3e9d3775f4cabd 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -291,6 +291,25 @@ test('a test that creates asynchronous activity', (t) => { }); ``` +## Watch mode + + + +> Stability: 1 - Experimental + +The Node.js test runner supports running in watch mode by passing the `--watch` flag: + +```bash +node --test --watch +``` + +In watch mode, the test runner will watch for changes to test files and +their dependencies. When a change is detected, the test runner will +rerun the tests affected by the change. +The test runner will continue to run until the process is terminated. + ## Running tests from the command line The Node.js test runner can be invoked from the command line by passing the diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 12753f3bbbf6dc..f7165a0288cf9e 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -3,6 +3,7 @@ const { prepareMainThreadExecution, markBootstrapComplete } = require('internal/process/pre_execution'); +const { getOptionValue } = require('internal/options'); const { isUsingInspector } = require('internal/util/inspector'); const { run } = require('internal/test_runner/runner'); const { exitCodes: { kGenericUserError } } = internalBinding('errors'); @@ -20,7 +21,7 @@ if (isUsingInspector()) { inspectPort = process.debugPort; } -const tapStream = run({ concurrency, inspectPort }); +const tapStream = run({ concurrency, inspectPort, watch: getOptionValue('--watch') }); tapStream.pipe(process.stdout); tapStream.once('test:fail', () => { process.exitCode = kGenericUserError; diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index c82799a30ac6af..f1e536c493f200 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -10,6 +10,9 @@ const { ObjectAssign, PromisePrototypeThen, SafePromiseAll, + SafePromiseAllReturnVoid, + SafePromiseAllSettledReturnVoid, + SafeMap, SafeSet, } = primordials; @@ -17,13 +20,14 @@ const { spawn } = require('child_process'); const { readdirSync, statSync } = require('fs'); // TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern. const { createInterface } = require('readline'); +const { FilesWatcher } = require('internal/watch_mode/files_watcher'); const console = require('internal/console/global'); const { codes: { ERR_TEST_FAILURE, }, } = require('internal/errors'); -const { validateArray } = require('internal/validators'); +const { validateArray, validateBoolean } = require('internal/validators'); const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector'); const { kEmptyObject } = require('internal/util'); const { createTestTree } = require('internal/test_runner/harness'); @@ -34,9 +38,12 @@ const { } = require('internal/test_runner/utils'); const { basename, join, resolve } = require('path'); const { once } = require('events'); -const { exitCodes: { kGenericUserError } } = internalBinding('errors'); +const { + triggerUncaughtException, + exitCodes: { kGenericUserError }, +} = internalBinding('errors'); -const kFilterArgs = ['--test']; +const kFilterArgs = ['--test', '--watch']; // TODO(cjihrig): Replace this with recursive readdir once it lands. function processPath(path, testFiles, options) { @@ -113,17 +120,28 @@ function getRunArgs({ path, inspectPort }) { return argv; } +const runningProcesses = new SafeMap(); +const runningSubtests = new SafeMap(); -function runTestFile(path, root, inspectPort) { +function runTestFile(path, root, inspectPort, filesWatcher) { const subtest = root.createSubtest(Test, path, async (t) => { const args = getRunArgs({ path, inspectPort }); + const stdio = ['pipe', 'pipe', 'pipe']; + const env = { ...process.env }; + if (filesWatcher) { + stdio.push('ipc'); + env.WATCH_REPORT_DEPENDENCIES = '1'; + } - const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8' }); + const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8', env, stdio }); + runningProcesses.set(path, child); // TODO(cjihrig): Implement a TAP parser to read the child's stdout // instead of just displaying it all if the child fails. let err; let stderr = ''; + filesWatcher?.watchChildProcessModules(child, path); + child.on('error', (error) => { err = error; }); @@ -146,6 +164,8 @@ function runTestFile(path, root, inspectPort) { child.stdout.toArray({ signal: t.signal }), ]); + runningProcesses.delete(path); + runningSubtests.delete(path); if (code !== 0 || signal !== null) { if (!err) { err = ObjectAssign(new ERR_TEST_FAILURE('test failed', kSubtestsFailed), { @@ -166,21 +186,57 @@ function runTestFile(path, root, inspectPort) { return subtest.start(); } +function watchFiles(testFiles, root, inspectPort) { + const filesWatcher = new FilesWatcher({ throttle: 500, mode: 'filter' }); + filesWatcher.on('changed', ({ owners }) => { + filesWatcher.unfilterFilesOwnedBy(owners); + PromisePrototypeThen(SafePromiseAllReturnVoid(testFiles, async (file) => { + if (!owners.has(file)) { + return; + } + const runningProcess = runningProcesses.get(file); + if (runningProcess) { + runningProcess.kill(); + await once(runningProcess, 'exit'); + } + await runningSubtests.get(file); + runningSubtests.set(file, runTestFile(file, root, inspectPort, filesWatcher)); + }, undefined, (error) => { + triggerUncaughtException(error, true /* fromPromise */); + })); + }); + return filesWatcher; +} + function run(options) { if (options === null || typeof options !== 'object') { options = kEmptyObject; } - const { concurrency, timeout, signal, files, inspectPort } = options; + const { concurrency, timeout, signal, files, inspectPort, watch } = options; if (files != null) { validateArray(files, 'options.files'); } + if (watch != null) { + validateBoolean(watch, 'options.watch'); + } const root = createTestTree({ concurrency, timeout, signal }); const testFiles = files ?? createTestFileList(); - PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, inspectPort)), - () => root.postRun()); + let postRun = () => root.postRun(); + let filesWatcher; + if (watch) { + filesWatcher = watchFiles(testFiles, root, inspectPort); + postRun = undefined; + } + + PromisePrototypeThen(SafePromiseAllSettledReturnVoid(testFiles, (path) => { + const subtest = runTestFile(path, root, inspectPort, filesWatcher); + runningSubtests.set(path, subtest); + return subtest; + }), postRun); + return root.reporter; } diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index f2141051fceaf4..3c756c4b5d77c9 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -26,6 +26,8 @@ class FilesWatcher extends EventEmitter { #watchers = new SafeMap(); #filteredFiles = new SafeSet(); #throttling = new SafeSet(); + #depencencyOwners = new SafeMap(); + #ownerDependencies = new SafeMap(); #throttle; #mode; @@ -74,7 +76,8 @@ class FilesWatcher extends EventEmitter { return; } this.#throttling.add(trigger); - this.emit('changed'); + const owners = this.#depencencyOwners.get(trigger); + this.emit('changed', { owners }); setTimeout(() => this.#throttling.delete(trigger), this.#throttle).unref(); } @@ -95,7 +98,7 @@ class FilesWatcher extends EventEmitter { } } - filterFile(file) { + filterFile(file, owner) { if (!file) return; if (supportsRecursiveWatching) { this.watchPath(dirname(file)); @@ -105,24 +108,43 @@ class FilesWatcher extends EventEmitter { this.watchPath(file, false); } this.#filteredFiles.add(file); + if (owner) { + const owners = this.#depencencyOwners.get(file) ?? new SafeSet(); + const dependencies = this.#ownerDependencies.get(file) ?? new SafeSet(); + owners.add(owner); + dependencies.add(file); + this.#depencencyOwners.set(file, owners); + this.#ownerDependencies.set(owner, dependencies); + } } - watchChildProcessModules(child) { + watchChildProcessModules(child, key = null) { if (this.#mode !== 'filter') { return; } child.on('message', (message) => { try { if (ArrayIsArray(message['watch:require'])) { - ArrayPrototypeForEach(message['watch:require'], (file) => this.filterFile(file)); + ArrayPrototypeForEach(message['watch:require'], (file) => this.filterFile(file, key)); } if (ArrayIsArray(message['watch:import'])) { - ArrayPrototypeForEach(message['watch:import'], (file) => this.filterFile(fileURLToPath(file))); + ArrayPrototypeForEach(message['watch:import'], (file) => this.filterFile(fileURLToPath(file), key)); } } catch { // Failed watching file. ignore } }); } + unfilterFilesOwnedBy(owners) { + owners.forEach((owner) => { + this.#ownerDependencies.get(owner)?.forEach((dependency) => { + this.#filteredFiles.delete(dependency); + this.#depencencyOwners.delete(dependency); + }); + this.#filteredFiles.delete(owner); + this.#depencencyOwners.delete(owner); + this.#ownerDependencies.delete(owner); + }); + } clearFileFilters() { this.#filteredFiles.clear(); } @@ -130,6 +152,8 @@ class FilesWatcher extends EventEmitter { this.#watchers.forEach(this.#unwatch); this.#watchers.clear(); this.#filteredFiles.clear(); + this.#depencencyOwners.clear(); + this.#ownerDependencies.clear(); } } diff --git a/src/node_options.cc b/src/node_options.cc index ce353bbcf5cbbf..d3c493a36d5f89 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -152,9 +152,9 @@ void EnvironmentOptions::CheckOptions(std::vector* errors, errors->push_back("either --test or --interactive can be used, not both"); } - if (watch_mode) { - // TODO(MoLow): Support (incremental?) watch mode within test runner - errors->push_back("either --test or --watch can be used, not both"); + if (watch_mode_paths.size() > 0) { + errors->push_back( + "--watch-path cannot be used in combination with --test"); } #ifndef ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER diff --git a/test/fixtures/test-runner/dependency.js b/test/fixtures/test-runner/dependency.js new file mode 100644 index 00000000000000..f053ebf7976e37 --- /dev/null +++ b/test/fixtures/test-runner/dependency.js @@ -0,0 +1 @@ +module.exports = {}; diff --git a/test/fixtures/test-runner/dependency.mjs b/test/fixtures/test-runner/dependency.mjs new file mode 100644 index 00000000000000..cc798ff50da947 --- /dev/null +++ b/test/fixtures/test-runner/dependency.mjs @@ -0,0 +1 @@ +export const a = 1; diff --git a/test/fixtures/test-runner/dependent.js b/test/fixtures/test-runner/dependent.js new file mode 100644 index 00000000000000..c382b0f989e47b --- /dev/null +++ b/test/fixtures/test-runner/dependent.js @@ -0,0 +1,3 @@ +require('./dependency.js'); +import('./dependency.mjs'); +import('data:text/javascript,'); diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs new file mode 100644 index 00000000000000..6803ac4e349138 --- /dev/null +++ b/test/parallel/test-runner-watch-mode.mjs @@ -0,0 +1,46 @@ +// Flags: --expose-internals +import '../common/index.mjs'; +import { describe, it } from 'node:test'; +import { spawn } from 'node:child_process'; +import { writeFileSync, readFileSync } from 'node:fs'; +import util from 'internal/util'; +import * as fixtures from '../common/fixtures.mjs'; + +async function testWatch({ files, fileToUpdate }) { + const ran1 = util.createDeferredPromise(); + const ran2 = util.createDeferredPromise(); + const child = spawn(process.execPath, ['--watch', '--test', '--no-warnings', ...files], { encoding: 'utf8' }); + let stdout = ''; + child.stdout.on('data', (data) => { + stdout += data.toString(); + if (/ok 2/.test(stdout)) ran1.resolve(); + if (/ok 3/.test(stdout)) ran2.resolve(); + }); + + await ran1.promise; + writeFileSync(fileToUpdate, readFileSync(fileToUpdate, 'utf8')); + await ran2.promise; + child.kill(); +} + +describe('test runner watch mode', () => { + it('should run tests repeatedly', async () => { + const file1 = fixtures.path('test-runner/index.test.js'); + const file2 = fixtures.path('test-runner/subdir/subdir_test.js'); + await testWatch({ files: [file1, file2], fileToUpdate: file2 }); + }); + + it('should run tests with dependency repeatedly', async () => { + const file1 = fixtures.path('test-runner/index.test.js'); + const dependent = fixtures.path('test-runner/dependent.js'); + const dependency = fixtures.path('test-runner/dependency.js'); + await testWatch({ files: [file1, dependent], fileToUpdate: dependency }); + }); + + it('should run tests with ESM dependency', async () => { + const file1 = fixtures.path('test-runner/index.test.js'); + const dependent = fixtures.path('test-runner/dependent.js'); + const dependency = fixtures.path('test-runner/dependency.mjs'); + await testWatch({ files: [file1, dependent], fileToUpdate: dependency }); + }); +});