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.

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

test_runner: bail #48919

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: tap skip bailed test
  • Loading branch information
marco-ippolito committed Aug 29, 2023
commit faf412b93006ca0dc88cfd7f00c35d594377aa1c
2 changes: 1 addition & 1 deletion doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1514,7 +1514,7 @@ added:
- REPLACEME
-->

marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
Specifies the bailout behavior of the test runner when running tests.
Instructs the test runner to bail out if a test failure occurs.
See the documentation on [test bailout][] for more details.

### `--test-name-pattern`
Expand Down
7 changes: 0 additions & 7 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2721,13 +2721,6 @@ the token causing the error is available via the `cause` property.

This error represents a failed TAP validation.

<a id="ERR_TEST_BAILOUT"></a>

### `ERR_TEST_BAILOUT`

This error represents a test that has bailed out after failure.
This error occurs only when the flag `--test-bail` is passed.

<a id="ERR_TEST_FAILURE"></a>

### `ERR_TEST_FAILURE`
Expand Down
2 changes: 1 addition & 1 deletion doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ test('mocks setTimeout to be executed synchronously without having to actually w
});
```

## Test bail
## Bailing out

<!-- YAML
added:
Expand Down
4 changes: 0 additions & 4 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1615,10 +1615,6 @@ E('ERR_TAP_VALIDATION_ERROR', function(errorMsg) {
hideInternalStackFrames(this);
return errorMsg;
}, Error);
E('ERR_TEST_BAILOUT', function(errorMsg) {
hideInternalStackFrames(this);
return errorMsg;
}, Error);
E('ERR_TEST_FAILURE', function(error, failureType) {
hideInternalStackFrames(this);
assert(typeof failureType === 'string' || typeof failureType === 'symbol',
Expand Down
1 change: 1 addition & 0 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ function setup(root) {
topLevel: 0,
suites: 0,
},
bailedOut: false,
shouldColorizeTestFiles: false,
};
root.startTime = hrtime();
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const assert = require('assert');
const Transform = require('internal/streams/transform');
const { inspectWithNoCustomRetry } = require('internal/errors');
const { green, blue, red, white, gray, shouldColorize } = require('internal/util/colors');
const { kSubtestsFailed } = require('internal/test_runner/test');
const { kSubtestsFailed, kBailedOut } = require('internal/test_runner/test');
const { getCoverageReport } = require('internal/test_runner/utils');
const { relative } = require('path');

Expand Down Expand Up @@ -68,7 +68,7 @@ class SpecReporter extends Transform {
}
#formatTestReport(type, data, prefix = '', indent = '', hasChildren = false) {
// Do not report if test has bailed out
if (data.details?.error?.code === 'ERR_TEST_BAILOUT') {
if (data.details?.error?.failureType === kBailedOut) {
return '';
}
let color = colors[type] ?? white;
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/test_runner/reporter/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const {
const { inspectWithNoCustomRetry } = require('internal/errors');
const { isError, kEmptyObject } = require('internal/util');
const { getCoverageReport } = require('internal/test_runner/utils');
const { kBailedOut } = require('internal/test_runner/test');
const kDefaultIndent = ' '; // 4 spaces
const kFrameStartRegExp = /^ {4}at /;
const kLineBreakRegExp = /\n|\r\n/;
Expand All @@ -32,6 +33,8 @@ async function * tapReporter(source) {
for await (const { type, data } of source) {
switch (type) {
case 'test:fail': {
// If test bailed out dont report
if (data.details?.error?.failureType === kBailedOut) break;
yield reportTest(data.nesting, data.testNumber, 'not ok', data.name, data.skip, data.todo);
const location = `${data.file}:${data.line}:${data.column}`;
yield reportDetails(data.nesting, data.details, location);
Expand Down
16 changes: 8 additions & 8 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const {
codes: {
ERR_INVALID_ARG_TYPE,
ERR_TEST_FAILURE,
ERR_TEST_BAILOUT,
},
AbortError,
} = require('internal/errors');
Expand Down Expand Up @@ -67,6 +66,7 @@ const kParentAlreadyFinished = 'parentAlreadyFinished';
const kSubtestsFailed = 'subtestsFailed';
const kTestCodeFailure = 'testCodeFailure';
const kTestTimeoutFailure = 'testTimeoutFailure';
const kBailedOut = 'testBailedOut';
const kHookFailure = 'hookFailed';
const kDefaultTimeout = null;
const noop = FunctionPrototype;
Expand All @@ -78,7 +78,6 @@ const kUnwrapErrors = new SafeSet()
.add('uncaughtException').add('unhandledRejection');
const { testNamePatterns, testOnlyFlag, bail } = parseCommandLine();
let kResistStopPropagation;
let bailedOut = false;

function stopTest(timeout, signal) {
const deferred = createDeferredPromise();
Expand Down Expand Up @@ -471,15 +470,15 @@ class Test extends AsyncResource {
if (this.endTime !== null) {
return;
}

const unknownError = bailedOut ? new ERR_TEST_BAILOUT('test bailed out') : new ERR_TEST_FAILURE(
const cancelledError = this.root.harness.bailedOut ? new ERR_TEST_FAILURE(
'test bailed out',
kBailedOut,
) : new ERR_TEST_FAILURE(
'test did not finish before its parent and was cancelled',
kCancelledByParent,
);

this.fail(error ||
unknownError,
);
this.fail(error || cancelledError);
this.startTime = this.startTime || this.endTime; // If a test was canceled before it was started, e.g inside a hook
this.cancelled = true;
this.abortController.abort();
Expand All @@ -504,7 +503,7 @@ class Test extends AsyncResource {
this.endTime = hrtime();
this.passed = false;
this.error = err;
bailedOut = bail;
this.root.harness.bailedOut = bail;
}

pass() {
Expand Down Expand Up @@ -980,6 +979,7 @@ module.exports = {
kTestCodeFailure,
kTestTimeoutFailure,
kAborted,
kBailedOut,
kUnwrapErrors,
Suite,
Test,
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
&EnvironmentOptions::heap_prof_interval);
#endif // HAVE_INSPECTOR
AddOption("--test-bail",
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
"stop test execution when given number of tests have failed",
"stop test execution when a test has failed",
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
&EnvironmentOptions::test_bail);
AddOption("--max-http-header-size",
"set the maximum size of HTTP headers (default: 16384 (16KB))",
Expand Down
27 changes: 24 additions & 3 deletions test/parallel/test-runner-bail.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@ const assert = require('node:assert');
const testFile = fixtures.path('test-runner/bail/bail.js');
tmpdir.refresh();

describe('node:test bail', () => {
describe('node:test bail tap', () => {
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
it('should exit at first failure', async () => {
const child = spawnSync(process.execPath, ['--test', '--test-bail', testFile]);
assert.strictEqual(child.stderr.toString(), '');
assert.match(child.stdout.toString(), /ok 1 - first/);
assert.match(child.stdout.toString(), /not ok 2 - second/);
assert.match(child.stdout.toString(), /ok 3 - third/);
assert.doesNotMatch(child.stdout.toString(), /ok 3 - third/);
assert.match(child.stdout.toString(), /not ok 1 - nested/);
assert.doesNotMatch(child.stdout.toString(), /ok 1 - ok forth/);
assert.doesNotMatch(child.stdout.toString(), /not ok 2 - fifth/);
assert.doesNotMatch(child.stdout.toString(), /Subtest: top level/);
});

it('should exit not exit if bail isnt set', async () => {
Expand All @@ -35,3 +34,25 @@ describe('node:test bail', () => {
assert.match(child.stdout.toString(), /not ok 2 - top level/);
});
});

describe('node:test bail spec', () => {
it('should exit at first failure', async () => {
const child = spawnSync(process.execPath, ['--test', '--test-bail', '--test-reporter=spec', testFile]);
assert.strictEqual(child.stderr.toString(), '');
assert.match(child.stdout.toString(), /✔ first/);
assert.match(child.stdout.toString(), /✖ second/);
assert.doesNotMatch(child.stdout.toString(), /✖ third/);
assert.doesNotMatch(child.stdout.toString(), /✔ forth/);
assert.doesNotMatch(child.stdout.toString(), /✖ fifth/);
});

it('should exit not exit if bail isnt set', async () => {
const child = spawnSync(process.execPath, ['--test', '--test-reporter=spec', testFile]);
assert.strictEqual(child.stderr.toString(), '');
assert.match(child.stdout.toString(), /✔ first/);
assert.match(child.stdout.toString(), /✖ second/);
assert.match(child.stdout.toString(), /✖ third/);
assert.match(child.stdout.toString(), /✔ forth/);
assert.match(child.stdout.toString(), /✖ fifth/);
});
});