Skip to content

Commit 09def7f

Browse files
committed
test_runner: support expectFailure matchers and labels
Normalize expectFailure values to support string labels, direct matchers, and { label, match } objects while validating errors via assert.throws. Update TAP reporting and tests, including function matcher coverage and unexpected-pass behavior, and clarify docs for direct matcher usage.
1 parent 3819c7f commit 09def7f

File tree

5 files changed

+310
-11
lines changed

5 files changed

+310
-11
lines changed

doc/api/test.md

Lines changed: 57 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()` fail 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,45 @@ 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+
// To supply both a reason and specific error for `expectFailure`, use { label, match }.
314+
it('should fail with specific error and reason', {
315+
expectFailure: {
316+
label: 'reason for failure',
317+
match: /error message/,
318+
},
319+
}, () => {
320+
assert.strictEqual(doTheThing(), true);
321+
});
282322
```
283323

284324
`skip` and/or `todo` are mutually exclusive to `expectFailure`, and `skip` or `todo`
@@ -1684,6 +1724,18 @@ changes:
16841724
thread. If `false`, only one test runs at a time.
16851725
If unspecified, subtests inherit this value from their parent.
16861726
**Default:** `false`.
1727+
* `expectFailure` {boolean|string|RegExp|Function|Object|Error} If truthy, the
1728+
test is expected to fail. If a string is provided, that string is displayed
1729+
in the test results as the reason why the test is expected to fail. If a
1730+
[<RegExp>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp) |
1731+
[<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function) |
1732+
[<Object>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) |
1733+
[<Error>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error)
1734+
is provided directly (without wrapping in `{ match: ... }`), the test passes
1735+
only if the thrown error matches that value. Matching behavior follows
1736+
[`assert.throws`][]. To provide both a reason and validation, pass an object
1737+
with `label` (string) and `match` (RegExp, Function, Object, or Error).
1738+
**Default:** `false`.
16871739
* `only` {boolean} If truthy, and the test context is configured to run
16881740
`only` tests, then this test will be run. Otherwise, the test is skipped.
16891741
**Default:** `false`.
@@ -4122,6 +4174,7 @@ Can be used to abort test subtasks when the test has been aborted.
41224174
[`NODE_V8_COVERAGE`]: cli.md#node_v8_coveragedir
41234175
[`SuiteContext`]: #class-suitecontext
41244176
[`TestContext`]: #class-testcontext
4177+
[`assert.throws`]: assert.md#assertthrowsfn-error-message
41254178
[`context.diagnostic`]: #contextdiagnosticmessage
41264179
[`context.skip`]: #contextskipmessage
41274180
[`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: 77 additions & 4 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);
640676
this.skipped = skip !== undefined && skip !== false;
641677
this.isTodo = (todo !== undefined && todo !== false) || this.parent?.isTodo;
642678
this.startTime = null;
@@ -948,7 +984,27 @@ class Test extends AsyncResource {
948984
return;
949985
}
950986

951-
if (this.expectFailure === true) {
987+
if (this.expectFailure) {
988+
if (typeof this.expectFailure === 'object' &&
989+
this.expectFailure.match !== undefined) {
990+
const { match: validation } = this.expectFailure;
991+
try {
992+
const { throws } = assert;
993+
const errorToCheck = (err?.code === 'ERR_TEST_FAILURE' &&
994+
err?.failureType === kTestCodeFailure &&
995+
err.cause) ?
996+
err.cause : err;
997+
throws(() => { throw errorToCheck; }, validation);
998+
} catch (e) {
999+
this.passed = false;
1000+
this.error = new ERR_TEST_FAILURE(
1001+
'The test failed, but the error did not match the expected validation',
1002+
kTestCodeFailure,
1003+
);
1004+
this.error.cause = e;
1005+
return;
1006+
}
1007+
}
9521008
this.passed = true;
9531009
} else {
9541010
this.passed = false;
@@ -970,6 +1026,20 @@ class Test extends AsyncResource {
9701026
return;
9711027
}
9721028

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

@@ -1359,7 +1429,10 @@ class Test extends AsyncResource {
13591429
} else if (this.isTodo) {
13601430
directive = this.reporter.getTodo(this.message);
13611431
} else if (this.expectFailure) {
1362-
directive = this.reporter.getXFail(this.expectFailure); // TODO(@JakobJingleheimer): support specifying failure
1432+
const message = typeof this.expectFailure === 'object' ?
1433+
this.expectFailure.label :
1434+
this.expectFailure;
1435+
directive = this.reporter.getXFail(message);
13631436
}
13641437

13651438
if (this.reportedType) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ if (!process.env.NODE_TEST_CONTEXT) {
1010
stream.on('test:fail', common.mustCall((event) => {
1111
assert.strictEqual(event.expectFailure, true);
1212
assert.strictEqual(event.details.error.code, 'ERR_TEST_FAILURE');
13-
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.failureType, 'testCodeFailure');
14+
assert.strictEqual(event.details.error.cause, 'Test passed but was expected to fail');
1515
}, 1));
1616
} else {
1717
test('passing test', { expectFailure: true }, () => {});

0 commit comments

Comments
 (0)