fix(coderd/x/agenthooks/dispatch): deflake TestDispatcherTimeoutNoRetry - #28050
fix(coderd/x/agenthooks/dispatch): deflake TestDispatcherTimeoutNoRetry#28050ibetitsmike wants to merge 2 commits into
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdae156e22
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| t.Cleanup(server.Close) | ||
| return &http.Response{ | ||
| StatusCode: http.StatusOK, | ||
| Body: contextBlockedBody{ctx: req.Context()}, |
There was a problem hiding this comment.
Would just waiting for req.Context() to expire in the roundTripFunc have the same effect here?
| })} | ||
|
|
||
| _, _, err := newTestDispatcher(t, server.Client(), server.URL, 50*time.Millisecond).Dispatch( | ||
| _, _, err := newTestDispatcher(t, client, "https://hooks.example.com/coder", 50*time.Millisecond).Dispatch( |
There was a problem hiding this comment.
Why change the server URL here?
Fixes the
TestDispatcherTimeoutNoRetryflake from CODAGT-919.Problem
The test asserted
requests.Load() == 1where the counter was incremented inside anhttptesthandler, under a hard 50 ms dispatch timeout. On a loaded runner (observed on thetest-go-pg (windows-2022)gauntlet) the deadline expired before the handler was scheduled, so timeout classification succeeded but the request count observed 0.Fix
Replace the real server with a
roundTripFunctransport stub (helper already in the file) that records the attempt synchronously, blocks onreq.Context().Done(), and returns the context error. The dispatcher's own 50 ms deadline still genuinely expires and takes the timeout classification path inpost(), and a retry regression would synchronously invoke the stub a second time, so both assertions keep guarding the same behavior without depending on scheduler timing.Validation
go test ./coderd/x/agenthooks/dispatch -run '^TestDispatcherTimeoutNoRetry$' -count=100and-race -count=40pass; full package suite passes with-race.dispatcher.gofails the test withexpected: "timeout", actual: "protocol_error", confirming the test still guards that branch.