fix(#2490): add retry logic for flaky e2e TestAdminInstallUninstall - #2492
Conversation
Address two failure modes in TestAdminInstallUninstall: 1. 401 Bad credentials during `admin analyze`: Add tryRunCLI helper that returns an error instead of fataling, and wrap the analyze call in a retry loop with backoff (up to 3 attempts, 10s/20s delays). This handles transient GitHub propagation delays after repo creation. 2. Triage workflow not dispatched: Verify the shim workflow file exists on the test-repo default branch (with retries) before creating the test issue. The shim must be active before it can trigger on issues:opened events; without this check, a race between PR merge propagation and issue creation can cause the dispatch to silently not fire. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Site previewPreview: https://8ce141c9-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 9:56 AM UTC · Completed 10:07 AM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsLow
Labels: PR modifies e2e test infrastructure to fix flaky test failures |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| var analyzeOutput string | ||
| for attempt := range 3 { | ||
| if attempt > 0 { | ||
| delay := time.Duration(attempt*10) * time.Second |
There was a problem hiding this comment.
[low] retry-logging
The analyze retry log message uses 'Analyze attempt %d failed, retrying in %s...' which differs from the 'Attempt %d: ' format used by other retry loops in the same file (including the shim verification retry added in this same PR).
Suggested fix: Align with the established 'Attempt %d: ' log format for consistency within the file.
| delay := time.Duration(attempt*10) * time.Second | ||
| t.Logf("Analyze attempt %d failed, retrying in %s...", attempt, delay) | ||
| time.Sleep(delay) | ||
| } |
There was a problem hiding this comment.
[low] edge-case
If tryRunCLI succeeds but returns an empty string for analyzeOutput, the test proceeds silently. This is pre-existing behavior (the original runCLI call had the same property), not a regression.
| _, err = env.client.GetFileContent(ctx, env.org, forge.ConfigRepoName, layers.VendoredBinaryPath) | ||
| require.NoError(t, err, "vendored binary should exist at %s", layers.VendoredBinaryPath) | ||
| analyzeOutput := runCLI(t, env.binary, env.token, "admin", "analyze", env.org) | ||
| // Retry analyze with backoff to handle transient 401s from GitHub |
There was a problem hiding this comment.
[low] retry-pattern-consistency
The analyze retry uses increasing backoff (10s, 20s) while other retries in the file use fixed 3-5s delays. The variation is justified by the different failure mode (401 credential propagation vs. resource availability).
fullsend-ai-review
Bot
added
ready-for-merge
|
🤖 Finished Retro · ✅ Success · Started 10:47 AM UTC · Completed 10:54 AM UTC |
Retro: PR #2492 — add retry logic for flaky e2e TestAdminInstallUninstallOverall: This was a clean, efficient agent workflow. The code agent produced a correct fix in a single iteration, the review agent approved with only low-severity style observations, and a human approved and merged within ~50 minutes of PR creation. No rework cycles. Timeline
Key observationThe PR modifies e2e test infrastructure, but e2e tests were skipped because the bot author didn't have the Existing coverage
Proposals filed
|
…laky-retry fix(fullsend-ai#2490): add retry logic for flaky e2e TestAdminInstallUninstall
1 participant
Address two failure modes in TestAdminInstallUninstall:
401 Bad credentials during
admin analyze: Add tryRunCLI helper thatreturns an error instead of fataling, and wrap the analyze call in a
retry loop with backoff (up to 3 attempts, 10s/20s delays). This
handles transient GitHub propagation delays after repo creation.
Triage workflow not dispatched: Verify the shim workflow file exists
on the test-repo default branch (with retries) before creating the
test issue. The shim must be active before it can trigger on
issues:opened events; without this check, a race between PR merge
propagation and issue creation can cause the dispatch to silently
not fire.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Closes #2490
Post-script verification
agent/2490-e2e-flaky-retry)7bfb871b7cb7110ae563136761d5df2388399052..HEAD)