Skip to content

Commit

Permalink
test_runner,test: fix flaky test-runner-cli-concurrency.js
Browse files Browse the repository at this point in the history
This test was flaky on Windows when trying to clean up the
tmp directory, probably because it relied on child processes
timing out and being killed.

This commit updates the test to check for debug output
from the test runner. This should be adequate because the
original change was effectively:

let concurrency = getOptionValue('--test-concurrency') || true;

The test runner now logs the value of the concurrency variable.

Fixes: #50101
PR-URL: #50108
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
  • Loading branch information
cjihrig authored and targos committed Nov 11, 2023
1 parent 7b97c44 commit 64ef108
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 39 deletions.
14 changes: 12 additions & 2 deletions lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ const {
RegExpPrototypeExec,
StringPrototypeSplit,
} = primordials;
let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
debug = fn;
});

prepareMainThreadExecution(false);
markBootstrapComplete();
Expand Down Expand Up @@ -57,8 +60,15 @@ if (shardOption) {
};
}

run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters, shard })
.on('test:fail', (data) => {
const options = {
concurrency,
inspectPort,
watch: getOptionValue('--watch'),
setup: setupTestReporters,
shard,
};
debug('test runner configuration:', options);
run(options).on('test:fail', (data) => {
if (data.todo === undefined || data.todo === false) {
process.exitCode = kGenericUserError;
}
Expand Down
55 changes: 18 additions & 37 deletions test/parallel/test-runner-cli-concurrency.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,26 @@
'use strict';
const common = require('../common');
const tmpdir = require('../common/tmpdir');
const { deepStrictEqual, strictEqual } = require('node:assert');
require('../common');
const fixtures = require('../common/fixtures');
const assert = require('node:assert');
const { spawnSync } = require('node:child_process');
const { readdirSync, writeFileSync } = require('node:fs');
const { join } = require('node:path');
const { beforeEach, test } = require('node:test');
const { test } = require('node:test');
const cwd = fixtures.path('test-runner', 'default-behavior');
const env = { ...process.env, 'NODE_DEBUG': 'test_runner' };

function createTestFile(name) {
writeFileSync(join(tmpdir.path, name), `
const fs = require('node:fs');
fs.unlinkSync(__filename);
setTimeout(() => {}, 1_000_000_000);
`);
}

beforeEach(() => {
tmpdir.refresh();
createTestFile('test-1.js');
createTestFile('test-2.js');
test('default concurrency', async () => {
const args = ['--test'];
const cp = spawnSync(process.execPath, args, { cwd, env });
assert.match(cp.stderr.toString(), /concurrency: true,/);
});

test('concurrency of one', () => {
const cp = spawnSync(process.execPath, ['--test', '--test-concurrency=1'], {
cwd: tmpdir.path,
timeout: common.platformTimeout(1000),
});

strictEqual(cp.stderr.toString(), '');
strictEqual(cp.error.code, 'ETIMEDOUT');
deepStrictEqual(readdirSync(tmpdir.path), ['test-2.js']);
test('concurrency of one', async () => {
const args = ['--test', '--test-concurrency=1'];
const cp = spawnSync(process.execPath, args, { cwd, env });
assert.match(cp.stderr.toString(), /concurrency: 1,/);
});

test('concurrency of two', () => {
const cp = spawnSync(process.execPath, ['--test', '--test-concurrency=2'], {
cwd: tmpdir.path,
timeout: common.platformTimeout(1000),
});

strictEqual(cp.stderr.toString(), '');
strictEqual(cp.error.code, 'ETIMEDOUT');
deepStrictEqual(readdirSync(tmpdir.path), []);
test('concurrency of two', async () => {
const args = ['--test', '--test-concurrency=2'];
const cp = spawnSync(process.execPath, args, { cwd, env });
assert.match(cp.stderr.toString(), /concurrency: 2,/);
});

0 comments on commit 64ef108

Please sign in to comment.