test: compare bucket and raw chart points by timestamp - #17
Merged
Merged
Conversation
The index-aligned comparison assumed the two paths' windows can differ only at the leading point, but the raw path's startTime is its own wall clock per request: on a loaded runner the pair straddles the grid edge and the oldest bucket point is legitimately absent from raw (or present with a partial-window value). CI caught it as a 96-vs-95 divergence; eight spinning cores reproduced it at will. assertAlignedPoints now maps raw points by timestamp, tolerates at most one straddling edge at the oldest end (skipped, not compared), and requires exact agreement everywhere else. Subset-peak fields compare under the never-invent bound (bucket peak <= raw peak) instead of equality; the formatted peak strings are excluded from the strict branch for the same reason. 20 repetitions of all four equivalence tests under an eight-spinners load simulator pass, crossing the grid edge repeatedly.
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
1 participant
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.
Follow-up to #14/#15. The CI release gate on v0.3.34 caught the phase flake the earlier mitigations only narrowed, so the release was pulled back (tag and draft deleted;
latestis v0.3.33 and nothing broken was published).Root cause, finally
The index-aligned comparison assumed the bucket/raw request pair can differ only at the leading point. But each request derives
startTimefrom its own wall clock — on a loaded CI runner the seeding between the two calls crosses a 15-minute grid edge, and then the raw path legitimately drops the oldest display bucket (its samples predate that request'sstartTime) while the stored bucket cannot be re-cut. That is correct product behavior; the test asserted the wrong invariant.Reproduced deterministically with eight spinning-core loaders (8/8 failures before, 0/40 after), including runs that straddle the grid edge live.
Change
assertAlignedPointsmaps raw points by timestamp: at most one straddling edge is tolerated at the oldest end (skipped, never compared), everything after it must agree exactly. Subset-peak fields (rx/tx_peak_bps) compare under the documented never-invent bound (bucket <= raw) instead of equality when the selection is a proper subset; the matching*_formattedstrings are excluded from the strict branch. System and single-interface selections still require full equality of every value — that is where an aggregation bug would actually show.Verification
go test ./internal/...— 6/6 packages.-count=20under the eight-spinners load simulator — clean.After merge
Re-tag
v0.3.34and republish (the bump commit is already on master; only the gate was red).