test(rstest): run the win32 rename mock test in an isolated pool - #857
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fa531fefe
ℹ️ 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".
| ...fixtureProjectTestFiles, | ||
| ...integrationTestFiles, | ||
| ...mcpConformanceTestFiles, | ||
| ...moduleMockTestFiles, |
There was a problem hiding this comment.
Keep the rename test in the local merge gate
Excluding this file from the unit pool moves it exclusively to test:host-filesystem, but neither the standard check script (package.json:26) nor the local-CI legs (scripts/local-ci.mjs:373-376) invoke that pool. Consequently, the documented local merge gate can pass without exercising this regression at all; retain it in an isolated pool that the local gate runs, while optionally also including it in the cross-OS filesystem job.
AGENTS.md reference: AGENTS.md:L147-L154
Useful? React with 👍 / 👎.
Summary
Verify (fast)has failed on every Node leg since #844 (runs 36102691096 on 12a1ddc and 36178852750 on b038e1a).rstest-generated-module-win32-rename.test.tsmocksnode:fs/promiseswithrs.mock, but the unit pool runs withisolate: false. When another file in the same worker (for examplerstest-generated-module-write.test.ts) loadssrc/rstest/generated-module.tsfirst, the cached module keeps the realrenameand both tests fail. Local runs passed only because file-to-worker scheduling differed.This adds a
moduleMockTestFileslist torstest.integration-tests.ts. The unit pool excludes it, andtest:host-filesystem(per-file isolation, already run on ubuntu, macOS, and Windows) includes it. The pool-list guard now checks the entry like every other list.A
rs.spyOnon the builtin plussyncBuiltinESMExportswas tried and dropped: Rspack's interop binds the builtin at load, so the spy never ran.Verification
rstest --config rstest.unit.config.ts --pool.maxWorkers 1with the write test then the win32 test failed with the CI assertions.rstest --config rstest.host-filesystem.config.ts <win32 test>passes 2/2.rstest listshows it in that pool and absent from the unit pool.rstest-pool-lists.test.tspasses 34/34.pnpm lintandpnpm typecheckpass.pnpm test: unit 4293/4293, route-unit 91/91, projection 197/197, integration 1162/1165. The 3 integration failures are Playwright timing assertions in Workbench e2e files this change cannot reach (load average 120 to 180 on the host). A targeted rerun passed 2 of the 3 and failed the third on a different 253 ms assertion.pnpm test:host-filesystemhas 2dev-host-install.test.tsfailures with and without this change. Hosted host-filesystem jobs are green.