Skip to content

Commit 5e8df72

Browse files
test_runner: expose expectFailure message
This change exposes the expectFailure message in the test runner and adds edge cases for expectFailure ambiguity. PR-URL: #61563 Reviewed-By: vassudanagunta Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 parent 37ff1ea commit 5e8df72

File tree

5 files changed

+406
-12
lines changed

5 files changed

+406
-12
lines changed

doc/api/test.md

Lines changed: 62 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,50 @@ 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 handled.
296+
297+
Each of the following tests fails _despite_ being flagged `expectFailure`
298+
because the failure does not match the specific **expected** failure.
299+
300+
```js
301+
it('fails because regex does not match', {
302+
expectFailure: /expected message/,
303+
}, () => {
304+
throw new Error('different message');
305+
});
306+
307+
it('fails because object matcher does not match', {
308+
expectFailure: { code: 'ERR_EXPECTED' },
309+
}, () => {
310+
const err = new Error('boom');
311+
err.code = 'ERR_ACTUAL';
312+
throw err;
313+
});
314+
```
315+
316+
To supply both a reason and specific error for `expectFailure`, use `{ label, match }`.
317+
318+
```js
319+
it('should fail with specific error and reason', {
320+
expectFailure: {
321+
label: 'reason for failure',
322+
match: /error message/,
323+
},
324+
}, () => {
325+
assert.strictEqual(doTheThing(), true);
326+
});
282327
```
283328

284329
`skip` and/or `todo` are mutually exclusive to `expectFailure`, and `skip` or `todo`
@@ -1684,6 +1729,18 @@ changes:
16841729
thread. If `false`, only one test runs at a time.
16851730
If unspecified, subtests inherit this value from their parent.
16861731
**Default:** `false`.
1732+
* `expectFailure` {boolean|string|RegExp|Function|Object|Error} If truthy, the
1733+
test is expected to fail. If a non-empty string is provided, that string is displayed
1734+
in the test results as the reason why the test is expected to fail. If a
1735+
[<RegExp>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp),
1736+
[<Function>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function),
1737+
[<Object>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object), or
1738+
[<Error>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error)
1739+
is provided directly (without wrapping in `{ match: … }`), the test passes
1740+
only if the thrown error matches, following the behavior of
1741+
[`assert.throws`][]. To provide both a reason and validation, pass an object
1742+
with `label` (string) and `match` (RegExp, Function, Object, or Error).
1743+
**Default:** `false`.
16871744
* `only` {boolean} If truthy, and the test context is configured to run
16881745
`only` tests, then this test will be run. Otherwise, the test is skipped.
16891746
**Default:** `false`.
@@ -4122,6 +4179,7 @@ Can be used to abort test subtasks when the test has been aborted.
41224179
[`NODE_V8_COVERAGE`]: cli.md#node_v8_coveragedir
41234180
[`SuiteContext`]: #class-suitecontext
41244181
[`TestContext`]: #class-testcontext
4182+
[`assert.throws`]: assert.md#assertthrowsfn-error-message
41254183
[`context.diagnostic`]: #contextdiagnosticmessage
41264184
[`context.skip`]: #contextskipmessage
41274185
[`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' ? ` ${tapEscape(expectFailure)}` : ''}`;
8181
}
8282

8383
line += '\n';

