Skip to content

Commit 4f1426d

Browse files
MoLowaduh95
authored andcommitted
test_runner: fix hooks test context
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: #63285 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 6a4c4b7 commit 4f1426d

4 files changed

Lines changed: 91 additions & 17 deletions

File tree

lib/internal/test_runner/harness.js

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const {
2121
},
2222
} = require('internal/errors');
2323
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
24-
const { kCancelledByParent, Test, Suite, TestContext, SuiteContext } = require('internal/test_runner/test');
24+
const { kCancelledByParent, Test, Suite } = require('internal/test_runner/test');
2525
const {
2626
parseCommandLine,
2727
reporterScope,
@@ -439,16 +439,7 @@ function getTestContext() {
439439
if (test === undefined || test === reporterScope) {
440440
return undefined;
441441
}
442-
// For hooks (hookType is set), return the test/suite being hooked (the parent)
443-
const actualTest = test.hookType !== undefined ? test.parent : test;
444-
if (actualTest === undefined) {
445-
return undefined;
446-
}
447-
// Return SuiteContext for suites, TestContext for tests
448-
if (actualTest instanceof Suite) {
449-
return new SuiteContext(actualTest);
450-
}
451-
return new TestContext(actualTest);
442+
return test.getCtx();
452443
}
453444

454445
module.exports = {

lib/internal/test_runner/test.js

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,8 +1233,14 @@ class Test extends AsyncResource {
12331233
}
12341234
}
12351235

1236+
#ctx;
1237+
getCtx() {
1238+
this.#ctx ??= new TestContext(this);
1239+
return this.#ctx;
1240+
}
1241+
12361242
getRunArgs() {
1237-
const ctx = new TestContext(this);
1243+
const ctx = this.getCtx();
12381244
return { __proto__: null, ctx, args: [ctx] };
12391245
}
12401246

@@ -1703,6 +1709,11 @@ class TestHook extends Test {
17031709
this.#args = args;
17041710
return super.run();
17051711
}
1712+
1713+
getCtx() {
1714+
return this.parentTest.getCtx();
1715+
}
1716+
17061717
getRunArgs() {
17071718
return this.#args;
17081719
}
@@ -1794,6 +1805,12 @@ class Suite extends Test {
17941805
this.buildPhaseFinished = true;
17951806
}
17961807

1808+
#ctx;
1809+
getCtx() {
1810+
this.#ctx ??= new TestContext(this);
1811+
return this.#ctx;
1812+
}
1813+
17971814
getRunArgs() {
17981815
const ctx = new SuiteContext(this);
17991816
return { __proto__: null, ctx, args: [ctx] };
@@ -1866,6 +1883,4 @@ module.exports = {
18661883
kUnwrapErrors,
18671884
Suite,
18681885
Test,
1869-
TestContext,
1870-
SuiteContext,
18711886
};

test/parallel/test-runner-get-test-context.js

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
const assert = require('node:assert');
4-
const { test, getTestContext, describe, it } = require('node:test');
4+
const {
5+
test,
6+
getTestContext,
7+
describe,
8+
it,
9+
before,
10+
after,
11+
beforeEach,
12+
afterEach,
13+
} = require('node:test');
514

615
// Outside a test — must return undefined
716
assert.strictEqual(getTestContext(), undefined);
@@ -40,6 +49,63 @@ describe('getTestContext returns SuiteContext in suite', () => {
4049
});
4150
});
4251

52+
describe('getTestContext inside hooks', () => {
53+
const suiteName = 'getTestContext inside hooks';
54+
55+
before(common.mustCall((t) => {
56+
const ctx = getTestContext();
57+
assert.ok(ctx !== undefined);
58+
assert.strictEqual(ctx.name, suiteName);
59+
assert.strictEqual(ctx.name, t.name);
60+
}));
61+
62+
beforeEach(common.mustCall(() => {
63+
const ctx = getTestContext();
64+
assert.ok(ctx !== undefined);
65+
assert.strictEqual(ctx.name, suiteName);
66+
}));
67+
68+
afterEach(common.mustCall(() => {
69+
const ctx = getTestContext();
70+
assert.ok(ctx !== undefined);
71+
assert.strictEqual(ctx.name, suiteName);
72+
}));
73+
74+
after(common.mustCall((t) => {
75+
const ctx = getTestContext();
76+
assert.ok(ctx !== undefined);
77+
assert.strictEqual(ctx.name, suiteName);
78+
assert.strictEqual(ctx.name, t.name);
79+
}));
80+
81+
it('runs inside the suite', () => {
82+
const ctx = getTestContext();
83+
assert.ok(ctx !== undefined);
84+
assert.strictEqual(ctx.name, 'runs inside the suite');
85+
});
86+
});
87+
88+
test('getTestContext inside test-level hooks returns the parent test', async (t) => {
89+
const parentName = t.name;
90+
t.beforeEach(common.mustCall(() => {
91+
const ctx = getTestContext();
92+
assert.ok(ctx !== undefined);
93+
assert.strictEqual(ctx.name, parentName);
94+
}));
95+
96+
t.afterEach(common.mustCall(() => {
97+
const ctx = getTestContext();
98+
assert.ok(ctx !== undefined);
99+
assert.strictEqual(ctx.name, parentName);
100+
}));
101+
102+
await t.test('child', () => {
103+
const ctx = getTestContext();
104+
assert.ok(ctx !== undefined);
105+
assert.strictEqual(ctx.name, 'child');
106+
});
107+
});
108+
43109
test('getTestContext works in test body during async operations', async (t) => {
44110
const ctx = getTestContext();
45111
assert.ok(ctx !== undefined);

tools/eslint-rules/must-call-assert.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ function isMustCallOrMustCallAtLeast(str) {
6363
}
6464

6565
function isMustCallOrTest(str) {
66-
return str === 'test' || str === 'it' || isMustCallOrMustCallAtLeast(str);
66+
return str === 'test' || str === 'it' || str === 'describe' || str === 'suite' ||
67+
str === 'before' || str === 'after' || str === 'beforeEach' || str === 'afterEach' ||
68+
isMustCallOrMustCallAtLeast(str);
6769
}
6870

6971
module.exports = {

0 commit comments

Comments
 (0)