Repository navigation
test(server): make the busy-rescan re-arm assertion timing-free - #1733
Merged
Merged
Conversation
TestInformationalRescan_BusyServerIsRetriedNotDropped sampled the rescan timer map at one fixed instant. fireInformationalRescan drops the entry at fire time and re-creates it only after the gates and the claim ran, so the sample could land in that window under CI load (4 of 8 recent main runs). Read infoRescanSeq before scheduling and wait for a sequence above before+1, which only a busy re-arm can produce. No production change.
Deploying mcpproxy-docs with
|
| Latest commit: |
9871eed
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://44a4780d.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-contributor-issues-pr.mcpproxy-docs.pages.dev |
…absolute ceiling The two guards compared the single worst of 300 scans against 50*median+50ms; one GC pause or descheduled goroutine on a macOS runner hit 119ms against a 109ms bound. Bound p90 relative to the baseline and keep an absolute 2s ceiling on the worst scan, which no scheduler stall reaches but any wait behind the gate or a fetch deadline would.
This was referenced Oct 22, 2026
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 39750735140 --repo smart-mcp-proxy/mcpproxy-go
|
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.
Description
TestInformationalRescan_BusyServerIsRetriedNotDropped(Spec 118, #1689) failed onmainin 4 of the last 8 unit-test runs and was the only red check on contributor PRs #1727, #1725 and #1715.Cause.
fireInformationalRescandeletes the server's entry frominfoRescanTimerswhen the debounce timer fires and re-creates it only after the gates and the claim ran. On every busy fire there is a short window in which "is a timer armed" is false. The test sampled the map at one fixed instant (time.Sleep(4 * debounce)), which under CI load lands in that window. Reproduced locally: 1 failure in 150 runs withGOMAXPROCS=1under 2×ncpu CPU burners.Fix (test only). Read
infoRescanSequnder the mutex before scheduling; the change itself takesbefore+1, sorequire.Eventually(rescanSeq("srv") > before+1)is a timing-free proof that a busy fire re-armed the timer. The "no scan attempted on top of a claimed slot" and "a busy slot is not a failed attempt" assertions are unchanged.Verification. 400 runs of the fixed test under the same CPU contention: 0 failures.
go test -race -count=3 -run TestInformationalRescan ./internal/server/green.golangci-lintwith the CI v2 config clean. Cross-reviewed with opencode (gpt-6.1-sol), two rounds: round 1 found the baseline read could hit the same gap (fixed by reading the counter before scheduling), round 2 CLEAN.No production code changes.
Second flake, same PR
TestRefreshConcurrency_ScansDuringASlowSelfCheckSeeOldOrNew(from #1720) failed the darwin-amd64 Build Binaries job on this PR:119.6ms is not less than 109.7ms (baseline median 1.19ms). Both refresh-concurrency guards compared the single worst of 300 scans against50*median+50ms, which one GC pause on a macOS runner exceeds. They now bound p90 relative to the baseline and keep an absolute 2 s ceiling on the worst scan (no scheduler stall reaches it, any wait behind the gate or a fetch deadline does). 30 runs under CPU contention: 0 failures. opencode Sol 6.1 round 1 flagged that p90 alone could hide one scan absorbing a timed-out wait; the absolute ceiling is the answer to that.