refactor(tests): adopt libtest-mimic and custom snapshot_test crate#339
refactor(tests): adopt libtest-mimic and custom snapshot_test crate#339branchseer merged 7 commits intomainfrom
Conversation
…nesses Replace ad-hoc argument parsing with libtest-mimic to get proper cargo test integration (--filter, --list, --exact, formatted output). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Create `crates/snapshot_test` with `check_snapshot` and `check_json_snapshot` that return `Result<(), String>` with unified diffs, integrating cleanly with libtest-mimic's `Failed` type. - Drop `insta` dependency and all its transitive deps. - Plan JSON snapshots use `.jsonc` extension with `// vp run ...` comment headers; e2e snapshots keep `.snap`. - Strip insta YAML headers from all existing snapshot files. - Exclude snapshot directories from oxfmt to prevent trailing comma insertion in `.jsonc` files. - Support `UPDATE_SNAPSHOTS=1` to accept new output in-place. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
branchseer
force-pushed
the
use-libtest-mimic
branch
from
fe0e4a1 to
e9e3132
Compare
![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: fe0e4a11c3
ℹ️ 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".
- Normalize \r\n to \n when reading snapshot files for Windows compat - Replace redundant closures with Into::into - Remove unfulfilled #[expect] attributes - Add disallowed_methods to snapshot_test module-level expect Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
libtest-mimic defaults to `available_parallelism()` threads, but the previous custom `fn main()` harness ran fixtures sequentially. Running e2e fixtures in parallel causes PTY and signal-routing contention, especially on the ctrl-c test, which intermittently fails on Linux/musl CI when task B gets launched before vt processes the ctrl-c signal. Force `test_threads = 1` unless the user explicitly overrides via `--test-threads`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
macOS and Windows CI were unaffected by the ctrl-c flakiness. Scope the single-thread override to Linux only so other platforms keep the faster parallel execution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
branchseer
changed the title
Explain why we didn't use `insta`: its panic-based assertion API prints the diff to stderr, which doesn't make it into libtest-mimic's failure summary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace stale `INSTA_UPDATE=always` references with `UPDATE_SNAPSHOTS=1` in CLAUDE.md and CONTRIBUTING.md to match the new snapshot_test crate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 775c31b255
ℹ️ 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".
| value: &impl Serialize, | ||
| ) -> Result<(), String> { | ||
| let json = serde_json::to_string_pretty(value).expect("failed to serialize snapshot value"); | ||
| self.check_snapshot(&format!("{name}.jsonc"), &format!("// {comment}\n{json}\n")) |
There was a problem hiding this comment.
.jsonc.new artifacts
JSON snapshot assertions now produce {name}.jsonc.new on failure, but the repository ignore rule only covers *.snap.new. That means a normal failing plan snapshot run leaves untracked .jsonc.new files in the worktree that are easy to accidentally commit, adding noise and friction to snapshot workflows.
Useful? React with 👍 / 👎.
1 participant
Why
Before this PR, both
e2e_snapshotsandplan_snapshotsusedharness = falsewith a hand-rolledfn main()that:args().nth(1)), and nothing else — no--list, no--exact, no--ignored, no--test-threads.insta's own stderr diff output, with no structured "passed / failed / filtered out" summary.Two problems fall out of that:
AI-agent friction. Agents (Claude Code, Cursor, etc.) drive tests through
cargo testand expect the built-in harness contract: filter a subset with--exact, enumerate with--list, read the standard "N passed; M failed" summary to know what happened. Custom output and missing flags force the agent to grep through ad-hoc progress prints or re-run the full suite just to isolate one fixture. Moving tolibtest-mimicremoves that friction — the harness behaves exactly like a normal Rust test binary.No parallelism.
plan_snapshotsin particular is CPU-bound and embarrassingly parallel per fixture, but the old harness ran it single-threaded.libtest-mimicruns fixtures in parallel by default (viaavailable_parallelism()), using the hardware the CI already pays for. (E2E tests spawn PTY children and send signals; on Linux we pin them back to single-threaded — see the "Platform-specific fixes" section below.)Summary
Two-part migration of the e2e and plan snapshot test harnesses:
1. Adopt
libtest-mimicfor propercargo testintegrationMigrating to
libtest-mimicgives:cargo testflag support:--filter,--list,--exact,--ignored,--test-threadsrunning N tests/test X ... ok/test result: ok. ... passed; ... failed)2. Replace
instawith a customsnapshot_testcrateCreated
crates/snapshot_testwith two methods that returnResult<(), String>— containing a unified diff — so failures integrate cleanly withlibtest-mimic::Failed:check_snapshot(name, actual)— compares raw textcheck_json_snapshot(name, comment, value)— serializes to pretty JSON, prepends// {comment}header, writes to.jsoncKey behaviors:
instadependency and its transitive depsfailures:section of test output (seecrates/snapshot_test/README.mdfor whyinstadidn't fit).jsoncextension with// run ...command headers (e.g.// run build --recursive); e2e snapshots keep.snapinstaYAML headers stripped from all 310 snapshot filesUPDATE_SNAPSHOTS=1env var accepts new output in-place (replacesINSTA_UPDATE)oxfmtvia.oxfmtrc.jsonto prevent trailing-comma insertion in.jsoncfilesPlatform-specific fixes
test_threads = 1), matching the pre-PR custom-harness behavior.libtest-mimicdefaults toavailable_parallelism(), which caused PTY/signal-routing races in thectrl-ctest. macOS and Windows keep parallel execution.Test plan
cargo test -p vite_task_plan --test plan_snapshots— 45 tests passcargo test -p vite_task_bin --test e2e_snapshots— 36 tests passcargo test -- --listlists all fixture namescargo test -- --exact <name>filters correctly.snap/.jsoncfile produces a unified diff in the failure message, pointing to the.snap.new/.jsonc.newfileUPDATE_SNAPSHOTS=1accepts new output in-place and clears.newfiles🤖 Generated with Claude Code