From 06550883c168bc839cdb13dee0ce2e099d8fd2ae Mon Sep 17 00:00:00 2001 From: Carrie Edwards Date: Thu, 7 Mar 2024 09:41:03 -0800 Subject: [PATCH] Clean up of tests and test utils Co-authored by: Fiona Liao : Signed-off-by: Carrie Edwards --- tsdb/db_test.go | 26 +++++++++++++------------- tsdb/head_test.go | 2 +- tsdb/ooo_head_read_test.go | 22 +++++----------------- tsdb/testutil.go | 13 +++++++------ 4 files changed, 26 insertions(+), 37 deletions(-) diff --git a/tsdb/db_test.go b/tsdb/db_test.go index 3d6d55c90..a9c7dee75 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -4572,7 +4572,7 @@ func testOOOCompaction(t *testing.T, scenario sampleTypeScenario) { require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } verifyDBSamples() // Before any compaction. @@ -4639,7 +4639,7 @@ func testOOOCompaction(t *testing.T, scenario sampleTypeScenario) { require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } // Checking for expected data in the blocks. @@ -4779,7 +4779,7 @@ func testOOOCompactionWithNormalCompaction(t *testing.T, scenario sampleTypeScen require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } // Checking for expected data in the blocks. @@ -4888,7 +4888,7 @@ func testOOOCompactionWithDisabledWriteLog(t *testing.T, scenario sampleTypeScen require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } // Checking for expected data in the blocks. @@ -4990,7 +4990,7 @@ func testOOOQueryAfterRestartWithSnapshotAndRemovedWBL(t *testing.T, scenario sa require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } // Checking for expected ooo data from mmap chunks. @@ -5262,7 +5262,7 @@ func testOOOAppendAndQuery(t *testing.T, scenario sampleTypeScenario) { expSamples[k] = append(expSamples[k], s) } } - requireEqualSamples(t, expSamples, seriesSet, true) + requireEqualSeries(t, expSamples, seriesSet, true) requireEqualOOOSamples(t, totalSamples-2, db) } @@ -5379,7 +5379,7 @@ func testOOODisabled(t *testing.T, scenario sampleTypeScenario) { require.NoError(t, err) seriesSet := query(t, querier, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.")) - requireEqualSamples(t, expSamples, seriesSet, true) + requireEqualSeries(t, expSamples, seriesSet, true) requireEqualOOOSamples(t, 0, db) require.Equal(t, float64(failedSamples), prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType))+prom_testutil.ToFloat64(db.head.metrics.outOfBoundSamples.WithLabelValues(scenario.sampleType)), @@ -5446,7 +5446,7 @@ func testWBLAndMmapReplay(t *testing.T, scenario sampleTypeScenario) { }) exp[k] = v } - requireEqualSamples(t, exp, seriesSet, true) + requireEqualSeries(t, exp, seriesSet, true) } // In-order samples. @@ -5712,7 +5712,7 @@ func testOOOCompactionFailure(t *testing.T, scenario sampleTypeScenario) { q, err := NewBlockQuerier(block, math.MinInt64, math.MaxInt64) require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } // Checking for expected data in the blocks. @@ -5948,7 +5948,7 @@ func testOOOMmapCorruption(t *testing.T, scenario sampleTypeScenario) { require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } verifySamples(allSamples) @@ -6076,7 +6076,7 @@ func testOutOfOrderRuntimeConfig(t *testing.T, scenario sampleTypeScenario) { require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } doOOOCompaction := func(t *testing.T, db *DB) { @@ -6287,7 +6287,7 @@ func testNoGapAfterRestartWithOOO(t *testing.T, scenario sampleTypeScenario) { require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } cases := []struct { @@ -6415,7 +6415,7 @@ func testWblReplayAfterOOODisableAndRestart(t *testing.T, scenario sampleTypeSce require.NoError(t, err) actRes := query(t, q, labels.MustNewMatcher(labels.MatchRegexp, "foo", "bar.*")) - requireEqualSamples(t, expRes, actRes, true) + requireEqualSeries(t, expRes, actRes, true) } verifySamples(allSamples) diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 1eb0c0534..fa4834516 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -5232,7 +5232,7 @@ func testHeadMinOOOTimeUpdate(t *testing.T, scenario sampleTypeScenario) { appendSample := func(ts int64) { app := h.Appender(context.Background()) - _, _, err = scenario.appendFunc(app, labels.FromStrings("a", "b"), ts*time.Minute.Milliseconds(), 99.0) + _, _, err = scenario.appendFunc(app, labels.FromStrings("a", "b"), ts*time.Minute.Milliseconds(), ts) require.NoError(t, err) require.NoError(t, app.Commit()) } diff --git a/tsdb/ooo_head_read_test.go b/tsdb/ooo_head_read_test.go index 9eaab13dc..ed9a9f88c 100644 --- a/tsdb/ooo_head_read_test.go +++ b/tsdb/ooo_head_read_test.go @@ -469,10 +469,6 @@ func TestOOOHeadChunkReader_Chunk(t *testing.T) { } } -// TestOOOHeadChunkReader_Chunk tests that the Chunk method works as expected. -// It does so by appending out of order samples to the db and then initializing -// an OOOHeadChunkReader to read chunks from it. -// //nolint:revive // unexported-return. func testOOOHeadChunkReader_Chunk(t *testing.T, scenario sampleTypeScenario) { opts := DefaultOptions() @@ -822,7 +818,8 @@ func testOOOHeadChunkReader_Chunk(t *testing.T, scenario sampleTypeScenario) { db := newTestDBWithOpts(t, opts) app := db.Appender(context.Background()) - s1Ref, _, _ := scenario.appendFunc(app, s1, tc.firstInOrderSampleAt, tc.firstInOrderSampleAt/1*time.Minute.Milliseconds()) + s1Ref, _, err := scenario.appendFunc(app, s1, tc.firstInOrderSampleAt, tc.firstInOrderSampleAt/1*time.Minute.Milliseconds()) + require.NoError(t, err) require.NoError(t, app.Commit()) // OOO few samples for s1. @@ -838,7 +835,7 @@ func testOOOHeadChunkReader_Chunk(t *testing.T, scenario sampleTypeScenario) { ir := NewOOOHeadIndexReader(db.head, tc.queryMinT, tc.queryMaxT, 0) var chks []chunks.Meta var b labels.ScratchBuilder - err := ir.Series(s1Ref, &b, &chks) + err = ir.Series(s1Ref, &b, &chks) require.NoError(t, err) require.Equal(t, len(tc.expChunksSamples), len(chks)) @@ -851,7 +848,7 @@ func testOOOHeadChunkReader_Chunk(t *testing.T, scenario sampleTypeScenario) { it := iterable.Iterator(nil) resultSamples := samplesFromIterator(t, it) - compareSamples(t, s1.String(), tc.expChunksSamples[i], resultSamples, true) + requireEqualSamples(t, s1.String(), tc.expChunksSamples[i], resultSamples, true) } }) } @@ -873,15 +870,6 @@ func TestOOOHeadChunkReader_Chunk_ConsistentQueryResponseDespiteOfHeadExpanding( } } -// TestOOOHeadChunkReader_Chunk_ConsistentQueryResponseDespiteOfHeadExpanding tests -// that if a query comes and performs a Series() call followed by a Chunks() call -// the response is consistent with the data seen by Series() even if the OOO -// head receives more samples before Chunks() is called. -// An example: -// - Response A comes from: Series() then Chunk() -// - Response B comes from : Series(), in parallel new samples added to the head, then Chunk() -// - A == B -// //nolint:revive // unexported-return. func testOOOHeadChunkReader_Chunk_ConsistentQueryResponseDespiteOfHeadExpanding(t *testing.T, scenario sampleTypeScenario) { opts := DefaultOptions() @@ -1054,7 +1042,7 @@ func testOOOHeadChunkReader_Chunk_ConsistentQueryResponseDespiteOfHeadExpanding( it := iterable.Iterator(nil) resultSamples := samplesFromIterator(t, it) - compareSamples(t, s1.String(), tc.expChunksSamples[i], resultSamples, true) + requireEqualSamples(t, s1.String(), tc.expChunksSamples[i], resultSamples, true) } }) } diff --git a/tsdb/testutil.go b/tsdb/testutil.go index 0412cc2c2..3f7df2d3a 100644 --- a/tsdb/testutil.go +++ b/tsdb/testutil.go @@ -44,6 +44,7 @@ type sampleTypeScenario struct { sampleFunc func(ts, value int64) sample } +// TODO: native histogram sample types will be added as part of out-of-order native histogram support; see #11220. var sampleTypeScenarios = map[string]sampleTypeScenario{ float: { sampleType: sampleMetricTypeFloat, @@ -102,12 +103,12 @@ var sampleTypeScenarios = map[string]sampleTypeScenario{ // }, } -// requireEqualSamples checks that the actual series are equal to the expected ones. It ignores the counter reset hints for histograms. -func requireEqualSamples(t *testing.T, expected, actual map[string][]chunks.Sample, ignoreCounterResets bool) { +// requireEqualSeries checks that the actual series are equal to the expected ones. It ignores the counter reset hints for histograms. +func requireEqualSeries(t *testing.T, expected, actual map[string][]chunks.Sample, ignoreCounterResets bool) { for name, expectedItem := range expected { actualItem, ok := actual[name] require.True(t, ok, "Expected series %s not found", name) - compareSamples(t, name, expectedItem, actualItem, ignoreCounterResets) + requireEqualSamples(t, name, expectedItem, actualItem, ignoreCounterResets) } for name := range actual { _, ok := expected[name] @@ -122,12 +123,12 @@ func requireEqualOOOSamples(t *testing.T, expectedSamples int, db *DB) { "number of ooo appended samples mismatch") } -func compareSamples(t *testing.T, name string, expected, actual []chunks.Sample, ignoreCounterResets bool) { - require.Equal(t, len(expected), len(actual), "Length not expected for %s", name) +func requireEqualSamples(t *testing.T, name string, expected, actual []chunks.Sample, ignoreCounterResets bool) { + require.Equal(t, len(expected), len(actual), "Length not equal to expected for %s", name) for i, s := range expected { expectedSample := s actualSample := actual[i] - require.Equal(t, expectedSample.T(), expectedSample.T(), "Different timestamps for %s[%d]", name, i) + require.Equal(t, expectedSample.T(), actualSample.T(), "Different timestamps for %s[%d]", name, i) require.Equal(t, expectedSample.Type().String(), actualSample.Type().String(), "Different types for %s[%d] at ts %d", name, i, expectedSample.T()) switch { case s.H() != nil: