Fix e2e test flakes#1619
Merged
Merged
Conversation
Replace sleep-based synchronization in e2e tests with health checks, port polling, and Eventually assertions to eliminate timing-dependent flakes. Key changes: - Add TestMain to install Dapr once per test binary instead of reinstalling in every test function. - Add health-check helpers (waitForDaprHealth, waitForAppHealthy, waitForAppsListed, waitForLogContent) to replace time.Sleep - Add waitForPortsFree to prevent port contention between tests - Add startDaprRun/startDaprRunRetry helpers with proper cleanup ordering (WaitGroup) to prevent "log after test completed" panics - Add collectOutput helper with context cancellation for reliable run-template output collection - Set cmd.WaitDelay in CommandExecWithContext to prevent blocking forever on zombie process pipes (macOS) - Handle EPERM in killProcessGroup for macOS zombie process groups - Add safety goroutine in executeAgainstRunningDapr to kill stuck processes after 5 minutes - Use countSchedulerEntries instead of line-count assertions - Poll for workflow terminal state before rerun operations - Purge stale workflow instances before workflow list assertions Signed-off-by: joshvanl <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces timing-dependent flakes in the standalone e2e test suite by installing Dapr once per test binary and replacing many time.Sleep synchronizations with health checks, port polling, and require.Eventually assertions.
Changes:
- Add
TestMainto perform a single Dapr install/uninstall per test binary and remove per-test install/uninstall calls. - Introduce new test helpers (health checks, port-availability polling, run-template output collection, safer background
dapr runorchestration). - Harden process execution/cleanup behavior (exec
WaitDelay, macOS EPERM handling when killing process groups) and update multiple tests to use polling assertions.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/standalone/main_test.go | Adds TestMain to install/uninstall Dapr once for the suite. |
| tests/e2e/standalone/utils.go | Adds health/polling helpers (waitFor*), startDaprRun*, and output collection utilities to reduce sleeps/flakes. |
| tests/e2e/standalone/workflow_test.go | Replaces sleep-based workflow test setup with startDaprRun + health checks and adds polling for terminal states. |
| tests/e2e/standalone/scheduler_test.go | Switches to retryable background startup and more resilient scheduler output assertions. |
| tests/e2e/standalone/run_template_test.go | Replaces fixed sleeps with port checks, health checks, log polling, and bounded context timeouts for dapr run -f. |
| tests/e2e/standalone/stop_with_run_template_test.go | Removes most sleeps and replaces with port polling + Eventually for list-derived readiness. |
| tests/e2e/standalone/list_test.go | Adds polling to wait for externally started daprd to appear in dapr list. |
| tests/e2e/standalone/init_test.go | Adjusts version resolution and ensures Dapr is reinstalled after init lifecycle tests. |
| tests/e2e/standalone/uninstall_test.go | Ensures Dapr is reinstalled after uninstall tests so later tests continue to work. |
| tests/e2e/standalone/init_negative_test.go | Reinstalls Dapr in cleanup after negative init scenarios. |
| tests/e2e/standalone/init_run_custom_path_test.go | Reinstalls Dapr in cleanup after non-default path tests. |
| tests/e2e/standalone/windows_run_template_test.go | Removes per-test Dapr installation now covered by TestMain. |
| tests/e2e/standalone/version_test.go | Removes per-test Dapr installation now covered by TestMain. |
| tests/e2e/standalone/run_test.go | Removes per-test uninstall cleanup now covered by TestMain; relies on context cancellation. |
| tests/e2e/standalone/stop_test.go | Removes per-test install/uninstall + sleeps; relies on suite-level setup. |
| tests/e2e/standalone/publish_test.go | Removes per-test Dapr installation now covered by TestMain. |
| tests/e2e/standalone/invoke_test.go | Removes per-test Dapr installation now covered by TestMain. |
| tests/e2e/spawn/spawn.go | Sets exec.Cmd.WaitDelay to avoid CombinedOutput hanging on inherited pipes (notably macOS). |
| cmd/run_unix.go | Handles EPERM during process-group polling on macOS to improve shutdown robustness. |
Comments suppressed due to low confidence (1)
tests/e2e/standalone/stop_with_run_template_test.go:79
- This
time.Sleep(10 * time.Second)is the remaining sleep-based synchronization in this test; it can still flake on slow runners. Replace it with polling (waitForAppsListed,getCLIPID, or a log/health check) to know when the run-template apps are actually started.
waitForPortsFree(t, 3510, 3511)
go ensureAllAppsStartedWithRunTemplate(t)
time.Sleep(10 * time.Second)
output, err := cmdStopWithRunTemplate("../testdata/invalid-dir")
assert.Contains(t, output, "Failed to get run file path")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: joshvanl <[email protected]>
Signed-off-by: joshvanl <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cicoyle
approved these changes
Apr 6, 2026
dapr-bot
pushed a commit
that referenced
this pull request
Apr 6, 2026
Fix e2e test flakes (cherry picked from commit 4b40955) Signed-off-by: dapr-bot <[email protected]>
JoshVanL
pushed a commit
that referenced
this pull request
Apr 6, 2026
Fix e2e test flakes (cherry picked from commit 4b40955) Signed-off-by: dapr-bot <[email protected]> Co-authored-by: Cassie Coyle <[email protected]>
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
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.
Replace sleep-based synchronization in e2e tests with health checks, port polling, and Eventually assertions to eliminate timing-dependent flakes. Key changes: