fix(tests): correct stale expected values in Windows-gated unit tests (closes #53, #54) - #55
Merged
Merged
Conversation
Closes #53, #54. Both tests were surfaced by PR #52 preview lane (run 24873800282 on 2026-04-24) as "Windows portability" failures. Investigation revealed that neither is actually platform-dependent \u2014 both are just stale hardcoded expected values that had NEVER EXECUTED IN CI before the preview lane ran them for the first time. ## Bug #5 (uffs-security fnv1a_known_vector) Old: 0x8584899336065430 (wrong) New: 0x85944171F73967E8 (canonical FNV-1a-64 of "foobar" per isthe.com reference) Would fail on any OS; never ran because pipe.rs is `#![cfg(windows)]` and pr-fast Tests runs on ubuntu. ## Bug #6 (uffs-mft test_pipelined_reader_creation) Old: hardcoded 64 * 1024 for all drive types New: derived from DriveType::optimal_chunk_size() itself Eliminates the drift vector so future tuning changes cannot silently break this test again. Same gating as #5: `mod pipelined;` is `#[cfg(windows)]`. ## Lesson (in \u00a710.5) Any test gated behind a platform cfg not exercised by pr-fast.yml is effectively dead code. Phase 5 preview lane is the first CI location that ever runs them.
…cked PR #55 preview run 24889490616 produced the first fully green end-to-end preview bake ever: all 6 jobs succeeded including `manifest` emission. This commit upgrades §10.3 and §10.6 with the real evidence that supersedes the out-of-band sha256 placeholders from PR #52. ## §10.3 updates - **Item #2** (Same-SHA integrity): ticked with real manifest.json contents (git_sha, tested_sha, cargo_lock_sha256, rustc_version, nextest_version, target, build_os, files[]). All 17 files[].sha256 values verified against locally downloaded artifacts on 2026-04-24. - **Item #3** (Nextest archive round-trip): upgraded from 1320/1322 partial to 1322/1322 full pass after bugs #5 and #6 fixed. - **Item #5** (Fork-PR behaviour): upgraded from static-grep-only to a full security-model analysis covering runners, secrets, label-trigger, and concurrency. Live fork-PR bake deferred to first natural external contribution (with explicit TODO list for what to verify). ## §10.6 updates Moved to Resolved: - Phase 5 wrap-up + full green end-to-end bake (2026-04-24) - Issue #53 (uffs-security fnv1a) — closed by this PR - Issue #54 (uffs-mft pipelined buffer) — closed by this PR Remaining Active: Polars-ops xwin-debug DX blocker, Real-world bake gaps, Phase 4b release.yml permissions refactor — all unrelated to Phase 5.
githubrobbi
added a commit
that referenced
this pull request
Apr 24, 2026
Dashboard row was stale after PR #55. Flips to ✅, adds full 4-PR commit chain, and a Phase 5 sub-status paragraph mirroring the Phase 4 sub-status format.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both tests were surfaced by PR #52's preview lane (run 24873800282 on 2026-04-24) as "Windows portability" failures. Investigation revealed that neither is actually platform-dependent — both are just stale hardcoded expected values that had NEVER EXECUTED IN CI before the preview lane ran them for the first time.
Closes #53.
Closes #54.
Root cause (common to both)
Each test is inside a
#[cfg(windows)]gate (module-level forpipe.rs,mod pipelined;declaration forpipelined.rs). In the current CI setup:pr-fast.ymlTestsjob runs onubuntu-22.04→ the gated modules are excluded → tests aren't compiled or executed.pr-fast.ymlWindows compile check→ only runscargo check --target x86_64-pc-windows-msvc→ modules compile but tests don't execute.preview-artifacts.ymlsmoke-windows(first ran 2026-04-24) → first CI location that ever executed these tests on Windows.Result: hardcoded expected values drifted from reality over the lifetime of the codebase without anyone noticing, because the assertions were dead code.
Bug #5 —
uffs-security::pipe::tests::fnv1a_known_vectorThe new value is the canonical FNV-1a-64 of
"foobar"per the reference implementation. Cross-checked with a pure-Python implementation using the sameOFFSET = 0xCBF29CE484222325/PRIME = 0x100000001B3constants as the Rust impl.The old value was just wrong from the start. Not a platform dependency — the original #53 issue's endianness / encoding theories are withdrawn.
Bug #6 —
uffs-mft::io::readers::pipelined::tests::test_pipelined_reader_creationPlus analogous for
HddandUnknown.optimal_chunk_size()returns 2 MiB / 1 MiB / 1 MiB respectively. The old hardcoded64 * 1024was a stale prototype value, never updated when the chunk-size tuning was revised.Deriving the expected value from
optimal_chunk_size()itself eliminates the drift vector: future tuning changes can't silently re-break this test.Not a Windows vs Linux allocation-granularity thing — the original #54 issue's
dwAllocationGranularitytheory is withdrawn.Why one PR not two
Both bugs share the exact same root cause (Windows-gated tests never validated in CI) and have the same mechanical fix shape. Single logical change, single commit, closes both tracking issues. Separate PRs would duplicate the diagnostic narrative without adding value.
Validation plan
Once PR Fast CI goes green, this PR will be labeled
preview-binariesto re-bake the preview lane end-to-end. Expected outcome:smoke-windows: 1322/1322 tests pass (up from 1320/1322 on PR fix(preview): move build-test-archive to windows-latest; reconcile plan doc #52)manifestjob: emitsmanifest.jsonfor the first time (previously blocked bysmoke-windowsfailure vianeeds:coupling)After this PR merges, the Phase 5 wrap-up docs-only PR will swap the out-of-band sha256 evidence in
§10.3for the realmanifest.jsonand close the§10.6Phase 5 wrap-up item.Lesson (captured in
§10.5)Any test gated behind a platform
cfgthat is NOT exercised bypr-fast.ymlis effectively dead code. The Phase 5 preview lane is currently the only CI location that ever exercises#[cfg(windows)]test bodies. Follow-up idea for a future phase: periodic scheduled job that runs full cross-platform nextest onmain(not gated by fast-CI budget) to catch drift proactively in test bodies that the PR lane can't afford to run.