test_runner: run afterEach for runtime t.skip()

[igor-shevelenkov]

Ensure afterEach runs when a test is skipped at runtime (t.skip()), while keeping static { skip: true } behavior unchanged.

Runtime t.skip() previously set skipped and prevented afterEach, even if beforeEach ran.
This left resources uncleaned and differed from user expectations.

Fixes: #61462

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[jsumners-nr]

Looks good to me.

[ljharb]

This looks fine, but t.skip() shouldn't be marking the overarching test as skipped at all - its purpose is to generate a string comment that will show up as a skipped assertion.

[igor-shevelenkov]

Thanks @ljharb. I see from tape-testing/tape#545 that in tape, t.skip() is designed as an assertion-level skip. Not a test-level skip. That aligns with TAP semantics.
However, Node's test runner has established t.skip() as a test-level runtime skip. It marks the whole test as skipped, not just an assertion. If we choose to change that i think it'll be a breaking change.
I think something like a separate t.skipAssertion() method would be better

[cjihrig]

You don't need the beforeEachTotal or afterEachTotal variables. The common.mustCall()s will enforce those for you.

[ljharb]

@igor-shevelenkov deviating from TAP semantics seems like a pretty big design issue, but fair that it'd be a breaking change. I'd still suggest making it (in a different PR ofc)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.73%. Comparing base (abddfc9) to head (2127b04).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #61525   +/-   ##
=======================================
  Coverage   89.72%   89.73%           
=======================================
  Files         676      676           
  Lines      205988   205990    +2     
  Branches    39490    39489    -1     
=======================================
+ Hits       184830   184846   +16     
+ Misses      13295    13291    -4     
+ Partials     7863     7853   -10     

Files with missing lines	Coverage Δ
lib/internal/test_runner/test.js	97.28% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[igor-shevelenkov]

@ljharb i'll research this topic, pros and cons and come back with possible solutions

[pmarchini]

@igor-shevelenkov deviating from TAP semantics seems like a pretty big design issue, but fair that it'd be a breaking change. I'd still suggest making it (in a different PR ofc)

I think that this might be a good discussion point for the next Node.js Test Runner Bi-Weekly Meeting 👀

cc @nodejs/test_runner

[cjihrig]

FWIW, I would strongly advise against letting TAP influence any decisions outside of the TAP reporter itself.

[ljharb]

@cjihrig true, it's not TAP specifically, it's the prior art of pre-existing TAP-generating test frameworks, most of which (possibly all) don't have a dynamic mechanism to skip a test from inside the test that's already begun.

[cjihrig]

I can't speak to whether or not anything else supports them, but they have been there for three and a half years (#43730) at this point. It doesn't seem like a good idea to break users who are depending on it at this point when this seems to be a straightforward fix.

[JakobJingleheimer]

This was discussed in a couple test runner team meetings, and we reached consensus that this PR is 👍 I read through the remaining comments and I see no outstanding items that weren't addressed in the meeting. I'm adding author-ready and will land in 2 days assuming nobody speaks out.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/71459/

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/61525
✔  Done loading data for nodejs/node/pull/61525
----------------------------------- PR info ------------------------------------
Title      test_runner: run afterEach for runtime t.skip() (#61525)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     igor-shevelenkov:main -> nodejs:main
Labels     author ready, needs-ci, commit-queue-squash, test_runner
Commits    4
 - test_runner: run afterEach on runtime skip
 - Merge branch 'nodejs:main' into main
 - test: track beforeEach calls in runtime-skip test
 - Merge branch 'nodejs:main' into main
Committers 2
 - Igor <igorshevelenkov4@gmail.com>
 - GitHub <noreply@github.com>
PR-URL: https://github.com/nodejs/node/pull/61525
Fixes: https://github.com/nodejs/node/issues/61462
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/61525
Fixes: https://github.com/nodejs/node/issues/61462
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 26 Jan 2026 10:50:20 GMT
   ✔  Approvals: 4
   ✔  - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/61525#pullrequestreview-3707520585
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/61525#pullrequestreview-3713408859
   ✔  - Moshe Atlow (@MoLow): https://github.com/nodejs/node/pull/61525#pullrequestreview-3718102648
   ✔  - Pietro Marchini (@pmarchini): https://github.com/nodejs/node/pull/61525#pullrequestreview-3846531064
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-02-25T18:44:10Z: https://ci.nodejs.org/job/node-test-pull-request/71459/
- Querying data for job/node-test-pull-request/71459/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 61525
From https://github.com/nodejs/node
 * branch                  refs/pull/61525/merge -> FETCH_HEAD
✔  Fetched commits as 764e9a756082..3a29ef0407fa
--------------------------------------------------------------------------------
Auto-merging lib/internal/test_runner/test.js
error: commit 332f669ee2e3a7674327867145e02ac3d5053846 is a merge but no -m option was given.
fatal: cherry-pick failed
[main e85a169712] test_runner: run afterEach on runtime skip
 Author: Igor <igorshevelenkov4@gmail.com>
 Date: Mon Jan 26 13:06:41 2026 +0300
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-runner-aftereach-runtime-skip.js
   ✘  Failed to apply patches

https://github.com/nodejs/node/actions/runs/22413356910

[JakobJingleheimer]

Uuuugh. @igor-shevelenkov I think there's a conflict with main. Could you try rebasing your branch onto the latest from main?

If that doesn't produce a conflict, it's something else (don't push).

If it does produce a conflict, could you resolve it and force-push? I'll have to re-run CI etc.

[igor-shevelenkov]

@JakobJingleheimer, I rebased my branch onto the latest main and got no conflicts.

[JakobJingleheimer]

Ahh, I see the problem:

Our tooling does not support merges. The merge commits will have to be removed. The easiest way is:

1. create a new branch (with the same name as the original) off of nodejs/node's latest main
2. cherry-pick 062059d and 9979c2f
3. force-push to the original remote branch (overwriting it)

I'll re-run CI etc when that's done.

[github-actions]

Failed to start CI

- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✘  Failed to start PR CI: 200 OK

https://github.com/nodejs/node/actions/runs/22451470137

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/71470/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/71471/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/71472/

[nodejs-github-bot]

Landed in 2ae6d8f