From bf86e025a58e65b9f00e9e8e5698e9909fb51bbf Mon Sep 17 00:00:00 2001 From: Romain Menke Date: Sun, 11 May 2025 19:23:20 +0200 Subject: [PATCH] Revert "test_runner: remove promises returned by t.test()" This reverts commit 1a2eb15bc610926789a8a3a002ec722c39768c20. PR-URL: https://github.com/nodejs/node/pull/58282 Fixes: https://github.com/nodejs/node/issues/58227 Reviewed-By: Matteo Collina Reviewed-By: LiviaMedeiros --- doc/api/test.md | 62 +++++++++++-------- lib/internal/test_runner/test.js | 2 +- .../test-runner/output/dot_reporter.snapshot | 2 + .../test-runner/output/hooks.snapshot | 5 ++ .../output/hooks_spec_reporter.snapshot | 5 ++ .../output/junit_reporter.snapshot | 5 +- .../test-runner/output/output.snapshot | 2 + .../test-runner/output/output_cli.snapshot | 2 + .../test-runner/output/spec_reporter.snapshot | 2 + .../output/spec_reporter_cli.snapshot | 2 + test/parallel/test-runner-module-mocking.js | 14 ++--- 11 files changed, 67 insertions(+), 36 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index eeec5e961b..c9905fde32 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -105,11 +105,11 @@ top level test with two subtests. ```js test('top level test', async (t) => { - t.test('subtest 1', (t) => { + await t.test('subtest 1', (t) => { assert.strictEqual(1, 1); }); - t.test('subtest 2', (t) => { + await t.test('subtest 2', (t) => { assert.strictEqual(2, 2); }); }); @@ -118,7 +118,12 @@ test('top level test', async (t) => { > **Note:** `beforeEach` and `afterEach` hooks are triggered > between each subtest execution. -Any subtest failures cause the parent test to fail. +In this example, `await` is used to ensure that both subtests have completed. +This is necessary because tests do not wait for their subtests to +complete, unlike tests created within suites. +Any subtests that are still outstanding when their parent finishes +are cancelled and treated as failures. Any subtest failures cause the parent +test to fail. ## Skipping tests @@ -236,20 +241,20 @@ that are not executed are omitted from the test runner output. // The suite's 'only' option is set, so these tests are run. test('this test is run', { only: true }, async (t) => { // Within this test, all subtests are run by default. - t.test('running subtest'); + await t.test('running subtest'); // The test context can be updated to run subtests with the 'only' option. t.runOnly(true); - t.test('this subtest is now skipped'); - t.test('this subtest is run', { only: true }); + await t.test('this subtest is now skipped'); + await t.test('this subtest is run', { only: true }); // Switch the context back to execute all tests. t.runOnly(false); - t.test('this subtest is now run'); + await t.test('this subtest is now run'); // Explicitly do not run these tests. - t.test('skipped subtest 3', { only: false }); - t.test('skipped subtest 4', { skip: true }); + await t.test('skipped subtest 3', { only: false }); + await t.test('skipped subtest 4', { skip: true }); }); // The 'only' option is not set, so this test is skipped. @@ -304,13 +309,13 @@ multiple times (e.g. `--test-name-pattern="test 1"`, ```js test('test 1', async (t) => { - t.test('test 2'); - t.test('test 3'); + await t.test('test 2'); + await t.test('test 3'); }); test('Test 4', async (t) => { - t.test('Test 5'); - t.test('test 6'); + await t.test('Test 5'); + await t.test('test 6'); }); ``` @@ -3388,9 +3393,12 @@ before each subtest of the current test. ```js test('top level test', async (t) => { t.beforeEach((t) => t.diagnostic(`about to run ${t.name}`)); - t.test('This is a subtest', (t) => { - assert.ok('some relevant assertion here'); - }); + await t.test( + 'This is a subtest', + (t) => { + assert.ok('some relevant assertion here'); + }, + ); }); ``` @@ -3448,9 +3456,12 @@ after each subtest of the current test. ```js test('top level test', async (t) => { t.afterEach((t) => t.diagnostic(`finished running ${t.name}`)); - t.test('This is a subtest', (t) => { - assert.ok('some relevant assertion here'); - }); + await t.test( + 'This is a subtest', + (t) => { + assert.ok('some relevant assertion here'); + }, + ); }); ``` @@ -3702,8 +3713,10 @@ no-op. test('top level test', (t) => { // The test context can be set to run subtests with the 'only' option. t.runOnly(true); - t.test('this subtest is now skipped'); - t.test('this subtest is run', { only: true }); + return Promise.all([ + t.test('this subtest is now skipped'), + t.test('this subtest is run', { only: true }), + ]); }); ``` @@ -3775,10 +3788,6 @@ added: - v18.0.0 - v16.17.0 changes: - - version: - - v24.0.0 - pr-url: https://github.com/nodejs/node/pull/56664 - description: This function no longer returns a `Promise`. - version: - v18.8.0 - v16.18.0 @@ -3823,13 +3832,14 @@ changes: to this function is a [`TestContext`][] object. If the test uses callbacks, the callback function is passed as the second argument. **Default:** A no-op function. +* Returns: {Promise} Fulfilled with `undefined` once the test completes. This function is used to create subtests under the current test. This function behaves in the same fashion as the top level [`test()`][] function. ```js test('top level test', async (t) => { - t.test( + await t.test( 'This is a subtest', { only: false, skip: false, concurrency: 1, todo: false, plan: 1 }, (t) => { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 245b3c0979..6b3d087b67 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -370,7 +370,7 @@ class TestContext { Test, name, options, fn, overrides, ); - subtest.start(); + return subtest.start(); } before(fn, options) { diff --git a/test/fixtures/test-runner/output/dot_reporter.snapshot b/test/fixtures/test-runner/output/dot_reporter.snapshot index 533622f95d..fc2b58cfef 100644 --- a/test/fixtures/test-runner/output/dot_reporter.snapshot +++ b/test/fixtures/test-runner/output/dot_reporter.snapshot @@ -158,6 +158,8 @@ Failed tests: * * * + * + * ✖ subtest sync throw fails (*ms) '2 subtests failed' ✖ timed out async test (*ms) diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot index f1f5b7573c..4ba957d539 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -576,6 +576,7 @@ not ok 16 - t.after throws - no subtests * * * + * ... 1..2 not ok 17 - t.beforeEach throws @@ -606,6 +607,8 @@ not ok 17 - t.beforeEach throws * * * + * + * ... # Subtest: 2 not ok 2 - 2 @@ -626,6 +629,7 @@ not ok 17 - t.beforeEach throws * * * + * ... 1..2 not ok 18 - t.afterEach throws @@ -753,6 +757,7 @@ not ok 21 - afterEach context when test fails * * * + * ... 1..2 not ok 22 - afterEach throws and test fails diff --git a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot index 8ee710e845..8c267672b9 100644 --- a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot @@ -363,6 +363,7 @@ * * * + * * 1 (*ms) @@ -375,6 +376,8 @@ * * * + * + * * 2 (*ms) @@ -388,6 +391,7 @@ * * * + * * 1 (*ms) @@ -435,6 +439,7 @@ * * * + * * t.after() is called if test body throws (*ms) diff --git a/test/fixtures/test-runner/output/junit_reporter.snapshot b/test/fixtures/test-runner/output/junit_reporter.snapshot index 9084fdf45d..aaa5dcd6ff 100644 --- a/test/fixtures/test-runner/output/junit_reporter.snapshot +++ b/test/fixtures/test-runner/output/junit_reporter.snapshot @@ -355,7 +355,8 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first -[Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second] { +Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second + * { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: Error: thrown from subtest sync throw fails at second @@ -365,6 +366,8 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first * * * + * + * } diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot index cf464c09a0..36d9c289fa 100644 --- a/test/fixtures/test-runner/output/output.snapshot +++ b/test/fixtures/test-runner/output/output.snapshot @@ -606,6 +606,8 @@ not ok 51 - custom inspect symbol that throws fail * * * + * + * ... 1..2 not ok 52 - subtest sync throw fails diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot index 9fad5ba240..4546269836 100644 --- a/test/fixtures/test-runner/output/output_cli.snapshot +++ b/test/fixtures/test-runner/output/output_cli.snapshot @@ -614,6 +614,8 @@ not ok 51 - custom inspect symbol that throws fail * * * + * + * ... 1..2 not ok 52 - subtest sync throw fails diff --git a/test/fixtures/test-runner/output/spec_reporter.snapshot b/test/fixtures/test-runner/output/spec_reporter.snapshot index 7eedb8b170..1892069327 100644 --- a/test/fixtures/test-runner/output/spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter.snapshot @@ -289,6 +289,8 @@ * * * + * + * * timed out async test (*ms) diff --git a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot index 6a0c7e381c..52dc40bb36 100644 --- a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot @@ -292,6 +292,8 @@ * * * + * + * * timed out async test (*ms) diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js index 1a1ef67632..89e08a9e22 100644 --- a/test/parallel/test-runner-module-mocking.js +++ b/test/parallel/test-runner-module-mocking.js @@ -477,15 +477,13 @@ test('mocks are automatically restored', async (t) => { assert.strictEqual(mocked.fn(), 43); }); - t.test('checks original behavior', async () => { - const cjsMock = require(cjsFixture); - const esmMock = await import(esmFixture); + const cjsMock = require(cjsFixture); + const esmMock = await import(esmFixture); - assert.strictEqual(cjsMock.string, 'original cjs string'); - assert.strictEqual(cjsMock.fn, undefined); - assert.strictEqual(esmMock.string, 'original esm string'); - assert.strictEqual(esmMock.fn, undefined); - }); + assert.strictEqual(cjsMock.string, 'original cjs string'); + assert.strictEqual(cjsMock.fn, undefined); + assert.strictEqual(esmMock.string, 'original esm string'); + assert.strictEqual(esmMock.fn, undefined); }); test('mocks can be restored independently', async (t) => {