From fb8cb1d7291b334e4dfd7f44d78718f286ac45a4 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sun, 5 May 2024 01:55:50 +0300 Subject: [PATCH] test_runner: preserve hook promise when executed twice PR-URL: https://github.com/nodejs/node/pull/52791 Reviewed-By: Chemi Atlow Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum --- lib/internal/test_runner/test.js | 8 +-- lib/internal/util.js | 9 ++-- test/fixtures/test-runner/output/hooks.js | 51 ++++++++++++++++++- .../test-runner/output/hooks.snapshot | 22 ++++++-- .../output/hooks_spec_reporter.snapshot | 8 ++- 5 files changed, 86 insertions(+), 12 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index d6855b300b8674..737f45a4163799 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -91,6 +91,8 @@ let kResistStopPropagation; let findSourceMap; let noopTestStream; +const kRunOnceOptions = { __proto__: null, preserveReturnValue: true }; + function lazyFindSourceMap(file) { if (findSourceMap === undefined) { ({ findSourceMap } = require('internal/source_map/source_map_cache')); @@ -567,7 +569,7 @@ class Test extends AsyncResource { // eslint-disable-next-line no-use-before-define const hook = new TestHook(fn, options); if (name === 'before' || name === 'after') { - hook.run = runOnce(hook.run); + hook.run = runOnce(hook.run, kRunOnceOptions); } if (name === 'before' && this.startTime !== null) { // Test has already started, run the hook immediately @@ -710,7 +712,7 @@ class Test extends AsyncResource { if (this.parent?.hooks.afterEach.length > 0 && !this.skipped) { await this.parent.runHook('afterEach', hookArgs); } - }); + }, kRunOnceOptions); let stopPromise; @@ -1081,7 +1083,7 @@ class Suite extends Test { const hookArgs = this.getRunArgs(); let stopPromise; - const after = runOnce(() => this.runHook('after', hookArgs)); + const after = runOnce(() => this.runHook('after', hookArgs), kRunOnceOptions); try { this.parent.activeSubtests++; await this.buildSuite; diff --git a/lib/internal/util.js b/lib/internal/util.js index 7bf210afce1078..29e425e86ca67c 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -514,12 +514,15 @@ function isInsideNodeModules() { return false; } -function once(callback) { +function once(callback, { preserveReturnValue = false } = kEmptyObject) { let called = false; + let returnValue; return function(...args) { - if (called) return; + if (called) return returnValue; called = true; - return ReflectApply(callback, this, args); + const result = ReflectApply(callback, this, args); + returnValue = preserveReturnValue ? result : undefined; + return result; }; } diff --git a/test/fixtures/test-runner/output/hooks.js b/test/fixtures/test-runner/output/hooks.js index 8a397726feeff4..0d67079b287305 100644 --- a/test/fixtures/test-runner/output/hooks.js +++ b/test/fixtures/test-runner/output/hooks.js @@ -2,6 +2,7 @@ const common = require('../../../common'); const assert = require('assert'); const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test'); +const { setTimeout } = require('node:timers/promises'); before((t) => t.diagnostic('before 1 called')); after((t) => t.diagnostic('after 1 called')); @@ -148,7 +149,6 @@ test('test hooks', async (t) => { })); }); - test('test hooks - no subtests', async (t) => { const testArr = []; @@ -253,5 +253,54 @@ describe('run after when before throws', () => { it('1', () => {}); }); + +test('test hooks - async', async (t) => { + const testArr = []; + + t.before(async (t) => { + testArr.push('before starting ' + t.name); + await setTimeout(10); + testArr.push('before ending ' + t.name); + }); + t.after(async (t) => { + testArr.push('after starting ' + t.name); + await setTimeout(10); + testArr.push('after ending ' + t.name); + }); + t.beforeEach(async (t) => { + testArr.push('beforeEach starting ' + t.name); + await setTimeout(10); + testArr.push('beforeEach ending ' + t.name); + }); + t.afterEach(async (t) => { + testArr.push('afterEach starting ' + t.name); + await setTimeout(10); + testArr.push('afterEach ending ' + t.name); + }); + await t.test('1', async () => { + testArr.push('1 starting'); + await setTimeout(10); + testArr.push('1 ending'); + }); + await t.test('2', async () => { + testArr.push('2 starting'); + await setTimeout(10); + testArr.push('2 ending'); + }); + + t.after(common.mustCall(() => { + assert.deepStrictEqual(testArr, [ + 'before starting test hooks - async', 'before ending test hooks - async', + 'beforeEach starting 1', 'beforeEach ending 1', + '1 starting', '1 ending', + 'afterEach starting 1', 'afterEach ending 1', + 'beforeEach starting 2', 'beforeEach ending 2', + '2 starting', '2 ending', + 'afterEach starting 2', 'afterEach ending 2', + 'after starting test hooks - async', 'after ending test hooks - async', + ]); + })); +}); + before((t) => t.diagnostic('before 2 called')); after((t) => t.diagnostic('after 2 called')); diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot index b9fab9e186a8ca..6b9d6d26a90e39 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -767,14 +767,30 @@ not ok 24 - run after when before throws * * ... -1..24 +# Subtest: test hooks - async + # Subtest: 1 + ok 1 - 1 + --- + duration_ms: * + ... + # Subtest: 2 + ok 2 - 2 + --- + duration_ms: * + ... + 1..2 +ok 25 - test hooks - async + --- + duration_ms: * + ... +1..25 # before 1 called # before 2 called # after 1 called # after 2 called -# tests 49 +# tests 52 # suites 12 -# pass 19 +# pass 22 # fail 27 # cancelled 3 # skipped 0 diff --git a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot index ce748bba437050..b5c5ab7d1965e5 100644 --- a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot @@ -392,13 +392,17 @@ * * + test hooks - async + 1 (*ms) + 2 (*ms) + test hooks - async (*ms) before 1 called before 2 called after 1 called after 2 called - tests 49 + tests 52 suites 12 - pass 19 + pass 22 fail 27 cancelled 3 skipped 0