Conversation
… guides Hardens the skill based on eval findings: adds explicit red lines for two-phase legacy-runner lifecycle, literal API substitution (no shims), call-site-only replacement, and coverage threshold preservation. Prunes deltas references to defer field/API mapping to the official Rstest guides and keep only skill-side enforcement rules. Drops the rstest-compat-pitfalls reference (fixed in rstest v0.9.3).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1a45f7e90
ℹ️ 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".
Addresses review feedback: principle 6 phase (b) conflicted with the mixed-mode policy in detect-test-framework.md — when a monorepo migrates one scope at a time, the first migrated scope going green must not trigger repo-wide removal of shared legacy devDeps, since that breaks still-on-Vitest/Jest packages. Now phase (b) splits into scope-local config/setup deletion (immediate on green) and shared devDep removal (deferred until no other scope still relies on them — final scope only in partial migrations). Deltas references updated to match.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20db02f9b5
ℹ️ 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".
…ope-local files Addresses review feedback: vitest.workspace.* is a shared multi-project entrypoint, not a scope-local artifact — deleting it when the first scope goes green breaks discovery/execution for remaining Vitest scopes in mixed-mode monorepos. Same pattern applies to root jest.config.* with projects: [...]. Both move into the shared-infrastructure bucket (drop only when no other scope still relies on them), leaving only per-scope config/setup in the scope-local deletion bucket.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Hardens the
migrate-to-rstestskill based on eval findings — every rule added here maps to a failure class the baseline (no-skill) run hit.jest-migration-deltas.mdandvitest-migration-deltas.mdto defer field/API mapping to the official Rstest guides and keep only skill-side enforcement (legacy file enumeration, snapshot re-record timing).rstest-compat-pitfalls.md(the only entry was the pre-0.9.3 re-export mock bug, now fixed upstream).Eval results (with-skill)
From the skill's evaluation run — every assertion the with-skill config was graded against passed, and the same failure classes drove the principle hardening above.
Aggregate
Skill lift: +21.3 pts pass rate; mean token spend ~42% lower, mean wall time ~53% lower.
Per-eval (with_skill)
Caveat regression checks (with_skill)
Two caveats from the prior eval iteration are gone in this run:
/* istanbul | c8 | v8 ignore */directives in anysrc/file across all 5 fixtures — principle 3 now pins this down.@rsbuild/plugin-reactsubstitute for@vitejs/plugin-react.vitest-multiprojectcorrectly removed the React plugin entirely and configured SWC's automatic JSX runtime viatools.swc.jsc.transform.react.runtime: 'automatic'.Notable agent choices (with_skill)
jest-basicmanual-mock resolution: Rstest resolves aliased manual mocks from<root>/__mocks__/<alias>/rather thansrc/__mocks__/. Agent created__mocks__/@app/logger.tsand left the originalsrc/__mocks__/logger.tsin place.jest-multiprojectglobalSetup wiring: RstestglobalSetupis per-project in multi-project mode and does not inherit from root. Agent placedglobalSetup: ['./rstest.global-setup.ts']under the consuming project;process.env.__DB_SEED__propagates to workers.vitest-multiprojectadapter rewrite:@testing-library/jest-dom/vitestcorrectly replaced withexpect.extend(matchers)from@testing-library/jest-dom/matchers;afterEach(cleanup)preserved.vitest-multiproject-partialisolation: web migrated standalone —packages/web/rstest.config.tsdoes not import fromvitest/config, rootvitest.shared.tsand root devDeps untouched, the remaining three packages still run on Vitest under the samepnpm -r test.Test plan
pnpm exec prettier --check skills/migrate-to-rstest/— all files formattedpnpm run lint:write(pre-commit) — rslint + prettier both clean