Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 40b3879

Browse filesBrowse files
MoLowtargos
MoLow
authored and
targos
committed
test_runner: fix test runner concurrency
PR-URL: #47675 Fixes: #47365 Fixes: #47696 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 2d7cac0 commit 40b3879
Copy full SHA for 40b3879

File tree

Expand file treeCollapse file tree

8 files changed

+71
-19
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+71
-19
lines changed
Open diff view settings
Collapse file

‎lib/internal/per_context/primordials.js‎

Copy file name to clipboardExpand all lines: lib/internal/per_context/primordials.js
+1-7Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -565,13 +565,7 @@ primordials.SafePromiseAllSettled = (promises, mapFn) =>
565565
* @returns {Promise<void>}
566566
*/
567567
primordials.SafePromiseAllSettledReturnVoid = async (promises, mapFn) => {
568-
for (let i = 0; i < promises.length; i++) {
569-
try {
570-
await (mapFn != null ? mapFn(promises[i], i) : promises[i]);
571-
} catch {
572-
// In all settled, we can ignore errors.
573-
}
574-
}
568+
await primordials.SafePromiseAllSettled(promises, mapFn);
575569
};
576570

577571
/**
Collapse file

‎lib/internal/test_runner/runner.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/runner.js
+18-8Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,6 @@ class FileTest extends Test {
207207
const testNumber = nesting === 0 ? (this.root.harness.counters.topLevel + 1) : node.id;
208208
const method = pass ? 'ok' : 'fail';
209209
this.reporter[method](nesting, this.name, testNumber, node.description, diagnostics, directive);
210-
if (nesting === 0) {
211-
this.failedSubtests ||= !pass;
212-
}
213-
this.#reportedChildren++;
214210
countCompletedTest({
215211
name: node.description,
216212
finished: true,
@@ -237,22 +233,36 @@ class FileTest extends Test {
237233
break;
238234
}
239235
}
236+
#accumulateReportItem({ kind, node, comments, nesting = 0 }) {
237+
if (kind !== TokenKind.TAP_TEST_POINT) {
238+
return;
239+
}
240+
this.#reportedChildren++;
241+
if (nesting === 0 && !node.status.pass) {
242+
this.failedSubtests = true;
243+
}
244+
}
245+
#drainBuffer() {
246+
if (this.#buffer.length > 0) {
247+
ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
248+
this.#buffer = [];
249+
}
250+
}
240251
addToReport(ast) {
252+
this.#accumulateReportItem(ast);
241253
if (!this.isClearToSend()) {
242254
ArrayPrototypePush(this.#buffer, ast);
243255
return;
244256
}
245-
this.reportStarted();
257+
this.#drainBuffer();
246258
this.#handleReportItem(ast);
247259
}
248260
reportStarted() {}
249261
report() {
262+
this.#drainBuffer();
250263
const skipReporting = this.#skipReporting();
251264
if (!skipReporting) {
252265
super.reportStarted();
253-
}
254-
ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
255-
if (!skipReporting) {
256266
super.report();
257267
}
258268
}
Collapse file

‎lib/internal/test_runner/test.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/test.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ class Test extends AsyncResource {
658658
this.reporter.coverage(this.nesting, kFilename, coverage);
659659
}
660660

661-
this.reporter.push(null);
661+
this.reporter.end();
662662
}
663663
}
664664

Collapse file

‎lib/internal/test_runner/tests_stream.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/tests_stream.js
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ class TestsStream extends Readable {
5959
this.#emit('test:coverage', { __proto__: null, nesting, file, summary });
6060
}
6161

62+
end() {
63+
this.#tryPush(null);
64+
}
65+
6266
#emit(type, data) {
6367
this.emit(type, data);
6468
this.#tryPush({ type, data });
Collapse file

‎test/fixtures/test-runner/concurrency/a.mjs‎

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import tmpdir from '../../../common/tmpdir.js';
2+
import { setTimeout } from 'node:timers/promises';
3+
import fs from 'node:fs/promises';
4+
import path from 'node:path';
5+
6+
await fs.writeFile(path.resolve(tmpdir.path, 'test-runner-concurrency'), 'a.mjs');
7+
while (true) {
8+
const file = await fs.readFile(path.resolve(tmpdir.path, 'test-runner-concurrency'), 'utf8');
9+
if (file === 'b.mjs') {
10+
break;
11+
}
12+
await setTimeout(10);
13+
}
Collapse file

‎test/fixtures/test-runner/concurrency/b.mjs‎

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import tmpdir from '../../../common/tmpdir.js';
2+
import { setTimeout } from 'node:timers/promises';
3+
import fs from 'node:fs/promises';
4+
import path from 'node:path';
5+
6+
while (true) {
7+
const file = await fs.readFile(path.resolve(tmpdir.path, 'test-runner-concurrency'), 'utf8');
8+
if (file === 'a.mjs') {
9+
await fs.writeFile(path.resolve(tmpdir.path, 'test-runner-concurrency'), 'b.mjs');
10+
break;
11+
}
12+
await setTimeout(10);
13+
}
Collapse file

‎test/parallel/test-primordials-promise.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-primordials-promise.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,11 @@ assertIsPromise(SafePromisePrototypeFinally(test(), common.mustCall()));
5555

5656
assertIsPromise(SafePromiseAllReturnArrayLike([test()]));
5757
assertIsPromise(SafePromiseAllReturnVoid([test()]));
58-
assertIsPromise(SafePromiseAllSettledReturnVoid([test()]));
5958
assertIsPromise(SafePromiseAny([test()]));
6059
assertIsPromise(SafePromiseRace([test()]));
6160

6261
assertIsPromise(SafePromiseAllReturnArrayLike([]));
6362
assertIsPromise(SafePromiseAllReturnVoid([]));
64-
assertIsPromise(SafePromiseAllSettledReturnVoid([]));
6563

6664
{
6765
const val1 = Symbol();
@@ -108,9 +106,11 @@ Object.defineProperties(Array.prototype, {
108106

109107
assertIsPromise(SafePromiseAll([test()]));
110108
assertIsPromise(SafePromiseAllSettled([test()]));
109+
assertIsPromise(SafePromiseAllSettledReturnVoid([test()]));
111110

112111
assertIsPromise(SafePromiseAll([]));
113112
assertIsPromise(SafePromiseAllSettled([]));
113+
assertIsPromise(SafePromiseAllSettledReturnVoid([]));
114114

115115
async function test() {
116116
const catchFn = common.mustCall();
Collapse file

‎test/parallel/test-runner-concurrency.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-runner-concurrency.js
+19-1Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
'use strict';
22
const common = require('../common');
3+
const tmpdir = require('../common/tmpdir');
4+
const fixtures = require('../common/fixtures');
35
const { describe, it, test } = require('node:test');
4-
const assert = require('assert');
6+
const assert = require('node:assert');
7+
const path = require('node:path');
8+
const fs = require('node:fs/promises');
9+
const os = require('node:os');
10+
11+
tmpdir.refresh();
512

613
describe('Concurrency option (boolean) = true ', { concurrency: true }, () => {
714
let isFirstTestOver = false;
@@ -62,3 +69,14 @@ describe(
6269
it('should run after other suites', expectedTestTree);
6370
});
6471
}
72+
73+
test('--test multiple files', { skip: os.availableParallelism() < 3 }, async () => {
74+
await fs.writeFile(path.resolve(tmpdir.path, 'test-runner-concurrency'), '');
75+
const { code, stderr } = await common.spawnPromisified(process.execPath, [
76+
'--test',
77+
fixtures.path('test-runner', 'concurrency', 'a.mjs'),
78+
fixtures.path('test-runner', 'concurrency', 'b.mjs'),
79+
]);
80+
assert.strictEqual(stderr, '');
81+
assert.strictEqual(code, 0);
82+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.