Skip to content

Commit

Permalink
test_runner: flatten TAP output when running using --test
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@MoLow @juanarbol
MoLow authored and juanarbol committed Mar 5, 2023
1 parent 391ff0d commit 5b3c606
Show file tree
Hide file tree
Showing 9 changed files with 726 additions and 697 deletions.
89 changes: 56 additions & 33 deletions lib/internal/test_runner/runner.js
View file
Expand Up @@ -8,9 +8,11 @@ const {
ArrayPrototypeSlice,
ArrayPrototypeSome,
ArrayPrototypeSort,
FunctionPrototypeCall,
Number,
ObjectAssign,
ObjectKeys,
PromisePrototypeThen,
SafePromiseAll,
SafePromiseAllReturnVoid,
SafePromiseAllSettledReturnVoid,
SafeMap,
Expand All @@ -35,7 +37,14 @@ const { validateArray, validateBoolean } = require('internal/validators');
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
const { kEmptyObject } = require('internal/util');
const { createTestTree } = require('internal/test_runner/harness');
const { kSubtestsFailed, Test } = require('internal/test_runner/test');
const {
kAborted,
kCancelledByParent,
kSubtestsFailed,
kTestCodeFailure,
kTestTimeoutFailure,
Test,
} = require('internal/test_runner/test');
const { TapParser } = require('internal/test_runner/tap_parser');
const { YAMLToJs } = require('internal/test_runner/yaml_to_js');
const { TokenKind } = require('internal/test_runner/tap_lexer');
Expand All @@ -54,6 +63,9 @@ const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
const kDiagnosticsFilterArgs = ['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];

const kCanceledTests = new SafeSet()
.add(kCancelledByParent).add(kAborted).add(kTestTimeoutFailure);

// TODO(cjihrig): Replace this with recursive readdir once it lands.
function processPath(path, testFiles, options) {
const stats = statSync(path);
Expand Down Expand Up @@ -132,6 +144,11 @@ function getRunArgs({ path, inspectPort }) {

class FileTest extends Test {
#buffer = [];
#counters = { __proto__: null, all: 0, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
failedSubtests = false;
#skipReporting() {
return this.#counters.all > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
}
#checkNestedComment({ comment }) {
const firstSpaceIndex = StringPrototypeIndexOf(comment, ' ');
if (firstSpaceIndex === -1) return false;
Expand All @@ -140,8 +157,6 @@ class FileTest extends Test {
ArrayPrototypeIncludes(kDiagnosticsFilterArgs, StringPrototypeSlice(comment, 0, firstSpaceIndex));
}
#handleReportItem({ kind, node, comments, nesting = 0 }) {
nesting += 1;

if (comments) {
ArrayPrototypeForEach(comments, (comment) => this.reporter.diagnostic(nesting, this.name, comment));
}
Expand All @@ -152,17 +167,20 @@ class FileTest extends Test {
break;

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

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

case TokenKind.TAP_TEST_POINT:
// eslint-disable-next-line no-case-declarations
case TokenKind.TAP_TEST_POINT: {

const { todo, skip, pass } = node.status;
// eslint-disable-next-line no-case-declarations

let directive;

if (skip) {
Expand All @@ -173,29 +191,22 @@ class FileTest extends Test {
directive = kEmptyObject;
}

if (pass) {
this.reporter.ok(
nesting,
this.name,
node.id,
node.description,
YAMLToJs(node.diagnostics),
directive
);
} else {
this.reporter.fail(
nesting,
this.name,
node.id,
node.description,
YAMLToJs(node.diagnostics),
directive
);
const diagnostics = YAMLToJs(node.diagnostics);
const cancelled = kCanceledTests.has(diagnostics.error?.failureType);
const testNumber = nesting === 0 ? (Number(node.id) + this.testNumber - 1) : node.id;
const method = pass ? 'ok' : 'fail';
this.reporter[method](nesting, this.name, testNumber, node.description, diagnostics, directive);
if (nesting === 0) {
FunctionPrototypeCall(super.countSubtest,
{ finished: true, skipped: skip, isTodo: todo, passed: pass, cancelled },
this.#counters);
this.failedSubtests ||= !pass;
}
break;

}
case TokenKind.COMMENT:
if (nesting === 1 && this.#checkNestedComment(node)) {
if (nesting === 0 && this.#checkNestedComment(node)) {
// Ignore file top level diagnostics
break;
}
Expand All @@ -215,10 +226,24 @@ class FileTest extends Test {
this.reportStarted();
this.#handleReportItem(ast);
}
countSubtest(counters) {
if (this.#counters.all === 0) {
return super.countSubtest(counters);
}
ArrayPrototypeForEach(ObjectKeys(counters), (key) => {
counters[key] += this.#counters[key];
});
}
reportStarted() {}
report() {
this.reportStarted();
const skipReporting = this.#skipReporting();
if (!skipReporting) {
super.reportStarted();
}
ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
super.report();
if (!skipReporting) {
super.report();
}
}
}

Expand Down Expand Up @@ -273,16 +298,14 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
subtest.addToReport(ast);
});

const { 0: { 0: code, 1: signal } } = await SafePromiseAll([
once(child, 'exit', { signal: t.signal }),
child.stdout.toArray({ signal: t.signal }),
]);
const { 0: code, 1: signal } = await once(child, 'exit', { signal: t.signal });

runningProcesses.delete(path);
runningSubtests.delete(path);
if (code !== 0 || signal !== null) {
if (!err) {
err = ObjectAssign(new ERR_TEST_FAILURE('test failed', kSubtestsFailed), {
const failureType = subtest.failedSubtests ? kSubtestsFailed : kTestCodeFailure;
err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
__proto__: null,
exitCode: code,
signal: signal,
Expand Down
64 changes: 37 additions & 27 deletions lib/internal/test_runner/test.js
View file
Expand Up @@ -57,6 +57,7 @@ const { cpus } = require('os');
const { bigint: hrtime } = process.hrtime;
const kCallbackAndPromisePresent = 'callbackAndPromisePresent';
const kCancelledByParent = 'cancelledByParent';
const kAborted = 'testAborted';
const kParentAlreadyFinished = 'parentAlreadyFinished';
const kSubtestsFailed = 'subtestsFailed';
const kTestCodeFailure = 'testCodeFailure';
Expand Down Expand Up @@ -390,10 +391,12 @@ class Test extends AsyncResource {
}

#abortHandler = () => {
this.cancel(this.#outerSignal?.reason || new AbortError('The test was aborted'));
const error = this.#outerSignal?.reason || new AbortError('The test was aborted');
error.failureType = kAborted;
this.#cancel(error);
};

cancel(error) {
#cancel(error) {
if (this.endTime !== null) {
return;
}
Expand Down Expand Up @@ -470,7 +473,7 @@ class Test extends AsyncResource {
return true;
}
if (this.#outerSignal?.aborted) {
this.cancel(this.#outerSignal.reason || new AbortError('The test was aborted'));
this.#abortHandler();
return true;
}
}
Expand Down Expand Up @@ -563,7 +566,7 @@ class Test extends AsyncResource {
try { await afterEach(); } catch { /* test is already failing, let's the error */ }
if (isTestFailureError(err)) {
if (err.failureType === kTestTimeoutFailure) {
this.cancel(err);
this.#cancel(err);
} else {
this.fail(err);
}
Expand All @@ -577,9 +580,31 @@ class Test extends AsyncResource {
this.postRun();
}

postRun(pendingSubtestsError) {
const counters = { __proto__: null, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 };
countSubtest(counters) {
// Check SKIP and TODO tests first, as those should not be counted as
// failures.
if (this.skipped) {
counters.skipped++;
} else if (this.isTodo) {
counters.todo++;
} else if (this.cancelled) {
counters.cancelled++;
} else if (!this.passed) {
counters.failed++;
} else {
counters.passed++;
}

if (!this.passed) {
counters.totalFailed++;
}
counters.all++;
}

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

if (!subtest.finished) {
subtest.cancel(pendingSubtestsError);
subtest.#cancel(pendingSubtestsError);
subtest.postRun(pendingSubtestsError);
}

// Check SKIP and TODO tests first, as those should not be counted as
// failures.
if (subtest.skipped) {
counters.skipped++;
} else if (subtest.isTodo) {
counters.todo++;
} else if (subtest.cancelled) {
counters.cancelled++;
} else if (!subtest.passed) {
counters.failed++;
} else {
counters.passed++;
}

if (!subtest.passed) {
counters.totalFailed++;
}
subtest.countSubtest(counters);
}

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

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

this.reporter.diagnostic(this.nesting, kFilename, `tests ${this.subtests.length}`);
this.reporter.diagnostic(this.nesting, kFilename, `tests ${counters.all}`);
this.reporter.diagnostic(this.nesting, kFilename, `pass ${counters.passed}`);
this.reporter.diagnostic(this.nesting, kFilename, `fail ${counters.failed}`);
this.reporter.diagnostic(this.nesting, kFilename, `cancelled ${counters.cancelled}`);
Expand Down Expand Up @@ -825,6 +833,8 @@ module.exports = {
kCancelledByParent,
kSubtestsFailed,
kTestCodeFailure,
kTestTimeoutFailure,
kAborted,
kUnwrapErrors,
Suite,
Test,
Expand Down
10 changes: 10 additions & 0 deletions test/message/test_runner_abort.out
View file
Expand Up @@ -40,6 +40,7 @@ TAP version 13
not ok 7 - not ok 3
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand All @@ -58,6 +59,7 @@ TAP version 13
not ok 8 - not ok 4
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand All @@ -76,6 +78,7 @@ TAP version 13
not ok 9 - not ok 5
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand All @@ -94,6 +97,7 @@ TAP version 13
not ok 1 - promise timeout signal
---
duration_ms: *
failureType: 'testAborted'
error: 'The operation was aborted due to timeout'
code: 23
stack: |-
Expand All @@ -106,6 +110,7 @@ not ok 1 - promise timeout signal
not ok 2 - promise abort signal
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand Down Expand Up @@ -160,6 +165,7 @@ not ok 2 - promise abort signal
not ok 7 - not ok 3
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand All @@ -178,6 +184,7 @@ not ok 2 - promise abort signal
not ok 8 - not ok 4
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand All @@ -196,6 +203,7 @@ not ok 2 - promise abort signal
not ok 9 - not ok 5
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand All @@ -214,6 +222,7 @@ not ok 2 - promise abort signal
not ok 3 - callback timeout signal
---
duration_ms: *
failureType: 'testAborted'
error: 'The operation was aborted due to timeout'
code: 23
stack: |-
Expand All @@ -226,6 +235,7 @@ not ok 3 - callback timeout signal
not ok 4 - callback abort signal
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand Down
2 changes: 2 additions & 0 deletions test/message/test_runner_abort_suite.out
View file
Expand Up @@ -64,6 +64,7 @@ TAP version 13
not ok 1 - describe timeout signal
---
duration_ms: *
failureType: 'testAborted'
error: 'The operation was aborted due to timeout'
code: 23
stack: |-
Expand All @@ -76,6 +77,7 @@ not ok 1 - describe timeout signal
not ok 2 - describe abort signal
---
duration_ms: *
failureType: 'testAborted'
error: 'This operation was aborted'
code: 20
stack: |-
Expand Down

0 comments on commit 5b3c606

Please sign in to comment.