perf(ci): enable full Go test caching (fixed ldflags + stable mtimes)#19618
Draft
perf(ci): enable full Go test caching (fixed ldflags + stable mtimes)#19618
Conversation
Tests don't need real version info. Use fixed version strings (0.0.0-test) instead of git-describe values for test ldflags. This makes link ActionIDs stable across commits, enabling Go's test result cache to hit. Also adds -buildvcs=false to stop Go 1.18+'s VCS stamping which independently changes link ActionIDs on every commit. Builds keep the current XDef/status.sh mechanism unchanged. Verified locally: tests cache across commits with fixed ldflags. Combined with the mtime fix (#19395), this enables full warm test caching in CI (2.5-8.2x faster test jobs). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Go's test cache validates inputs by (path, size, mtime) — not content. git checkout sets all file mtimes to the current time, which differs between CI runs, causing test results to never be cached. Fix: set all tracked file mtimes to a fixed date (2001-01-01) after checkout and cache restore. This makes the test cache hit across runs when file content hasn't changed. Measured impact (from PR #19201): - go unit tests: 35m → 8m (4.5x) - go-postgres: 31m → 36s (52x) - sensor-integration: 26m → 2.5m (11x) - Test cache hit rate: 97.6% (681/698 packages) Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
9 tasks
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
git ls-files | xargs touchmtime stabilization currently applies to every tracked file; consider narrowing this to files relevant to Go builds/tests (or excluding known problematic paths like large assets or build outputs) to avoid surprising other tools that rely on mtimes during the same workflow. - The hard-coded
VERSION_PKG="github.com/stackrox/rox/pkg/version/internal"ingo-tool.shwill silently break if the version package is moved/renamed; consider deriving this import path fromgo listor centralizing it in a single shared variable/source of truth.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `git ls-files | xargs touch` mtime stabilization currently applies to every tracked file; consider narrowing this to files relevant to Go builds/tests (or excluding known problematic paths like large assets or build outputs) to avoid surprising other tools that rely on mtimes during the same workflow.
- The hard-coded `VERSION_PKG="github.com/stackrox/rox/pkg/version/internal"` in `go-tool.sh` will silently break if the version package is moved/renamed; consider deriving this import path from `go list` or centralizing it in a single shared variable/source of truth.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Images are ready for the commit at 9a81a36. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19618 +/- ##
=======================================
Coverage 49.28% 49.28%
=======================================
Files 2735 2735
Lines 206215 206215
=======================================
+ Hits 101627 101635 +8
+ Misses 97046 97039 -7
+ Partials 7542 7541 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
2 participants
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.
Summary
Combines two fixes that together enable full Go test result caching across CI commits. Test jobs are 2.5-8.2x faster.
Commit 1 (from #19617): Use fixed version ldflags for tests — stabilizes test binary ActionIDs (Phase 1).
Commit 2 (from #19395): Stabilize file mtimes after checkout — fixes test input validation (Phase 2).
Both phases must hit for Go's test cache to return cached results.
Why both are needed
Go's test cache has two phases:
Expected CI results
From earlier testing on the same branch with both fixes:
Partially generated by AI.