🌱 [DO NOT MERGE] Debugging flaky test in spokecluster_aws_joining_test in github workflow #1192
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amrcoder The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughCommented-out the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/pre.yml (2)
53-74: Don’t comment out jobs; gate them with a condition tied to the debug PR.Commenting the unit job drops coverage uploads and invites config drift. Keep the job defined and skip it only for this “[DO NOT MERGE]” debug PR via an
if:guard.Apply this diff to re-enable the job while conditionally skipping it on this PR:
- # unit: - # name: unit - # runs-on: ubuntu-latest - # steps: - # - name: checkout code - # uses: actions/checkout@v5 - # - name: install Go - # uses: actions/setup-go@v5 - # with: - # go-version: ${{ env.GO_VERSION }} - # - name: unit - # run: make test - # - name: report coverage - # uses: codecov/codecov-action@v5 - # with: - # token: ${{ secrets.CODECOV_UPLOAD_TOKEN }} - # files: ./coverage.out - # flags: unit - # name: unit - # verbose: true - # fail_ci_if_error: false + unit: + # Skip unit job only for PRs explicitly marked DO NOT MERGE (keeps it on for all other PRs and on main) + if: ${{ !(github.event_name == 'pull_request' && contains(github.event.pull_request.title, '[DO NOT MERGE]')) }} + name: unit + runs-on: ubuntu-latest + steps: + - name: checkout code + uses: actions/checkout@v5 + - name: install Go + uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: unit + run: make test + - name: report coverage + uses: codecov/codecov-action@v5 + with: + token: ${{ secrets.CODECOV_UPLOAD_TOKEN }} + files: ./coverage.out + flags: unit + name: unit + verbose: true + fail_ci_if_error: false
86-86: Amplify flake reproduction signal and narrow scope.Run the target test repeatedly with shuffle and upload logs to artifacts.
Apply this diff to exercise the flake harder (adjust the regex to the exact test name you’re chasing):
- run: ARGS="-test.v" make test-registration-integration + run: ARGS='-test.v -test.count=20 -test.shuffle=on -test.run="SpokeCluster.*AWS.*Joining"' make test-registration-integrationAdd this step right after to persist logs (so you can diff runs):
- name: upload integration logs if: always() uses: actions/upload-artifact@v4 with: name: integration-logs path: | **/*.log **/test-results/*.json **/test-output/*.txtQuestions to confirm:
- Does test-registration-integration pass ARGS directly to go test? If not, I’ll adapt to the Makefile.
- What’s the exact test func regex (e.g., TestSpokeClusterAWSJoining)? I can tune -test.run accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pre.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: e2e-singleton
- GitHub Check: build
- GitHub Check: verify
- GitHub Check: integration
- GitHub Check: e2e
- GitHub Check: e2e-hosted
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/integration-test.mk (1)
34-34: Make the focus pattern opt-in to avoid accidental commits.Gate the focus behind an env var so default runs remain unchanged; set it in the workflow when debugging.
- ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast -ginkgo.focus="Joining Process for aws flow" ${ARGS} + ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast $(if $(GINKGO_FOCUS),-ginkgo.focus="$(GINKGO_FOCUS)",) ${ARGS}Tip: To reproduce flakes faster without changing Makefile behavior, pass via ARGS in CI:
- ARGS='-ginkgo.until-it-fails' or ARGS='-ginkgo.flake-attempts=3'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/integration-test.mk(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: e2e
- GitHub Check: e2e-hosted
- GitHub Check: e2e-singleton
- GitHub Check: verify
- GitHub Check: integration
- GitHub Check: build
| test-registration-integration: ensure-kubebuilder-tools | ||
| go test -c ./test/integration/registration -o ./registration-integration.test | ||
| ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast ${ARGS} | ||
| ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast --ginkgo.focus "Joining Process for aws flow" ${ARGS} |
There was a problem hiding this comment.
🧩 Analysis chain
Use -ginkgo.focus with the compiled test binary (single dash).
--ginkgo.focus is for the ginkgo CLI; the compiled test binary expects -ginkgo.focus. Using the wrong form risks ignoring the filter or flag-parsing errors.
Apply this diff:
- ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast --ginkgo.focus "Joining Process for aws flow" ${ARGS}
+ ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast -ginkgo.focus="Joining Process for aws flow" ${ARGS}Run to confirm the focus pattern matches an actual spec title:
🏁 Script executed:
#!/bin/bash
# Verify the focused spec exists (case/spacing sensitive).
rg -nP -C2 'Joining Process.*aws flow' test/integration/registration --type go || {
echo "No exact match found. Nearby candidates:" >&2
rg -nP -C2 'Joining Process|aws flow' test/integration/registration --type go
}Length of output: 565
Use -ginkgo.focus with the compiled test binary (single dash).
Spec found at test/integration/registration/spokecluster_aws_joining_test.go:28 — change the flag so the compiled test binary receives the focus.
- ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast --ginkgo.focus "Joining Process for aws flow" ${ARGS}
+ ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast -ginkgo.focus="Joining Process for aws flow" ${ARGS}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast --ginkgo.focus "Joining Process for aws flow" ${ARGS} | |
| ./registration-integration.test -ginkgo.slow-spec-threshold=15s -ginkgo.v -ginkgo.fail-fast -ginkgo.focus="Joining Process for aws flow" ${ARGS} |
🤖 Prompt for AI Agents
In test/integration-test.mk around line 34, the Makefile passes the Ginkgo focus
flag to the compiled test binary as "--ginkgo.focus" but the compiled binary
expects the single-dash form; replace "--ginkgo.focus" (or "-ginkgo.focus") with
"-focus" and keep the existing quoted pattern and other flags intact so the
command becomes the compiled test binary receiving -focus "Joining Process for
aws flow" ${ARGS}.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/integration/registration/spokecluster_aws_joining_test.go (1)
260-260: Prefer GinkgoWriter for CI‑captured debug logs.fmt.Printf can be dropped by reporters; GinkgoWriter is consistently captured in GitHub Actions output.
- fmt.Printf("testing flaky test attempt 1\n") + ginkgo.GinkgoWriter.Printf("testing flaky test attempt 1\n")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/integration/registration/spokecluster_aws_joining_test.go(2 hunks)
🧰 Additional context used
🪛 GitHub Check: integration
test/integration/registration/spokecluster_aws_joining_test.go
[failure] 28-28:
expected declaration, found ginkgo
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: e2e-singleton
- GitHub Check: e2e
- GitHub Check: e2e-hosted
- GitHub Check: build
- GitHub Check: verify
1 participant
Summary
Debugging flaky test in spokecluster_aws_joining_test in github workflow
Related issue(s)
Fixes # #1154
Summary by CodeRabbit
Tests
Chores
Notes