fix(core): surface testPath with OS-native separators#1467
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2176e67bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return [formatTestEntryName(relativePath), entry]; | ||
| // The entry name (key) stays slash-free; the value is the runtime | ||
| // `testPath`, surfaced to users as an OS-native path (#1465). | ||
| return [formatTestEntryName(relativePath), toNativePath(entry)]; |
There was a problem hiding this comment.
On Windows when the CommonJS/non-output.module loader is used, returning a native testPath here makes loadModule inject __filename with backslashes, but it still derives __dirname with pathe.dirname(testPath) in packages/core/src/runtime/worker/loadModule.ts, which normalizes back to C:/.... That breaks the normal Node invariant that __dirname === node:path.dirname(__filename) for CJS tests once this native path flows through; update the dirname injection to use native path handling when testPath is native.
Useful? React with 👍 / 👎.
fi3ework
force-pushed
the
fix-testpath-native-path
branch
2 times, most recently
from
9891a16 to
7877a1f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7877a1f03d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `expect.getState().testPath` is user-facing; expose the OS-native | ||
| // path (equal to `import.meta.filename`). Internal consumers | ||
| // (snapshot, reporter, related) keep the POSIX `test.testPath` (#1465). | ||
| testPath: toNativePath(test.testPath), |
There was a problem hiding this comment.
This only pins the global expect state to the native path for the duration of a test. When a test uses the public per-test context.expect, createExpect immediately calls freshExpectState, whose getter still returns workerState.testPath in the internal POSIX form, so on Windows context.expect.getState().testPath remains forward-slashed while the global expect.getState().testPath matches import.meta.filename; the same getter is also what top-level or pre-test reads use before this line runs.
Useful? React with 👍 / 👎.
`expect.getState().testPath` and the lifecycle hook `ctx.filepath` are real on-disk paths surfaced to users, but they were emitted in POSIX form (forward slashes), so on Windows they diverged from `import.meta.filename` / `__filename` (which the rspack plugin injects in native form). A user comparing them — e.g. `expect.getState().testPath === import.meta.filename` — failed on Windows. Internally `testPath` must stay POSIX: the snapshot client, reporter relative paths, and the related-graph lookup all run it through `pathe`, which only understands `/`. So convert to native separators only at the two user-facing boundaries (the `setState` testPath and the hook context `filepath`) via a small `toNativePath` helper, leaving every internal consumer untouched. Closes #1465
fi3ework
force-pushed
the
fix-testpath-native-path
branch
from
7877a1f to
f33a39b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f33a39be7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const toNativePath = (filePath: string): string => | ||
| nativeNormalize(filePath); |
There was a problem hiding this comment.
When browser mode runs on Windows, this helper is bundled through @rstest/core/internal/browser-runtime (browserRuntime.ts -> runtime/api -> runner.ts), and that web build uses pluginNodePolyfill() in packages/core/rslib.config.ts, which maps node:path to path-browserify; its normalize() is POSIX-only and leaves C:/... unchanged. The browser client also builds workerState.testPath with pathe.normalize, so expect.getState().testPath/ctx.filepath in browser tests will still be forward-slashed while the Rstest plugin injects native __filename/import.meta.filename, leaving the Windows browser-mode case unfixed.
Useful? React with 👍 / 👎.
1 participant
Summary
expect.getState().testPathand the lifecycle hookctx.filepathare real on-disk paths surfaced to users. They were emitted in POSIX form (forward slashes), so on Windows they diverged fromimport.meta.filename/__filename(which the rspack plugin injects in OS-native form) and from any path a user builds withnode:path. A user-side comparison likeexpect.getState().testPath === import.meta.filenametherefore failed on Windows. AtestPathis a real filesystem path, so blanket-forcing POSIX onto it is semantically wrong — these user-facing surfaces should carry OS-native separators.Internally
testPathmust stay POSIX: the snapshot client, the reporter relative paths, and the related-graph lookup all run it throughpathe, which only understands/. So the conversion happens only at the two user-facing boundaries — thesetStatetestPathand the hook contextfilepath— via a smalltoNativePathhelper, leaving every internal consumer untouched. The change is a no-op on POSIX platforms (native separator is already/).Related Links
Checklist