Repository navigation
Reuse Bazel build caches in GitHub Actions - #12253
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🧪 Test quality review — PR #12253
Compared merge base df9b675dd7b49b6bbd18775d013e05bc5af8a89a with head 6009d9c8a955d168f4e04b46d2718c9e122dd7ad. Evidence applies to this head, not every intermediate commit.
Test-value evidence
No MSTest/MTP test methods changed (test_count=0). The changed contracts are CI/build tooling (eng/bazel.ps1, bazel.yml, a Python contract script, docs). This review ran no code. Findings below are static predictions or the author's claims, not executed proof.
| Contract | Protecting test | Evidence status |
|---|---|---|
Persistent download and action stores, with the repository-content cache kept under the fresh output root (New-BazelInvocationConfig) |
test_bazel_cache.py::test_only_downloads_and_actions_are_persistent_in_paths_with_spaces |
Author-reported executed feature-removal counterfactual (7 pass, 1 fail, then 8 pass). Static prediction: it detects removal of the disk-cache setting. Not independently verified here. |
Main-only cache writes and compatible snapshot keys (bazel.yml) |
test_successful_main_seeds_and_prs_only_restore, test_snapshots_are_separate_and_reusable_across_source_revisions |
Static configuration guards. Hosted cache authorization is unverified. |
| Validation never skipped on cache hits | test_cache_hits_never_skip_validation_or_test_execution |
Static guard. The author reports BEP records, which are not accessible here. |
| Unique profiles outside the persistent stores; server stopped before save | test_profiles_are_unique_and_outside_persistent_stores, test_server_stops_before_save_and_profiles_remain_evidence |
Static guards. |
| Path validation (absolute, single-line paths) | Covered by the InvocationConfigTests probe |
Not assessed individually. |
| Docs, decision record, action-pin workflow trigger additions | Not applicable | Non-executable. |
Evidence gaps: no paired execution records are accessible to this workflow. Live GitHub cache service behavior, and the first compatible main seed, remain unverified, as the PR states.
🧵 Parallel safety
NOT_APPLICABLE. No changed tests or parallelization configuration were in scope.
Per-test assessment
No C#/MSTest test methods were added or modified, so there is nothing to grade (Not applicable). The Python unittest file .github/scripts/test_bazel_cache.py is outside the MSTest grading rubric. No inline findings were raised.
<sub>This advisory review is heuristic and non-blocking.
Re-run the combined review with /review-tests, or only the parallel-safety specialist with /parallel-audit.</sub>
🤖 Automated content by GitHub Copilot. Generated by the Test Quality Review on PR (on open / sync) workflow. · copilot · auto · ~29 AIC · ⌖ ~1 AIC · ⊞ 8K
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Reviewed the Bazel cache workflow, launcher config, tests and docs. The design is sound: saves are gated to successful non-PR main runs, PRs only restore, --nocache_test_results is retained, and the rc file is per-invocation and cleaned up. Two findings are posted inline. Neither was executed here; they are static analysis only.
Findings
- Unbounded disk-cache growth and quota pressure (inline,
bazel.yml). Cumulative snapshots without GC, saved on every relevantmainpush, can evict the repo's other caches under the 10 GB limit. - Windows-style separators on the Ubuntu leg (inline,
bazel.yml). The cache and profile paths are hard-coded with\. Linux works only through the launcher's\→/rewrite, not for the cache and upload steps.
Smaller notes
docs/decisions/bazel-build.mdcites aLinuxCacheAndProfilePathsWithSpacesLinux check, but no such test exists in the repo. The new Python suite runspwshon ubuntu-latest incheck-action-pins.yml, so it already covers Linux path generation. Either point to that or add the missing test.test_cache_hits_never_skip_validation_or_test_executionreadseng/bazel/validate.ps1, butcheck-action-pins.ymlpath filters list neither it norbazel.ymlchanges outside the cache test. Addeng/bazel/validate*.ps1and.github/workflows/bazel.ymlto that workflow's paths so the contract runs when they change.- The
Remove-Iteminfinallycan mask the Bazel exit status if it throws. Consider-ErrorAction SilentlyContinue. - The evidence is local and session-stored; hosted cache behavior, Linux execution and transfer costs are acknowledged as unverified. Please confirm the first
mainseed and a PR restore in CI before relying on the savings.
Test-value evidence: The new Python contract tests were not run here. The decision record states a feature-removal counterfactual (--disk_cache removed → one failure). I could not verify it.
There was a problem hiding this comment.
🔵 Needs a closer look
The hosted-cache flow lacks inspectable execution artifacts confirming restore/save authorization, a successful main seed consumed by a PR, and full Ubuntu validation.
No open issues 🎉
What changed in this PR
Adds persistent build caching to TestFx’s experimental Bazel GitHub Actions workflow, following #12204. Test execution remains uncached, and the required MSBuild/Arcade pipeline is unchanged.
Changes:
- Restore separate download and action caches across compatible runs; only successful
mainruns save snapshots. - Add opt-in cache/profile configuration, fresh extracted-repository storage, and per-command evidence.
- Add eight cache-contract tests, CI integration, and documentation of validation and limitations.
Test-value evidence: The PR reports eight passing contracts at 6009d9c8, a failing disk-cache-removal counterfactual, and local restored-cache runs with forced test execution. These results were not independently executed here; hosted cache behavior and full Linux validation remain explicit gaps.
| File | Description |
|---|---|
eng/bazel.ps1 |
Adds temporary invocation configuration for caches and profiles. |
docs/decisions/bazel-build.md |
Records cache design, experiments, evidence, and remaining gates. |
docs/bazel.md |
Documents cache behavior, local configuration, and measurement guidance. |
.github/workflows/check-action-pins.yml |
Runs cache-contract tests and expands relevant triggers. |
.github/workflows/bazel.yml |
Adds main-branch seeding, cache restore/save steps, and profile artifacts. |
.github/scripts/test_bazel_cache.py |
Adds launcher and workflow cache-contract tests. |
Review effort: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🧪 Test quality review — PR #12253
Compared merge base df9b675dd7b49b6bbd18775d013e05bc5af8a89a with head 66f2b962b22ff5d82db01d855398c170c6297d66. Evidence applies to this head, not every intermediate commit.
Test-value evidence
No MSTest/MTP test methods changed (test_count=0). Changed files are CI/build tooling (eng/bazel.ps1, bazel.yml, check-action-pins.yml, the Python contract script test_bazel_cache.py) and docs. This review ran no code, so everything below is a static prediction or an author claim, not executed proof.
| Contract | Protecting test | Evidence status |
|---|---|---|
| Persistent download and action stores, with the repository-content cache kept under the fresh output root | test_only_downloads_and_actions_are_persistent_in_paths_with_spaces |
Author-reported executed counterfactual (7 pass, 1 fail, then 8 pass). Static prediction: removing the disk-cache setting is detected. Not independently verified. |
| Main-only cache writes and compatible snapshot keys | test_successful_main_seeds_and_prs_only_restore, test_snapshots_are_separate_and_reusable_across_source_revisions |
Static configuration guards. Hosted cache authorization is unverified. |
| Validation and test execution never skipped on cache hits | test_cache_hits_never_skip_validation_or_test_execution |
Static guard. The author-reported BEP records are not accessible here. |
| Unique profiles outside persistent stores; server stopped before save | test_profiles_are_unique_and_outside_persistent_stores, test_server_stops_before_save_and_profiles_remain_evidence |
Static guards. |
| Snapshot size bounding | test_snapshot_limit_keeps_newest_files_and_bounds_payload |
Author-reported counterfactual (3 intended failures with deletion disabled). Not independently verified. |
| Guard triggers cover each inspected input | test_guard_runs_when_each_inspected_input_changes |
Author-reported counterfactual (removing the two validator filters fails the push and PR subcases). Not independently verified. |
| Docs and decision record | Not applicable | Non-executable. |
Evidence gaps: no paired execution records are accessible to this workflow. Live GitHub cache-service behavior, the first compatible main seed, and full Linux execution remain unverified, as the PR states.
🧵 Parallel safety
NOT_APPLICABLE. No changed tests or parallelization configuration were in scope.
Per-test assessment
No C#/MSTest test methods were added or modified, so there is nothing to grade (Not applicable). The Python file .github/scripts/test_bazel_cache.py is outside the MSTest grading rubric. No inline findings were raised.
<sub>This advisory review is heuristic and non-blocking.
Re-run the combined review with /review-tests, or only the parallel-safety specialist with /parallel-audit.</sub>
🤖 Automated content by GitHub Copilot. Generated by the Test Quality Review on PR (on open / sync) workflow. · copilot · auto · ~22 AIC · ⌖ ~1 AIC · ⊞ 8K
Pull request
Summary
The experimental Bazel workflow currently discards reusable build state with every hosted runner. Persist downloaded archives and compiled/generated action outputs so subsequent runs can reuse unchanged work across commits, without reusing test results.
mainruns; PRs only restore. Use platform/toolchain-compatible prefixes and unique per-run keys rather than SHA-only lookup.--nocache_test_resultsand unconditional validation on cache hits or misses.The required MSBuild/Arcade pipeline is unchanged. Follow-up to #12204; no linked source issue.
Design and decision record
Maintained cache contract, research, experiments and evidence and usage guide.
GitHub archive snapshots are the simplest initial transport; a remote action-cache service is deferred until measured transfer overhead warrants it. Only dedicated content-addressed stores persist. Bazel 9 otherwise defaults its extracted repository-content cache inside the download store, so the wrapper explicitly redirects that state under the fresh output root. The new cache/profile environment variables are opt-in; existing local behavior is unchanged when unset.
Snapshot limits run synchronously after server shutdown, rather than relying on Bazel's background idle GC completing before save. Missing blobs remain normal cache misses and rebuild/download when needed. This bounds accumulated store payloads, not repository-wide archive history or other workflows' quota use.
Demonstration
A text transcript is more useful than screenshots for this nonvisual build-cache change. On Windows, with four build workers, the same native source and a restored archive in a separate checkout/new output root, the initial implementation produced:
This single local pair is not a representative CI speedup. The 1.16 GB local gzip snapshot took 70.58s to create and 13.38s to extract; these are not GitHub zstd/network timings and are excluded from the command times above. No new product UI is introduced; the hosted cache/step surface remains unverified until CI runs this PR.
Validation and regression proof
Current head:
66f2b962b22ff5d82db01d855398c170c6297d66. All 13 cache contracts and the action-pin audit were rerun successfully on this committed head.Initial validated commit:
6009d9c8a955d168f4e04b46d2718c9e122dd7ad. The complete native/package runs below were executed before that commit on basedf9b675dd7b49b6bbd18775d013e05bc5af8a89aplus the initial implementation; their exact original executable-file hashes are recorded in the decision record. The review correction changes snapshot maintenance and guard triggers, not product source or forced test commands; the focused lifecycle evidence was rerun, not the full product suite.Native producer/consumers used Bazel 9.1.1 and SDK 11.0.100-rc.3.26522.148, unchanged test scheduling and net8/net9/net10/net11 runtime layouts.
$ebelow denotes the task-owned local evidence directory ending inbazel-cache-4461d332; the restored runs used the isolated comparison checkout.python .github\scripts\test_bazel_cache.pypython .github\scripts\check_action_pins.pygit diff --cached --check.\eng\bazel\validate.ps1 -Jobs 4 -EvidenceRoot (Join-Path $e 'cold-native').\eng\bazel\validate-packages.ps1 -Jobs 4 -EvidenceRoot (Join-Path $e 'cold-packages').\eng\bazel\validate.ps1 -Jobs 4 -EvidenceRoot (Join-Path $e 'warm-native').\eng\bazel\validate-packages.ps1 -Jobs 4 -EvidenceRoot (Join-Path $e 'warm-packages')Review follow-up evidence is retained under
bazel-review-d9e8c1dc. The real pinned-Bazel pruning harness reduced 25,925 bytes to its 9,541-byte fixture limit by evicting one old 16,384-byte CAS blob. A fresh-output build then passed with one disk-cache hit and one local rebuild, with both payload hashes independently verified. The exact workflowJoin-Pathexpressions also produced the correct native cache/profile directories on Ubuntu PowerShell 7.4.6 before any launcher rewrite, so the separator finding required no path change.eng/bazel/validate.ps1is now covered by both contract-workflow event filters;.github/workflows/**already covered the Bazel YAML.Profiles, BEP, raw logs, TRXs, package/provenance records and counterfactual hashes are retained locally, not uploaded CI artifacts. The decision record preserves the reproducible commands and source bindings. Owned output/cache roots, snapshot and comparison checkout were cleaned after verification; the review-probe servers were also stopped and their output roots removed.
Remaining gates: live GitHub cache-service authorization and restore/save costs, first successful compatible
mainseed, and full Linux workflow execution on the current head. Network-disabled Ubuntu PowerShell checks are local path evidence, not full Linux Bazel validation. No hosted percentage saving is claimed.Test-value evidence
test_only_downloads_and_actions_are_persistent_in_paths_with_spacestest_successful_main_seeds_and_prs_only_restore,test_snapshots_are_separate_and_reusable_across_source_revisionstest_cache_hits_never_skip_validation_or_test_executiontest_profiles_are_unique_and_outside_persistent_stores,test_server_stops_before_save_and_profiles_remain_evidenceChangedSourceInvalidatesCachedPassingExplorerTestingPlatformExplorer.UnitTests.SomeTests.TestMethod1to throwCACHE_INVALIDATION_SENTINELrebuilt it and produced one executed failure (Bazel exit 3, host exit 2). Restoring the exact original bytes and repeating the same command executed one pass, exit 0; no cached test result in either run.test_snapshot_limit_keeps_newest_files_and_bounds_payloadlru,tie,oversized), exit 1. Restoring deletion returned 13 passes, exit 0. Missing stores, invalid budgets and links also have explicit failure coverage.test_guard_runs_when_each_inspected_input_changesEvictedBlobRebuildsWhileRetainedBlobHitsThe initial input-invalidation command in both source states was:
The maintained decision record contains the original/counterfactual/restored source hashes and patch identities, including the review-correction bindings. No test assertion or scheduling was weakened to obtain these results.