From 30b191c0238d221110d82f4280f5b38098ff087c Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 19 Jan 2025 20:06:50 -0500 Subject: [PATCH] test_runner: remove promises returned by t.test() This commit updates the TestContext.prototype.test() API to no longer return a Promise. Fixes: https://github.com/nodejs/node/issues/51292 --- 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, 36 insertions(+), 67 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index c165efeb48d1b8..7040fa1397d4c9 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) => { - await t.test('subtest 1', (t) => { + t.test('subtest 1', (t) => { assert.strictEqual(1, 1); }); - await t.test('subtest 2', (t) => { + t.test('subtest 2', (t) => { assert.strictEqual(2, 2); }); }); @@ -118,12 +118,7 @@ test('top level test', async (t) => { > **Note:** `beforeEach` and `afterEach` hooks are triggered > between each subtest execution. -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. +Any subtest failures cause the parent test to fail. ## Skipping tests @@ -241,20 +236,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. - await t.test('running subtest'); + t.test('running subtest'); // The test context can be updated to run subtests with the 'only' option. t.runOnly(true); - await t.test('this subtest is now skipped'); - await t.test('this subtest is run', { only: true }); + t.test('this subtest is now skipped'); + t.test('this subtest is run', { only: true }); // Switch the context back to execute all tests. t.runOnly(false); - await t.test('this subtest is now run'); + t.test('this subtest is now run'); // Explicitly do not run these tests. - await t.test('skipped subtest 3', { only: false }); - await t.test('skipped subtest 4', { skip: true }); + t.test('skipped subtest 3', { only: false }); + t.test('skipped subtest 4', { skip: true }); }); // The 'only' option is not set, so this test is skipped. @@ -309,13 +304,13 @@ multiple times (e.g. `--test-name-pattern="test 1"`, ```js test('test 1', async (t) => { - await t.test('test 2'); - await t.test('test 3'); + t.test('test 2'); + t.test('test 3'); }); test('Test 4', async (t) => { - await t.test('Test 5'); - await t.test('test 6'); + t.test('Test 5'); + t.test('test 6'); }); ``` @@ -3175,12 +3170,9 @@ before each subtest of the current test. ```js test('top level test', async (t) => { t.beforeEach((t) => t.diagnostic(`about to run ${t.name}`)); - await t.test( - 'This is a subtest', - (t) => { - assert.ok('some relevant assertion here'); - }, - ); + t.test('This is a subtest', (t) => { + assert.ok('some relevant assertion here'); + }); }); ``` @@ -3238,12 +3230,9 @@ after each subtest of the current test. ```js test('top level test', async (t) => { t.afterEach((t) => t.diagnostic(`finished running ${t.name}`)); - await t.test( - 'This is a subtest', - (t) => { - assert.ok('some relevant assertion here'); - }, - ); + t.test('This is a subtest', (t) => { + assert.ok('some relevant assertion here'); + }); }); ``` @@ -3458,10 +3447,8 @@ no-op. test('top level test', (t) => { // The test context can be set to run subtests with the 'only' option. t.runOnly(true); - return Promise.all([ - t.test('this subtest is now skipped'), - t.test('this subtest is run', { only: true }), - ]); + t.test('this subtest is now skipped'); + t.test('this subtest is run', { only: true }); }); ``` @@ -3533,6 +3520,10 @@ added: - v18.0.0 - v16.17.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/56664 + description: This function no longer returns a `Promise`. - version: - v18.8.0 - v16.18.0 @@ -3577,14 +3568,13 @@ 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) => { - await t.test( + 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 9bc090b3836116..489490baef32c6 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -304,7 +304,7 @@ class TestContext { Test, name, options, fn, overrides, ); - return subtest.start(); + 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 4ef804951dc99f..5abbb979667cfd 100644 --- a/test/fixtures/test-runner/output/dot_reporter.snapshot +++ b/test/fixtures/test-runner/output/dot_reporter.snapshot @@ -154,8 +154,6 @@ 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 4ba957d539ce70..f1f5b7573c9814 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -576,7 +576,6 @@ not ok 16 - t.after throws - no subtests * * * - * ... 1..2 not ok 17 - t.beforeEach throws @@ -607,8 +606,6 @@ not ok 17 - t.beforeEach throws * * * - * - * ... # Subtest: 2 not ok 2 - 2 @@ -629,7 +626,6 @@ not ok 17 - t.beforeEach throws * * * - * ... 1..2 not ok 18 - t.afterEach throws @@ -757,7 +753,6 @@ 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 8c267672b9a951..8ee710e845f39c 100644 --- a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot @@ -363,7 +363,6 @@ * * * - * * 1 (*ms) @@ -376,8 +375,6 @@ * * * - * - * * 2 (*ms) @@ -391,7 +388,6 @@ * * * - * * 1 (*ms) @@ -439,7 +435,6 @@ * * * - * * 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 d1868bd4b6eaa9..3b1d15022af704 100644 --- a/test/fixtures/test-runner/output/junit_reporter.snapshot +++ b/test/fixtures/test-runner/output/junit_reporter.snapshot @@ -351,8 +351,7 @@ 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 @@ -362,8 +361,6 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second * * * - * - * } diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot index 044ac4137fa78d..ffbe91759bb859 100644 --- a/test/fixtures/test-runner/output/output.snapshot +++ b/test/fixtures/test-runner/output/output.snapshot @@ -598,8 +598,6 @@ 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 eaa085d97d06d1..7f989f14c619cf 100644 --- a/test/fixtures/test-runner/output/output_cli.snapshot +++ b/test/fixtures/test-runner/output/output_cli.snapshot @@ -606,8 +606,6 @@ 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 17eb9d01451d32..6c11b9ba6d4a39 100644 --- a/test/fixtures/test-runner/output/spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter.snapshot @@ -285,8 +285,6 @@ * * * - * - * * 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 2dd9e92deb1b38..a428b1140ac812 100644 --- a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot @@ -288,8 +288,6 @@ * * * - * - * * timed out async test (*ms) diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js index 8502d4aa99a9b6..f2db563f959e55 100644 --- a/test/parallel/test-runner-module-mocking.js +++ b/test/parallel/test-runner-module-mocking.js @@ -449,13 +449,15 @@ test('mocks are automatically restored', async (t) => { assert.strictEqual(mocked.fn(), 43); }); - const cjsMock = require(cjsFixture); - const esmMock = await import(esmFixture); + t.test('checks original behavior', async () => { + 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) => {