Skip to content

test(build): #231 hermetic regression gate for #147 cache-survives-tar-extract DoD#232

Merged
zackees merged 1 commit into
mainfrom
test/231-cache-survives-tar-extract
May 11, 2026
Merged

test(build): #231 hermetic regression gate for #147 cache-survives-tar-extract DoD#232
zackees merged 1 commit into
mainfrom
test/231-cache-survives-tar-extract

Conversation

@zackees
Copy link
Copy Markdown
Member

@zackees zackees commented May 11, 2026

Summary

Closes #231. Adds the hermetic regression gate called out in the issue: a three-case integration test that pins the invariants whose union closed #147.

  • fingerprint_survives_tar_roundtrip — tar a project, extract into a fresh location, stomp mtimes, assert hash_watch_set_stamps returns the same hash. Catches any future change that re-introduces mtime into the watch hash.
  • fingerprint_survives_workspace_relocation — extract under a different parent directory; assert the hash is unchanged. Pins the relative_path_for_hash invariant against absolute-path leakage.
  • compiler_signature_survives_toolchain_path_change — two compiler paths with the same filename produce identical build_rebuild_signatures; a third with a different filename produces a distinct one. Pins the compiler_identity substitution at compiler.rs:307.

Together with commit 2e8bc4c (AC#5 ≤ 50 ms warm threshold in bench-fastled-examples), this gives #147 both an empirical speed gate and a hermetic correctness gate.

The tests pass on main today — the fixes are already in tree. Their job is to turn RED instead of silently going slow on the next AC run if anyone re-introduces mtime or absolute paths into the fingerprint contract.

Test plan

  • uv run --script ci/test.py -p fbuild-build --test cache_survives_tar_extract — all 3 pass locally
  • uv run --script ci/test.py -p fbuild-build — full crate suite passes (acceptance tests ignored as expected for CI-only)
  • uv run soldr cargo clippy --workspace --all-targets -- -D warnings — clean

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests

  • Added regression tests to ensure build cache fingerprinting remains stable across project extraction, relocation to different directories, and varying toolchain paths.

Review Change Stack

…r-extract DoD

Adds `crates/fbuild-build/tests/cache_survives_tar_extract.rs` with three cases
that pin the invariants closed by #147:

1. `fingerprint_survives_tar_roundtrip` -- tars a project, extracts into a
   fresh location, stomps mtimes to a known-different value, asserts
   `hash_watch_set_stamps` returns the same hash. Catches any regression that
   reintroduces mtime into the watch hash.
2. `fingerprint_survives_workspace_relocation` -- extracts under a different
   parent path; asserts the hash is unchanged. Pins the
   `relative_path_for_hash` invariant against absolute-path leakage.
3. `compiler_signature_survives_toolchain_path_change` -- two compiler paths
   with the same filename produce identical `build_rebuild_signature`s; a
   third with a different filename produces a distinct one. Pins the
   `compiler_identity` substitution at compiler.rs:307.

Adds `tar = { workspace = true }` to fbuild-build dev-dependencies. The tests
pass on main today; their job is to turn RED instead of silently going slow
in `bench-fastled-examples` if anyone re-introduces mtime or absolute paths
into the fingerprint contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR adds a regression test suite and corresponding tar dev dependency to validate that fbuild's build cache fingerprinting remains stable across cross-runner scenarios: tar extraction (which resets mtimes), workspace relocation (which changes absolute paths), and toolchain path changes. Three hermetic test cases pin invariants on hash_watch_set_stamps and build_rebuild_signature that must hold for cache reuse across CI runs.

Changes

Build Cache Fingerprint Invariance

Layer / File(s) Summary
Dev Dependency Addition
crates/fbuild-build/Cargo.toml
tar crate added to [dev-dependencies] with workspace = true.
Test Harness & Helpers
crates/fbuild-build/tests/cache_survives_tar_extract.rs (lines 1–83)
Module documentation and embedded C/C++ source fixtures establish invariants being pinned. Helper functions populate project directories from sources, construct FingerprintWatch configs, create/extract tarballs, reset mtimes, and read filesystem timestamps.
Cache Invariance Test Cases
crates/fbuild-build/tests/cache_survives_tar_extract.rs (lines 85–193)
fingerprint_survives_tar_roundtrip validates watch-set hashes survive tar roundtrip with mtime reset. fingerprint_survives_workspace_relocation validates watch-set hashes survive extraction under different parent paths. compiler_signature_survives_toolchain_path_change validates rebuild signatures survive absolute toolchain path changes while still differentiating compiler identities (e.g., gcc vs clang).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • FastLED/fbuild#155: Modifies the fingerprinting logic and compiler-path handling that these new tests directly validate for invariance across tar extraction, relocation, and toolchain path changes.

Poem

🐰 A tar-archive goes round and round,
Its mtimes lost, its paths unbound,
Yet hashes hold their steady ground—
No mtime tricks, no absolute frowns,
Just content truth in every round! 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a hermetic regression test for issue #147's cache-survives-tar-extract definition of done, which matches the changeset's addition of a new regression test file.
Linked Issues check ✅ Passed The PR fully implements the three test cases required by #231: fingerprint_survives_tar_roundtrip, fingerprint_survives_workspace_relocation, and compiler_signature_survives_toolchain_path_change, all of which directly validate the three invariants protecting the fbuild cache across tar-extract and workspace relocation scenarios specified in #147.
Out of Scope Changes check ✅ Passed All changes are in scope: the tar dev-dependency addition is necessary for the new tests to run on Windows, and the new test file directly implements the hermetic regression gate required by #231 without introducing unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/231-cache-survives-tar-extract

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zackees zackees merged commit 8b2c8f2 into main May 11, 2026
80 of 81 checks passed
zackees added a commit that referenced this pull request May 11, 2026
#233)

Adds `cache_survives_tar_extract_uno` to `crates/fbuild-build/tests/avr_build.rs`
as the integration-level companion to the hermetic unit test landed in #232. The
unit test exercises the watch-set hash and `build_rebuild_signature` directly;
this test drives a real AVR Uno build through `AvrOrchestrator.build()`, tars
the resulting project tree, extracts it under a different parent directory,
stomps mtimes, and rebuilds. It then asserts the warm rebuild hits the
fast-path (`reused cached artifacts` in `BuildResult::message`), produces a
byte-identical firmware.hex, and runs faster than the cold build.

Catches failure modes the unit test cannot:
  - orchestrator state persisted outside the watch set
  - fast-path predicate bugs that pass per-layer unit tests but reject a
    legitimately-cached `BuildResult`
  - absolute paths baked into `build_fingerprint.json` or other artifacts
    the predicate reads

Test sets `FBUILD_NO_ZCCACHE=1` (via an RAII guard) on purpose: zccache has
its own fingerprint-state machinery that is covered by
`zccache_hit_across_workspace_rename.rs`, and decoupling lets this test focus
on the fbuild-owned fast-path predicate that the #147 fix actually changed.

Gated `#[ignore]` per the established AVR-test pattern (downloads avr-gcc +
Arduino-AVR core on first run).

Also re-adds `tar = { workspace = true }` to fbuild-build dev-deps (the
dependency was introduced on the #232 branch and the test on this branch
also uses it).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

test: hermetic regression gate for #147 cache-survives-tar-extract DoD [META] fbuild build cache must survive CI tar-extract / cross-runner restore

1 participant