Skip to content

Commit

Permalink
test_runner: fix support watch with run(), add globPatterns option
Browse files Browse the repository at this point in the history
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #53866
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
  • Loading branch information
mcollina authored and RafaelGSS committed Aug 5, 2024
1 parent ee97c04 commit 893c864
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 49 deletions.
6 changes: 6 additions & 0 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,9 @@ added:
- v18.9.0
- v16.19.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/53866
description: Added the `globPatterns` option.
- version: v22.0.0
pr-url: https://github.com/nodejs/node/pull/52038
description: Added the `forceExit` option.
Expand All @@ -1263,6 +1266,9 @@ changes:
* `forceExit`: {boolean} Configures the test runner to exit the process once
all known tests have finished executing even if the event loop would
otherwise remain active. **Default:** `false`.
* `globPatterns`: {Array} An array containing the list of glob patterns to
match test files. This option cannot be used together with `files`.
**Default:** matching files from [test runner execution model][].
* `inspectPort` {number|Function} Sets inspector port of test child process.
This can be a number, or a function that takes no arguments and returns a
number. If a nullish value is provided, each process gets its own port,
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
'use strict';

const {
ArrayPrototypeSlice,
} = primordials;

const {
prepareMainThreadExecution,
markBootstrapComplete,
Expand Down Expand Up @@ -42,6 +46,7 @@ const options = {
setup: setupTestReporters,
timeout: perFileTimeout,
shard,
globPatterns: ArrayPrototypeSlice(process.argv, 1),
};
debug('test runner configuration:', options);
run(options).on('test:fail', (data) => {
Expand Down
30 changes: 22 additions & 8 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
'use strict';

const {
ArrayIsArray,
ArrayPrototypeEvery,
Expand All @@ -10,7 +11,6 @@ const {
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeShift,
ArrayPrototypeSlice,
ArrayPrototypeSome,
ArrayPrototypeSort,
ObjectAssign,
Expand Down Expand Up @@ -87,10 +87,12 @@ const kCanceledTests = new SafeSet()

let kResistStopPropagation;

function createTestFileList() {
function createTestFileList(patterns) {
const cwd = process.cwd();
const hasUserSuppliedPattern = process.argv.length > 1;
const patterns = hasUserSuppliedPattern ? ArrayPrototypeSlice(process.argv, 1) : [kDefaultPattern];
const hasUserSuppliedPattern = patterns != null;
if (!patterns || patterns.length === 0) {
patterns = [kDefaultPattern];
}
const glob = new Glob(patterns, {
__proto__: null,
cwd,
Expand Down Expand Up @@ -344,7 +346,6 @@ function runTestFile(path, filesWatcher, opts) {

let err;


child.on('error', (error) => {
err = error;
});
Expand Down Expand Up @@ -418,8 +419,8 @@ function watchFiles(testFiles, opts) {
opts.root.harness.watching = true;

watcher.on('changed', ({ owners, eventType }) => {
if (eventType === 'rename') {
const updatedTestFiles = createTestFileList();
if (!opts.hasFiles && eventType === 'rename') {
const updatedTestFiles = createTestFileList(opts.globPatterns);

const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x));
const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x));
Expand Down Expand Up @@ -482,6 +483,7 @@ function run(options = kEmptyObject) {
watch,
setup,
only,
globPatterns,
} = options;

if (files != null) {
Expand All @@ -502,6 +504,16 @@ function run(options = kEmptyObject) {
if (only != null) {
validateBoolean(only, 'options.only');
}
if (globPatterns != null) {
validateArray(globPatterns, 'options.globPatterns');
}

if (globPatterns?.length > 0 && files?.length > 0) {
throw new ERR_INVALID_ARG_VALUE(
'options.globPatterns', globPatterns, 'is not supported when specifying \'options.files\'',
);
}

if (shard != null) {
validateObject(shard, 'options.shard');
// Avoid re-evaluating the shard object in case it's a getter
Expand Down Expand Up @@ -557,7 +569,7 @@ function run(options = kEmptyObject) {
root.postRun();
return root.reporter;
}
let testFiles = files ?? createTestFileList();
let testFiles = files ?? createTestFileList(globPatterns);

if (shard) {
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
Expand All @@ -573,6 +585,8 @@ function run(options = kEmptyObject) {
inspectPort,
testNamePatterns,
testSkipPatterns,
hasFiles: files != null,
globPatterns,
only,
forceExit,
};
Expand Down
24 changes: 24 additions & 0 deletions test/fixtures/test-runner-watch.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { run } from 'node:test';
import { tap } from 'node:test/reporters';
import { parseArgs } from 'node:util';

const options = {
file: {
type: 'string',
},
};
const {
values,
positionals,
} = parseArgs({ args: process.argv.slice(2), options });

let files;

if (values.file) {
files = [values.file];
}

run({
files,
watch: true
}).compose(tap).pipe(process.stdout);
61 changes: 61 additions & 0 deletions test/parallel/test-runner-run-files-undefined.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import * as common from '../common/index.mjs';
import tmpdir from '../common/tmpdir.js';
import { describe, it, run, beforeEach } from 'node:test';
import { dot, spec, tap } from 'node:test/reporters';
import { fork } from 'node:child_process';
import assert from 'node:assert';

if (common.hasCrypto) {
console.log('1..0 # Skipped: no crypto');
process.exit(0);
}

if (process.env.CHILD === 'true') {
describe('require(\'node:test\').run with no files', { concurrency: true }, () => {
beforeEach(() => {
tmpdir.refresh();
process.chdir(tmpdir.path);
});

it('should neither pass or fail', async () => {
const stream = run({
files: undefined
}).compose(tap);
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustNotCall());

// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
});

it('can use the spec reporter', async () => {
const stream = run({
files: undefined
}).compose(spec);
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustNotCall());

// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
});

it('can use the dot reporter', async () => {
const stream = run({
files: undefined
}).compose(dot);
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustNotCall());

// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
});
});
} else if (common.isAIX) {
console.log('1..0 # Skipped: test runner without specifying files fails on AIX');
} else {
fork(import.meta.filename, [], {
env: { CHILD: 'true' }
}).on('exit', common.mustCall((code) => {
assert.strictEqual(code, 0);
}));
}
175 changes: 175 additions & 0 deletions test/parallel/test-runner-run-watch.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// Flags: --expose-internals
import * as common from '../common/index.mjs';
import { describe, it, beforeEach } from 'node:test';
import assert from 'node:assert';
import { spawn } from 'node:child_process';
import { once } from 'node:events';
import { writeFileSync, renameSync, unlinkSync, existsSync } from 'node:fs';
import util from 'internal/util';
import tmpdir from '../common/tmpdir.js';
import { join } from 'node:path';

if (common.isIBMi)
common.skip('IBMi does not support `fs.watch()`');

// This test updates these files repeatedly,
// Reading them from disk is unreliable due to race conditions.
const fixtureContent = {
'dependency.js': 'module.exports = {};',
'dependency.mjs': 'export const a = 1;',
'test.js': `
const test = require('node:test');
require('./dependency.js');
import('./dependency.mjs');
import('data:text/javascript,');
test('test has ran');`,
};

let fixturePaths;

function refresh() {
tmpdir.refresh();

fixturePaths = Object.keys(fixtureContent)
.reduce((acc, file) => ({ ...acc, [file]: tmpdir.resolve(file) }), {});
Object.entries(fixtureContent)
.forEach(([file, content]) => writeFileSync(fixturePaths[file], content));
}

const runner = join(import.meta.dirname, '..', 'fixtures', 'test-runner-watch.mjs');

async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.path }) {
const ran1 = util.createDeferredPromise();
const ran2 = util.createDeferredPromise();
const args = [runner];
if (file) args.push('--file', file);
const child = spawn(process.execPath,
args,
{ encoding: 'utf8', stdio: 'pipe', cwd });
let stdout = '';
let currentRun = '';
const runs = [];

child.stdout.on('data', (data) => {
stdout += data.toString();
currentRun += data.toString();
const testRuns = stdout.match(/# duration_ms\s\d+/g);
if (testRuns?.length >= 1) ran1.resolve();
if (testRuns?.length >= 2) ran2.resolve();
});

const testUpdate = async () => {
await ran1.promise;
const content = fixtureContent[fileToUpdate];
const path = fixturePaths[fileToUpdate];
const interval = setInterval(() => writeFileSync(path, content), common.platformTimeout(1000));
await ran2.promise;
runs.push(currentRun);
clearInterval(interval);
child.kill();
await once(child, 'exit');
for (const run of runs) {
assert.doesNotMatch(run, /run\(\) is being called recursively/);
assert.match(run, /# tests 1/);
assert.match(run, /# pass 1/);
assert.match(run, /# fail 0/);
assert.match(run, /# cancelled 0/);
}
};

const testRename = async () => {
await ran1.promise;
const fileToRenamePath = tmpdir.resolve(fileToUpdate);
const newFileNamePath = tmpdir.resolve(`test-renamed-${fileToUpdate}`);
const interval = setInterval(() => renameSync(fileToRenamePath, newFileNamePath), common.platformTimeout(1000));
await ran2.promise;
runs.push(currentRun);
clearInterval(interval);
child.kill();
await once(child, 'exit');

for (const run of runs) {
assert.doesNotMatch(run, /run\(\) is being called recursively/);
if (action === 'rename2') {
assert.match(run, /MODULE_NOT_FOUND/);
} else {
assert.doesNotMatch(run, /MODULE_NOT_FOUND/);
}
assert.match(run, /# tests 1/);
assert.match(run, /# pass 1/);
assert.match(run, /# fail 0/);
assert.match(run, /# cancelled 0/);
}
};

const testDelete = async () => {
await ran1.promise;
const fileToDeletePath = tmpdir.resolve(fileToUpdate);
const interval = setInterval(() => {
if (existsSync(fileToDeletePath)) {
unlinkSync(fileToDeletePath);
} else {
ran2.resolve();
}
}, common.platformTimeout(1000));
await ran2.promise;
runs.push(currentRun);
clearInterval(interval);
child.kill();
await once(child, 'exit');

for (const run of runs) {
assert.doesNotMatch(run, /MODULE_NOT_FOUND/);
}
};

action === 'update' && await testUpdate();
action === 'rename' && await testRename();
action === 'rename2' && await testRename();
action === 'delete' && await testDelete();
}

describe('test runner watch mode', () => {
beforeEach(refresh);
it('should run tests repeatedly', async () => {
await testWatch({ file: 'test.js', fileToUpdate: 'test.js' });
});

it('should run tests with dependency repeatedly', async () => {
await testWatch({ file: 'test.js', fileToUpdate: 'dependency.js' });
});

it('should run tests with ESM dependency', async () => {
await testWatch({ file: 'test.js', fileToUpdate: 'dependency.mjs' });
});

it('should support running tests without a file', async () => {
await testWatch({ fileToUpdate: 'test.js' });
});

it('should support a watched test file rename', async () => {
await testWatch({ fileToUpdate: 'test.js', action: 'rename' });
});

it('should not throw when deleting a watched test file', { skip: common.isAIX }, async () => {
await testWatch({ fileToUpdate: 'test.js', action: 'delete' });
});

it('should run tests with dependency repeatedly in a different cwd', async () => {
await testWatch({
file: join(tmpdir.path, 'test.js'),
fileToUpdate: 'dependency.js',
cwd: import.meta.dirname,
action: 'rename2'
});
});

it('should handle renames in a different cwd', async () => {
await testWatch({
file: join(tmpdir.path, 'test.js'),
fileToUpdate: 'test.js',
cwd: import.meta.dirname,
action: 'rename2'
});
});
});
Loading

0 comments on commit 893c864

Please sign in to comment.