Skip to content

Commit 3a1a7fa

Browse files
MoLowtargos
authored andcommitted
test_runner: flatten TAP output when running using --test
PR-URL: #46440 Fixes: #45833 Refs: #45833 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent c972612 commit 3a1a7fa

9 files changed

+724
-697
lines changed

lib/internal/test_runner/runner.js

+56-33
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ const {
88
ArrayPrototypeSlice,
99
ArrayPrototypeSome,
1010
ArrayPrototypeSort,
11+
FunctionPrototypeCall,
12+
Number,
1113
ObjectAssign,
14+
ObjectKeys,
1215
PromisePrototypeThen,
13-
SafePromiseAll,
1416
SafePromiseAllReturnVoid,
1517
SafePromiseAllSettledReturnVoid,
1618
SafeMap,
@@ -35,7 +37,14 @@ const { validateArray, validateBoolean } = require('internal/validators');
3537
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
3638
const { kEmptyObject } = require('internal/util');
3739
const { createTestTree } = require('internal/test_runner/harness');
38-
const { kSubtestsFailed, Test } = require('internal/test_runner/test');
40+
const {
41+
kAborted,
42+
kCancelledByParent,
43+
kSubtestsFailed,
44+
kTestCodeFailure,
45+
kTestTimeoutFailure,
46+
Test,
47+
} = require('internal/test_runner/test');
3948
const { TapParser } = require('internal/test_runner/tap_parser');
4049
const { YAMLToJs } = require('internal/test_runner/yaml_to_js');
4150
const { TokenKind } = require('internal/test_runner/tap_lexer');
@@ -55,6 +64,9 @@ const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
5564
const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
5665
const kDiagnosticsFilterArgs = ['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];
5766

67+
const kCanceledTests = new SafeSet()
68+
.add(kCancelledByParent).add(kAborted).add(kTestTimeoutFailure);
69+
5870
// TODO(cjihrig): Replace this with recursive readdir once it lands.
5971
function processPath(path, testFiles, options) {
6072
const stats = statSync(path);
@@ -133,6 +145,11 @@ function getRunArgs({ path, inspectPort }) {
133145

134146
class FileTest extends Test {
135147
#buffer = [];
148+
#counters = { __proto__: null, all: 0, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
149+
failedSubtests = false;
150+
#skipReporting() {
151+
return this.#counters.all > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
152+
}
136153
#checkNestedComment({ comment }) {
137154
const firstSpaceIndex = StringPrototypeIndexOf(comment, ' ');
138155
if (firstSpaceIndex === -1) return false;
@@ -141,8 +158,6 @@ class FileTest extends Test {
141158
ArrayPrototypeIncludes(kDiagnosticsFilterArgs, StringPrototypeSlice(comment, 0, firstSpaceIndex));
142159
}
143160
#handleReportItem({ kind, node, comments, nesting = 0 }) {
144-
nesting += 1;
145-
146161
if (comments) {
147162
ArrayPrototypeForEach(comments, (comment) => this.reporter.diagnostic(nesting, this.name, comment));
148163
}
@@ -153,17 +168,20 @@ class FileTest extends Test {
153168
break;
154169

155170
case TokenKind.TAP_PLAN:
171+
if (nesting === 0 && this.#skipReporting()) {
172+
break;
173+
}
156174
this.reporter.plan(nesting, this.name, node.end - node.start + 1);
157175
break;
158176

159177
case TokenKind.TAP_SUBTEST_POINT:
160178
this.reporter.start(nesting, this.name, node.name);
161179
break;
162180

163-
case TokenKind.TAP_TEST_POINT:
164-
// eslint-disable-next-line no-case-declarations
181+
case TokenKind.TAP_TEST_POINT: {
182+
165183
const { todo, skip, pass } = node.status;
166-
// eslint-disable-next-line no-case-declarations
184+
167185
let directive;
168186

169187
if (skip) {
@@ -174,29 +192,22 @@ class FileTest extends Test {
174192
directive = kEmptyObject;
175193
}
176194

177-
if (pass) {
178-
this.reporter.ok(
179-
nesting,
180-
this.name,
181-
node.id,
182-
node.description,
183-
YAMLToJs(node.diagnostics),
184-
directive,
185-
);
186-
} else {
187-
this.reporter.fail(
188-
nesting,
189-
this.name,
190-
node.id,
191-
node.description,
192-
YAMLToJs(node.diagnostics),
193-
directive,
194-
);
195+
const diagnostics = YAMLToJs(node.diagnostics);
196+
const cancelled = kCanceledTests.has(diagnostics.error?.failureType);
197+
const testNumber = nesting === 0 ? (Number(node.id) + this.testNumber - 1) : node.id;
198+
const method = pass ? 'ok' : 'fail';
199+
this.reporter[method](nesting, this.name, testNumber, node.description, diagnostics, directive);
200+
if (nesting === 0) {
201+
FunctionPrototypeCall(super.countSubtest,
202+
{ finished: true, skipped: skip, isTodo: todo, passed: pass, cancelled },
203+
this.#counters);
204+
this.failedSubtests ||= !pass;
195205
}
196206
break;
197207

208+
}
198209
case TokenKind.COMMENT:
199-
if (nesting === 1 && this.#checkNestedComment(node)) {
210+
if (nesting === 0 && this.#checkNestedComment(node)) {
200211
// Ignore file top level diagnostics
201212
break;
202213
}
@@ -216,10 +227,24 @@ class FileTest extends Test {
216227
this.reportStarted();
217228
this.#handleReportItem(ast);
218229
}
230+
countSubtest(counters) {
231+
if (this.#counters.all === 0) {
232+
return super.countSubtest(counters);
233+
}
234+
ArrayPrototypeForEach(ObjectKeys(counters), (key) => {
235+
counters[key] += this.#counters[key];
236+
});
237+
}
238+
reportStarted() {}
219239
report() {
220-
this.reportStarted();
240+
const skipReporting = this.#skipReporting();
241+
if (!skipReporting) {
242+
super.reportStarted();
243+
}
221244
ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
222-
super.report();
245+
if (!skipReporting) {
246+
super.report();
247+
}
223248
}
224249
}
225250

@@ -274,16 +299,14 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
274299
subtest.addToReport(ast);
275300
});
276301

277-
const { 0: { 0: code, 1: signal } } = await SafePromiseAll([
278-
once(child, 'exit', { signal: t.signal }),
279-
child.stdout.toArray({ signal: t.signal }),
280-
]);
302+
const { 0: code, 1: signal } = await once(child, 'exit', { signal: t.signal });
281303

282304
runningProcesses.delete(path);
283305
runningSubtests.delete(path);
284306
if (code !== 0 || signal !== null) {
285307
if (!err) {
286-
err = ObjectAssign(new ERR_TEST_FAILURE('test failed', kSubtestsFailed), {
308+
const failureType = subtest.failedSubtests ? kSubtestsFailed : kTestCodeFailure;
309+
err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
287310
__proto__: null,
288311
exitCode: code,
289312
signal: signal,

lib/internal/test_runner/test.js

+37-27
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ const { availableParallelism } = require('os');
5555
const { bigint: hrtime } = process.hrtime;
5656
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
5757
const kCancelledByParent = 'cancelledByParent';
58+
const kAborted = 'testAborted';
5859
const kParentAlreadyFinished = 'parentAlreadyFinished';
5960
const kSubtestsFailed = 'subtestsFailed';
6061
const kTestCodeFailure = 'testCodeFailure';
@@ -379,10 +380,12 @@ class Test extends AsyncResource {
379380
}
380381

381382
#abortHandler = () => {
382-
this.cancel(this.#outerSignal?.reason || new AbortError('The test was aborted'));
383+
const error = this.#outerSignal?.reason || new AbortError('The test was aborted');
384+
error.failureType = kAborted;
385+
this.#cancel(error);
383386
};
384387

385-
cancel(error) {
388+
#cancel(error) {
386389
if (this.endTime !== null) {
387390
return;
388391
}
@@ -459,7 +462,7 @@ class Test extends AsyncResource {
459462
return true;
460463
}
461464
if (this.#outerSignal?.aborted) {
462-
this.cancel(this.#outerSignal.reason || new AbortError('The test was aborted'));
465+
this.#abortHandler();
463466
return true;
464467
}
465468
}
@@ -552,7 +555,7 @@ class Test extends AsyncResource {
552555
try { await afterEach(); } catch { /* test is already failing, let's the error */ }
553556
if (isTestFailureError(err)) {
554557
if (err.failureType === kTestTimeoutFailure) {
555-
this.cancel(err);
558+
this.#cancel(err);
556559
} else {
557560
this.fail(err);
558561
}
@@ -566,9 +569,31 @@ class Test extends AsyncResource {
566569
this.postRun();
567570
}
568571

569-
postRun(pendingSubtestsError) {
570-
const counters = { __proto__: null, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
572+
countSubtest(counters) {
573+
// Check SKIP and TODO tests first, as those should not be counted as
574+
// failures.
575+
if (this.skipped) {
576+
counters.skipped++;
577+
} else if (this.isTodo) {
578+
counters.todo++;
579+
} else if (this.cancelled) {
580+
counters.cancelled++;
581+
} else if (!this.passed) {
582+
counters.failed++;
583+
} else {
584+
counters.passed++;
585+
}
586+
587+
if (!this.passed) {
588+
counters.totalFailed++;
589+
}
590+
counters.all++;
591+
}
571592

593+
postRun(pendingSubtestsError) {
594+
const counters = {
595+
__proto__: null, all: 0, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0,
596+
};
572597
// If the test was failed before it even started, then the end time will
573598
// be earlier than the start time. Correct that here.
574599
if (this.endTime < this.startTime) {
@@ -583,27 +608,10 @@ class Test extends AsyncResource {
583608
const subtest = this.subtests[i];
584609

585610
if (!subtest.finished) {
586-
subtest.cancel(pendingSubtestsError);
611+
subtest.#cancel(pendingSubtestsError);
587612
subtest.postRun(pendingSubtestsError);
588613
}
589-
590-
// Check SKIP and TODO tests first, as those should not be counted as
591-
// failures.
592-
if (subtest.skipped) {
593-
counters.skipped++;
594-
} else if (subtest.isTodo) {
595-
counters.todo++;
596-
} else if (subtest.cancelled) {
597-
counters.cancelled++;
598-
} else if (!subtest.passed) {
599-
counters.failed++;
600-
} else {
601-
counters.passed++;
602-
}
603-
604-
if (!subtest.passed) {
605-
counters.totalFailed++;
606-
}
614+
subtest.countSubtest(counters);
607615
}
608616

609617
if ((this.passed || this.parent === null) && counters.totalFailed > 0) {
@@ -623,13 +631,13 @@ class Test extends AsyncResource {
623631
this.parent.processPendingSubtests();
624632
} else if (!this.reported) {
625633
this.reported = true;
626-
this.reporter.plan(this.nesting, kFilename, this.subtests.length);
634+
this.reporter.plan(this.nesting, kFilename, counters.all);
627635

628636
for (let i = 0; i < this.diagnostics.length; i++) {
629637
this.reporter.diagnostic(this.nesting, kFilename, this.diagnostics[i]);
630638
}
631639

632-
this.reporter.diagnostic(this.nesting, kFilename, `tests ${this.subtests.length}`);
640+
this.reporter.diagnostic(this.nesting, kFilename, `tests ${counters.all}`);
633641
this.reporter.diagnostic(this.nesting, kFilename, `pass ${counters.passed}`);
634642
this.reporter.diagnostic(this.nesting, kFilename, `fail ${counters.failed}`);
635643
this.reporter.diagnostic(this.nesting, kFilename, `cancelled ${counters.cancelled}`);
@@ -806,6 +814,8 @@ module.exports = {
806814
kCancelledByParent,
807815
kSubtestsFailed,
808816
kTestCodeFailure,
817+
kTestTimeoutFailure,
818+
kAborted,
809819
kUnwrapErrors,
810820
Suite,
811821
Test,

test/message/test_runner_abort.out

+10
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ TAP version 13
4040
not ok 7 - not ok 3
4141
---
4242
duration_ms: *
43+
failureType: 'testAborted'
4344
error: 'This operation was aborted'
4445
code: 20
4546
stack: |-
@@ -58,6 +59,7 @@ TAP version 13
5859
not ok 8 - not ok 4
5960
---
6061
duration_ms: *
62+
failureType: 'testAborted'
6163
error: 'This operation was aborted'
6264
code: 20
6365
stack: |-
@@ -76,6 +78,7 @@ TAP version 13
7678
not ok 9 - not ok 5
7779
---
7880
duration_ms: *
81+
failureType: 'testAborted'
7982
error: 'This operation was aborted'
8083
code: 20
8184
stack: |-
@@ -94,6 +97,7 @@ TAP version 13
9497
not ok 1 - promise timeout signal
9598
---
9699
duration_ms: *
100+
failureType: 'testAborted'
97101
error: 'The operation was aborted due to timeout'
98102
code: 23
99103
stack: |-
@@ -106,6 +110,7 @@ not ok 1 - promise timeout signal
106110
not ok 2 - promise abort signal
107111
---
108112
duration_ms: *
113+
failureType: 'testAborted'
109114
error: 'This operation was aborted'
110115
code: 20
111116
stack: |-
@@ -160,6 +165,7 @@ not ok 2 - promise abort signal
160165
not ok 7 - not ok 3
161166
---
162167
duration_ms: *
168+
failureType: 'testAborted'
163169
error: 'This operation was aborted'
164170
code: 20
165171
stack: |-
@@ -178,6 +184,7 @@ not ok 2 - promise abort signal
178184
not ok 8 - not ok 4
179185
---
180186
duration_ms: *
187+
failureType: 'testAborted'
181188
error: 'This operation was aborted'
182189
code: 20
183190
stack: |-
@@ -196,6 +203,7 @@ not ok 2 - promise abort signal
196203
not ok 9 - not ok 5
197204
---
198205
duration_ms: *
206+
failureType: 'testAborted'
199207
error: 'This operation was aborted'
200208
code: 20
201209
stack: |-
@@ -214,6 +222,7 @@ not ok 2 - promise abort signal
214222
not ok 3 - callback timeout signal
215223
---
216224
duration_ms: *
225+
failureType: 'testAborted'
217226
error: 'The operation was aborted due to timeout'
218227
code: 23
219228
stack: |-
@@ -226,6 +235,7 @@ not ok 3 - callback timeout signal
226235
not ok 4 - callback abort signal
227236
---
228237
duration_ms: *
238+
failureType: 'testAborted'
229239
error: 'This operation was aborted'
230240
code: 20
231241
stack: |-

test/message/test_runner_abort_suite.out

+2
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ TAP version 13
6464
not ok 1 - describe timeout signal
6565
---
6666
duration_ms: *
67+
failureType: 'testAborted'
6768
error: 'The operation was aborted due to timeout'
6869
code: 23
6970
stack: |-
@@ -76,6 +77,7 @@ not ok 1 - describe timeout signal
7677
not ok 2 - describe abort signal
7778
---
7879
duration_ms: *
80+
failureType: 'testAborted'
7981
error: 'This operation was aborted'
8082
code: 20
8183
stack: |-

0 commit comments

Comments
 (0)