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.

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

test_runner: support watch mode #45214

Merged
merged 2 commits into from Nov 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 12 additions & 3 deletions doc/api/cli.md
Expand Up @@ -1209,11 +1209,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
MoLow marked this conversation as resolved.
Show resolved Hide resolved
`--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`

Expand Down Expand Up @@ -1575,6 +1580,10 @@ will be chosen.

<!-- YAML
added: v18.11.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/45214
description: Test runner now supports running in watch mode.
-->

> Stability: 1 - Experimental
Expand Down Expand Up @@ -1608,7 +1617,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
Expand Down
19 changes: 19 additions & 0 deletions doc/api/test.md
Expand Up @@ -291,6 +291,25 @@ test('a test that creates asynchronous activity', (t) => {
});
```

## Watch mode

<!-- YAML
added: REPLACEME
-->

MoLow marked this conversation as resolved.
Show resolved Hide resolved
> 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
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/main/test_runner.js
Expand Up @@ -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');
Expand All @@ -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;
Expand Down
72 changes: 64 additions & 8 deletions lib/internal/test_runner/runner.js
Expand Up @@ -10,20 +10,24 @@ const {
ObjectAssign,
PromisePrototypeThen,
SafePromiseAll,
SafePromiseAllReturnVoid,
SafePromiseAllSettledReturnVoid,
SafeMap,
SafeSet,
} = primordials;

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');
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
});
Expand All @@ -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), {
Expand All @@ -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 */);
}));
MoLow marked this conversation as resolved.
Show resolved Hide resolved
});
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;
}
Expand Down
34 changes: 29 additions & 5 deletions lib/internal/watch_mode/files_watcher.js
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}

Expand All @@ -95,7 +98,7 @@ class FilesWatcher extends EventEmitter {
}
}

filterFile(file) {
filterFile(file, owner) {
if (!file) return;
if (supportsRecursiveWatching) {
this.watchPath(dirname(file));
Expand All @@ -105,31 +108,52 @@ 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();
}
clear() {
this.#watchers.forEach(this.#unwatch);
this.#watchers.clear();
this.#filteredFiles.clear();
this.#depencencyOwners.clear();
this.#ownerDependencies.clear();
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/node_options.cc
Expand Up @@ -152,9 +152,9 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* 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
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/test-runner/dependency.js
@@ -0,0 +1 @@
module.exports = {};
1 change: 1 addition & 0 deletions test/fixtures/test-runner/dependency.mjs
@@ -0,0 +1 @@
export const a = 1;
3 changes: 3 additions & 0 deletions test/fixtures/test-runner/dependent.js
@@ -0,0 +1,3 @@
require('./dependency.js');
import('./dependency.mjs');
import('data:text/javascript,');
46 changes: 46 additions & 0 deletions 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 });
MoLow marked this conversation as resolved.
Show resolved Hide resolved
});

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 });
});
});