test_runner: support expecting a test-case to fail #60669

JakobJingleheimer · 2025-11-10T18:22:17Z

This PR adds support for an xfail directive (similar to skip and todo), whereby a user can flag a test-case as expected to fail.

Co-Authored-By: Alejandro Espa <98526766+vespa7@users.noreply.github.com>

nodejs-github-bot · 2025-11-10T18:22:22Z

Review requested:

@nodejs/test_runner

lib/internal/test_runner/reporter/tap.js

test/fixtures/test-runner/output/describe_it.js

vassudanagunta · 2025-11-24T14:21:08Z

There is long-established precedent for xfail:

pytest. See also XFail: mark test functions as expected to fail and How and why I use pytest's xfail
- pytest.xfail is distinct from pytest.fail
Mozilla
Jest, which calls it failing, though the original proposal used the term xfail.
DejaGnu Testing Framework
The Greg testing framework
See also:
- What's the purpose of xfail tests?
- TAP Directives for XPASS and XFAIL tests with lib/tap-driver.sh

What is the commitment level and timeline for this? I ask because I've already implemented xfail, ypass and some other additional feature extensions to node:test in a yet-to-be-released node-test-extra package (like fs-extra is to node:fs). I'd be delighted if xfail was subsumed into node:test. Because I've put a lot of thought into it, I might have something to contribute to nodejs on this. I could write up my thoughts, collaborate with @JakobJingleheimer, and/or create an alternate PR.

lmk, please

JakobJingleheimer · 2025-11-24T14:30:17Z

Ah, sure. I'm not particular about the name as long as it's intuitive (which xfail seems to be).

I expect this could land pretty quickly: quite a few collaborators had previously voiced support for it, and to my knowledge there have been no general concerns raised against the feature itself.

Aside from the name (which is trivial to update in the PR), the remaining work is:

find where the result data gets passed to reporters (and add this property to that—the actual implementation work there is probably ~5 seconds)
update the docs

Creating competing PRs is never desirable and generally pisses off the person / people already working on it.

Happy to collaborate on the feature; but aside from influencing the name, I think this is very straightforward and has very little implication work, especially almost none remaining.

In terms of timeline to finish this, I expect to have time this week (the past ~2 weeks were very tumultuous for me).

vassudanagunta · 2025-11-24T15:35:14Z

Additional features that my version has:

option to specify the expected error. I think this is really important. If for example you are documenting/reproducing/tracking via a test a bug that is in released code, you will want it to xfail, since it ought not fail CI, but only if it fails in the expected way.
command line option to treat xfails as regular tests, i.e. treating them as test failures. This is useful when you want to see the failure details, which most reporters only show for failed tests. With node-test-extra, it will also show all debug/console logging, which is otherwise suppressed.
- I am considering augmenting that command line option with a filter arg, so you can filter which xfails are treated as regular tests.

I think that's it, though I'll have to take a look at

JakobJingleheimer · 2025-11-24T15:57:22Z

Alex and I considered those and pushed them to v2 to get the base implementation landed sooner (many users probably won't need the v2 extras, so no need to make them wait for things they don't want/need). If you already have an implementation for that, I'm happy to defer to you for that once this lands 🙂

vassudanagunta · 2025-11-24T16:16:09Z

Deferring to v2 makes sense, but I think you should quickly consider how it would be added to the API, just to avoid potential API thrashing or getting painted into a corner. In the implementation, you have a string value given for xfail that is treated as a reporting message, just like todo. That makes sense:

test('this should to that', { xfail: 'bug #382' }, (t) => {
   //...
});

How would you, in v2, specify the expected failure? An xfail-error property that specifies the error message or pattern? Something that can be parsed out of the existing xfail property? Or within the test function, e.g. via a new assert.xfail method that specifies both the expected value and also the expected erroneous value? Or...?

vassudanagunta · 2025-11-24T16:29:05Z

Arguably a new kind of assert might be better than adding an xfail marker as a test property.

assert.xfail(expected, actual, expectedErroneous?, message?)

OTOH, assertions of failures maybe should be reserved for testing that the code throws the correct exception under specific conditions.

JakobJingleheimer · 2025-11-24T16:41:44Z

That's contrary to the intention of this feature: the test should be identical to the non-failing version. This is flag to flip.

assert.xfail is a different feature 🙂

JakobJingleheimer · 2025-11-24T16:48:33Z

you have a string value given for xfail that is treated as a reporting message, just like todo. That makes sense:
test('this should to that', { xfail: 'bug #382' }, (t) => {

   //...

});

We were thinking in the case of it.fail, any failure is okay.

In the case of { fail } it would accept, true, a string or regex to compare against err.message (and maybe an instance of Error/AssertionError).

For all of these, there is no API thrashing as they're all truthy / falsy, which makes them additive to the current.

