Skip to content

Commit 01cf2c4

Browse files
committed
test_runner: enhance expectFailure option
Normalize expectFailure values to support string labels, direct matchers, and { label, match } objects while validating errors via assert.throws. Allow tests to inherit the 'expectFailure' option from their parent suite. Update TAP reporting and tests, including function matcher coverage and unexpected-pass behavior, and clarify docs for direct matcher usage. Resolves: #61570
1 parent 3d30c30 commit 01cf2c4

File tree

5 files changed

+338
-12
lines changed

5 files changed

+338
-12
lines changed

doc/api/test.md

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -265,11 +265,12 @@ added:
265265
- v25.5.0
266266
-->
267267

268-
This flips the pass/fail reporting for a specific test or suite: A flagged test/test-case must throw
269-
in order to "pass"; a test/test-case that does not throw, fails.
268+
This flips the pass/fail reporting for a specific test or suite: a flagged test
269+
case must throw in order to pass, and a flagged test case that does not throw
270+
fails.
270271

271-
In the following, `doTheThing()` returns _currently_ `false` (`false` does not equal `true`, causing
272-
`strictEqual` to throw, so the test-case passes).
272+
In each of the following, `doTheThing()` fails to return `true`, but since the
273+
tests are flagged `expectFailure`, they pass.
273274

274275
```js
275276
it.expectFailure('should do the thing', () => {
@@ -279,6 +280,48 @@ it.expectFailure('should do the thing', () => {
279280
it('should do the thing', { expectFailure: true }, () => {
280281
assert.strictEqual(doTheThing(), true);
281282
});
283+
284+
it('should do the thing', { expectFailure: 'feature not implemented' }, () => {
285+
assert.strictEqual(doTheThing(), true);
286+
});
287+
```
288+
289+
If the value of `expectFailure` is a
290+
[<RegExp>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp) |
291+
[<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function) |
292+
[<Object>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) |
293+
[<Error>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error),
294+
the tests will pass only if they throw a matching value.
295+
See [`assert.throws`][] for how each value type is interpreted.
296+
297+
Each of the following tests fails _despite_ being flagged `expectFailure`
298+
because the failure was not the expected one.
299+
300+
```js
301+
it('fails because regex does not match', { expectFailure: /expected message/ }, () => {
302+
throw new Error('different message');
303+
});
304+
305+
it('fails because object matcher does not match', {
306+
expectFailure: { code: 'ERR_EXPECTED' },
307+
}, () => {
308+
const err = new Error('boom');
309+
err.code = 'ERR_ACTUAL';
310+
throw err;
311+
});
312+
```
313+
314+
To supply both a reason and specific error for `expectFailure`, use `{ label, match }`.
315+
316+
```js
317+
it('should fail with specific error and reason', {
318+
expectFailure: {
319+
label: 'reason for failure',
320+
match: /error message/,
321+
},
322+
}, () => {
323+
assert.strictEqual(doTheThing(), true);
324+
});
282325
```
283326

284327
`skip` and/or `todo` are mutually exclusive to `expectFailure`, and `skip` or `todo`
@@ -1684,6 +1727,18 @@ changes:
16841727
thread. If `false`, only one test runs at a time.
16851728
If unspecified, subtests inherit this value from their parent.
16861729
**Default:** `false`.
1730+
* `expectFailure` {boolean|string|RegExp|Function|Object|Error} If truthy, the
1731+
test is expected to fail. If a string is provided, that string is displayed
1732+
in the test results as the reason why the test is expected to fail. If a
1733+
[<RegExp>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp) |
1734+
[<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function) |
1735+
[<Object>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) |
1736+
[<Error>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error)
1737+
is provided directly (without wrapping in `{ match: ... }`), the test passes
1738+
only if the thrown error matches that value. Matching behavior follows
1739+
[`assert.throws`][]. To provide both a reason and validation, pass an object
1740+
with `label` (string) and `match` (RegExp, Function, Object, or Error).
1741+
**Default:** `false`.
16871742
* `only` {boolean} If truthy, and the test context is configured to run
16881743
`only` tests, then this test will be run. Otherwise, the test is skipped.
16891744
**Default:** `false`.
@@ -4122,6 +4177,7 @@ Can be used to abort test subtasks when the test has been aborted.
41224177
[`NODE_V8_COVERAGE`]: cli.md#node_v8_coveragedir
41234178
[`SuiteContext`]: #class-suitecontext
41244179
[`TestContext`]: #class-testcontext
4180+
[`assert.throws`]: assert.md#assertthrowsfn-error-message
41254181
[`context.diagnostic`]: #contextdiagnosticmessage
41264182
[`context.skip`]: #contextskipmessage
41274183
[`context.todo`]: #contexttodomessage

lib/internal/test_runner/reporter/tap.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ function reportTest(nesting, testNumber, status, name, skip, todo, expectFailure
7777
} else if (todo !== undefined) {
7878
line += ` # TODO${typeof todo === 'string' && todo.length ? ` ${tapEscape(todo)}` : ''}`;
7979
} else if (expectFailure !== undefined) {
80-
line += ' # EXPECTED FAILURE';
80+
line += ` # EXPECTED FAILURE${typeof expectFailure === 'string' && expectFailure.length ? ` ${tapEscape(expectFailure)}` : ''}`;
8181
}
8282

8383
line += '\n';

