Skip to content

Replace --compilers/--procs with -p flag, upgrade Ginkgo to v2.28.3#1949

Open
madmecodes wants to merge 1 commit into
Kuadrant:mainfrom
madmecodes:fix/ginkgo-parallel-flag
Open

Replace --compilers/--procs with -p flag, upgrade Ginkgo to v2.28.3#1949
madmecodes wants to merge 1 commit into
Kuadrant:mainfrom
madmecodes:fix/ginkgo-parallel-flag

Conversation

@madmecodes

@madmecodes madmecodes commented May 7, 2026

Copy link
Copy Markdown

Summary

  • Upgrade Ginkgo from v2.22.0 to v2.28.3 (above v2.23.4 threshold for -p flag support)
  • Replace --compilers and --procs flags with -p in integration test targets for automatic parallelization
  • Preserve --procs=1 in test-bare-k8s-integration (single Kuadrant CR limitation)
  • Remove unused INTEGRATION_TEST_NUM_CORES and INTEGRATION_TEST_NUM_PROCESSES variables

Fixes #1420

Test plan

  • Verify go mod tidy produces no changes
  • Verify integration test targets still work with -p flag
  • Verify test-bare-k8s-integration continues to run serially with --procs=1

Summary by CodeRabbit

  • Chores
    • Updated project dependencies to newer versions to improve stability and testing toolchain.
    • Simplified integration test configuration and invocation: removed prior default core/process counts and replaced per-target compiler/process flags with streamlined parallelism settings for more consistent test runs.

@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f3fb9ecb-a696-460e-94b9-e96b634bbbc4

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6c7f9 and da282ce.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • go.mod
  • make/integration-tests.mk
🚧 Files skipped from review as they are similar to previous changes (2)
  • go.mod
  • make/integration-tests.mk

📝 Walkthrough

Walkthrough

Bumps Ginkgo/Gomega and several indirect Go modules in go.mod; removes INTEGRATION_TEST_NUM_CORES/INTEGRATION_TEST_NUM_PROCESSES from make/integration-tests.mk; replaces Ginkgo --compilers/--procs flags with -p in integration targets (keeping --procs=1 for bare-k8s).

Changes

Ginkgo Upgrade and Integration Test Configuration

Layer / File(s) Summary
Dependency Updates
go.mod
Direct dependencies github.com/onsi/ginkgo/v2 and github.com/onsi/gomega are bumped; indirect github.com/google/pprof pseudo-version updated; adds go.yaml.in/yaml/v3 and bumps golang.org/x/* indirect modules.
Top-level Makefile variables
make/integration-tests.mk
Removed default variable definitions INTEGRATION_TEST_NUM_CORES ?= 4 and INTEGRATION_TEST_NUM_PROCESSES ?= 10; other top-level vars remain.
Integration Test Invocation Changes
make/integration-tests.mk
Integration targets (test-bare-k8s-integration, test-gatewayapi-env-integration, test-istio-env-integration, test-envoygateway-env-integration, test-integration) replace --compilers=.../--procs=... with -p in Ginkgo invocations; test-bare-k8s-integration retains --procs=1.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through modules, bumps in tow,
Swapped flags for -p so parallel runs go,
Makefile trimmed, variables free,
Tests march onward, light and speedy,
A rabbit's cheer for builds that flow.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarises the main changes: upgrading Ginkgo and replacing flags with -p in integration tests.
Linked Issues check ✅ Passed Changes meet all issue #1420 requirements: Ginkgo upgraded to v2.28.3 (exceeds v2.23.4 minimum), -p flag replaces --compilers/--procs across all targets, --procs=1 preserved for test-bare-k8s-integration, and unused variables removed.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #1420 objectives; no extraneous modifications are present in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@madmecodes madmecodes force-pushed the fix/ginkgo-parallel-flag branch from ab648af to 5a6c7f9 Compare May 8, 2026 07:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
make/integration-tests.mk (1)

36-36: ⚡ Quick win

Coverage profile bloat on high-core CI machines.

When -p is active, each parallel node produces its own coverage data that Ginkgo merges into cover.out. Running in parallel on high-core-count machines can lead to many duplicated lines in the merged output — on a 64-core machine the merged cover.out was reported to reach 7.9 GB, versus 130 MB without -p.

If your CI runners are provisioned with many cores and memory is constrained, consider capping parallelism with --procs=N (e.g., matching the old INTEGRATION_TEST_NUM_PROCESSES=10) rather than fully auto-detected -p, or verify that the runners have sufficient memory headroom for the merged profile.

Also applies to: 55-55, 73-73, 92-92

🤖 Prompt for 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.

In `@make/integration-tests.mk` at line 36, Replace the Ginkgo '-p' auto-parallel
flag to avoid coverage-profile bloat by capping parallelism: change occurrences
of the literal '-p' in make/integration-tests.mk to
'--procs=$(INTEGRATION_TEST_NUM_PROCESSES)' (and add/ensure the
INTEGRATION_TEST_NUM_PROCESSES variable is set or documented), i.e., update each
place that passes '-p' to use '--procs=...' so CI can be limited (applies to the
'-p' instances referenced around lines 36, 55, 73, 92).
🤖 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.

Nitpick comments:
In `@make/integration-tests.mk`:
- Line 36: Replace the Ginkgo '-p' auto-parallel flag to avoid coverage-profile
bloat by capping parallelism: change occurrences of the literal '-p' in
make/integration-tests.mk to '--procs=$(INTEGRATION_TEST_NUM_PROCESSES)' (and
add/ensure the INTEGRATION_TEST_NUM_PROCESSES variable is set or documented),
i.e., update each place that passes '-p' to use '--procs=...' so CI can be
limited (applies to the '-p' instances referenced around lines 36, 55, 73, 92).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c465d21b-9719-44f3-bf14-a7522f598c79

📥 Commits

Reviewing files that changed from the base of the PR and between ab648af and 5a6c7f9.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • go.mod
  • make/integration-tests.mk
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod

Upgrade ginkgo from v2.22.0 to v2.28.3 and replace separate
--compilers and --procs flags with -p for automatic parallelization
in integration test targets. Preserve --procs=1 for
test-bare-k8s-integration due to single Kuadrant CR limitation.
Remove unused INTEGRATION_TEST_NUM_CORES and
INTEGRATION_TEST_NUM_PROCESSES variables.

Fixes Kuadrant#1420

Signed-off-by: madmecodes <ayushguptadev1@gmail.com>
@madmecodes madmecodes force-pushed the fix/ginkgo-parallel-flag branch from 5a6c7f9 to da282ce Compare May 9, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace --compilers and --procs with -p flag after Ginkgo v2.23.4 upgrade

1 participant