FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: fix test runner concurrency · nodejs/node@40b3879 · GitHub

/ node Public

Commit 40b3879

Browse files
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

8 files changed

Lines changed: 71 additions & 19 deletions

File tree

‎lib/internal/per_context/primordials.js‎

Lines 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
/**

‎lib/internal/test_runner/runner.js‎

Lines 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
}

‎lib/internal/test_runner/test.js‎

Lines 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

‎lib/internal/test_runner/tests_stream.js‎

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

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

Lines 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+
}

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

Lines 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+
}

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

Lines 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();

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

Lines 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)
Back | FazBrowse Home | New Git URL