lib/internal/test_runner/test.js

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const {
1313
MathMax,
1414
Number,
1515
NumberPrototypeToFixed,
16+
ObjectKeys,
1617
ObjectSeal,
1718
Promise,
1819
PromisePrototypeThen,
@@ -40,6 +41,7 @@ const {
4041
AbortError,
4142
codes: {
4243
ERR_INVALID_ARG_TYPE,
44+
ERR_INVALID_ARG_VALUE,
4345
ERR_TEST_FAILURE,
4446
},
4547
} = require('internal/errors');
@@ -56,7 +58,8 @@ const {
5658
once: runOnce,
5759
setOwnProperty,
5860
} = require('internal/util');
59-
const { isPromise } = require('internal/util/types');
61+
const assert = require('assert');
62+
const { isPromise, isRegExp } = require('internal/util/types');
6063
const {
6164
validateAbortSignal,
6265
validateFunction,
@@ -487,6 +490,39 @@ class SuiteContext {
487490
}
488491
}
489492

493+
function parseExpectFailure(expectFailure) {
494+
if (expectFailure === undefined || expectFailure === false) {
495+
return false;
496+
}
497+
498+
if (typeof expectFailure === 'string') {
499+
return { __proto__: null, label: expectFailure, match: undefined };
500+
}
501+
502+
if (typeof expectFailure === 'function' || isRegExp(expectFailure)) {
503+
return { __proto__: null, label: undefined, match: expectFailure };
504+
}
505+
506+
if (typeof expectFailure !== 'object') {
507+
return { __proto__: null, label: undefined, match: undefined };
508+
}
509+
510+
const keys = ObjectKeys(expectFailure);
511+
if (keys.length === 0) {
512+
throw new ERR_INVALID_ARG_VALUE('options.expectFailure', expectFailure, 'must not be an empty object');
513+
}
514+
515+
if (keys.every((k) => k === 'match' || k === 'label')) {
516+
return {
517+
__proto__: null,
518+
label: expectFailure.label,
519+
match: expectFailure.match,
520+
};
521+
}
522+
523+
return { __proto__: null, label: undefined, match: expectFailure };
524+
}
525+
490526
class Test extends AsyncResource {
491527
reportedType = 'test';
492528
abortController;
@@ -636,7 +672,7 @@ class Test extends AsyncResource {
636672
this.plan = null;
637673
this.expectedAssertions = plan;
638674
this.cancelled = false;
639-
this.expectFailure = expectFailure !== undefined && expectFailure !== false;
675+
this.expectFailure = parseExpectFailure(expectFailure) || this.parent?.expectFailure;
640676
this.skipped = skip !== undefined && skip !== false;
641677
this.isTodo = (todo !== undefined && todo !== false) || this.parent?.isTodo;
642678
this.startTime = null;
@@ -950,7 +986,27 @@ class Test extends AsyncResource {
950986
return;
951987
}
952988

953-
if (this.expectFailure === true) {
989+
if (this.expectFailure) {
990+
if (typeof this.expectFailure === 'object' &&
991+
this.expectFailure.match !== undefined) {
992+
const { match: validation } = this.expectFailure;
993+
try {
994+
const { throws } = assert;
995+
const errorToCheck = (err?.code === 'ERR_TEST_FAILURE' &&
996+
err?.failureType === kTestCodeFailure &&
997+
err.cause) ?
998+
err.cause : err;
999+
throws(() => { throw errorToCheck; }, validation);
1000+
} catch (e) {
1001+
this.passed = false;
1002+
this.error = new ERR_TEST_FAILURE(
1003+
'The test failed, but the error did not match the expected validation',
1004+
kTestCodeFailure,
1005+
);
1006+
this.error.cause = e;
1007+
return;
1008+
}
1009+
}
9541010
this.passed = true;
9551011
} else {
9561012
this.passed = false;
@@ -960,7 +1016,7 @@ class Test extends AsyncResource {
9601016
}
9611017

9621018
pass() {
963-
if (this.error == null && this.expectFailure === true && !this.skipped) {
1019+
if (this.error == null && this.expectFailure && !this.skipped) {
9641020
this.passed = false;
9651021
this.error = new ERR_TEST_FAILURE(
9661022
'test was expected to fail but passed',
@@ -972,6 +1028,20 @@ class Test extends AsyncResource {
9721028
return;
9731029
}
9741030

1031+
if (this.skipped || this.isTodo) {
1032+
this.passed = true;
1033+
return;
1034+
}
1035+
1036+
if (this.expectFailure) {
1037+
this.passed = false;
1038+
this.error = new ERR_TEST_FAILURE(
1039+
'Test passed but was expected to fail',
1040+
kTestCodeFailure,
1041+
);
1042+
return;
1043+
}
1044+
9751045
this.passed = true;
9761046
}
9771047

@@ -1361,7 +1431,10 @@ class Test extends AsyncResource {
13611431
} else if (this.isTodo) {
13621432
directive = this.reporter.getTodo(this.message);
13631433
} else if (this.expectFailure) {
1364-
directive = this.reporter.getXFail(this.expectFailure); // TODO(@JakobJingleheimer): support specifying failure
1434+
const message = typeof this.expectFailure === 'object' ?
1435+
this.expectFailure.label :
1436+
this.expectFailure;
1437+
directive = this.reporter.getXFail(message);
13651438
}
13661439

13671440
if (this.reportedType) {

test/parallel/test-runner-expect-error-but-pass.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@ if (!process.env.NODE_TEST_CONTEXT) {
88

99
stream.on('test:pass', common.mustNotCall());
1010
stream.on('test:fail', common.mustCall((event) => {
11-
assert.strictEqual(event.expectFailure, true);
1211
assert.strictEqual(event.details.error.code, 'ERR_TEST_FAILURE');
1312
assert.strictEqual(event.details.error.failureType, 'expectedFailure');
14-
assert.strictEqual(event.details.error.cause, 'test was expected to fail but passed');
13+
assert.strictEqual(event.details.error.message, 'test was expected to fail but passed');
1514
}, 1));
1615
} else {
1716
test('passing test', { expectFailure: true }, () => {});

0 commit comments

Comments
 (0)