Skip to content

test: split test-runner-run-watch.mjs #60653

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.

Sign up for GitHub

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

Merged
nodejs-github-bot merged 1 commit into from
Nov 11, 2025
Merged

test: split test-runner-run-watch.mjs #60653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 102 additions & 28 deletions test/common/watch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,58 @@ function refreshForTestRunnerWatch() {
}
}

async function performFileOperation(operation, useRunApi, timeout = 1000) {
if (useRunApi) {
const interval = setInterval(() => {
operation();
clearInterval(interval);
}, common.platformTimeout(timeout));
} else {
operation();
await setTimeout(common.platformTimeout(timeout));
}
}

function assertTestOutput(run, shouldCheckRecursion = false) {
if (shouldCheckRecursion) {
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/);
}

async function testRunnerWatch({
fileToUpdate,
file,
action = 'update',
fileToCreate,
isolation,
useRunApi = false,
cwd = tmpdir.path,
runnerCwd,
}) {
const ran1 = Promise.withResolvers();
const ran2 = Promise.withResolvers();
const child = spawn(process.execPath,
['--watch', '--test', '--test-reporter=spec',
isolation ? `--test-isolation=${isolation}` : '',
file ? fixturePaths[file] : undefined].filter(Boolean),
{ encoding: 'utf8', stdio: 'pipe', cwd: tmpdir.path });

let args;
if (useRunApi) {
// Use the fixture that calls run() API
const runner = fixtures.path('test-runner-watch.mjs');
args = [runner];
if (file) args.push('--file', file);
if (runnerCwd) args.push('--cwd', runnerCwd);
if (isolation) args.push('--isolation', isolation);
} else {
// Use CLI --watch --test flags
args = ['--watch', '--test', '--test-reporter=spec',
isolation ? `--test-isolation=${isolation}` : '',
file ? fixturePaths[file] : undefined].filter(Boolean);
}

const child = spawn(process.execPath, args,
{ encoding: 'utf8', stdio: 'pipe', cwd });
let stdout = '';
let currentRun = '';
const runs = [];
Expand All @@ -79,20 +117,28 @@ async function testRunnerWatch({
currentRun = '';
const content = fixtureContent[fileToUpdate];
const path = fixturePaths[fileToUpdate];
writeFileSync(path, content);
await setTimeout(common.platformTimeout(1000));
await ran2.promise;

if (useRunApi) {
const interval = setInterval(
() => writeFileSync(path, content),
common.platformTimeout(1000),
);
await ran2.promise;
clearInterval(interval);
Copy link
Member Author

@joyeecheung joyeecheung Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the original logic in

node/test/parallel/test-runner-run-watch.mjs

Lines 80 to 83 in 9524908

const interval = setInterval(() => writeFileSync(path, content), common.platformTimeout(1000));
await ran2.promise;
runs.push(currentRun);
clearInterval(interval);
here because I want to keep this PR strictly a split, and not change logic for now, so this is not reusing that performFileOperation() helper; test-run-watch-dependency-repeatedly.mjs flaked in https://ci.nodejs.org/job/node-test-pull-request/70115/ and I think the culprit might be in this logic - the interval fires too fast and before the test finishes, the write fires, then the watched process get killed with SIGTERM to run again. I think the follow up to deflake it might be to just do what's in the branch below and not do the interval.

(I am somewhat puzzled why it's using an interval in the original logic, I think update + timeout like the branch below which was split from the --watch test should suffice; repeated updates could trigger additional events that kill the process being watched unexpectedly @mcollina )

} else {
writeFileSync(path, content);
await setTimeout(common.platformTimeout(1000));
await ran2.promise;
}

runs.push(currentRun);
child.kill();
await once(child, 'exit');

assert.strictEqual(runs.length, 2);

for (const run of runs) {
assert.match(run, /tests 1/);
assert.match(run, /pass 1/);
assert.match(run, /fail 0/);
assert.match(run, /cancelled 0/);
assertTestOutput(run, useRunApi);
}
};

Expand All @@ -102,31 +148,53 @@ async function testRunnerWatch({
currentRun = '';
const fileToRenamePath = tmpdir.resolve(fileToUpdate);
const newFileNamePath = tmpdir.resolve(`test-renamed-${fileToUpdate}`);
renameSync(fileToRenamePath, newFileNamePath);
await setTimeout(common.platformTimeout(1000));

await performFileOperation(
() => renameSync(fileToRenamePath, newFileNamePath),
useRunApi,
);
await ran2.promise;

runs.push(currentRun);
child.kill();
await once(child, 'exit');

assert.strictEqual(runs.length, 2);

for (const run of runs) {
assert.match(run, /tests 1/);
assert.match(run, /pass 1/);
assert.match(run, /fail 0/);
assert.match(run, /cancelled 0/);
const [firstRun, secondRun] = runs;
assertTestOutput(firstRun, useRunApi);

if (action === 'rename2') {
assert.match(secondRun, /MODULE_NOT_FOUND/);
return;
}

assertTestOutput(secondRun, useRunApi);
};

const testDelete = async () => {
await ran1.promise;
runs.push(currentRun);
currentRun = '';
const fileToDeletePath = tmpdir.resolve(fileToUpdate);
unlinkSync(fileToDeletePath);
await setTimeout(common.platformTimeout(2000));
ran2.resolve();

if (useRunApi) {
const { existsSync } = require('node:fs');
const interval = setInterval(() => {
if (existsSync(fileToDeletePath)) {
unlinkSync(fileToDeletePath);
} else {
ran2.resolve();
clearInterval(interval);
}
}, common.platformTimeout(1000));
await ran2.promise;
} else {
unlinkSync(fileToDeletePath);
await setTimeout(common.platformTimeout(2000));
ran2.resolve();
}

runs.push(currentRun);
child.kill();
await once(child, 'exit');
Expand All @@ -143,25 +211,29 @@ async function testRunnerWatch({
runs.push(currentRun);
currentRun = '';
const newFilePath = tmpdir.resolve(fileToCreate);
writeFileSync(newFilePath, 'module.exports = {};');
await setTimeout(common.platformTimeout(1000));

await performFileOperation(
() => writeFileSync(newFilePath, 'module.exports = {};'),
useRunApi,
);
await ran2.promise;

runs.push(currentRun);
child.kill();
await once(child, 'exit');

for (const run of runs) {
assert.match(run, /tests 1/);
assert.match(run, /pass 1/);
assert.match(run, /fail 0/);
assert.match(run, /cancelled 0/);
assertTestOutput(run, false);
}
};

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

return runs;
}


Expand All @@ -170,4 +242,6 @@ module.exports = {
skipIfNoWatchModeSignals,
testRunnerWatch,
refreshForTestRunnerWatch,
fixtureContent,
fixturePaths,
};
8 changes: 0 additions & 8 deletions test/parallel/parallel.status
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ test-snapshot-incompatible: SKIP
test-inspector-network-fetch: PASS, FLAKY
# https://github.com/nodejs/node/issues/54808
test-async-context-frame: PASS, FLAKY
# https://github.com/nodejs/node/issues/54534
test-runner-run-watch: PASS, FLAKY
# https://github.com/nodejs/node/issues/59636
test-fs-cp-sync-error-on-exist: SKIP
test-fs-cp-sync-symlink-points-to-dest-error: SKIP
Expand All @@ -43,8 +41,6 @@ test-without-async-context-frame: PASS, FLAKY
test-performance-function: PASS, FLAKY
# https://github.com/nodejs/node/issues/54346
test-esm-loader-hooks-inspect-wait: PASS, FLAKY
# https://github.com/nodejs/node/issues/54534
test-runner-run-watch: PASS, FLAKY

[$system==linux && $arch==s390x]
# https://github.com/nodejs/node/issues/58353
Expand All @@ -54,8 +50,6 @@ test-http2-debug: PASS, FLAKY
# https://github.com/nodejs/node/issues/42741
test-http-server-headers-timeout-keepalive: PASS,FLAKY
test-http-server-request-timeout-keepalive: PASS,FLAKY
# https://github.com/nodejs/node/issues/54534
test-runner-run-watch: PASS, FLAKY
# https://github.com/nodejs/node/issues/60050
test-cluster-dgram-1: SKIP

Expand Down Expand Up @@ -85,8 +79,6 @@ test-esm-loader-hooks-inspect-wait: PASS, FLAKY
test-fs-promises-watch-iterator: SKIP
# https://github.com/nodejs/node/issues/50050
test-tick-processor-arguments: SKIP
# https://github.com/nodejs/node/issues/54534
test-runner-run-watch: PASS, FLAKY

[$system==freebsd]
# https://github.com/nodejs/node/issues/54346
Expand Down
Loading
Loading