Repository navigation
test: flakes get their root causes fixed - #555
Merged
Merged
Conversation
TestChangingTheLimitMovesAnOpenTunnel failed 2/40 under -race on linux/arm64: a limit switch lets through the unlimited chunk already in flight (splice checks the kicked deadline between chunks), which reached the test's 512 KiB target in under a millisecond. The test now measures the new rate from 50 ms after the switch, and egress.md and SetLimit say the switch lands after the chunk in flight. TestDisablingARunningClaimRevokesItsPacketPath timed out under full-suite load: the claim's first tun_up runs in a goroutine, and a disable that beats it records down, up, down, while the test waited for up, down. It now waits for the first tun_up before disabling, and a new test pins that a disable beating the first tun_up still leaves the guest down.
…s a redundant down The diagnostic showed which copy moved the extra bytes: the tx spliced copy, parked in waitRead when the kick set its past deadline, woke on data instead. runtime.poll_runtime_pollWait returns ready when the fd is readable even with the deadline expired, so spliceDrain moved what the socket held, up to maxSpliceSize (1 MiB), before the next prepareRead returned the timeout. The test now runs at 2 MiB/s, requires a burst, one splice chunk and 0.75 s at the new cap to take 700 ms or more, and fails when more than the burst and one chunk pass the cap's pace, which a 50 ms late kick does. egress.md and SetLimit name that bound. The tun test accepts the down the arm goroutine repeats when a disable lands between tun_up and its armed check.
…and the remaining at-once docs name the splice bound Taking the time before loading the sink count let a scheduler stall between them count as bytes past the pace. The local rate constant no longer shadows x/time/rate. sandboxd-api.md and SetEgressLimit carry the same one-more-splice caveat as egress.md, which is rewrapped.
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.
Root-causes and fixes the two flaky tests on the v0.1.31 round's open list. Both causes are on the test side; no product behaviour changes.
TestChangingTheLimitMovesAnOpenTunnel(egress; 2/40 under-raceon linux/arm64)kicksets a past read deadline, but splice checks it only between chunks.egress.mdand theSetLimitdoc say the switch lands once the chunk in flight is through. The comment is edited in place.-raceon linux/arm64;kick()onSetLimitfails 3/3.TestDisablingARunningClaimRevokesItsPacketPath(pool; under full-suite load only)tun_upruns in a goroutine. A disable that beats it recordsdown, up, down; the arm goroutine re-checks and downs the guest again, so the guest ends down, which is correct. The test waited for exactlyup, down. Forced withtunUpGate, the verbs are[down, up, down].tun_upbefore it disables.TestADisableBeforeTheFirstTunUpLeavesTheGuestDownpins that convergence.-raceon linux/arm64;tun_upfails it.Cost. Tests and docs only.
/simplify: test-only diff of +28/−6. Nothing to fold: the egress fix reuses
waitBytesand the tun fix reusestunVerbsOfanddoorOpen.Gates (fail-fast ship script):
0 issues.on darwin and linux; fmt clean; asl clean;utilstest;SetLimitdoc edit.