Skip to content

refactor(migrate-to-rstest): tighten principles and defer to official guides#51

Merged
fi3ework merged 4 commits intomainfrom
refine-migrate-to-rstest-from-eval
Apr 21, 2026
Merged

refactor(migrate-to-rstest): tighten principles and defer to official guides#51
fi3ework merged 4 commits intomainfrom
refine-migrate-to-rstest-from-eval

Conversation

@fi3ework
Copy link
Copy Markdown
Member

Summary

Hardens the migrate-to-rstest skill based on eval findings — every rule added here maps to a failure class the baseline (no-skill) run hit.

  • SKILL.md principles: adds explicit red lines for (6) two-phase legacy-runner lifecycle, (7) literal API substitution with no shims, (8) call-site-only replacement (no test-name mutation), (9) coverage thresholds are not negotiable.
  • Deltas references: pruned jest-migration-deltas.md and vitest-migration-deltas.md to defer field/API mapping to the official Rstest guides and keep only skill-side enforcement (legacy file enumeration, snapshot re-record timing).
  • Global API reference: rewritten around three red lines (no shims, no test-name mutation, global-only scope) instead of restating the mapping table.
  • Removed: 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

Config Pass Pct Tokens (mean) Time (s)
with_skill 64/64 100% ~45,000 ~318
without_skill 48/61 78.7% 77,675 676

Skill lift: +21.3 pts pass rate; mean token spend ~42% lower, mean wall time ~53% lower.

Per-eval (with_skill)

Eval Pass Time
jest-basic 11/11 283.9s
vitest-basic 12/12 175.7s
vitest-multiproject 16/16 238.2s
vitest-multiproject-partial 12/12 219.3s
jest-multiproject 13/13 552.4s

Caveat regression checks (with_skill)

Two caveats from the prior eval iteration are gone in this run:

  1. No /* istanbul | c8 | v8 ignore */ directives in any src/ file across all 5 fixtures — principle 3 now pins this down.
  2. No @rsbuild/plugin-react substitute for @vitejs/plugin-react. vitest-multiproject correctly removed the React plugin entirely and configured SWC's automatic JSX runtime via tools.swc.jsc.transform.react.runtime: 'automatic'.

Notable agent choices (with_skill)

  • jest-basic manual-mock resolution: Rstest resolves aliased manual mocks from <root>/__mocks__/<alias>/ rather than src/__mocks__/. Agent created __mocks__/@app/logger.ts and left the original src/__mocks__/logger.ts in place.
  • jest-multiproject globalSetup wiring: Rstest globalSetup is per-project in multi-project mode and does not inherit from root. Agent placed globalSetup: ['./rstest.global-setup.ts'] under the consuming project; process.env.__DB_SEED__ propagates to workers.
  • vitest-multiproject adapter rewrite: @testing-library/jest-dom/vitest correctly replaced with expect.extend(matchers) from @testing-library/jest-dom/matchers; afterEach(cleanup) preserved.
  • vitest-multiproject-partial isolation: web migrated standalone — packages/web/rstest.config.ts does not import from vitest/config, root vitest.shared.ts and root devDeps untouched, the remaining three packages still run on Vitest under the same pnpm -r test.

Test plan

  • pnpm exec prettier --check skills/migrate-to-rstest/ — all files formatted
  • pnpm run lint:write (pre-commit) — rslint + prettier both clean
  • Spot-check SKILL.md workflow numbering / section titles render correctly on the docs surface

… 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).
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread skills/migrate-to-rstest/SKILL.md Outdated
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.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread skills/migrate-to-rstest/references/vitest-migration-deltas.md Outdated
…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.
@fi3ework
Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ 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".

@fi3ework fi3ework requested a review from 9aoy April 20, 2026 11:59
@fi3ework fi3ework merged commit 7b67abf into main Apr 21, 2026
4 checks passed
@fi3ework fi3ework deleted the refine-migrate-to-rstest-from-eval branch April 21, 2026 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants