From 29698674e18110325af80ef14ebf662486cd87ad Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 21 Jul 2023 19:26:27 +0300 Subject: [PATCH 1/5] test_runner: fix global before not called when no global test exists --- lib/internal/test_runner/test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9a333e1457fbc0..07853c5af3a13e 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -821,6 +821,10 @@ class Suite extends Test { return; } + if (this.parent?.hooks.before.length > 0) { + await this.parent.runHook('before', this.parent?.getRunArgs()); + } + await this.runHook('before', hookArgs); const stopPromise = stopTest(this.timeout, this.signal); From 99786aef0a44568907794482d33d9b2607b96c48 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sat, 22 Jul 2023 21:05:54 +0300 Subject: [PATCH 2/5] add test --- .../output/hooks-with-no-global-test.js | 84 +++++++++++++++++++ .../output/hooks-with-no-global-test.snapshot | 44 ++++++++++ test/parallel/test-runner-output.mjs | 1 + 3 files changed, 129 insertions(+) create mode 100644 test/fixtures/test-runner/output/hooks-with-no-global-test.js create mode 100644 test/fixtures/test-runner/output/hooks-with-no-global-test.snapshot diff --git a/test/fixtures/test-runner/output/hooks-with-no-global-test.js b/test/fixtures/test-runner/output/hooks-with-no-global-test.js new file mode 100644 index 00000000000000..69fe25aefc6222 --- /dev/null +++ b/test/fixtures/test-runner/output/hooks-with-no-global-test.js @@ -0,0 +1,84 @@ +'use strict'; +const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test'); +const assert = require("assert"); + +// This file should not have any global tests to reproduce bug #48844 +const testArr = []; + +before(() => testArr.push('global before')); +after(() => { + testArr.push('global after'); + + try { + assert.deepStrictEqual(testArr, [ + 'global before', + 'describe before', + + 'describe beforeEach', + 'describe it 1', + 'describe afterEach', + + 'describe beforeEach', + 'describe test 2', + 'describe afterEach', + + 'describe nested before', + + 'describe beforeEach', + 'describe nested beforeEach', + 'describe nested it 1', + 'describe afterEach', + 'describe nested afterEach', + + 'describe beforeEach', + 'describe nested beforeEach', + 'describe nested test 2', + 'describe afterEach', + 'describe nested afterEach', + + 'describe nested after', + 'describe after', + 'global after', + ]); + } catch (e) { + // TODO(rluvaton): remove the try catch after #48867 is fixed + console.error(e); + process.exit(1); + } +}); + +describe('describe hooks with no global tests', () => { + before(() => { + testArr.push('describe before'); + }); + after(()=> { + testArr.push('describe after'); + }); + beforeEach(() => { + testArr.push('describe beforeEach'); + }); + afterEach(() => { + testArr.push('describe afterEach'); + }); + + it('1', () => testArr.push('describe it 1')); + test('2', () => testArr.push('describe test 2')); + + describe('nested', () => { + before(() => { + testArr.push('describe nested before') + }); + after(() => { + testArr.push('describe nested after') + }); + beforeEach(() => { + testArr.push('describe nested beforeEach') + }); + afterEach(() => { + testArr.push('describe nested afterEach') + }); + + it('nested 1', () => testArr.push('describe nested it 1')); + test('nested 2', () => testArr.push('describe nested test 2')); + }); +}); \ No newline at end of file diff --git a/test/fixtures/test-runner/output/hooks-with-no-global-test.snapshot b/test/fixtures/test-runner/output/hooks-with-no-global-test.snapshot new file mode 100644 index 00000000000000..722a3a4ca2ceac --- /dev/null +++ b/test/fixtures/test-runner/output/hooks-with-no-global-test.snapshot @@ -0,0 +1,44 @@ +TAP version 13 +# Subtest: describe hooks with no global tests + # Subtest: 1 + ok 1 - 1 + --- + duration_ms: * + ... + # Subtest: 2 + ok 2 - 2 + --- + duration_ms: * + ... + # Subtest: nested + # Subtest: nested 1 + ok 1 - nested 1 + --- + duration_ms: * + ... + # Subtest: nested 2 + ok 2 - nested 2 + --- + duration_ms: * + ... + 1..2 + ok 3 - nested + --- + duration_ms: * + type: 'suite' + ... + 1..3 +ok 1 - describe hooks with no global tests + --- + duration_ms: * + type: 'suite' + ... +1..1 +# tests 4 +# suites 2 +# pass 4 +# fail 0 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms * diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 0d670c37bc9319..76c511117ea091 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -36,6 +36,7 @@ const tests = [ { name: 'test-runner/output/describe_it.js' }, { name: 'test-runner/output/describe_nested.js' }, { name: 'test-runner/output/hooks.js' }, + { name: 'test-runner/output/hooks-with-no-global-test.js' }, { name: 'test-runner/output/no_refs.js' }, { name: 'test-runner/output/no_tests.js' }, { name: 'test-runner/output/only_tests.js' }, From 5b2c5d66bba8010c7648c9ad20bf88901d23fae8 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 23 Jul 2023 10:09:54 +0300 Subject: [PATCH 3/5] remove ?. --- lib/internal/test_runner/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 07853c5af3a13e..1fe036cf455a15 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -822,7 +822,7 @@ class Suite extends Test { } if (this.parent?.hooks.before.length > 0) { - await this.parent.runHook('before', this.parent?.getRunArgs()); + await this.parent.runHook('before', this.parent.getRunArgs()); } await this.runHook('before', hookArgs); From 6c5a5af36ac0a7306807c36d0e89dd205be39c6c Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 23 Jul 2023 13:32:13 +0300 Subject: [PATCH 4/5] Update test/fixtures/test-runner/output/hooks-with-no-global-test.js Co-authored-by: Antoine du Hamel --- test/fixtures/test-runner/output/hooks-with-no-global-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/test-runner/output/hooks-with-no-global-test.js b/test/fixtures/test-runner/output/hooks-with-no-global-test.js index 69fe25aefc6222..844aa6ff3c2d59 100644 --- a/test/fixtures/test-runner/output/hooks-with-no-global-test.js +++ b/test/fixtures/test-runner/output/hooks-with-no-global-test.js @@ -81,4 +81,4 @@ describe('describe hooks with no global tests', () => { it('nested 1', () => testArr.push('describe nested it 1')); test('nested 2', () => testArr.push('describe nested test 2')); }); -}); \ No newline at end of file +}); From f2c551c70ca4dd4a3c556303c66e7749040677f9 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 23 Jul 2023 23:29:52 +0300 Subject: [PATCH 5/5] remove ?. --- lib/internal/test_runner/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 1fe036cf455a15..1ca52853c88812 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -821,7 +821,7 @@ class Suite extends Test { return; } - if (this.parent?.hooks.before.length > 0) { + if (this.parent.hooks.before.length > 0) { await this.parent.runHook('before', this.parent.getRunArgs()); }