Fix flaky pkg/flare TestProcessAgentFullConfig (FLREM-152) - #54961
Fix flaky pkg/flare TestProcessAgentFullConfig (FLREM-152)#54961pducolin wants to merge 1 commit into
Conversation
The "verify auth" subtest picked a free port, closed the listener, then started the process-agent API server on that same port number. Between the close and the real bind, another process on the CI host could grab the port first, failing the server with "address already in use". This caused the flaky failures tracked in FLREM-152 on macOS ARM64 runners. setupProcessAPIServer now retries with a fresh port when it hits this bind error, and the port selection lives in that shared helper so both call sites (TestProcessAgentFullConfig and TestProcessAgentChecks) get the fix.
|
@codex review |
github-actions
Bot
added
the
short review
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84bb722167
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| t.Cleanup(func() { _ = app.Stop(context.Background()) }) | ||
| return | ||
| } | ||
| if attempt >= maxAttempts || !strings.Contains(startErr.Error(), "address already in use") { |
There was a problem hiding this comment.
When the port is stolen on Windows, net.Listen reports the platform-specific message “Only one usage of each socket address” (already asserted in test/new-e2e/tests/agent-subcommands/run_win_test.go:52), so this substring check is false and the helper fails on the first collision instead of retrying. This leaves both auth tests flaky in the Windows unit-test job; classify the wrapped error using a portable address-in-use check rather than matching the Unix/macOS text.
AGENTS.md reference: AGENTS.md:L177-L178
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
|
1 participant
What broke
pkg/flare.TestProcessAgentFullConfig failed on CI. The subtest was "verify_auth". This is tracked in FLREM-152.
Root cause
The test picks a free TCP port. It does this by opening a listener, reading its port, and closing it right away. Then it starts a real API server on that same port number.
There is a gap between closing the listener and starting the real server. In that gap, another process on the same machine can take the port. When that happens, the server fails to start with "address already in use".
This is more likely on CI hosts. CI runs many test binaries at the same time.
The same pattern also exists in TestProcessAgentChecks.
Fix
The helper
setupProcessAPIServernow picks the port and starts the server together. If the start fails because the port is taken, it picks a new port and tries again, up to 5 times.The two call sites no longer pick the port themselves. They just call the helper. This removes the duplicated, unsafe code.
Testing
Ran
pkg/flaretests locally on macOS ARM64, the same platform as the failing CI job. All 47 tests pass, including TestProcessAgentFullConfig and TestProcessAgentChecks.Ref: FLREM-152