Skip to content

doc(minutes): add 09 Feb 2026 #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JakobJingleheimer wants to merge 1 commit into
base: main
Choose a base branch
from
Open

doc(minutes): add 09 Feb 2026 #15

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions minutes/2026-02-09 test-runner meeting minutes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# 09 February 2026

## Attendees

* Alex Vespa [@vespa7](https://github.com/vespa7)
* Ethan Arrowood [@Ethan-Arrowood](https://github.com/Ethan-Arrowood)
* Jacob Smith [@JakobJingleheimer](https://github.com/JakobJingleheimer) (chair)
* Jordan Harband [@ljharb](https://github.com/ljharb)
* Pietro Marchini [@pmarchini](https://github.com/pmarchini)

## Topics

* `t.skip()` Others (tape, etc) to not behave like this.
* standardising test runner design
* https://github.com/WinterTC55/proposal-minimum-common-api/issues/32
* https://github.com/WinterTC55/proposal-minimum-common-api/issues/68
* [doc(proposal): `expectFailure` label and/or matcher](https://github.com/nodejs/test-runner/pull/10)
* [doc(proposal): un/break `--test`](https://github.com/nodejs/test-runner/pull/13)

## Outcomes

* General agreement that `t.skip()`'s behaviour is wrong (likely an accident/bug/mistake, which the docs seem to support).
Copy link
Member

@pmarchini pmarchini Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just checked the rest of the documentation, and the behavior is definitely intended:
https://nodejs.org/docs/v24.13.0/api/test.html#contextskipmessage.

When checking other test runners, like Mocha, this behavior also exists (e.g. https://mochajs.org/declaring/inclusive-tests/#_top).

Given that this behavior is "by design" and aligned with other test runners,IMHO , it's just a matter of personal preference...and I don't think it's enough to justify a major breaking change

P.S.: the same is true for Vitest as well (https://vitest.dev/guide/test-context.html#test-context)

Copy link
Member Author

@JakobJingleheimer JakobJingleheimer Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting. Thanks for checking!

If it's actually fairly common, then sure, it's already there. Kinda weird, but some people like liquorice too 🤷‍♂️

Copy link
Member

@pmarchini pmarchini Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to understand how common it is, but I'm not sure that, without a standard, it makes sense to compare different tools...

cc @ljharb

Copy link
Member

@ljharb ljharb Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these runners all have the API, then that’s a very different story, but it still seems weird to me given the tap-like API the test runner has - which leads into the design philosophy discussion.

Copy link
Member Author

@JakobJingleheimer JakobJingleheimer Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* General agreement that `t.skip()`'s behaviour is wrong (likely an accident/bug/mistake, which the docs seem to support).
* ~Initially general agreement that `t.skip()`'s behaviour is wrong (likely an accident/bug/mistake, which the docs seem to support).~
* Further investigation shows this behaviour
* exists in others (including [mocha](https://mochajs.org/declaring/inclusive-tests/#_top))
* was intended: https://nodejs.org/docs/v24.13.0/api/test.html#contextskipmessage

Copy link

@vassudanagunta vassudanagunta Feb 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find it written anywhere what exactly was deemed wrong about t.skip's behavior.

Copy link
Member

@pmarchini pmarchini Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testCtx.skip skips the test from which it is invoked dynamically, while test.skip creates a skipped test (test with option skip: true)

Copy link
Member Author

@JakobJingleheimer JakobJingleheimer Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there actually wasn't consensus, let's update the minutes to just describe the problem (and merge the minutes), and then we can continue the discussion (either here or in a separate issue/discussion)?

Copy link
Member

@ljharb ljharb Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep - one mental model is "t.skip skips t" and another is "t.skip is a way to generate a skipped assertion underneath t". The latter is how tap and tape work.

Copy link

@vassudanagunta vassudanagunta Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't t.test.skip create a skipped test underneath t?

Copy link

@vassudanagunta vassudanagunta Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, skip never generates a test anywhere in the API. It changes the state of the test to which it is applied1. If applied to a test context, it changes the state of that test. Consistent, yeah?

Footnotes

  1. Conceptually speaking. Technically skip is a function in the test namespace that creates a test, but if you consider the following I think you will get what I mean:

    test('yada', fn)

test('yada', {skip: true}, fn)

test.skip('yada', fn)
    ↩

Copy link

@vassudanagunta vassudanagunta Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't t.test.skip create a skipped test underneath t?

Nope: TypeError: t.test.skip is not a function

But it should be. IMHO, that would result in a consistent API, with t.skip staying as it is.

* General agreement that it would be good to standardise node's test-runner at the WinterTC55 level, if we can, but might be a very uphill battle. Perhaps better to audit our and others' design and adjust (and then take that to WinterTC).
* https://github.com/nodejs/test-runner/pull/10 ready to approve
* Un/break `--test`
* option 3
* leave `--watch` as-is
* in `test` mode, its value (`--watch` only enables watch mode)
* support `--watch-path` in test mode
* only `--test` N-times 1-to-1 with arg list, making it position-independent

## Todos

* Decide if fixing `t.skip()` is a breaking change.
* Jordan to find & share his audit of test runner features to use as base-point.
* Jacob to incorporate "option 3" to https://github.com/nodejs/test-runner/pull/13 and tag team for review.