vassudanagunta · 2025-11-24T22:14:48Z

it all sounds good to me. I'm glad this is getting into node:test. Thank you!

As to reporting, I think you have three options:

xfail is a special case of pass. If you want existing reporters that don't know about xfail to report it as a regular pass.
xfail is a special case of todo. If you want existing reporters that don't know about xfail to report it as a todo. this makes conceptual sense since it is something that shouldn't be treated as "all green", something that should be highlighted as needing to be addressed.
you don't worry about breaking existing reporters, and you want xfail to be reported distinctly from pass and todo.

In my implementation, I chose option 2. A failing xfail generates a test todo event, where the todo message is prefixed with "xfail:". A passing xfail generates a test fail event, with an error indicating that it was an xfail that unexpectedly passed.

This allows my xfail aware reporter to produce a more informative report (it formats xfail results differently from both pass and todo), while degrading gracefully for legacy reporters.

JakobJingleheimer · 2025-11-24T23:11:35Z

I (and I think Alex too?) was imagining the result of it.xfail would go into the existing pass (when it throws) or fail (when it doesn't throw) and would additionally have a property expectedFail: true (not married to the prop name either—if a well known one exists that isn't crazy, happy to go with that, especially if reports will already support it).

Reporters that support the extra prop do so, and those that don't just behave "normally" (even the uninformed report is still correct).

vassudanagunta · 2025-11-25T04:26:56Z

ok, that's what i meant by option 1. Since I've implemented an xfail-aware custom reporter, it's all good for my purposes.

JakobJingleheimer · 2025-12-06T12:53:51Z

Huzzah! TAP reporter is now consuming/surfacing xfail. The others aren't yet because I can't find where they compose their output text

doc/api/test.md

vassudanagunta · 2025-12-06T17:53:52Z

The others aren't yet because I can't find where they compose their output text

Jacob, both spec and dot reporters rely on:

node/lib/internal/test_runner/reporter/utils.js

