Skip to content

perf(ci): Combine roxvet runs into single pass#19958

Merged
AlexVulaj merged 6 commits intomasterfrom
AlexVulaj/roxvet-combine-runs
Apr 14, 2026
Merged

perf(ci): Combine roxvet runs into single pass#19958
AlexVulaj merged 6 commits intomasterfrom
AlexVulaj/roxvet-combine-runs

Conversation

@AlexVulaj
Copy link
Copy Markdown
Contributor

@AlexVulaj AlexVulaj commented Apr 13, 2026

Description

This PR optimizes the roxvet Makefile target by combining two sequential runs into a single pass, reducing execution time by ~44%.

Previously, roxvet ran twice:

  1. First pass: with test build tags (sql_integration, test_e2e, etc.), enabling donotcompareproto and gogoprotofunctions analyzers
  2. Second pass: without tags or those analyzer flags, checking production code

The change removes the sequential execution overhead and ensures stricter checks apply to all code.

Running all analyzers on previously-unchecked tagged files exposed violations that this PR also fixes:

testtags analyzer bug (tools/roxvet/analyzers/testtags/analyzer.go): CommentGroup.Text() strips // prefixes, so the check strings.HasPrefix(comment.Text(), "//go:build") never matched. Fixed to check raw Comment.Text field instead. This was a latent bug — the analyzer was always broken, but never ran on these files before.

validateimports exceptions (tools/roxvet/analyzers/validateimports/analyzer.go):

  • Added pkg/fixtures to migrator's allowed imports (used by migration tests for test data helpers)
  • Added exception for pkg to import from tools/generate-helpers/pg-table-bindings and central/processlisteningonport/store (test-only imports for postgres search testing)

Forbidden stdlib imports (4 files): Swapped syncpkg/sync in test files behind sql_integration tags

Forbidden third-party import (central/complianceoperator/v2/report/datastore/datastore_impl_test.go): Swapped github.com/google/uuidpkg/uuid

Needless format calls (8 occurrences across 5 files): fmt.Errorf("static string")errors.New(...), fmt.Printffmt.Print

Import alias consistency (tests/compliance_operator_v2_test.go): Renamed dynclientctrlClient to match codebase convention

sort.Strings (central/views/imagecomponentflat/view_test.go): → slices.Sort

All fixes are in test files or roxvet analyzers — no production code changes.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  • Local A/B benchmarking with cold cache: master (5m24s) vs this branch (3m00)
  • Verified all roxvet violations from the initial CI run are resolved
  • Confirmed all changes are test-only — no production code modified

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.57%. Comparing base (8d9c528) to head (872aef4).
⚠️ Report is 20 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19958      +/-   ##
==========================================
+ Coverage   49.56%   49.57%   +0.01%     
==========================================
  Files        2764     2765       +1     
  Lines      208357   208481     +124     
==========================================
+ Hits       103276   103362      +86     
- Misses      97430    97464      +34     
- Partials     7651     7655       +4     
Flag Coverage Δ
go-unit-tests 49.57% <100.00%> (+0.01%) ⬆️

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.

@AlexVulaj AlexVulaj changed the title Combine roxvet runs into single pass perf(ci): Combine roxvet runs into single pass Apr 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 13, 2026

🚀 Build Images Ready

Images are ready for commit 13998bd. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-652-g13998bd1db

@AlexVulaj AlexVulaj force-pushed the AlexVulaj/roxvet-combine-runs branch from 5aabb57 to 18aedd0 Compare April 13, 2026 15:28
@AlexVulaj AlexVulaj marked this pull request as draft April 13, 2026 16:13
@AlexVulaj AlexVulaj marked this pull request as ready for review April 13, 2026 17:11
@AlexVulaj AlexVulaj marked this pull request as draft April 13, 2026 19:09
@AlexVulaj AlexVulaj marked this pull request as ready for review April 14, 2026 14:29
@AlexVulaj AlexVulaj requested a review from a team as a code owner April 14, 2026 14:29
Copy link
Copy Markdown
Contributor

@davdhacs davdhacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

Comment thread tools/roxvet/analyzers/validateimports/analyzer.go
Comment thread tools/roxvet/analyzers/validateimports/analyzer.go Outdated
Copy link
Copy Markdown
Contributor

@dashrews78 dashrews78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like the exception for the uuid_test. Please come up with a different solution for that.

I'm not really a fan of the fixtures import allowance, but I can accept that one.

Comment thread tools/roxvet/analyzers/validateimports/analyzer.go Outdated
@AlexVulaj AlexVulaj requested a review from dashrews78 April 14, 2026 17:49
Copy link
Copy Markdown
Contributor

@dashrews78 dashrews78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for humoring me on my suggestions.

@AlexVulaj AlexVulaj merged commit 13998bd into master Apr 14, 2026
101 of 105 checks passed
@AlexVulaj AlexVulaj deleted the AlexVulaj/roxvet-combine-runs branch April 14, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants