tests(CI): restore file mtime by janisz · Pull Request #15609 · stackrox/stackrox

janisz · 2025-06-06T12:50:29Z

This PR restore file creation time to allow usage of go test cache. So only changed packages (including dependent packages) will be retested. This should reduce test time as less tests will be executed.

sourcery-ai

Hey @janisz - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 1 issue found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2025-06-06T12:52:53Z

.github/actions/cache-go-dependencies/action.yaml

 runs:
  using: composite
  steps:
+    - uses: chetan/git-restore-mtime-action@v2


suggestion: Add a descriptive name to this step

Using a name field here will align with the rest of the workflow and make the step clearer.

Suggested change

- uses: chetan/git-restore-mtime-action@v2

- name: Restore git file modification times

uses: chetan/git-restore-mtime-action@v2

rhacs-bot · 2025-06-06T13:13:27Z

Images are ready for the commit at 124486b.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.9.x-26-g124486b140.

codecov · 2025-06-06T14:38:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.80%. Comparing base (8105c79) to head (69573d4).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #15609   +/-   ##
=======================================
  Coverage   48.80%   48.80%           
=======================================
  Files        2590     2590           
  Lines      190471   190471           
=======================================
  Hits        92966    92966           
- Misses      90207    90208    +1     
+ Partials     7298     7297    -1

Flag	Coverage Δ
go-unit-tests	`48.80% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kylape · 2025-06-06T17:16:29Z

.github/actions/cache-go-dependencies/action.yaml

 runs:
  using: composite
  steps:
+    - uses: chetan/git-restore-mtime-action@v2


this is a minor point and i know we don't follow this elsewhere, but it would be more secure to point to a SHA instead of a version tag, particularly because this is a random guy's code repo.

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

sourcery-ai

Hey @janisz - I've reviewed your changes - here's some feedback:

Consider centralizing the CI container image tag (e.g. via a shared env var or matrix) so you don’t have to update it in every workflow file.
Verify that removing the -coverprofile flags from the go-test invocations still satisfies any coverage collection or reporting requirements you rely on.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz · 2025-06-09T14:51:09Z

Closing. We can check on this in the future. For now it's not working out of the box. Tests cache is working only when the same params are passed. Since we do not differentiate cache between release and non release we need to fix it. Also test cache is not supported when coverprofile is used (this will be fixed in Go 1.25)

Go's test cache computes a test binary ID that includes -X ldflags. Since -X MainVersion and -X GitShortSha change on every commit, the test binary ID changes too, causing 100% test cache misses (Phase 1). Fix: generate pkg/version/internal/zversion.go with an init() function instead of passing version data via -X ldflags. For tests, use only the base tag (e.g. "4.11.x") which is stable across commits. For builds, use the full git-describe version. This eliminates -X ldflags entirely from the link step, stabilizing the link ActionID. Result: test binary IDs are constant across commits (fixing Phase 1), and build cache hits improve from ~0% to ~99.9% (only 2/4403 packages recompile — version packages whose output is byte-identical anyway). Also adds -buildvcs=false to disable Go 1.18+'s unused VCS stamping, which otherwise adds git state to every binary. This is the companion to PR #19395 (mtime fix for Phase 2). Together they enable 4.5-52x test speedup. Addresses concerns from PR #15609: -coverprofile caching is fixed in Go 1.25 (we're on 1.25.5), and GOTAGS=release shares 93% of cache entries with default — separation is counterproductive. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Go's test cache computes a test binary ID that includes -X ldflags. Since -X MainVersion and -X GitShortSha change on every commit, the test binary ID changes too, causing 100% test cache misses (Phase 1). Fix: generate pkg/version/internal/zversion.go with an init() function instead of passing version data via -X ldflags. For tests, use only the base tag (e.g. "4.11.x") which is stable across commits. For builds, use the full git-describe version. This eliminates -X ldflags entirely from the link step, stabilizing the link ActionID. Result: test binary IDs are constant across commits (fixing Phase 1), and build cache hits improve from ~0% to ~99.9% (only 2/4403 packages recompile — version packages whose output is byte-identical anyway). This is the companion to PR #19395 (mtime fix for Phase 2). Together they enable 4.5-52x test speedup. Addresses concerns from PR #15609: -coverprofile caching is fixed in Go 1.25 (we're on 1.25.5), and GOTAGS=release shares 93% of cache entries with default — separation is counterproductive. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

janisz marked this pull request as draft June 6, 2025 12:50

openshift-ci bot added the do-not-merge/work-in-progress label Jun 6, 2025

github-actions bot added the area/ci label Jun 6, 2025

sourcery-ai bot reviewed Jun 6, 2025

View reviewed changes

janisz changed the title ~~tests(CI): retore file mtime~~ tests(CI): restore file mtime Jun 6, 2025

kylape reviewed Jun 6, 2025

View reviewed changes

janisz force-pushed the restore_mtime branch from abdf3ba to 5e7f96f Compare June 9, 2025 10:30

janisz marked this pull request as ready for review June 9, 2025 12:38

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 9, 2025

janisz marked this pull request as draft June 9, 2025 12:38

openshift-ci bot added the do-not-merge/work-in-progress label Jun 9, 2025

janisz force-pushed the restore_mtime branch from 5e7f96f to b203748 Compare June 9, 2025 12:40

janisz added 3 commits June 9, 2025 14:40

tests(CI): retore file mtime

1e8e5a1

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

WIP

835134a

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

debug

2d9f17a

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

sourcery-ai bot reviewed Jun 9, 2025

View reviewed changes

remove coverage to enable cache

124486b

Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

janisz force-pushed the restore_mtime branch from b203748 to 124486b Compare June 9, 2025 12:41

janisz closed this Jun 9, 2025

janisz deleted the restore_mtime branch September 16, 2025 09:56

janisz mentioned this pull request Mar 13, 2026

perf(ci): stabilize file mtimes for Go test cache #19395

Merged

5 tasks

davdhacs mentioned this pull request Mar 24, 2026

perf(ci): version refactor (remove Xref/status.sh) #19424

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests(CI): restore file mtime#15609

tests(CI): restore file mtime#15609
janisz wants to merge 4 commits intomasterfrom
restore_mtime

janisz commented Jun 6, 2025

Uh oh!

sourcery-ai bot left a comment

Uh oh!

sourcery-ai bot Jun 6, 2025

Uh oh!

rhacs-bot commented Jun 6, 2025 •

edited

Loading

Uh oh!

codecov bot commented Jun 6, 2025 •

edited

Loading

Uh oh!

kylape Jun 6, 2025 •

edited

Loading

Uh oh!

sourcery-ai bot left a comment

Uh oh!

janisz commented Jun 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	- uses: chetan/git-restore-mtime-action@v2
	- name: Restore git file modification times
	uses: chetan/git-restore-mtime-action@v2

Conversation

janisz commented Jun 6, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

rhacs-bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kylape Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

janisz commented Jun 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rhacs-bot commented Jun 6, 2025 •

edited

Loading

codecov bot commented Jun 6, 2025 •

edited

Loading

kylape Jun 6, 2025 •

edited

Loading