FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: expose expectFailure message · nodejs/node@fce2930 · GitHub

/ node Public

Commit fce2930

Browse files
Han5991
authored and
aduh95
committed
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 Fixes: #61570 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Jacob Smith <jacob@frende.me>
1 parent ea2df2a commit fce2930

5 files changed

Lines changed: 405 additions & 12 deletions

File tree

‎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`.
@@ -4181,6 +4238,7 @@ Can be used to abort test subtasks when the test has been aborted.
41814238
[`NODE_V8_COVERAGE`]: cli.md#node_v8_coveragedir
41824239
[`SuiteContext`]: #class-suitecontext
41834240
[`TestContext`]: #class-testcontext
4241+
[`assert.throws`]: assert.md#assertthrowsfn-error-message
41844242
[`context.diagnostic`]: #contextdiagnosticmessage
41854243
[`context.skip`]: #contextskipmessage
41864244
[`context.todo`]: #contexttodomessage

‎lib/internal/test_runner/reporter/tap.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ function reportTest(nesting, testNumber, status, name, skip, todo, expectFailure
8787
} else if (todo !== undefined) {
8888
line += ` # TODO${typeof todo === 'string' && todo.length ? ` ${tapEscape(todo)}` : ''}`;
8989
} else if (expectFailure !== undefined) {
90-
line += ' # EXPECTED FAILURE';
90+
line += ` # EXPECTED FAILURE${typeof expectFailure === 'string' ? ` ${tapEscape(expectFailure)}` : ''}`;
9191
}
9292

9393
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,
@@ -492,6 +496,39 @@ class SuiteContext {
492496
}
493497
}
494498

499+
function parseExpectFailure(expectFailure) {
500+
if (expectFailure === undefined || expectFailure === false) {
501+
return false;
502+
}
503+
504+
if (typeof expectFailure === 'string') {
505+
return { __proto__: null, label: expectFailure, match: undefined };
506+
}
507+
508+
if (typeof expectFailure === 'function' || isRegExp(expectFailure)) {
509+
return { __proto__: null, label: undefined, match: expectFailure };
510+
}
511+
512+
if (typeof expectFailure !== 'object') {
513+
return { __proto__: null, label: undefined, match: undefined };
514+
}
515+
516+
const keys = ObjectKeys(expectFailure);
517+
if (keys.length === 0) {
518+
throw new ERR_INVALID_ARG_VALUE('options.expectFailure', expectFailure, 'must not be an empty object');
519+
}
520+
521+
if (ArrayPrototypeEvery(keys, (k) => k === 'match' || k === 'label')) {
522+
return {
523+
__proto__: null,
524+
label: expectFailure.label,
525+
match: expectFailure.match,
526+
};
527+
}
528+
529+
return { __proto__: null, label: undefined, match: expectFailure };
530+
}
531+
495532
class Test extends AsyncResource {
496533
reportedType = 'test';
497534
abortController;
@@ -641,7 +678,7 @@ class Test extends AsyncResource {
641678
this.plan = null;
642679
this.expectedAssertions = plan;
643680
this.cancelled = false;
644-
this.expectFailure = expectFailure !== undefined && expectFailure !== false;
681+
this.expectFailure = parseExpectFailure(expectFailure) || this.parent?.expectFailure;
645682
this.skipped = skip !== undefined && skip !== false;
646683
this.isTodo = (todo !== undefined && todo !== false) || this.parent?.isTodo;
647684
this.startTime = null;
@@ -955,7 +992,30 @@ class Test extends AsyncResource {
955992
return;
956993
}
957994

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

9671027
pass() {
968-
if (this.error == null && this.expectFailure === true && !this.skipped) {
1028+
if (this.error == null && this.expectFailure && !this.skipped) {
9691029
this.passed = false;
9701030
this.error = new ERR_TEST_FAILURE(
9711031
'test was expected to fail but passed',
@@ -977,6 +1037,20 @@ class Test extends AsyncResource {
9771037
return;
9781038
}
9791039

1040+
if (this.skipped || this.isTodo) {
1041+
this.passed = true;
1042+
return;
1043+
}
1044+
1045+
if (this.expectFailure) {
1046+
this.passed = false;
1047+
this.error = new ERR_TEST_FAILURE(
1048+
'Test passed but was expected to fail',
1049+
kTestCodeFailure,
1050+
);
1051+
return;
1052+
}
1053+
9801054
this.passed = true;
9811055
}
9821056

@@ -1366,7 +1440,10 @@ class Test extends AsyncResource {
13661440
} else if (this.isTodo) {
13671441
directive = this.reporter.getTodo(this.message);
13681442
} else if (this.expectFailure) {
1369-
directive = this.reporter.getXFail(this.expectFailure); // TODO(@JakobJingleheimer): support specifying failure
1443+
const message = typeof this.expectFailure === 'object' ?
1444+
this.expectFailure.label :
1445+
this.expectFailure;
1446+
directive = this.reporter.getXFail(message);
13701447
}
13711448

13721449
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)
Back | FazBrowse Home | New Git URL