internal: server/git, Fix TestGitServer_Timeout flakiness on Windows CI#2129
Open
aymanbagabas wants to merge 1 commit into
Open
internal: server/git, Fix TestGitServer_Timeout flakiness on Windows CI#2129aymanbagabas wants to merge 1 commit into
aymanbagabas wants to merge 1 commit into
Conversation
The test measured elapsed time from the client goroutine while the server arms its idle deadline on a separate goroutine. On loaded Windows CI runners, the server could arm the deadline before the client captured start := time.Now(), causing elapsed to fall short of the configured Timeout and tripping the lower-bound assertion. Anchor the two clocks by sending a 1-byte probe before timing: pkt-line decoding needs 4 bytes for the length header, so the server consumes the byte and re-enters Read, arming a fresh idle deadline at approximately the same wall-clock moment as the client's start. Recording start before the Write ensures elapsed includes any scheduling latency, making the lower-bound robust. Assisted-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]> Signed-off-by: Ayman Bagabas <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates TestGitServer_Timeout to reduce timing flakiness on Windows CI by aligning the client-side elapsed measurement with the server’s idle deadline refresh.
Changes:
- Sends a one-byte probe before waiting for the idle timeout.
- Starts elapsed timing before the probe write so scheduling/write latency is included.
- Updates the test comments to document the timing rationale.
Show a summary per file
| File | Description |
|---|---|
internal/server/git/git_test.go |
Adjusts the timeout test to anchor server/client timing before asserting the idle timeout lower bound. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
AriehSchneier
approved these changes
May 16, 2026
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.
Fixes a flaky
TestGitServer_Timeoutreported by @AriehSchneier in #2053.The test compares an elapsed duration measured on the client goroutine against the server's idle timeout, but the server arms its deadline on a separate goroutine. On loaded Windows runners the server can arm the deadline before the client captures
start := time.Now(), causing elapsed to fall short.This change anchors the two clocks by sending a 1-byte probe before timing: pkt-line decoding needs 4 bytes for the length header, so the server consumes the byte and re-enters
Read, arming a fresh idle deadline at approximately the same wall-clock moment as the client'sstart. Recordingstartbefore theWritekeeps any scheduling latency on the elapsed side, making the lower-bound assertion robust.Note: the server-side proposal in #2053 (deferring the initial
updateDeadline()to firstRead) does not eliminate the race becauseserverConn.Readalready callsupdateDeadline()at the start of every read.cc @AriehSchneier