Fetch the ginkgo binary version from the e2e tests module - #533
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses issues related to Ginkgo version mismatches between the main project and the end-to-end testing module. By updating the dependency version and introducing a verification step in the build process, it ensures that the testing environment remains synchronized with the project's core dependencies, preventing potential runtime errors during test execution. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the github.com/onsi/ginkgo/v2 dependency to v2.32.0 and introduces a new check-ginkgo-version Makefile target to ensure version alignment between the main module and the e2e tests. Feedback on these changes includes stripping the 'v' prefix from the Ginkgo version check in the Makefile to prevent redundant installations, and integrating the new version check target into the lint pipeline so it runs automatically.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| CONTROLLER_TOOLS_VERSION ?= v0.19.0 | ||
| KUBECTL_VERSION ?= v1.27.0 | ||
| GINKGO_VERSION ?= $(shell go list -m -f '{{.Version}}' github.com/onsi/ginkgo/v2) | ||
| GINKGO_VERSION ?= $(shell cd e2etests && go list -m -f '{{.Version}}' github.com/onsi/ginkgo/v2) |
There was a problem hiding this comment.
The go list command returns the version with a leading v (e.g., v2.32.0). However, ginkgo version prints the version without the v prefix (e.g., Ginkgo Version 2.32.0). This causes the grep -q $(GINKGO_VERSION) check in the ginkgo target (line 326) to always fail, leading to redundant go install executions on every run.
Since go install requires the v prefix, we should keep GINKGO_VERSION with the v prefix, but strip it when performing the version check in the ginkgo target. For example, you can update line 326 to:
test -s $(LOCALBIN)/ginkgo && $(LOCALBIN)/ginkgo version | grep -q $(GINKGO_VERSION:v%=%) || \| .PHONY: check-ginkgo-version | ||
| check-ginkgo-version: ## Verify ginkgo version is aligned between go.mod and e2etests/go.mod | ||
| @MAIN_VERSION=$$(go list -m -f '{{.Version}}' github.com/onsi/ginkgo/v2); \ | ||
| E2E_VERSION=$$(cd e2etests && go list -m -f '{{.Version}}' github.com/onsi/ginkgo/v2); \ | ||
| if [ "$$MAIN_VERSION" != "$$E2E_VERSION" ]; then \ | ||
| echo "ERROR: ginkgo version mismatch: go.mod has $$MAIN_VERSION, e2etests/go.mod has $$E2E_VERSION"; \ | ||
| exit 1; \ | ||
| fi |
There was a problem hiding this comment.
The new check-ginkgo-version target is defined but not integrated into any of the standard pipeline targets (such as lint or test). This means the check will not run automatically during local development or CI workflows unless explicitly invoked.
Consider adding check-ginkgo-version as a dependency to the lint target to ensure version alignment is automatically verified:
.PHONY: lint
lint: $(GOLANGCI_LINT_CUSTOM_BIN) check-ginkgo-version
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Makefile now reads Ginkgo’s version from the e2e module and checks it against the root module. A standalone CI job runs this validation. ChangesGinkgo version alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Read the ginkgo CLI version from e2etests/go.mod instead of the main go.mod, since that is the module where ginkgo is used as a test runner. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump ginkgo from v2.28.3 to v2.32.0 in the main module to match the version in e2etests/go.mod, avoiding CLI/package version mismatch when building the host validator. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Line 79: Update the CI workflow around the unconditional validation step
containing make check-ginkgo-version so it runs for Dependabot updates as well
as other pull requests. Keep the existing generated-code regeneration exclusion
for dependabot[bot], but ensure the Ginkgo synchronization check is not gated by
that condition.
In `@Makefile`:
- Around line 435-440: Update the check-ginkgo-version recipe around the
MAIN_VERSION and E2E_VERSION assignments to fail closed when either go list
command fails, such as by enabling errexit or explicitly checking each command’s
status. Ensure a failed lookup cannot leave both variables empty and allow the
version comparison to return success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6f77a30-828b-46d1-960c-12e6cd2e2f61
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
.github/workflows/ci.yamlMakefile
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 87-97: Add a job-level permissions block to check-ginkgo-version
granting only contents: read, while preserving the existing checkout, Go setup,
and make check-ginkgo-version steps.
- Around line 90-91: Update the actions/checkout step in the CI workflow to set
persist-credentials to false, preventing the GITHUB_TOKEN from being stored in
the local Git configuration before the repository Makefile runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 77858775-0427-466c-a14c-37557406fb35
📒 Files selected for processing (2)
.github/workflows/ci.yamlMakefile
Add a check-ginkgo-version Makefile target that fails if the ginkgo version in go.mod and e2etests/go.mod diverge. Run it in CI as part of the check-generated job. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Is this a BUG FIX or a FEATURE ?:
What this PR does / why we need it:
Fixes the gingko mistmatch error when we bump the e2e go mod file but we forget to update the binary, end ensure both version are aligned.
Special notes for your reviewer:
Release note:
AI Guidelines Acknowledgment:
Summary by CodeRabbit