test(windows): restore consolidation coverage#452
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 757fdb79f1
ℹ️ 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".
| } | ||
|
|
||
| #[cfg(all(test, not(windows)))] | ||
| #[cfg(test)] |
There was a problem hiding this comment.
Gate the Windows-incompatible test assertion
With this cfg widened, the full consolidation suite now runs on Windows, but untracked_branch_databases_are_recovered_into_the_destination still compares a recovered db_file string to the Unix-only literal "branches/orphan-target.db". On Windows that recovered value is built from a filesystem relative path via relative.to_str(), so it uses backslashes (branches\\orphan-target.db), making the assertion false and keeping Windows CI red whenever this suite runs; normalize the assertion or keep that specific test gated.
Useful? React with 👍 / 👎.
Summary
Follow-up to #450. That PR merged while GitHub still cached head
6a33ffe4; two already-pushed descendants were not included in the merge.This restores the Windows consolidation suite that an overlapping session temporarily gated while diagnosing Windows byte-range locking. The suite must execute, not disappear.
Correctness
src/migrate/consolidate/tests.rson Windows.BEGIN IMMEDIATEpreventing the same process from reading its own database files.Why this follow-up is required
Windows SQLite uses byte-range locks. The real production safety gate rejects consolidation before mutation when holder discovery cannot prove the profile offline. Unit fixtures are isolated and separately test that fail-closed production policy. Holding a Windows
BEGIN IMMEDIATEwhile then copying the fixture database tests an impossible same-process lock arrangement rather than migration behavior, so only that fixture guard is substituted.The prior broad
#[cfg(all(test, not(windows)))]removed all 36 consolidation tests and could let future Windows regressions ship silently. This PR restores the platform-neutral suite and lets CI prove the scoped fixture isolation works.Validation
cargo check --tests --target x86_64-pc-windows-gnu: passedcargo clippy --all-targets --all-features -- -D warnings: passedDo not merge release PR #451 until this follow-up is green and merged.