From 413493cab9d39dfa00cbc602a8a6d9b938bcd56a Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 28 Feb 2023 22:26:58 +0200 Subject: [PATCH] test_runner: avoid running twice tests in describe PR-URL: https://github.com/nodejs/node/pull/46888 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel --- doc/api/test.md | 14 +++++++++----- lib/internal/test_runner/harness.js | 27 ++++++++++++--------------- test/message/test_runner_hooks.js | 16 ++++++++-------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index 1b39f0c683dab4..00cd2636908647 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -767,7 +767,8 @@ 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} Resolved with `undefined` once the test completes. +* Returns: {Promise} Resolved with `undefined` once + the test completes, or immediately if the test runs within [`describe()`][]. The `test()` function is the value imported from the `test` module. Each invocation of this function results in reporting the test to the {TestsStream}. @@ -776,10 +777,12 @@ The `TestContext` object passed to the `fn` argument can be used to perform actions related to the current test. Examples include skipping the test, adding additional diagnostic information, or creating subtests. -`test()` returns a `Promise` that resolves once the test completes. The return -value can usually be discarded for top level tests. However, the return value -from subtests should be used to prevent the parent test from finishing first -and cancelling the subtest as shown in the following example. +`test()` returns a `Promise` that resolves once the test completes. +if `test()` is called within a `describe()` block, it resolve immediately. +The return value can usually be discarded for top level tests. +However, the return value from subtests should be used to prevent the parent +test from finishing first and cancelling the subtest +as shown in the following example. ```js test('top level test', async (t) => { @@ -1700,6 +1703,7 @@ added: v18.7.0 [`context.diagnostic`]: #contextdiagnosticmessage [`context.skip`]: #contextskipmessage [`context.todo`]: #contexttodomessage +[`describe()`]: #describename-options-fn [`run()`]: #runoptions [`test()`]: #testname-options-fn [describe options]: #describename-options-fn diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 748edd9fa21017..874f1c253e110b 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -1,6 +1,7 @@ 'use strict'; const { ArrayPrototypeForEach, + PromiseResolve, SafeMap, } = primordials; const { @@ -185,31 +186,27 @@ async function startSubtest(subtest) { await subtest.start(); } -function test(name, options, fn) { - const parent = testResources.get(executionAsyncId()) || getGlobalRoot(); - const subtest = parent.createSubtest(Test, name, options, fn); - return startSubtest(subtest); -} - -function runInParentContext(Factory) { +function runInParentContext(Factory, addShorthands = true) { function run(name, options, fn, overrides) { const parent = testResources.get(executionAsyncId()) || getGlobalRoot(); const subtest = parent.createSubtest(Factory, name, options, fn, overrides); - if (parent === getGlobalRoot()) { - startSubtest(subtest); + if (!(parent instanceof Suite)) { + return startSubtest(subtest); } + return PromiseResolve(); } - const cb = (name, options, fn) => { - run(name, options, fn); - }; + const test = (name, options, fn) => run(name, options, fn); + if (!addShorthands) { + return test; + } ArrayPrototypeForEach(['skip', 'todo', 'only'], (keyword) => { - cb[keyword] = (name, options, fn) => { + test[keyword] = (name, options, fn) => { run(name, options, fn, { [keyword]: true }); }; }); - return cb; + return test; } function hook(hook) { @@ -221,7 +218,7 @@ function hook(hook) { module.exports = { createTestTree, - test, + test: runInParentContext(Test, false), describe: runInParentContext(Suite), it: runInParentContext(Test), before: hook('before'), diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js index dab607f862ff5f..bd5adb762d86fd 100644 --- a/test/message/test_runner_hooks.js +++ b/test/message/test_runner_hooks.js @@ -32,7 +32,7 @@ describe('describe hooks', () => { }); it('1', () => testArr.push('1')); - it('2', () => testArr.push('2')); + test('2', () => testArr.push('2')); describe('nested', () => { before(function() { @@ -48,44 +48,44 @@ describe('describe hooks', () => { testArr.push('afterEach ' + this.name); }); it('nested 1', () => testArr.push('nested 1')); - it('nested 2', () => testArr.push('nested 2')); + test('nested 2', () => testArr.push('nested 2')); }); }); describe('before throws', () => { before(() => { throw new Error('before'); }); it('1', () => {}); - it('2', () => {}); + test('2', () => {}); }); describe('after throws', () => { after(() => { throw new Error('after'); }); it('1', () => {}); - it('2', () => {}); + test('2', () => {}); }); describe('beforeEach throws', () => { beforeEach(() => { throw new Error('beforeEach'); }); it('1', () => {}); - it('2', () => {}); + test('2', () => {}); }); describe('afterEach throws', () => { afterEach(() => { throw new Error('afterEach'); }); it('1', () => {}); - it('2', () => {}); + test('2', () => {}); }); describe('afterEach when test fails', () => { afterEach(common.mustCall(2)); it('1', () => { throw new Error('test'); }); - it('2', () => {}); + test('2', () => {}); }); describe('afterEach throws and test fails', () => { afterEach(() => { throw new Error('afterEach'); }); it('1', () => { throw new Error('test'); }); - it('2', () => {}); + test('2', () => {}); }); test('test hooks', async (t) => {