Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test_runner: flatten TAP output when running using --test #46440

Merged
merged 2 commits into from
Feb 18, 2023
Merged

test_runner: flatten TAP output when running using --test #46440

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd1e3ba
test_runner: flatten TAP output when running using `--test`
MoLow Jan 31, 2023
a1f6e9e
CR
MoLow Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
87 changes: 54 additions & 33 deletions lib/internal/test_runner/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ const {
ArrayPrototypeSlice,
ArrayPrototypeSome,
ArrayPrototypeSort,
Number,
ObjectAssign,
ObjectKeys,
PromisePrototypeThen,
SafePromiseAll,
SafePromiseAllReturnVoid,
SafePromiseAllSettledReturnVoid,
SafeMap,
Expand All @@ -35,7 +36,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 @@ -55,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 @@ -133,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 @@ -141,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 @@ -153,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 @@ -174,29 +191,21 @@ 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) {
super.countSubtest
.call({ finished: true, skipped: skip, isTodo: todo, passed: pass, cancelled }, this.#counters);
MoLow marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -216,10 +225,24 @@ class FileTest extends Test {
this.reportStarted();
cjihrig marked this conversation as resolved.
Show resolved Hide resolved
this.#handleReportItem(ast);
}
countSubtest(counters) {
if (this.#counters.all === 0) {
return super.countSubtest(counters);
}
ArrayPrototypeForEach(ObjectKeys(counters), (key) => {
counters[key] += this.#counters[key];
});
}
reportStarted() {}
cjihrig marked this conversation as resolved.
Show resolved Hide resolved
report() {
this.reportStarted();
const skipReporting = this.#skipReporting();
if (!skipReporting) {
super.reportStarted();
MoLow marked this conversation as resolved.
Show resolved Hide resolved
}
ArrayPrototypeForEach(this.#buffer, (ast) => this.#handleReportItem(ast));
super.report();
if (!skipReporting) {
super.report();
}
}
}

Expand Down Expand Up @@ -274,16 +297,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const { availableParallelism } = 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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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