From 60db1ec456ea10ba90715c2d1aa008dc54853e77 Mon Sep 17 00:00:00 2001 From: Felipe Coelho Date: Sun, 8 Mar 2026 16:16:55 -0300 Subject: [PATCH 1/2] test_runner: add skip, todo, only, and expectFailure to subtest context The top-level test() function exposes test.skip(), test.todo(), test.only(), and test.expectFailure() variants, but these were missing from TestContext's test() method used in subtests, causing TypeError. Move test() from the class prototype to an arrow function in the constructor, allowing variants to be attached as properties. Extract a shared runSubtest() helper to avoid duplicating the plan counting and createSubtest logic. This trades one shared prototype method for 5 closures per TestContext instance (one base + four variants), which is acceptable given V8's closure optimization for same-shape functions. Includes tests for: variant existence, skip preventing callback execution, todo with and without callback, plan counting with variants, and nested subtest variant availability. Fixes: https://github.com/nodejs/node/issues/50665 --- lib/internal/test_runner/test.js | 52 ++++++++++++------- .../test-runner-subtest-skip-todo-only.js | 48 +++++++++++++++++ 2 files changed, 81 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-runner-subtest-skip-todo-only.js diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 2203bbd3497659..23c0f5837de1b6 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,6 +1,7 @@ 'use strict'; const { ArrayPrototypeEvery, + ArrayPrototypeForEach, ArrayPrototypePush, ArrayPrototypePushApply, ArrayPrototypeShift, @@ -259,6 +260,38 @@ class TestContext { constructor(test) { this.#test = test; + + const runSubtest = (name, options, fn, loc, extraOverrides) => { + const overrides = { + __proto__: null, + ...extraOverrides, + loc, + }; + + const { plan } = this.#test; + if (plan !== null) { + plan.count(); + } + + const subtest = this.#test.createSubtest( + // eslint-disable-next-line no-use-before-define + Test, name, options, fn, overrides, + ); + + return subtest.start(); + }; + + this.test = (name, options, fn) => + runSubtest(name, options, fn, getCallerLocation()); + ArrayPrototypeForEach( + ['expectFailure', 'skip', 'todo', 'only'], + (keyword) => { + this.test[keyword] = (name, options, fn) => + runSubtest(name, options, fn, getCallerLocation(), { + [keyword]: true, + }); + }, + ); } get signal() { @@ -358,25 +391,6 @@ class TestContext { this.#test.todo(message); } - test(name, options, fn) { - const overrides = { - __proto__: null, - loc: getCallerLocation(), - }; - - const { plan } = this.#test; - if (plan !== null) { - plan.count(); - } - - const subtest = this.#test.createSubtest( - // eslint-disable-next-line no-use-before-define - Test, name, options, fn, overrides, - ); - - return subtest.start(); - } - before(fn, options) { this.#test.createHook('before', fn, { __proto__: null, diff --git a/test/parallel/test-runner-subtest-skip-todo-only.js b/test/parallel/test-runner-subtest-skip-todo-only.js new file mode 100644 index 00000000000000..7c3e89e5ee64b8 --- /dev/null +++ b/test/parallel/test-runner-subtest-skip-todo-only.js @@ -0,0 +1,48 @@ +'use strict'; +const common = require('../common'); +const assert = require('node:assert'); +const { test } = require('node:test'); + +// Verify that all subtest variants exist on TestContext. +test('subtest variants exist on TestContext', common.mustCall(async (t) => { + assert.strictEqual(typeof t.test, 'function'); + assert.strictEqual(typeof t.test.skip, 'function'); + assert.strictEqual(typeof t.test.todo, 'function'); + assert.strictEqual(typeof t.test.only, 'function'); + assert.strictEqual(typeof t.test.expectFailure, 'function'); +})); + +// t.test.skip: callback must NOT be called. +test('t.test.skip prevents callback execution', common.mustCall(async (t) => { + await t.test.skip('skipped subtest', common.mustNotCall()); +})); + +// t.test.todo without callback: subtest is marked as todo and skipped. +test('t.test.todo without callback', common.mustCall(async (t) => { + await t.test.todo('todo subtest without callback'); +})); + +// t.test.todo with callback: callback runs but subtest is marked as todo. +test('t.test.todo with callback runs the callback', common.mustCall(async (t) => { + await t.test.todo('todo subtest with callback', common.mustCall()); +})); + +// Plan counting works with subtest variants. +test('plan counts subtest variants', common.mustCall(async (t) => { + t.plan(3); + await t.test('normal subtest', common.mustCall()); + await t.test.skip('skipped subtest'); + await t.test.todo('todo subtest'); +})); + +// Nested subtests also expose the variants. +test('nested subtests have variants', common.mustCall(async (t) => { + await t.test('level 1', common.mustCall(async (t2) => { + assert.strictEqual(typeof t2.test.skip, 'function'); + assert.strictEqual(typeof t2.test.todo, 'function'); + assert.strictEqual(typeof t2.test.only, 'function'); + assert.strictEqual(typeof t2.test.expectFailure, 'function'); + + await t2.test.skip('nested skipped', common.mustNotCall()); + })); +})); From 981cf8d3bd4b4f7a92dd7d3d039aa55a5207315a Mon Sep 17 00:00:00 2001 From: Felipe Coelho Date: Sun, 8 Mar 2026 17:26:57 -0300 Subject: [PATCH 2/2] test_runner: add __proto__: null to extraOverrides object literal Node.js core lint rule (node-core/set-proto-to-null-in-object) requires every object literal in lib/ to have __proto__: null to prevent prototype pollution. --- lib/internal/test_runner/test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 23c0f5837de1b6..4b30654304b3e3 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -288,6 +288,7 @@ class TestContext { (keyword) => { this.test[keyword] = (name, options, fn) => runSubtest(name, options, fn, getCallerLocation(), { + __proto__: null, [keyword]: true, }); },