Line 71 in 37d9cfc

    
           function formatTestReport(type, data, prefix = '', indent = '', hasChildren = false, showErrorDetails = true) {

I know this as I spent a lot of time trying to reverse engineer the semantics of TestsStream, which isn't documented nearly enough for people to write custom reporters without inferring semantics by reading all of the built-in reporter code. I will open a discussion or issue on that topic when I get the chance.

But as part of my reverse engineering, I ended up paying off some test_runner tech debt in precisely this part of the code: #59700. It's seems to be falling through the cracks in PR limbo. Maybe we can get that merged? It isn't strictly necessary for your PR, but since we'll have review eyes on this area of code, it would be efficient to do both at the same time?

vassudanagunta · 2025-12-06T18:11:40Z

test/fixtures/test-runner/output/describe_it.js



-it.todo('sync pass todo', () => {
+it.xfail('sync expect fail (method)', () => {


I think you should also add a "sync pass xfail with message" like there is with todo and skip.

This opens up the question of what that should look like for TAP. In my own reporter, I append #XFAIL to the message. The discussion on TAP Directives for XPASS and XFAIL that I linked to in the main thread debates #XFAIL vs #TODO. The latter makes a lot of sense too... see my comment that conceptually an xfail is a special case of todo.

Maybe the folks behind the TAP spec, TAPE and tapjs/node-tap can chime in? @isaacs @ljharb

Do you mean in the case of specifying it via config object? If so, the value that would subsequently be supported would be the expected failure we talked about #60669 (comment) so

it('should add', { xfail: 'Expected 2 to equal 1' }, () => { … });

where the test-case must fail with an error whose message is 'Expected 2 to equal 1'

Or as a regex:

it('should add', { xfail: /expected 2 to equal 1/i }, () => { … });

When it fails with a different error, xfail would then not "pass" (and would be marked "failed", reported as failed, etc).

That would then be used in the test-case reporter output like

✔ should add # Expected 2 to equal 1

✔ should add # /expected 2 to equal 1/i

✖ should add # Expected 2 to equal 1

✖ should add # /expected 2 to equal 1/i

But since that's part of v1.1, I think for now it should not handle any value other than truthy/falsy.

I think both a reason (like todo and skip) and expected error should be supported. Example reasons:

✔ should add #XFAIL Issue #389 ✔ should add #XFAIL Can't fix until SemVer Major increment

I'm building a system where released bugs are tracked both as a a bug tracking issue and as an XFAIL test case.

I took your comment to mean (update of the current documentation to illustrate):

skip <boolean> | <string> If truthy, the test is skipped. If a string is provided, that string is displayed in the test results as the reason for skipping the test. Default: false.

todo <boolean> | <string> If truthy, the test marked as TODO. If a string is provided, that string is displayed in the test results as the reason why the test is TODO. Default: false.

xfail <boolean> | <string> | <RegExp> If truthy, the test marked as XFAIL. If a string is provided, that string is displayed in the test results as the reason why the test is expected to fail. If a RegExp is provided, only a failure that matches is permitted. Default: false.

Now I see my interpretation was wrong. In any case, that approach would allow EITHER a reason OR an expected error, NOT BOTH. I think both should be supported.

But as you say, we can punt until v1.1. 🤫🤐🙃

JakobJingleheimer · 2025-12-07T19:08:21Z

Jacob, both spec and dot reporters rely on:

node/lib/internal/test_runner/reporter/utils.js

Line 71 in 37d9cfc

function formatTestReport(type, data, prefix = '', indent = '', hasChildren = false, showErrorDetails = true) {

Huzzah, yes, that's it. Thanks!

codecov · 2025-12-07T21:57:43Z

Codecov Report

❌ Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.53%. Comparing base (bd3a202) to head (241e5dd).
⚠️ Report is 204 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/reporter/utils.js	33.33%	1 Missing and 1 partial ⚠️
lib/internal/test_runner/tests_stream.js	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60669      +/-   ##
==========================================
- Coverage   88.55%   88.53%   -0.02%     
==========================================
  Files         703      703              
  Lines      208077   208449     +372     
  Branches    40083    40210     +127     
==========================================
+ Hits       184254   184548     +294     
- Misses      15841    15905      +64     
- Partials     7982     7996      +14

Files with missing lines	Coverage Δ
lib/internal/test_runner/harness.js	`91.82% <100.00%> (ø)`
lib/internal/test_runner/reporter/tap.js	`98.23% <100.00%> (+0.01%)`	⬆️
lib/internal/test_runner/test.js	`97.36% <100.00%> (-0.01%)`	⬇️
lib/internal/test_runner/tests_stream.js	`89.65% <75.00%> (-0.35%)`	⬇️
lib/internal/test_runner/reporter/utils.js	`90.38% <33.33%> (-2.76%)`	⬇️

... and 86 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JakobJingleheimer · 2025-12-11T18:49:25Z

PR is quasi-blocked by a bug in the Validate commit message workflow (pending fix: nodejs/core-validate-commit#130).

pmarchini · 2025-12-18T12:50:17Z

lib/internal/test_runner/reporter/tap.js

    switch (type) {
      case 'test:fail': {
-        yield reportTest(data.nesting, data.testNumber, 'not ok', data.name, data.skip, data.todo);
+        yield reportTest(data.nesting, data.testNumber, 'not ok', data.name, data.skip, data.todo, data.xfail);


If we decide to move forward with xfail, we will need to update all integrated reporters!

Didn't I already?

I can see only the integration for tap, did you check spec, dot etc?

Spec and dot use the same util, and I did Spec 🙂

puuuurfect, can we add some tests to cover this behaviour for the different embedded reporters? 😁

I think we should add the new xfail here: test/fixtures/test-runner/output/output.js.
This fixture should be used by the snapshot tests for all the different reporters

Are you thinking I do it there instead of the output one I did do or in addition? I can see reasons for either:

More coverage to catch something.

More things to change, which is a maintenance burden.

If I'm not mistaken, currently no tests cover the actual output of xfail across all integrated reporters.

My only concern is that any behavior that becomes clearly defined should be tested to avoid unintentional breakage.

IMHO, it's fine to remove the existing test if it no longer adds value once other tests are added.

Regarding the maintenance burden: these are snapshot tests, so they're usually quick to update.

pmarchini · 2025-12-18T12:55:19Z

doc/api/test.md

+These are tests written to otherwise pass but there is some issue (often
+upstream) causing them to fail. Rather than using `skip` or `todo`, which would
+effectively silence the failure, the test would instead be marked `xfail` (`x`
+from "expected" + `fail`). That way, when the issue is resolved, that is


IMHO, we should provide more behaviour-first documentation for this feature, rather than focusing on a specific use case!

I was thinking to add that in the test runner Learn article. The sentiment for API docs seems to be purely what it is. I don't have an opinion.

ava calls this "failing", which seems a bit clearer to me than "xfail"

My biggest concern about this feature isn't the feature itself, but the naming 😬

@ljharb @pmarchini See prior art: #60669 (comment)

jest and ava, so yes? 🤷🏾‍♂️

i think xfail is more intuitive but i defer to you guys. More important to me are semantics. Is failing a subclass of todo or pass? For protocols like TAP and downstream consumers that don't know about it, it should be mapped as a subclass of one or the other. Likewise for existing custom node:test reporters.

I think todo. A failing test is one that is flagged for future action. it shouldn't be easily ignored.

(my last argument for xfail is embedded above. there's gonna be a lot of confusion when people use the very common phrase "failing test".)

a failing test that fails correctly, as a todo, make sense to me, as long as a failing test that passes incorrectly, is a fail :-)

fair point on the discussion confusion - doesn't change my position, but it's a good thing for others to consider.

a failing test that fails correctly, as a todo, make sense to me, as long as a failing test that passes incorrectly, is a fail :-)

that's how i see it exactly

"Successfully failed" and "failed to fail"; not confusing at all 🤪

I don't particularly care about the name—I want the functionality.

I'm not sure I understand this "subclass of todo". What does todo have to do with this? They seem fairly different to me:

A todo can pass or fail without affecting the suite's result.

An xfail must fail correctly, and it does affect the suite's result.

pmarchini · 2025-12-18T12:58:14Z

test/fixtures/test-runner/output/describe_it.js



-it.todo('sync pass todo', () => {
+it.xfail('sync expect fail (method)', () => {


I would suggest adding a test similar to test-runner-todo-skip-tests.js too, as this specific test is mainly meant to test the output 😁

avivkeller

Neato!

test_runner: support expecting a test-case to fail

32bff8e

Co-Authored-By: Alejandro Espa <98526766+vespa7@users.noreply.github.com>

JakobJingleheimer requested review from MoLow and pmarchini November 10, 2025 18:22

JakobJingleheimer added the test_runner Issues and PRs related to the test runner subsystem. label Nov 10, 2025

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Nov 10, 2025

JakobJingleheimer commented Nov 10, 2025

View reviewed changes

lib/internal/test_runner/reporter/tap.js Outdated Show resolved Hide resolved

pmarchini reviewed Nov 24, 2025

View reviewed changes

lib/internal/test_runner/reporter/tap.js Show resolved Hide resolved

pmarchini reviewed Nov 24, 2025

View reviewed changes

test/fixtures/test-runner/output/describe_it.js Outdated Show resolved Hide resolved

JakobJingleheimer added 5 commits November 26, 2025 23:23

fixup!: rename fail → xfail

cb03a42

fixup!: add getXFail to TestStream

b8b5c6c

fixup!: test-case descriptions

4a682ec

fixup!: add doc for xfail

3353647

fixup!: tidy todo comment

4d29892

avivkeller reviewed Dec 6, 2025

View reviewed changes

doc/api/test.md Show resolved Hide resolved

vassudanagunta reviewed Dec 6, 2025

View reviewed changes

fixup!: add # EXPECTED FAILURE to other builtin reporter output

9c9691c

JakobJingleheimer marked this pull request as ready for review December 7, 2025 19:26

JakobJingleheimer added 4 commits December 8, 2025 20:54

fixup!: update snapshot

774cb57

fixup!: add meta to doc

f02497e

fixup!: remove unnecessary code comment

4638c04

fixup!: update snapshot

241e5dd

JakobJingleheimer requested review from cjihrig and pmarchini December 8, 2025 23:06

pmarchini reviewed Dec 18, 2025

View reviewed changes

avivkeller approved these changes Dec 18, 2025

View reviewed changes



		it.todo('sync pass todo', () => {
		it.xfail('sync expect fail (method)', () => {

Uh oh!

test_runner: support expecting a test-case to fail #60669

Are you sure you want to change the base?

test_runner: support expecting a test-case to fail #60669

Uh oh!

Conversation

JakobJingleheimer commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Nov 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vassudanagunta commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JakobJingleheimer commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vassudanagunta commented Nov 24, 2025

Uh oh!

JakobJingleheimer commented Nov 24, 2025

Uh oh!

vassudanagunta commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vassudanagunta commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JakobJingleheimer commented Nov 24, 2025

Uh oh!

JakobJingleheimer commented Nov 24, 2025

Uh oh!

vassudanagunta commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JakobJingleheimer commented Nov 24, 2025

Uh oh!

vassudanagunta commented Nov 25, 2025

Uh oh!

JakobJingleheimer commented Dec 6, 2025

Uh oh!

Uh oh!

vassudanagunta commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JakobJingleheimer Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JakobJingleheimer commented Dec 7, 2025

Uh oh!

codecov bot commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JakobJingleheimer commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

JakobJingleheimer commented Nov 10, 2025 •

edited

Loading

vassudanagunta commented Nov 24, 2025 •

edited

Loading

JakobJingleheimer commented Nov 24, 2025 •

edited

Loading

vassudanagunta commented Nov 24, 2025 •

edited

Loading

vassudanagunta commented Nov 24, 2025 •

edited

Loading

vassudanagunta commented Nov 24, 2025 •

edited

Loading

vassudanagunta commented Dec 6, 2025 •

edited

Loading

JakobJingleheimer Dec 7, 2025 •

edited

Loading

codecov bot commented Dec 7, 2025 •

edited

Loading

vassudanagunta Dec 18, 2025 •

edited

Loading