Skip to content

Commit 5b3c606

Browse files
MoLowjuanarbol
MoLow
authored and
juanarbol
committed
test_runner: flatten TAP output when running using --test
PR-URL: #46440 Backport-PR-URL: #46839 Fixes: #45833 Refs: #45833 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 391ff0d commit 5b3c606

9 files changed

+726
-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');
@@ -54,6 +63,9 @@ const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
5463
const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
5564
const kDiagnosticsFilterArgs = ['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];
5665

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

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

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

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

162-
case TokenKind.TAP_TEST_POINT:
163-
// eslint-disable-next-line no-case-declarations
180+
case TokenKind.TAP_TEST_POINT: {
181+
164182
const { todo, skip, pass } = node.status;
165-
// eslint-disable-next-line no-case-declarations
183+
166184
let directive;
167185

168186
if (skip) {
@@ -173,29 +191,22 @@ class FileTest extends Test {
173191
directive = kEmptyObject;
174192
}
175193

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

207+
}
197208
case TokenKind.COMMENT:
198-
if (nesting === 1 && this.#checkNestedComment(node)) {
209+
if (nesting === 0 && this.#checkNestedComment(node)) {
199210
// Ignore file top level diagnostics
200211
break;
201212
}
@@ -215,10 +226,24 @@ class FileTest extends Test {
215226
this.reportStarted();
216227
this.#handleReportItem(ast);
217228
}
229+
countSubtest(counters) {
230+
if (this.#counters.all === 0) {
231+
return super.countSubtest(counters);
232+
}
233+
ArrayPrototypeForEach(ObjectKeys(counters), (key) => {
234+
counters[key] += this.#counters[key];
235+
});
236+
}
237+
reportStarted() {}
218238
report() {
219-
this.reportStarted();
239+
const skipReporting = this.#skipReporting();
240+
if (!skipReporting) {
241+
super.reportStarted();
242+
}
220243
ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
221-
super.report();
244+
if (!skipReporting) {
245+
super.report();
246+
}
222247
}
223248
}
224249

@@ -273,16 +298,14 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
273298
subtest.addToReport(ast);
274299
});
275300

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

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

‎lib/internal/test_runner/test.js

+37-27
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const { cpus } = require('os');
5757
const { bigint: hrtime } = process.hrtime;
5858
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
5959
const kCancelledByParent = 'cancelledByParent';
60+
const kAborted = 'testAborted';
6061
const kParentAlreadyFinished = 'parentAlreadyFinished';
6162
const kSubtestsFailed = 'subtestsFailed';
6263
const kTestCodeFailure = 'testCodeFailure';
@@ -390,10 +391,12 @@ class Test extends AsyncResource {
390391
}
391392

392393
#abortHandler = () => {
393-
this.cancel(this.#outerSignal?.reason || new AbortError('The test was aborted'));
394+
const error = this.#outerSignal?.reason || new AbortError('The test was aborted');
395+
error.failureType = kAborted;
396+
this.#cancel(error);
394397
};
395398

396-
cancel(error) {
399+
#cancel(error) {
397400
if (this.endTime !== null) {
398401
return;
399402
}
@@ -470,7 +473,7 @@ class Test extends AsyncResource {
470473
return true;
471474
}
472475
if (this.#outerSignal?.aborted) {
473-
this.cancel(this.#outerSignal.reason || new AbortError('The test was aborted'));
476+
this.#abortHandler();
474477
return true;
475478
}
476479
}
@@ -563,7 +566,7 @@ class Test extends AsyncResource {
563566
try { await afterEach(); } catch { /* test is already failing, let's the error */ }
564567
if (isTestFailureError(err)) {
565568
if (err.failureType === kTestTimeoutFailure) {
566-
this.cancel(err);
569+
this.#cancel(err);
567570
} else {
568571
this.fail(err);
569572
}
@@ -577,9 +580,31 @@ class Test extends AsyncResource {
577580
this.postRun();
578581
}
579582

580-
postRun(pendingSubtestsError) {
581-
const counters = { __proto__: null, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
583+
countSubtest(counters) {
584+
// Check SKIP and TODO tests first, as those should not be counted as
585+
// failures.
586+
if (this.skipped) {
587+
counters.skipped++;
588+
} else if (this.isTodo) {
589+
counters.todo++;
590+
} else if (this.cancelled) {
591+
counters.cancelled++;
592+
} else if (!this.passed) {
593+
counters.failed++;
594+
} else {
595+
counters.passed++;
596+
}
597+
598+
if (!this.passed) {
599+
counters.totalFailed++;
600+
}
601+
counters.all++;
602+
}
582603

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

596621
if (!subtest.finished) {
597-
subtest.cancel(pendingSubtestsError);
622+
subtest.#cancel(pendingSubtestsError);
598623
subtest.postRun(pendingSubtestsError);
599624
}
600-
601-
// Check SKIP and TODO tests first, as those should not be counted as
602-
// failures.
603-
if (subtest.skipped) {
604-
counters.skipped++;
605-
} else if (subtest.isTodo) {
606-
counters.todo++;
607-
} else if (subtest.cancelled) {
608-
counters.cancelled++;
609-
} else if (!subtest.passed) {
610-
counters.failed++;
611-
} else {
612-
counters.passed++;
613-
}
614-
615-
if (!subtest.passed) {
616-
counters.totalFailed++;
617-
}
625+
subtest.countSubtest(counters);
618626
}
619627

620628
if ((this.passed || this.parent === null) && counters.totalFailed > 0) {
@@ -634,13 +642,13 @@ class Test extends AsyncResource {
634642
this.parent.processPendingSubtests();
635643
} else if (!this.reported) {
636644
this.reported = true;
637-
this.reporter.plan(this.nesting, kFilename, this.subtests.length);
645+
this.reporter.plan(this.nesting, kFilename, counters.all);
638646

639647
for (let i = 0; i < this.diagnostics.length; i++) {
640648
this.reporter.diagnostic(this.nesting, kFilename, this.diagnostics[i]);
641649
}
642650

643-
this.reporter.diagnostic(this.nesting, kFilename, `tests ${this.subtests.length}`);
651+
this.reporter.diagnostic(this.nesting, kFilename, `tests ${counters.all}`);
644652
this.reporter.diagnostic(this.nesting, kFilename, `pass ${counters.passed}`);
645653
this.reporter.diagnostic(this.nesting, kFilename, `fail ${counters.failed}`);
646654
this.reporter.diagnostic(this.nesting, kFilename, `cancelled ${counters.cancelled}`);
@@ -825,6 +833,8 @@ module.exports = {
825833
kCancelledByParent,
826834
kSubtestsFailed,
827835
kTestCodeFailure,
836+
kTestTimeoutFailure,
837+
kAborted,
828838
kUnwrapErrors,
829839
Suite,
830840
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)