experiment: skip Go tests on UI-only PRs#20737
Draft
davdhacs wants to merge 46 commits into
Draft
Conversation
Rename cache-go-dependencies to setup-go. Add a "Setup Go" step that finds the best Go from hostedtoolcache (matching go.mod's minimum version), adds it to PATH, and sets GOTOOLCHAIN=auto. This replaces both actions/setup-go and the GOTOOLCHAIN env var workaround (golang/go#75031). Benefits: - Uses writable hostedtoolcache Go (covdata works) - Sets GOTOOLCHAIN=auto (not local, so sub-module tools work) - No separate setup-go step needed in workflows - No GOCACHE caching conflicts (setup-go's cache disabled) - Instant activation (~4ms, no download) All workflow references updated from cache-go-dependencies to setup-go. GOTOOLCHAIN env var workaround removed from unit-tests. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Clarify that setup-go's default caching uses a single key across all jobs, causing cross-contamination and unbounded growth. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Show the three differences between actions/setup-go and our action side-by-side with rationale for each choice. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The action now prefers the toolchain directive from go.mod (the intended build version) over the go directive (the minimum). This matches Go's own semantics: go.mod can specify both `go 1.25.0` (minimum language version) and `toolchain go1.25.10` (build version). Falls back to the go directive when no toolchain directive is set. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace column layout with two clear sections: problems with actions/setup-go and what this action does instead. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirror the order of the "problems" list: GOTOOLCHAIN, GOCACHE, then version selection. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
[AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Explain that cache keys are per-job by default, with key-suffix for matrix entry separation. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The action falls back to the go directive (minimum language version) when no toolchain directive is set. This silently uses an older Go than intended. Emit a GHA warning so it's visible in the CI summary. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reflects that the action now handles both Go setup and caching. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Register the same problem matcher as actions/setup-go so Go compilation errors show as inline annotations on PR diffs. Same matchers.json format used by the upstream action. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Run Go unit test jobs directly on ubuntu-latest instead of inside the apollo-ci container. This eliminates ~55s container init overhead per job and enables using the host's pre-installed postgres. Changes: - Remove container blocks from go, go-postgres, go-bench jobs - Replace container postgres (initdb/pg_ctl) with host postgres: - default: pre-installed via systemctl - 15: sclorg docker container (same base as production) - Add pg: ['default', '15'] matrix to go-postgres for multi-version - Add POSTGRES_PASSWORD env for sclorg container auth - Remove fetch-depth: 0 (not needed without container git issues) - Add key-suffix with matrix.pg for per-variant GOCACHE separation Depends on PR #20483 (custom setup-go action). [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace cache-go-dependencies with a setup-go composite action that handles both Go activation and GOCACHE management. Default path: bash hostedtoolcache lookup (~0.3s). Reads go.mod's toolchain directive (falls back to go), finds the best matching Go pre-installed on the runner, sets GOTOOLCHAIN=auto. Fallback path (use-setup-go: 'true'): delegates to actions/setup-go (~8s). Same version resolution from go.mod, but uses the upstream action for installation. Available as a fallback if the default path has issues. Both paths share: per-job GOCACHE keys with stale-entry trimming, mtime stabilization for test caching, Go problem matchers, and the toolchain directive warning. See golang/go#75031 for the covdata bug that motivated this work. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If neither toolchain nor go directive is found, an empty version would match any Go in hostedtoolcache. Fail with an error instead. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove if-guard that was accidentally attached to the PR labels step instead of the setup-go step (caught by sourcery-ai) - Make column number optional in Go problem matcher regex so it matches both "file.go:10: msg" and "file.go:10:5: msg" (caught by coderabbitai) [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…krox into davdhacs/setup-go-combined
…ner-unittests # Conflicts: # .github/actions/setup-go/action.yaml # .github/actions/setup-go/matchers.json # .github/workflows/build.yaml # .github/workflows/scanner-build.yaml # .github/workflows/scanner-db-integration-tests.yaml # .github/workflows/style.yaml # .github/workflows/test-bundle-helpers.yaml # .github/workflows/unit-tests.yaml
[AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The docker postgres container (pg 15) uses --network=host and needs pg_isready -h 127.0.0.1 since there's no unix socket. Pre-installed postgres (default) uses unix sockets and works without -h. On timeout, dump postgres logs for debugging: - docker: docker logs postgres - pre-installed: journalctl -u postgresql [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Run Go unit test jobs directly on ubuntu-latest instead of inside the apollo-ci container. Bump go directive to 1.25.10 which includes the backported covdata fix (golang/go#75031, golang/go#78411) so `go test -cover` works with the system Go's GOTOOLCHAIN auto-download. Changes to unit-tests.yaml: - Remove container blocks from go, go-postgres, go-bench jobs - Replace container postgres with host postgres (systemctl for default, sclorg docker for pg 15) - Add pg: ['default', '15'] matrix to go-postgres - Add POSTGRES_PASSWORD env for sclorg container auth - Add -h 127.0.0.1 to pg_isready for docker postgres - Dump postgres logs on readiness timeout for debugging go.mod: - Bump go directive from 1.25.7 to 1.25.10 (covdata fix) [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The go.mod now requires go 1.25.10 (covdata fix backport). The ceiling check must match. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…krox/stackrox into davdhacs/remove-container-unittests
Konflux builders have Go 1.25.9 (not 1.25.10) and set GOTOOLCHAIN=local, so go 1.25.10 in go.mod breaks Konflux builds. Use the toolchain directive instead: - go 1.25.7: minimum version, compatible with Konflux (1.25.9 >= 1.25.7) - toolchain go1.25.10: auto-downloaded on GHA runners where GOTOOLCHAIN=auto, provides the covdata fix (golang/go#75031) On Konflux: GOTOOLCHAIN=local ignores the toolchain directive, uses the local 1.25.9. Builds don't use go test -cover so the covdata bug doesn't apply. On GHA: GOTOOLCHAIN=auto downloads 1.25.10, covdata works. Reverts go-version-ceiling to 1.25.7 to match the go directive. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add .github/test-images/postgres-15/Dockerfile with the sclorg postgres 15 image used by go-postgres unit tests. Dependabot will auto-bump the digest via the docker ecosystem config. The workflow still has the SHA inline (postgres starts before checkout so the Dockerfile isn't available yet). When Dependabot bumps the Dockerfile, the reviewer updates the SHA in unit-tests.yaml to match. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…krox/stackrox into davdhacs/remove-container-unittests
The Dockerfile approach requires a manual sync step (reviewer copies the SHA into the workflow) since postgres starts before checkout. Simpler to keep the image inline and update manually. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…age" This reverts commit 3491fe3.
Dependabot's docker file_fetcher matches /dockerfile/i in any filename, so postgres.dockerfile works without a subdirectory. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Build from .github/test-images/postgres.dockerfile instead of inlining the image SHA in the workflow. The Dockerfile is the single source of truth — Dependabot bumps it, no manual sync needed. Moves checkout before the postgres start step since docker build needs the Dockerfile. The docker build for a single FROM line is just a pull+tag, no additional build time. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Split UI jobs into unit-tests-ui.yaml with paths filter for ui/. Add paths-ignore: ui/** to unit-tests.yaml so Go tests skip on UI-only PRs. Add passthrough workflows that satisfy required status checks when the real workflow is skipped: - unit-tests-go-passthrough.yaml: reports go/go-postgres/go-bench/ local-roxctl check names on UI-only PRs - unit-tests-ui-passthrough.yaml: reports ui check name on non-UI PRs No ruleset changes needed — passthrough jobs use the same names as the required checks. [AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
unit-tests-ui.yaml, consider adding.github/actions/job-preamble/**to thepathslist (as it was part ofrun_allbefore) so that changes to the shared preamble that affect UI tests still reliably trigger this workflow. - To fully achieve the goal of skipping Go tests for UI-only changes, you may want to align the path filters so that
.github/actions/cache-ui-dependencies/**(and any other strictly-UI infra) is included inpaths-ignorefor the Go workflow and in thepathsforunit-tests-go-passthrough.yaml, mirroring the UI workflow’s configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `unit-tests-ui.yaml`, consider adding `.github/actions/job-preamble/**` to the `paths` list (as it was part of `run_all` before) so that changes to the shared preamble that affect UI tests still reliably trigger this workflow.
- To fully achieve the goal of skipping Go tests for UI-only changes, you may want to align the path filters so that `.github/actions/cache-ui-dependencies/**` (and any other strictly-UI infra) is included in `paths-ignore` for the Go workflow and in the `paths` for `unit-tests-go-passthrough.yaml`, mirroring the UI workflow’s configuration.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
🚀 Build Images ReadyImages are ready for commit a94cc55. To use with deploy scripts: export MAIN_IMAGE_TAG=4.12.x-105-ga94cc5570c |
[AI-assisted] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Apply the same optimizations from PRs #20738, #20741, #20774: - Remove containers from both UI jobs - Add setup-node, cache-ui-dependencies, make -C ui deps pattern - Use cypress-io/github-action with npm run test-component - Update test-summary action SHA to match master Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Description
Split unit-tests.yaml into Go and UI workflows with native
pathsfilters so UI-only PRs skip Go tests (~44 min saved) and Go-only PRs skip UI tests.Passthrough workflows with matching job names satisfy required status checks when the real workflow is skipped (GitHub leaves skipped checks as "Pending" which blocks merge).
Stacked on #20484 (container removal).
Do not merge — experiment to verify the approach works.
User-facing documentation
Testing and quality
How I validated my change
Verify that this PR (which changes only workflow YAML) triggers both the passthrough and real workflows correctly.