test_runner: run afterEach for runtime t.skip() by igor-shevelenkov · Pull Request #61525 · nodejs/node

igor-shevelenkov · 2026-01-26T10:50:20Z

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 · 2026-01-26T10:50:26Z

Review requested:

@nodejs/test_runner

jsumners-nr

Looks good to me.

ljharb · 2026-01-26T16:26:30Z

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 · 2026-01-26T18:49:42Z

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 · 2026-01-26T18:58:40Z

test/parallel/test-runner-aftereach-runtime-skip.js

+process.on('exit', () => {
+  assert.strictEqual(beforeEachTotal, 2);
+  assert.strictEqual(afterEachRuntimeSkip, 1);
+  assert.strictEqual(afterEachTotal, 2);
+});


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

ljharb · 2026-01-26T19:02:46Z

@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 · 2026-01-26T19:39:47Z

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 · 2026-01-27T10:48:27Z

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

pmarchini · 2026-01-27T22:45:48Z

@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 · 2026-01-27T23:26:14Z

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

ljharb · 2026-01-27T23:33:06Z

@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 · 2026-01-27T23:45:43Z

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 · 2026-02-23T19:42:54Z

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 · 2026-02-25T18:44:10Z

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

nodejs-github-bot · 2026-02-25T19:56:11Z

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 <[email protected]>
 - GitHub <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/61525
Fixes: https://github.com/nodejs/node/issues/61462
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/61525
Fixes: https://github.com/nodejs/node/issues/61462
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
--------------------------------------------------------------------------------
   ℹ  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 <[email protected]>
 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 · 2026-02-25T21:55:30Z

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 · 2026-02-26T08:52:26Z

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

JakobJingleheimer · 2026-02-26T09:14:49Z

Ahh, I see the problem:

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

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

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

Signed-off-by: Igor <[email protected]>

Increment a beforeEach counter and assert it runs twice when a test is skipped at runtime. Signed-off-by: Igor <[email protected]>

github-actions · 2026-02-26T16:35:52Z

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 · 2026-02-26T17:51:45Z

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

nodejs-github-bot · 2026-02-26T19:10:02Z

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

nodejs-github-bot · 2026-02-26T20:43:57Z

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

nodejs-github-bot · 2026-02-26T21:36:25Z

Landed in 2ae6d8f

nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jan 26, 2026

jsumners-nr approved these changes Jan 26, 2026

View reviewed changes

cjihrig approved these changes Jan 26, 2026

View reviewed changes

atlowChemi approved these changes Jan 27, 2026

View reviewed changes

MoLow approved these changes Jan 28, 2026

View reviewed changes

JakobJingleheimer mentioned this pull request Feb 23, 2026

Node.js Test Runner Meeting 2026-02-23 nodejs/test-runner#16

Open

JakobJingleheimer added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 23, 2026

pmarchini approved these changes Feb 24, 2026

View reviewed changes

JakobJingleheimer added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 25, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 25, 2026

This comment has been minimized.

Sign in to view

JakobJingleheimer added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 25, 2026

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 25, 2026

JakobJingleheimer added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 25, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 25, 2026

igor-shevelenkov added 2 commits February 26, 2026 12:37

test_runner: run afterEach on runtime skip

25b0646

Signed-off-by: Igor <[email protected]>

test: track beforeEach calls in runtime-skip test

2127b04

Increment a beforeEach counter and assert it runs twice when a test is skipped at runtime. Signed-off-by: Igor <[email protected]>

igor-shevelenkov force-pushed the main branch from 3a29ef0 to 2127b04 Compare February 26, 2026 09:38

JakobJingleheimer approved these changes Feb 26, 2026

View reviewed changes

JakobJingleheimer added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 26, 2026

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2026

JakobJingleheimer added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2026

github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 26, 2026

avivkeller added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 26, 2026

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 26, 2026

nodejs-github-bot merged commit 2ae6d8f into nodejs:main Feb 26, 2026
70 checks passed

Uh oh!

Conversation

igor-shevelenkov commented Jan 26, 2026

Uh oh!

nodejs-github-bot commented Jan 26, 2026

Uh oh!

jsumners-nr left a comment

Choose a reason for hiding this comment

Uh oh!

ljharb commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igor-shevelenkov commented Jan 26, 2026

Uh oh!

cjihrig Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

ljharb commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

igor-shevelenkov commented Jan 27, 2026

Uh oh!

pmarchini commented Jan 27, 2026

Uh oh!

cjihrig commented Jan 27, 2026

Uh oh!

ljharb commented Jan 27, 2026

Uh oh!

cjihrig commented Jan 27, 2026

Uh oh!

JakobJingleheimer commented Feb 23, 2026

Uh oh!

This comment has been minimized.

nodejs-github-bot commented Feb 25, 2026

Uh oh!

nodejs-github-bot commented Feb 25, 2026

Uh oh!

JakobJingleheimer commented Feb 25, 2026

Uh oh!

igor-shevelenkov commented Feb 26, 2026

Uh oh!

JakobJingleheimer commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

nodejs-github-bot commented Feb 26, 2026

Uh oh!

nodejs-github-bot commented Feb 26, 2026

Uh oh!

nodejs-github-bot commented Feb 26, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ljharb commented Jan 26, 2026 •

edited

Loading

ljharb commented Jan 26, 2026 •

edited

Loading

codecov bot commented Jan 26, 2026 •

edited

Loading