From 60db1ec456ea10ba90715c2d1aa008dc54853e77 Mon Sep 17 00:00:00 2001 From: Felipe Coelho Date: Sun, 8 Mar 2026 16:16:55 -0300 Subject: [PATCH 1/3] 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/3] 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, }); }, From 3d6951bea853f69c692dafb35e3276902e14c331 Mon Sep 17 00:00:00 2001 From: Felipe Coelho Date: Sun, 15 Mar 2026 23:35:02 -0300 Subject: [PATCH 3/3] refactor(test_runner): use private method instead of constructor closure --- lib/internal/test_runner/test.js | 48 ++++++------ .../test-runner-subtest-skip-todo-only.js | 78 ++++++++++--------- 2 files changed, 65 insertions(+), 61 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 4b30654304b3e3..123d5a80baf2db 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -261,33 +261,15 @@ 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(); - }; - + // Attach .skip, .todo, .only, .expectFailure variants to this.test, + // matching the same set exposed on the top-level test() in harness.js. this.test = (name, options, fn) => - runSubtest(name, options, fn, getCallerLocation()); + this.#runTest(name, options, fn, getCallerLocation()); ArrayPrototypeForEach( - ['expectFailure', 'skip', 'todo', 'only'], + ['expectFailure', 'only', 'skip', 'todo'], (keyword) => { this.test[keyword] = (name, options, fn) => - runSubtest(name, options, fn, getCallerLocation(), { + this.#runTest(name, options, fn, getCallerLocation(), { __proto__: null, [keyword]: true, }); @@ -295,6 +277,26 @@ class TestContext { ); } + #runTest(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(); + } + get signal() { return this.#test.signal; } diff --git a/test/parallel/test-runner-subtest-skip-todo-only.js b/test/parallel/test-runner-subtest-skip-todo-only.js index 7c3e89e5ee64b8..62b02af35704e9 100644 --- a/test/parallel/test-runner-subtest-skip-todo-only.js +++ b/test/parallel/test-runner-subtest-skip-todo-only.js @@ -1,48 +1,50 @@ 'use strict'; +// Regression test for https://github.com/nodejs/node/issues/50665 +// t.test() should expose the same .skip, .todo, .only, .expectFailure +// variants as the top-level test() function. 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'); -})); +test('context test function exposes all variant methods', async (t) => { + for (const variant of ['skip', 'todo', 'only', 'expectFailure']) { + assert.strictEqual( + typeof t.test[variant], 'function', + `t.test.${variant} should be a 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()); -})); +test('skip variant does not execute the callback', async (t) => { + await t.test.skip('this is skipped', 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'); -})); +test('todo variant without callback marks subtest as todo', async (t) => { + await t.test.todo('pending feature'); +}); -// 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()); -})); +test('todo variant with callback still runs', async (t) => { + let ran = false; + await t.test.todo('work in progress', () => { ran = true; }); + assert.ok(ran, 'todo callback should have been invoked'); +}); -// 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'); -})); +test('variants increment the plan counter', async (t) => { + t.plan(4); + await t.test('regular', common.mustCall()); + await t.test.skip('skipped'); + await t.test.todo('todo'); + await t.test.todo('todo with cb', common.mustCall()); +}); -// 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()); - })); -})); +test('variants propagate to nested subtests', async (t) => { + await t.test('outer', async (t2) => { + for (const variant of ['skip', 'todo', 'only', 'expectFailure']) { + assert.strictEqual( + typeof t2.test[variant], 'function', + `nested t.test.${variant} should be a function`, + ); + } + await t2.test.skip('inner skip', common.mustNotCall()); + }); +});