Skip to content

Commit 2a98faf

Browse files
sandy2008claude
andauthored
fix(integration): repair TestExpandedPostingsCacheFuzz latent bugs (#7549)
TestExpandedPostingsCacheFuzz had two independent defects that together neutered the test: 1. The query-generation loop at integration/query_fuzz_test.go:550-557 was inverted: `break` fired the moment isValidQuery returned true, exiting the outer `for i` loop, and only *invalid* expressions ever reached the append. The queries/matchers slices ended up holding at most testRun invalid exprs (or empty if the first draw was valid). Every downstream c1.Query / c1.QueryRange call then short-circuited with the same error on both clients, so cmp.Equal(err1, err2) trivially held and nothing was actually compared. 2. The Series diff branch at integration/query_fuzz_test.go:657 read `cmp.Equal(tc.sres1, tc.sres1, ...)` — self-comparison — so the Series API divergence check was dead code. Net effect: the test had been a no-op for divergences. Issue #7545 captures the empirical census. Fix: wrap the WalkRangeQuery call in a nested `for { ... }` that re-draws until isValidQuery is satisfied, mirroring the sibling fuzz tests in this file, and change `tc.sres1, tc.sres1` to `tc.sres1, tc.sres2`. No production code touched; enabledAggrs is already wired in. Two follow-ups worth flagging honestly: wall-clock will grow substantially (the previously-skipped ~9k HTTP round-trips now actually run), and any genuine v1.18.0-vs-HEAD divergence in the expanded-postings-cache code path will now surface as a real failure for the first time, to be triaged as a real bug rather than a regression from this PR. Fixes #7545 Signed-off-by: Sandy Chen <Yuxuan.Chen@morganstanley.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 76947fd commit 2a98faf

1 file changed

Lines changed: 7 additions & 4 deletions

File tree

integration/query_fuzz_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -550,9 +550,12 @@ func TestExpandedPostingsCacheFuzz(t *testing.T) {
550550
queries := make([]string, 0, testRun)
551551
matchers := make([]string, 0, testRun)
552552
for i := 0; i < testRun; i++ {
553-
expr := ps.WalkRangeQuery()
554-
if isValidQuery(expr, true) {
555-
break
553+
var expr parser.Expr
554+
for {
555+
expr = ps.WalkRangeQuery()
556+
if isValidQuery(expr, true) {
557+
break
558+
}
556559
}
557560
queries = append(queries, expr.Pretty(0))
558561
matchers = append(matchers, storepb.PromMatchersToString(
@@ -651,7 +654,7 @@ func TestExpandedPostingsCacheFuzz(t *testing.T) {
651654
} else if !cmp.Equal(tc.res1, tc.res2, comparer) {
652655
t.Logf("case %d results mismatch.\n%s: %s\nres1: %s\nres2: %s\n", i, tc.qt, tc.query, tc.res1.String(), tc.res2.String())
653656
failures++
654-
} else if !cmp.Equal(tc.sres1, tc.sres1, labelSetsComparer) {
657+
} else if !cmp.Equal(tc.sres1, tc.sres2, labelSetsComparer) {
655658
t.Logf("case %d results mismatch.\n%s: %s\nsres1: %s\nsres2: %s\n", i, tc.qt, tc.query, tc.sres1, tc.sres2)
656659
failures++
657660
}

0 commit comments

Comments
 (0)