Skip to content

Commit 41191b8

Browse files
committed
fix(core): late-bind hooks and onTestFinished to current file under isolate: false
Continues the #1376 family: under `isolate: false` a context-bound `@rstest/core` API value-copied into a module shared across files goes stale. The file-level hooks (`afterAll`/`beforeAll`/`afterEach`/`beforeEach`) were left bound to the first file's runner, so a shared helper re-exporting a hook (e.g. `export const beforeEach = beforeEach`) silently registered into the first file's torn-down collector from the second file on. `onTestFinished`/ `onTestFailed` had the same staleness against the per-file test runner and threw "can only be called inside a test". Route the hooks through `currentRuntime()` (the per-file `RSTEST_RUNTIME`, like `it`/`describe`), and `onTestFinished`/`onTestFailed` through a new per-file `globalThis.RSTEST_RUNNER` resolved at call time. The remaining `rstest`/`rs` utilities object is the one known, non-idiomatic residual (re-exporting the whole namespace), deferred until a real repro surfaces. Refs #1376.
1 parent 6a4cc28 commit 41191b8

9 files changed

Lines changed: 140 additions & 16 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { expect } from '@rstest/core';
2+
import { onTestFinished, test } from './sharedFinished';
3+
4+
let finishedRan = false;
5+
6+
test('finishedA: shared onTestFinished registers without throwing', () => {
7+
onTestFinished(() => {
8+
finishedRan = true;
9+
});
10+
expect(finishedRan).toBe(false);
11+
});
12+
13+
test('finishedA: shared onTestFinished ran after the previous test', () => {
14+
expect(finishedRan).toBe(true);
15+
});
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { expect } from '@rstest/core';
2+
import { onTestFinished, test } from './sharedFinished';
3+
4+
let finishedRan = false;
5+
6+
test('finishedB: shared onTestFinished still registers without throwing', () => {
7+
onTestFinished(() => {
8+
finishedRan = true;
9+
});
10+
expect(finishedRan).toBe(false);
11+
});
12+
13+
test('finishedB: shared onTestFinished still ran after the previous test', () => {
14+
expect(finishedRan).toBe(true);
15+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { expect } from '@rstest/core';
2+
import { beforeEach, test } from './sharedHooks';
3+
4+
let ran = false;
5+
beforeEach(() => {
6+
ran = true;
7+
});
8+
9+
test('hookA: shared beforeEach ran for this file', () => {
10+
expect(ran).toBe(true);
11+
});
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect } from '@rstest/core';
2+
import { beforeAll, beforeEach, afterEach, test } from './sharedHooks';
3+
4+
let beforeAllRan = false;
5+
let beforeEachRan = false;
6+
let afterEachRuns = 0;
7+
8+
beforeAll(() => {
9+
beforeAllRan = true;
10+
});
11+
beforeEach(() => {
12+
beforeEachRan = true;
13+
});
14+
afterEach(() => {
15+
afterEachRuns += 1;
16+
});
17+
18+
test('hookB: shared beforeAll/beforeEach ran for this file', () => {
19+
expect(beforeAllRan).toBe(true);
20+
expect(beforeEachRan).toBe(true);
21+
});
22+
23+
test('hookB: shared afterEach ran after the previous test', () => {
24+
expect(afterEachRuns).toBe(1);
25+
});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// A shared helper that value-captures `@rstest/core`'s `onTestFinished` at
2+
// module top-level. Under `isolate: false` this module is evaluated once per
3+
// worker, but `@rstest/core` is reset per file. A captured `onTestFinished`
4+
// must resolve the CURRENT file's runner — otherwise from the second file on it
5+
// throws "onTestFinished() can only be called inside a test" (the stale runner's
6+
// current test is undefined). See https://github.com/web-infra-dev/rstest/issues/1376.
7+
import { onTestFinished as rstestOnTestFinished, test } from '@rstest/core';
8+
9+
export const onTestFinished = rstestOnTestFinished;
10+
export { test };
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// A shared helper that value-captures `@rstest/core`'s file-level hooks at
2+
// module top-level — the common pattern of re-exporting hooks (often bundled
3+
// onto an extended `test`) from one workspace module.
4+
//
5+
// Under `isolate: false` this module is evaluated once per worker (#1373), but
6+
// `@rstest/core` is reset per file. A captured `beforeEach` must still register
7+
// against the CURRENT file's runner — otherwise from the second file on the
8+
// hook silently registers into the first file's torn-down collector and never
9+
// runs. See https://github.com/web-infra-dev/rstest/issues/1376.
10+
import {
11+
afterEach as rstestAfterEach,
12+
beforeAll as rstestBeforeAll,
13+
beforeEach as rstestBeforeEach,
14+
test,
15+
} from '@rstest/core';
16+
17+
export const beforeEach = rstestBeforeEach;
18+
export const afterEach = rstestAfterEach;
19+
export const beforeAll = rstestBeforeAll;
20+
export { test };

e2e/no-isolate/moduleSharing.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ describe('module state sharing under isolate: false', () => {
1616
// `@rstest/core` API captured in a module shared across files must still
1717
// resolve the current file's context, not the first file's torn-down one —
1818
// `test.extend(...)` against the current runner (extendA/extendB +
19-
// sharedFixture.ts) and `expect.poll(...)` against the current test
20-
// (pollA/pollB + sharedExpect.ts).
19+
// sharedFixture.ts), `expect.poll(...)` against the current test (pollA/pollB
20+
// + sharedExpect.ts), the file-level hooks (hookA/hookB + sharedHooks.ts),
21+
// and `onTestFinished` (finishedA/finishedB + sharedFinished.ts).
2122
it('shares imported module state across files while re-running setup', async ({
2223
onTestFinished,
2324
}) => {

packages/core/src/runtime/runner/index.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,27 @@ import { TestRunner } from './runner';
1212
import { createRuntimeAPI } from './runtime';
1313
import { traverseUpdateTest } from './task';
1414

15+
declare global {
16+
/**
17+
* The current test file's `TestRunner`. Reassigned per file (see
18+
* `createRunner`), so `onTestFinished`/`onTestFailed` captured by value in a
19+
* module shared across files under `isolate: false` (e.g.
20+
* `export const onTestFinished = onTestFinished`) resolve the running file's
21+
* runner instead of a torn-down one — the execution-phase sibling of
22+
* `RSTEST_RUNTIME`. See https://github.com/web-infra-dev/rstest/issues/1376.
23+
*/
24+
var RSTEST_RUNNER: TestRunner | undefined;
25+
}
26+
27+
const currentRunner = (): TestRunner => {
28+
if (!globalThis.RSTEST_RUNNER) {
29+
throw new Error(
30+
'Rstest runner is not registered yet, please make sure you are running in a rstest environment.',
31+
);
32+
}
33+
return globalThis.RSTEST_RUNNER;
34+
};
35+
1536
export const getFileTaskId = (testPath: string): string => {
1637
return `file:${testPath}`;
1738
};
@@ -45,15 +66,21 @@ export function createRunner({
4566
runtimeConfig: workerState.runtimeConfig,
4667
});
4768
const testRunner: TestRunner = new TestRunner(taskContext);
69+
// Publish this file's runner as the live indirection target (mirrors
70+
// `RSTEST_RUNTIME`), so a stale cross-file `onTestFinished`/`onTestFailed`
71+
// resolves the running file's runner. See FIX above and #1376.
72+
globalThis.RSTEST_RUNNER = testRunner;
4873

4974
return {
5075
api: {
5176
...runtime.api,
52-
onTestFinished: (fn, timeout) => {
53-
testRunner.onTestFinished(testRunner.getCurrentTest(), fn, timeout);
77+
onTestFinished: (...args) => {
78+
const runner = currentRunner();
79+
runner.onTestFinished(runner.getCurrentTest(), ...args);
5480
},
55-
onTestFailed: (fn, timeout) => {
56-
testRunner.onTestFailed(testRunner.getCurrentTest(), fn, timeout);
81+
onTestFailed: (...args) => {
82+
const runner = currentRunner();
83+
runner.onTestFailed(runner.getCurrentTest(), ...args);
5784
},
5885
},
5986
runner: {

packages/core/src/runtime/runner/runtime.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -729,16 +729,16 @@ export const createRuntimeAPI = ({
729729
describe,
730730
it,
731731
test: it,
732-
// Hooks bind eagerly to this file's `runtimeInstance` (not `currentRuntime()`)
733-
// because they are never reachable through a persisted `test.extend(...)`
734-
// result — `createTestAPI` exposes only test/each/for/extend, never hooks —
735-
// so they only flow through the per-file-reassigned `RSTEST_API` proxy. If a
736-
// hook is ever attached to the test API, switch these to `currentRuntime()`
737-
// too, or it would reopen https://github.com/web-infra-dev/rstest/issues/1376.
738-
afterAll: runtimeInstance.afterAll,
739-
beforeAll: runtimeInstance.beforeAll,
740-
afterEach: runtimeInstance.afterEach,
741-
beforeEach: runtimeInstance.beforeEach,
732+
// Hooks are late-bound through `currentRuntime()` for the same reason as
733+
// `it`/`describe`: a hook captured by value in a module shared across
734+
// files under `isolate: false` (e.g. `export const beforeEach = beforeEach`,
735+
// or attached onto an extended `test`) must register against the current
736+
// file's runner, not the file that first evaluated the helper.
737+
// See https://github.com/web-infra-dev/rstest/issues/1376.
738+
afterAll: (...args) => currentRuntime().afterAll(...args),
739+
beforeAll: (...args) => currentRuntime().beforeAll(...args),
740+
afterEach: (...args) => currentRuntime().afterEach(...args),
741+
beforeEach: (...args) => currentRuntime().beforeEach(...args),
742742
},
743743
instance: runtimeInstance,
744744
};

0 commit comments

Comments
 (0)