lib/internal/test_runner/test.js

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22
const {
3+
ArrayPrototypeEvery,
34
ArrayPrototypePush,
45
ArrayPrototypePushApply,
56
ArrayPrototypeShift,
@@ -13,6 +14,7 @@ const {
1314
MathMax,
1415
Number,
1516
NumberPrototypeToFixed,
17+
ObjectKeys,
1618
ObjectSeal,
1719
Promise,
1820
PromisePrototypeThen,
@@ -40,6 +42,7 @@ const {
4042
AbortError,
4143
codes: {
4244
ERR_INVALID_ARG_TYPE,
45+
ERR_INVALID_ARG_VALUE,
4346
ERR_TEST_FAILURE,
4447
},
4548
} = require('internal/errors');
@@ -56,7 +59,8 @@ const {
5659
once: runOnce,
5760
setOwnProperty,
5861
} = require('internal/util');
59-
const { isPromise } = require('internal/util/types');
62+
const assert = require('assert');
63+
const { isPromise, isRegExp } = require('internal/util/types');
6064
const {
6165
validateAbortSignal,
6266
validateFunction,
@@ -487,6 +491,39 @@ class SuiteContext {
487491
}
488492
}
489493

494+
function parseExpectFailure(expectFailure) {
495+
if (expectFailure === undefined || expectFailure === false) {
496+
return false;
497+
}
498+
499+
if (typeof expectFailure === 'string') {
500+
return { __proto__: null, label: expectFailure, match: undefined };
501+
}
502+
503+
if (typeof expectFailure === 'function' || isRegExp(expectFailure)) {
504+
return { __proto__: null, label: undefined, match: expectFailure };
505+
}
506+
507+
if (typeof expectFailure !== 'object') {
508+
return { __proto__: null, label: undefined, match: undefined };
509+
}
510+
511+
const keys = ObjectKeys(expectFailure);
512+
if (keys.length === 0) {
513+
throw new ERR_INVALID_ARG_VALUE('options.expectFailure', expectFailure, 'must not be an empty object');
514+
}
515+
516+
if (ArrayPrototypeEvery(keys, (k) => k === 'match' || k === 'label')) {
517+
return {
518+
__proto__: null,
519+
label: expectFailure.label,
520+
match: expectFailure.match,
521+
};
522+
}
523+
524+
return { __proto__: null, label: undefined, match: expectFailure };
525+
}
526+
490527
class Test extends AsyncResource {
491528
reportedType = 'test';
492529
abortController;
@@ -636,7 +673,7 @@ class Test extends AsyncResource {
636673
this.plan = null;
637674
this.expectedAssertions = plan;
638675
this.cancelled = false;
639-
this.expectFailure = expectFailure !== undefined && expectFailure !== false;
676+
this.expectFailure = parseExpectFailure(expectFailure) || this.parent?.expectFailure;
640677
this.skipped = skip !== undefined && skip !== false;
641678
this.isTodo = (todo !== undefined && todo !== false) || this.parent?.isTodo;
642679
this.startTime = null;
@@ -950,7 +987,30 @@ class Test extends AsyncResource {
950987
return;
951988
}
952989

953-
if (this.expectFailure === true) {
990+
if (this.expectFailure) {
991+
if (typeof this.expectFailure === 'object' &&
992+
this.expectFailure.match !== undefined) {
993+
const { match: validation } = this.expectFailure;
994+
try {
995+
const errorToCheck = (
996+
err?.code === 'ERR_TEST_FAILURE' &&
997+
err?.failureType === kTestCodeFailure &&
998+
err.cause
999+
) ?
1000+
err.cause :
1001+
err;
1002+
// eslint-disable-next-line no-restricted-syntax
1003+
assert.throws(() => { throw errorToCheck; }, validation);
1004+
} catch (e) {
1005+
this.passed = false;
1006+
this.error = new ERR_TEST_FAILURE(
1007+
'The test failed, but the error did not match the expected validation',
1008+
kTestCodeFailure,
1009+
);
1010+
this.error.cause = e;
1011+
return;
1012+
}
1013+
}
9541014
this.passed = true;
9551015
} else {
9561016
this.passed = false;
@@ -960,7 +1020,7 @@ class Test extends AsyncResource {
9601020
}
9611021

9621022
pass() {
963-
if (this.error == null && this.expectFailure === true && !this.skipped) {
1023+
if (this.error == null && this.expectFailure && !this.skipped) {
9641024
this.passed = false;
9651025
this.error = new ERR_TEST_FAILURE(
9661026
'test was expected to fail but passed',
@@ -972,6 +1032,20 @@ class Test extends AsyncResource {
9721032
return;
9731033
}
9741034

1035+
if (this.skipped || this.isTodo) {
1036+
this.passed = true;
1037+
return;
1038+
}
1039+
1040+
if (this.expectFailure) {
1041+
this.passed = false;
1042+
this.error = new ERR_TEST_FAILURE(
1043+
'Test passed but was expected to fail',
1044+
kTestCodeFailure,
1045+
);
1046+
return;
1047+
}
1048+
9751049
this.passed = true;
9761050
}
9771051

@@ -1361,7 +1435,10 @@ class Test extends AsyncResource {
13611435
} else if (this.isTodo) {
13621436
directive = this.reporter.getTodo(this.message);
13631437
} else if (this.expectFailure) {
1364-
directive = this.reporter.getXFail(this.expectFailure); // TODO(@JakobJingleheimer): support specifying failure
1438+
const message = typeof this.expectFailure === 'object' ?
1439+
this.expectFailure.label :
1440+
this.expectFailure;
1441+
directive = this.reporter.getXFail(message);
13651442
}
13661443

13671444
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)