From 5961f781861fbd91775fea212dc57a5d4a190512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Szulik?= Date: Thu, 2 Sep 2021 17:43:54 +0200 Subject: [PATCH 1/6] Refactor tsdb tests to use testify. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Szulik --- tsdb/db_test.go | 29 ++++++++----------- tsdb/fileutil/flock_test.go | 55 +++++++++++++------------------------ tsdb/head_test.go | 8 ++---- tsdb/index/postings_test.go | 28 +++++-------------- tsdb/querier_test.go | 25 ++--------------- tsdb/wal_test.go | 2 +- tsdb/wlog/reader_test.go | 12 ++++---- tsdb/wlog/wlog_test.go | 4 +-- 8 files changed, 50 insertions(+), 113 deletions(-) diff --git a/tsdb/db_test.go b/tsdb/db_test.go index 9e543ed500..fb36bbb868 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -27,7 +27,6 @@ import ( "path/filepath" "sort" "strconv" - "strings" "sync" "testing" "time" @@ -2327,9 +2326,7 @@ func TestBlockRanges(t *testing.T) { app := db.Appender(ctx) lbl := labels.FromStrings("a", "b") _, err = app.Append(0, lbl, firstBlockMaxT-1, rand.Float64()) - if err == nil { - t.Fatalf("appending a sample with a timestamp covered by a previous block shouldn't be possible") - } + require.Error(t, err, "appending a sample with a timestamp covered by a previous block shouldn't be possible") _, err = app.Append(0, lbl, firstBlockMaxT+1, rand.Float64()) require.NoError(t, err) _, err = app.Append(0, lbl, firstBlockMaxT+2, rand.Float64()) @@ -2347,9 +2344,8 @@ func TestBlockRanges(t *testing.T) { } require.Len(t, db.Blocks(), 2, "no new block created after the set timeout") - if db.Blocks()[0].Meta().MaxTime > db.Blocks()[1].Meta().MinTime { - t.Fatalf("new block overlaps old:%v,new:%v", db.Blocks()[0].Meta(), db.Blocks()[1].Meta()) - } + require.LessOrEqual(t, db.Blocks()[1].Meta().MinTime, db.Blocks()[0].Meta().MaxTime, + "new block overlaps old:%v,new:%v", db.Blocks()[0].Meta(), db.Blocks()[1].Meta()) // Test that wal records are skipped when an existing block covers the same time ranges // and compaction doesn't create an overlapping block. @@ -2389,9 +2385,8 @@ func TestBlockRanges(t *testing.T) { require.Len(t, db.Blocks(), 4, "no new block created after the set timeout") - if db.Blocks()[2].Meta().MaxTime > db.Blocks()[3].Meta().MinTime { - t.Fatalf("new block overlaps old:%v,new:%v", db.Blocks()[2].Meta(), db.Blocks()[3].Meta()) - } + require.LessOrEqual(t, db.Blocks()[3].Meta().MinTime, db.Blocks()[2].Meta().MaxTime, + "new block overlaps old:%v,new:%v", db.Blocks()[2].Meta(), db.Blocks()[3].Meta()) } // TestDBReadOnly ensures that opening a DB in readonly mode doesn't modify any files on the disk. @@ -3180,9 +3175,8 @@ func TestOpen_VariousBlockStates(t *testing.T) { var loaded int for _, l := range loadedBlocks { - if _, ok := expectedLoadedDirs[filepath.Join(tmpDir, l.meta.ULID.String())]; !ok { - t.Fatal("unexpected block", l.meta.ULID, "was loaded") - } + _, ok := expectedLoadedDirs[filepath.Join(tmpDir, l.meta.ULID.String())] + require.True(t, ok, "unexpected block", l.meta.ULID, "was loaded") loaded++ } require.Len(t, expectedLoadedDirs, loaded) @@ -3193,9 +3187,8 @@ func TestOpen_VariousBlockStates(t *testing.T) { var ignored int for _, f := range files { - if _, ok := expectedRemovedDirs[filepath.Join(tmpDir, f.Name())]; ok { - t.Fatal("expected", filepath.Join(tmpDir, f.Name()), "to be removed, but still exists") - } + _, ok := expectedRemovedDirs[filepath.Join(tmpDir, f.Name())] + require.False(t, ok, "expected", filepath.Join(tmpDir, f.Name()), "to be removed, but still exists") if _, ok := expectedIgnoredDirs[filepath.Join(tmpDir, f.Name())]; ok { ignored++ } @@ -3486,8 +3479,8 @@ func testQuerierShouldNotPanicIfHeadChunkIsTruncatedWhileReadingQueriedChunks(t // the "cannot populate chunk XXX: not found" error occurred. This error can occur // when the iterator tries to fetch an head chunk which has been offloaded because // of the head compaction in the meanwhile. - if firstErr != nil && !strings.Contains(firstErr.Error(), "cannot populate chunk") { - t.Fatalf("unexpected error: %s", firstErr.Error()) + if firstErr != nil { + require.ErrorContains(t, firstErr, "cannot populate chunk", "unexpected error: %s", firstErr.Error()) } } diff --git a/tsdb/fileutil/flock_test.go b/tsdb/fileutil/flock_test.go index f9cbbcf2b3..240e0954b1 100644 --- a/tsdb/fileutil/flock_test.go +++ b/tsdb/fileutil/flock_test.go @@ -18,6 +18,8 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/util/testutil" ) @@ -27,54 +29,35 @@ func TestLocking(t *testing.T) { fileName := filepath.Join(dir.Path(), "LOCK") - if _, err := os.Stat(fileName); err == nil { - t.Fatalf("File %q unexpectedly exists.", fileName) - } + _, err := os.Stat(fileName) + require.Error(t, err, "File %q unexpectedly exists.", fileName) lock, existed, err := Flock(fileName) - if err != nil { - t.Fatalf("Error locking file %q: %s", fileName, err) - } - if existed { - t.Errorf("File %q reported as existing during locking.", fileName) - } + require.NoError(t, err, "Error locking file %q: %s", fileName, err) + require.False(t, existed, "File %q reported as existing during locking.", fileName) // File must now exist. - if _, err = os.Stat(fileName); err != nil { - t.Errorf("Could not stat file %q expected to exist: %s", fileName, err) - } + _, err = os.Stat(fileName) + require.NoError(t, err, "Could not stat file %q expected to exist: %s", fileName, err) // Try to lock again. lockedAgain, existed, err := Flock(fileName) - if err == nil { - t.Fatalf("File %q locked twice.", fileName) - } - if lockedAgain != nil { - t.Error("Unsuccessful locking did not return nil.") - } - if !existed { - t.Errorf("Existing file %q not recognized.", fileName) - } + require.Error(t, err, "File %q locked twice.", fileName) + require.Nil(t, lockedAgain, "Unsuccessful locking did not return nil.") + require.True(t, existed, "Existing file %q not recognized.", fileName) - if err := lock.Release(); err != nil { - t.Errorf("Error releasing lock for file %q: %s", fileName, err) - } + err = lock.Release() + require.NoError(t, err, "Error releasing lock for file %q: %s", fileName, err) // File must still exist. - if _, err = os.Stat(fileName); err != nil { - t.Errorf("Could not stat file %q expected to exist: %s", fileName, err) - } + _, err = os.Stat(fileName) + require.NoError(t, err, "Could not stat file %q expected to exist: %s", fileName, err) // Lock existing file. lock, existed, err = Flock(fileName) - if err != nil { - t.Fatalf("Error locking file %q: %s", fileName, err) - } - if !existed { - t.Errorf("Existing file %q not recognized.", fileName) - } + require.NoError(t, err, "Error locking file %q: %s", fileName, err) + require.True(t, existed, "Existing file %q not recognized.", fileName) - if err := lock.Release(); err != nil { - t.Errorf("Error releasing lock for file %q: %s", fileName, err) - } + err = lock.Release() + require.NoError(t, err, "Error releasing lock for file %q: %s", fileName, err) } diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 90e187b58c..cec7c06bb6 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -206,7 +206,7 @@ func readTestWAL(t testing.TB, dir string) (recs []interface{}) { require.NoError(t, err) recs = append(recs, exemplars) default: - t.Fatalf("unknown record type") + require.Fail(t, "unknown record type") } } require.NoError(t, r.Err()) @@ -1371,7 +1371,7 @@ func TestDeletedSamplesAndSeriesStillInWALAfterCheckpoint(t *testing.T) { case []record.RefMetadata: metadata++ default: - t.Fatalf("unknown record type") + require.Fail(t, "unknown record type") } } require.Equal(t, 1, series) @@ -1620,9 +1620,7 @@ func TestComputeChunkEndTime(t *testing.T) { for testName, tc := range cases { t.Run(testName, func(t *testing.T) { got := computeChunkEndTime(tc.start, tc.cur, tc.max, tc.ratioToFull) - if got != tc.res { - t.Errorf("expected %d for (start: %d, cur: %d, max: %d, ratioToFull: %f), got %d", tc.res, tc.start, tc.cur, tc.max, tc.ratioToFull, got) - } + require.Equal(t, tc.res, got, "(start: %d, cur: %d, max: %d)", tc.start, tc.cur, tc.max) }) } } diff --git a/tsdb/index/postings_test.go b/tsdb/index/postings_test.go index e8df6dbd29..9e6bd23f8c 100644 --- a/tsdb/index/postings_test.go +++ b/tsdb/index/postings_test.go @@ -61,9 +61,7 @@ func TestMemPostings_ensureOrder(t *testing.T) { ok := sort.SliceIsSorted(l, func(i, j int) bool { return l[i] < l[j] }) - if !ok { - t.Fatalf("postings list %v is not sorted", l) - } + require.True(t, ok, "postings list %v is not sorted", l) } } } @@ -214,9 +212,7 @@ func TestIntersect(t *testing.T) { for _, c := range cases { t.Run("", func(t *testing.T) { - if c.res == nil { - t.Fatal("intersect result expectancy cannot be nil") - } + require.NotNil(t, c.res, "intersect result expectancy cannot be nil") expected, err := ExpandPostings(c.res) require.NoError(t, err) @@ -228,9 +224,7 @@ func TestIntersect(t *testing.T) { return } - if i == EmptyPostings() { - t.Fatal("intersect unexpected result: EmptyPostings sentinel") - } + require.NotEqual(t, EmptyPostings(), i, "intersect unexpected result: EmptyPostings sentinel") res, err := ExpandPostings(i) require.NoError(t, err) @@ -501,9 +495,7 @@ func TestMergedPostings(t *testing.T) { for _, c := range cases { t.Run("", func(t *testing.T) { - if c.res == nil { - t.Fatal("merge result expectancy cannot be nil") - } + require.NotNil(t, c.res, "merge result expectancy cannot be nil") ctx := context.Background() @@ -517,9 +509,7 @@ func TestMergedPostings(t *testing.T) { return } - if m == EmptyPostings() { - t.Fatal("merge unexpected result: EmptyPostings sentinel") - } + require.NotEqual(t, EmptyPostings(), m, "merge unexpected result: EmptyPostings sentinel") res, err := ExpandPostings(m) require.NoError(t, err) @@ -897,9 +887,7 @@ func TestWithoutPostings(t *testing.T) { for _, c := range cases { t.Run("", func(t *testing.T) { - if c.res == nil { - t.Fatal("without result expectancy cannot be nil") - } + require.NotNil(t, c.res, "without result expectancy cannot be nil") expected, err := ExpandPostings(c.res) require.NoError(t, err) @@ -911,9 +899,7 @@ func TestWithoutPostings(t *testing.T) { return } - if w == EmptyPostings() { - t.Fatal("without unexpected result: EmptyPostings sentinel") - } + require.NotEqual(t, EmptyPostings(), w, "without unexpected result: EmptyPostings sentinel") res, err := ExpandPostings(w) require.NoError(t, err) diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index c2bd717082..6e1a19ee4b 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -2702,22 +2702,7 @@ func TestFindSetMatches(t *testing.T) { } for _, c := range cases { - matches := findSetMatches(c.pattern) - if len(c.exp) == 0 { - if len(matches) != 0 { - t.Errorf("Evaluating %s, unexpected result %v", c.pattern, matches) - } - } else { - if len(matches) != len(c.exp) { - t.Errorf("Evaluating %s, length of result not equal to exp", c.pattern) - } else { - for i := 0; i < len(c.exp); i++ { - if c.exp[i] != matches[i] { - t.Errorf("Evaluating %s, unexpected result %s", c.pattern, matches[i]) - } - } - } - } + require.Equal(t, c.exp, findSetMatches(c.pattern), "Evaluating %s, unexpected result.", c.pattern) } } @@ -3016,9 +3001,7 @@ func TestPostingsForMatchers(t *testing.T) { } } require.NoError(t, p.Err()) - if len(exp) != 0 { - t.Errorf("Evaluating %v, missing results %+v", c.matchers, exp) - } + require.Empty(t, exp, "Evaluating %v", c.matchers) }) } } @@ -3101,9 +3084,7 @@ func TestClose(t *testing.T) { createBlock(t, dir, genSeries(1, 1, 10, 20)) db, err := Open(dir, nil, nil, DefaultOptions(), nil) - if err != nil { - t.Fatalf("Opening test storage failed: %s", err) - } + require.NoError(t, err, "Opening test storage failed: %s", err) defer func() { require.NoError(t, db.Close()) }() diff --git a/tsdb/wal_test.go b/tsdb/wal_test.go index 9004f5093f..8700a70754 100644 --- a/tsdb/wal_test.go +++ b/tsdb/wal_test.go @@ -528,7 +528,7 @@ func TestMigrateWAL_Fuzz(t *testing.T) { require.NoError(t, err) res = append(res, s) default: - t.Fatalf("unknown record type %d", dec.Type(rec)) + require.Fail(t, "unknown record type %d", dec.Type(rec)) } } require.NoError(t, r.Err()) diff --git a/tsdb/wlog/reader_test.go b/tsdb/wlog/reader_test.go index 0f510e0c1e..1546a97527 100644 --- a/tsdb/wlog/reader_test.go +++ b/tsdb/wlog/reader_test.go @@ -182,16 +182,14 @@ func TestReader(t *testing.T) { t.Logf("record %d", j) rec := r.Record() - if j >= len(c.exp) { - t.Fatal("received more records than expected") - } + require.Less(t, j, len(c.exp), "received more records than expected") require.Equal(t, c.exp[j], rec, "Bytes within record did not match expected Bytes") } - if !c.fail && r.Err() != nil { - t.Fatalf("unexpected error: %s", r.Err()) + if !c.fail { + require.NoError(t, r.Err(), "unexpected error: %s", r.Err()) } - if c.fail && r.Err() == nil { - t.Fatalf("expected error but got none") + if c.fail { + require.Error(t, r.Err(), "expected error but got none") } }) } diff --git a/tsdb/wlog/wlog_test.go b/tsdb/wlog/wlog_test.go index 8f4533e0ea..7d96920117 100644 --- a/tsdb/wlog/wlog_test.go +++ b/tsdb/wlog/wlog_test.go @@ -192,9 +192,7 @@ func TestWALRepair_ReadingError(t *testing.T) { require.Len(t, result, test.intactRecs, "Wrong number of intact records") for i, r := range result { - if !bytes.Equal(records[i], r) { - t.Fatalf("record %d diverges: want %x, got %x", i, records[i][:10], r[:10]) - } + require.True(t, bytes.Equal(records[i], r), "record %d diverges: want %x, got %x", i, records[i][:10], r[:10]) } // Make sure there is a new 0 size Segment after the corrupted Segment. From d5eb636a89d5ecbffe1c8b866b186e5dc560cf49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Szulik?= Date: Thu, 2 Sep 2021 17:54:19 +0200 Subject: [PATCH 2/6] Refactor cmd tests to use testify. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Szulik --- cmd/prometheus/main_test.go | 41 +++++++++++--------------------- cmd/prometheus/main_unix_test.go | 21 +++++++--------- cmd/promtool/unittest_test.go | 7 +++--- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/cmd/prometheus/main_test.go b/cmd/prometheus/main_test.go index f4fe3855c6..03f3a9bc39 100644 --- a/cmd/prometheus/main_test.go +++ b/cmd/prometheus/main_test.go @@ -126,12 +126,9 @@ func TestFailedStartupExitCode(t *testing.T) { require.Error(t, err) var exitError *exec.ExitError - if errors.As(err, &exitError) { - status := exitError.Sys().(syscall.WaitStatus) - require.Equal(t, expectedExitStatus, status.ExitStatus()) - } else { - t.Errorf("unable to retrieve the exit status for prometheus: %v", err) - } + require.ErrorAs(t, err, &exitError) + status := exitError.Sys().(syscall.WaitStatus) + require.Equal(t, expectedExitStatus, status.ExitStatus()) } type senderFunc func(alerts ...*notifier.Alert) @@ -194,9 +191,7 @@ func TestSendAlerts(t *testing.T) { tc := tc t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { senderFunc := senderFunc(func(alerts ...*notifier.Alert) { - if len(tc.in) == 0 { - t.Fatalf("sender called with 0 alert") - } + require.NotEmpty(t, tc.in, "sender called with 0 alert") require.Equal(t, tc.exp, alerts) }) rules.SendAlerts(senderFunc, "http://localhost:9090")(context.TODO(), "up", tc.in...) @@ -228,7 +223,7 @@ func TestWALSegmentSizeBounds(t *testing.T) { go func() { done <- prom.Wait() }() select { case err := <-done: - t.Errorf("prometheus should be still running: %v", err) + require.Fail(t, "prometheus should be still running: %v", err) case <-time.After(startupTime): prom.Process.Kill() <-done @@ -239,12 +234,9 @@ func TestWALSegmentSizeBounds(t *testing.T) { err = prom.Wait() require.Error(t, err) var exitError *exec.ExitError - if errors.As(err, &exitError) { - status := exitError.Sys().(syscall.WaitStatus) - require.Equal(t, expectedExitStatus, status.ExitStatus()) - } else { - t.Errorf("unable to retrieve the exit status for prometheus: %v", err) - } + require.ErrorAs(t, err, &exitError) + status := exitError.Sys().(syscall.WaitStatus) + require.Equal(t, expectedExitStatus, status.ExitStatus()) } } @@ -274,7 +266,7 @@ func TestMaxBlockChunkSegmentSizeBounds(t *testing.T) { go func() { done <- prom.Wait() }() select { case err := <-done: - t.Errorf("prometheus should be still running: %v", err) + require.Fail(t, "prometheus should be still running: %v", err) case <-time.After(startupTime): prom.Process.Kill() <-done @@ -285,12 +277,9 @@ func TestMaxBlockChunkSegmentSizeBounds(t *testing.T) { err = prom.Wait() require.Error(t, err) var exitError *exec.ExitError - if errors.As(err, &exitError) { - status := exitError.Sys().(syscall.WaitStatus) - require.Equal(t, expectedExitStatus, status.ExitStatus()) - } else { - t.Errorf("unable to retrieve the exit status for prometheus: %v", err) - } + require.ErrorAs(t, err, &exitError) + status := exitError.Sys().(syscall.WaitStatus) + require.Equal(t, expectedExitStatus, status.ExitStatus()) } } @@ -347,10 +336,8 @@ func getCurrentGaugeValuesFor(t *testing.T, reg prometheus.Gatherer, metricNames } require.Len(t, g.GetMetric(), 1) - if _, ok := res[m]; ok { - t.Error("expected only one metric family for", m) - t.FailNow() - } + _, ok := res[m] + require.False(t, ok, "expected only one metric family for", m) res[m] = *g.GetMetric()[0].GetGauge().Value } } diff --git a/cmd/prometheus/main_unix_test.go b/cmd/prometheus/main_unix_test.go index 417d062d66..59c5fe2bce 100644 --- a/cmd/prometheus/main_unix_test.go +++ b/cmd/prometheus/main_unix_test.go @@ -23,6 +23,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/util/testutil" ) @@ -37,9 +39,7 @@ func TestStartupInterrupt(t *testing.T) { prom := exec.Command(promPath, "-test.main", "--config.file="+promConfig, "--storage.tsdb.path="+t.TempDir(), "--web.listen-address=0.0.0.0"+port) err := prom.Start() - if err != nil { - t.Fatalf("execution error: %v", err) - } + require.NoError(t, err) done := make(chan error, 1) go func() { @@ -68,14 +68,11 @@ Loop: time.Sleep(500 * time.Millisecond) } - if !startedOk { - t.Fatal("prometheus didn't start in the specified timeout") - } - switch err := prom.Process.Kill(); { - case err == nil: - t.Errorf("prometheus didn't shutdown gracefully after sending the Interrupt signal") - case stoppedErr != nil && stoppedErr.Error() != "signal: interrupt": - // TODO: find a better way to detect when the process didn't exit as expected! - t.Errorf("prometheus exited with an unexpected error: %v", stoppedErr) + require.True(t, startedOk, "prometheus didn't start in the specified timeout") + err = prom.Process.Kill() + require.Error(t, err, "prometheus didn't shutdown gracefully after sending the Interrupt signal") + // TODO - find a better way to detect when the process didn't exit as expected! + if stoppedErr != nil { + require.EqualError(t, stoppedErr, "signal: interrupt", "prometheus exited with an unexpected error: %v", stoppedErr) } } diff --git a/cmd/promtool/unittest_test.go b/cmd/promtool/unittest_test.go index b8170d784e..971ddb40c5 100644 --- a/cmd/promtool/unittest_test.go +++ b/cmd/promtool/unittest_test.go @@ -16,6 +16,8 @@ package main import ( "testing" + "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/promql" ) @@ -178,9 +180,8 @@ func TestRulesUnitTestRun(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := RulesUnitTest(tt.queryOpts, tt.args.run, false, tt.args.files...); got != tt.want { - t.Errorf("RulesUnitTest() = %v, want %v", got, tt.want) - } + got := RulesUnitTest(tt.queryOpts, tt.args.run, false, tt.args.files...) + require.Equal(t, tt.want, got) }) } } From 6e9cca8158e924e810a24e58dea57f2c0c132707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Szulik?= Date: Fri, 3 Sep 2021 11:51:27 +0200 Subject: [PATCH 3/6] Refactor web tests to use testify. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Szulik --- web/api/v1/api_test.go | 129 +++++++++++------------------------------ web/web_test.go | 4 +- 2 files changed, 37 insertions(+), 96 deletions(-) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 3dc83548de..97a4c6220d 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -216,18 +216,11 @@ type rulesRetrieverMock struct { func (m *rulesRetrieverMock) CreateAlertingRules() { expr1, err := parser.ParseExpr(`absent(test_metric3) != 1`) - if err != nil { - m.testing.Fatalf("unable to parse alert expression: %s", err) - } + require.NoError(m.testing, err) expr2, err := parser.ParseExpr(`up == 1`) - if err != nil { - m.testing.Fatalf("Unable to parse alert expression: %s", err) - } - + require.NoError(m.testing, err) expr3, err := parser.ParseExpr(`vector(1)`) - if err != nil { - m.testing.Fatalf("Unable to parse alert expression: %s", err) - } + require.NoError(m.testing, err) rule1 := rules.NewAlertingRule( "test_metric3", @@ -302,9 +295,7 @@ func (m *rulesRetrieverMock) CreateRuleGroups() { } recordingExpr, err := parser.ParseExpr(`vector(1)`) - if err != nil { - m.testing.Fatalf("unable to parse alert expression: %s", err) - } + require.NoError(m.testing, err, "unable to parse alert expression: %s", err) recordingRule := rules.NewRecordingRule("recording-rule-1", recordingExpr, labels.Labels{}) r = append(r, recordingRule) @@ -714,9 +705,7 @@ func TestQueryExemplars(t *testing.T) { for _, te := range tc.exemplars { for _, e := range te.Exemplars { _, err := es.AppendExemplar(0, te.SeriesLabels, e) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } } @@ -2832,9 +2821,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E } req, err := request(method, test.query) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) tr.ResetMetadataStore() for _, tm := range test.metadata { @@ -2844,9 +2831,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E for _, te := range test.exemplars { for _, e := range te.Exemplars { _, err := es.AppendExemplar(0, te.SeriesLabels, e) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } } @@ -2882,17 +2867,11 @@ func describeAPIFunc(f apiFunc) string { func assertAPIError(t *testing.T, got *apiError, exp errorType) { t.Helper() - if got != nil { - if exp == errorNone { - t.Fatalf("Unexpected error: %s", got) - } - if exp != got.typ { - t.Fatalf("Expected error of type %q but got type %q (%q)", exp, got.typ, got) - } - return - } - if exp != errorNone { - t.Fatalf("Expected error of type %q but got none", exp) + if exp == errorNone { + require.Nil(t, got) + } else { + require.NotNil(t, got) + require.Equal(t, exp, got.typ, "(%q)", got) } } @@ -2906,13 +2885,7 @@ func assertAPIResponseLength(t *testing.T, got interface{}, expLen int) { t.Helper() gotLen := reflect.ValueOf(got).Len() - if gotLen != expLen { - t.Fatalf( - "Response length does not match, expected:\n%d\ngot:\n%d", - expLen, - gotLen, - ) - } + require.Equal(t, expLen, gotLen, "Response length does not match") } func assertAPIResponseMetadataLen(t *testing.T, got interface{}, expLen int) { @@ -2924,13 +2897,7 @@ func assertAPIResponseMetadataLen(t *testing.T, got interface{}, expLen int) { gotLen += len(m) } - if gotLen != expLen { - t.Fatalf( - "Amount of metadata in the response does not match, expected:\n%d\ngot:\n%d", - expLen, - gotLen, - ) - } + require.Equal(t, expLen, gotLen, "Amount of metadata in the response does not match") } type fakeDB struct { @@ -3271,26 +3238,18 @@ func TestRespondError(t *testing.T) { defer s.Close() resp, err := http.Get(s.URL) - if err != nil { - t.Fatalf("Error on test request: %s", err) - } + require.NoError(t, err, "Error on test request: %s", err) body, err := io.ReadAll(resp.Body) defer resp.Body.Close() - if err != nil { - t.Fatalf("Error reading response body: %s", err) - } - - if want, have := http.StatusServiceUnavailable, resp.StatusCode; want != have { - t.Fatalf("Return code %d expected in error response but got %d", want, have) - } - if h := resp.Header.Get("Content-Type"); h != "application/json" { - t.Fatalf("Expected Content-Type %q but got %q", "application/json", h) - } + require.NoError(t, err, "Error reading response body: %s", err) + want, have := http.StatusServiceUnavailable, resp.StatusCode + require.Equal(t, want, have, "Return code %d expected in error response but got %d", want, have) + h := resp.Header.Get("Content-Type") + require.Equal(t, "application/json", h, "Expected Content-Type %q but got %q", "application/json", h) var res Response - if err = json.Unmarshal(body, &res); err != nil { - t.Fatalf("Error unmarshaling JSON body: %s", err) - } + err = json.Unmarshal(body, &res) + require.NoError(t, err, "Error unmarshaling JSON body: %s", err) exp := &Response{ Status: statusError, @@ -3419,17 +3378,13 @@ func TestParseTime(t *testing.T) { for _, test := range tests { ts, err := parseTime(test.input) - if err != nil && !test.fail { - t.Errorf("Unexpected error for %q: %s", test.input, err) + if !test.fail { + require.NoError(t, err, "Unexpected error for %q: %s", test.input, err) + require.NotNil(t, ts) + require.True(t, ts.Equal(test.result), "Expected time %v for input %q but got %v", test.result, test.input, ts) continue } - if err == nil && test.fail { - t.Errorf("Expected error for %q but got none", test.input) - continue - } - if !test.fail && !ts.Equal(test.result) { - t.Errorf("Expected time %v for input %q but got %v", test.result, test.input, ts) - } + require.Error(t, err, "Expected error for %q but got none", test.input) } } @@ -3473,17 +3428,12 @@ func TestParseDuration(t *testing.T) { for _, test := range tests { d, err := parseDuration(test.input) - if err != nil && !test.fail { - t.Errorf("Unexpected error for %q: %s", test.input, err) + if !test.fail { + require.NoError(t, err, "Unexpected error for %q: %s", test.input, err) + require.Equal(t, test.result, d, "Expected duration %v for input %q but got %v", test.result, test.input, d) continue } - if err == nil && test.fail { - t.Errorf("Expected error for %q but got none", test.input) - continue - } - if !test.fail && d != test.result { - t.Errorf("Expected duration %v for input %q but got %v", test.result, test.input, d) - } + require.Error(t, err, "Expected error for %q but got none", test.input) } } @@ -3496,18 +3446,11 @@ func TestOptionsMethod(t *testing.T) { defer s.Close() req, err := http.NewRequest("OPTIONS", s.URL+"/any_path", nil) - if err != nil { - t.Fatalf("Error creating OPTIONS request: %s", err) - } + require.NoError(t, err, "Error creating OPTIONS request") client := &http.Client{} resp, err := client.Do(req) - if err != nil { - t.Fatalf("Error executing OPTIONS request: %s", err) - } - - if resp.StatusCode != http.StatusNoContent { - t.Fatalf("Expected status %d, got %d", http.StatusNoContent, resp.StatusCode) - } + require.NoError(t, err, "Error executing OPTIONS request") + require.Equal(t, http.StatusNoContent, resp.StatusCode) } func TestTSDBStatus(t *testing.T) { @@ -3546,9 +3489,7 @@ func TestTSDBStatus(t *testing.T) { api := &API{db: tc.db, gatherer: prometheus.DefaultGatherer} endpoint := tc.endpoint(api) req, err := http.NewRequest(tc.method, fmt.Sprintf("?%s", tc.values.Encode()), nil) - if err != nil { - t.Fatalf("Error when creating test request: %s", err) - } + require.NoError(t, err, "Error when creating test request") res := endpoint(req) assertAPIError(t, res.err, tc.errType) }) diff --git a/web/web_test.go b/web/web_test.go index 8832c28390..62bdb2ae31 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -440,7 +440,7 @@ func TestShutdownWithStaleConnection(t *testing.T) { select { case <-closed: case <-time.After(timeout + 5*time.Second): - t.Fatalf("Server still running after read timeout.") + require.FailNow(t, "Server still running after read timeout.") } } @@ -502,7 +502,7 @@ func TestHandleMultipleQuitRequests(t *testing.T) { select { case <-closed: case <-time.After(5 * time.Second): - t.Fatalf("Server still running after 5 seconds.") + require.FailNow(t, "Server still running after 5 seconds.") } } From b0c538787d9f89a7e3a9802223a51bcb84441485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Szulik?= Date: Sat, 4 Sep 2021 14:35:03 +0200 Subject: [PATCH 4/6] Refactor scrape tests to use testify. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Szulik --- scrape/manager_test.go | 60 ++++++++------------ scrape/scrape_test.go | 122 ++++++++++++++++------------------------- scrape/target_test.go | 102 +++++++++------------------------- 3 files changed, 98 insertions(+), 186 deletions(-) diff --git a/scrape/manager_test.go b/scrape/manager_test.go index 524424269e..a73b730786 100644 --- a/scrape/manager_test.go +++ b/scrape/manager_test.go @@ -458,9 +458,9 @@ func loadConfiguration(t testing.TB, c string) *config.Config { t.Helper() cfg := &config.Config{} - if err := yaml.UnmarshalStrict([]byte(c), cfg); err != nil { - t.Fatalf("Unable to load YAML config: %s", err) - } + err := yaml.UnmarshalStrict([]byte(c), cfg) + require.NoError(t, err, "Unable to load YAML config.") + return cfg } @@ -533,42 +533,38 @@ scrape_configs: } // Apply the initial configuration. - if err := scrapeManager.ApplyConfig(cfg1); err != nil { - t.Fatalf("unable to apply configuration: %s", err) - } + err = scrapeManager.ApplyConfig(cfg1) + require.NoError(t, err, "Unable to apply configuration.") select { case <-ch: - t.Fatal("reload happened") + require.FailNow(t, "Reload happened.") default: } // Apply a configuration for which the reload fails. - if err := scrapeManager.ApplyConfig(cfg2); err == nil { - t.Fatalf("expecting error but got none") - } + err = scrapeManager.ApplyConfig(cfg2) + require.Error(t, err, "Expecting error but got none.") select { case <-ch: - t.Fatal("reload happened") + require.FailNow(t, "Reload happened.") default: } // Apply a configuration for which the reload succeeds. - if err := scrapeManager.ApplyConfig(cfg3); err != nil { - t.Fatalf("unable to apply configuration: %s", err) - } + err = scrapeManager.ApplyConfig(cfg3) + require.NoError(t, err, "Unable to apply configuration.") select { case <-ch: default: - t.Fatal("reload didn't happen") + require.FailNow(t, "Reload didn't happen.") } // Re-applying the same configuration shouldn't trigger a reload. - if err := scrapeManager.ApplyConfig(cfg3); err != nil { - t.Fatalf("unable to apply configuration: %s", err) - } + err = scrapeManager.ApplyConfig(cfg3) + require.NoError(t, err, "Unable to apply configuration.") select { case <-ch: - t.Fatal("reload happened") + require.FailNow(t, "Reload happened.") default: } } @@ -595,7 +591,7 @@ func TestManagerTargetsUpdates(t *testing.T) { select { case ts <- tgSent: case <-time.After(10 * time.Millisecond): - t.Error("Scrape manager's channel remained blocked after the set threshold.") + require.Fail(t, "Scrape manager's channel remained blocked after the set threshold.") } } @@ -609,7 +605,7 @@ func TestManagerTargetsUpdates(t *testing.T) { select { case <-m.triggerReload: default: - t.Error("No scrape loops reload was triggered after targets update.") + require.Fail(t, "No scrape loops reload was triggered after targets update.") } } @@ -622,9 +618,8 @@ global: ` cfg := &config.Config{} - if err := yaml.UnmarshalStrict([]byte(cfgText), cfg); err != nil { - t.Fatalf("Unable to load YAML config cfgYaml: %s", err) - } + err := yaml.UnmarshalStrict([]byte(cfgText), cfg) + require.NoError(t, err, "Unable to load YAML config cfgYaml.") return cfg } @@ -636,25 +631,18 @@ global: // Load the first config. cfg1 := getConfig("ha1") - if err := scrapeManager.setOffsetSeed(cfg1.GlobalConfig.ExternalLabels); err != nil { - t.Error(err) - } + err = scrapeManager.setOffsetSeed(cfg1.GlobalConfig.ExternalLabels) + require.NoError(t, err) offsetSeed1 := scrapeManager.offsetSeed - if offsetSeed1 == 0 { - t.Error("Offset seed has to be a hash of uint64") - } + require.NotZero(t, offsetSeed1, "Offset seed has to be a hash of uint64.") // Load the first config. cfg2 := getConfig("ha2") - if err := scrapeManager.setOffsetSeed(cfg2.GlobalConfig.ExternalLabels); err != nil { - t.Error(err) - } + require.NoError(t, scrapeManager.setOffsetSeed(cfg2.GlobalConfig.ExternalLabels)) offsetSeed2 := scrapeManager.offsetSeed - if offsetSeed1 == offsetSeed2 { - t.Error("Offset seed should not be the same on different set of external labels") - } + require.NotEqual(t, offsetSeed1, offsetSeed2, "Offset seed should not be the same on different set of external labels.") } func TestManagerScrapePools(t *testing.T) { diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 95e4e182a0..f827ffc8da 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -72,15 +72,11 @@ func TestNewScrapePool(t *testing.T) { sp, _ = newScrapePool(cfg, app, 0, nil, nil, &Options{}, newTestScrapeMetrics(t)) ) - if a, ok := sp.appendable.(*nopAppendable); !ok || a != app { - t.Fatalf("Wrong sample appender") - } - if sp.config != cfg { - t.Fatalf("Wrong scrape config") - } - if sp.newLoop == nil { - t.Fatalf("newLoop function not initialized") - } + a, ok := sp.appendable.(*nopAppendable) + require.True(t, ok, "Failure to append.") + require.Equal(t, app, a, "Wrong sample appender.") + require.Equal(t, cfg, sp.config, "Wrong scrape config.") + require.NotNil(t, sp.newLoop, "newLoop function not initialized.") } func TestDroppedTargetsList(t *testing.T) { @@ -233,12 +229,10 @@ func TestScrapePoolStop(t *testing.T) { select { case <-time.After(5 * time.Second): - t.Fatalf("scrapeLoop.stop() did not return as expected") + require.Fail(t, "scrapeLoop.stop() did not return as expected") case <-done: // This should have taken at least as long as the last target slept. - if time.Since(stopTime) < time.Duration(numTargets*20)*time.Millisecond { - t.Fatalf("scrapeLoop.stop() exited before all targets stopped") - } + require.GreaterOrEqual(t, time.Since(stopTime), time.Duration(numTargets*20)*time.Millisecond, "scrapeLoop.stop() exited before all targets stopped") } mtx.Lock() @@ -324,12 +318,10 @@ func TestScrapePoolReload(t *testing.T) { select { case <-time.After(5 * time.Second): - t.Fatalf("scrapeLoop.reload() did not return as expected") + require.FailNow(t, "scrapeLoop.reload() did not return as expected") case <-done: // This should have taken at least as long as the last target slept. - if time.Since(reloadTime) < time.Duration(numTargets*20)*time.Millisecond { - t.Fatalf("scrapeLoop.stop() exited before all targets stopped") - } + require.GreaterOrEqual(t, time.Since(reloadTime), time.Duration(numTargets*20)*time.Millisecond, "scrapeLoop.stop() exited before all targets stopped") } mtx.Lock() @@ -703,13 +695,13 @@ func TestScrapeLoopStopBeforeRun(t *testing.T) { select { case <-stopDone: - t.Fatalf("Stopping terminated before run exited successfully") + require.FailNow(t, "Stopping terminated before run exited successfully.") case <-time.After(500 * time.Millisecond): } // Running the scrape loop must exit before calling the scraper even once. scraper.scrapeFunc = func(context.Context, io.Writer) error { - t.Fatalf("scraper was called for terminated scrape loop") + require.FailNow(t, "Scraper was called for terminated scrape loop.") return nil } @@ -722,13 +714,13 @@ func TestScrapeLoopStopBeforeRun(t *testing.T) { select { case <-runDone: case <-time.After(1 * time.Second): - t.Fatalf("Running terminated scrape loop did not exit") + require.FailNow(t, "Running terminated scrape loop did not exit.") } select { case <-stopDone: case <-time.After(1 * time.Second): - t.Fatalf("Stopping did not terminate after running exited") + require.FailNow(t, "Stopping did not terminate after running exited.") } } @@ -765,14 +757,13 @@ func TestScrapeLoopStop(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape wasn't stopped.") + require.FailNow(t, "Scrape wasn't stopped.") } // We expected 1 actual sample for each scrape plus 5 for report samples. // At least 2 scrapes were made, plus the final stale markers. - if len(appender.resultFloats) < 6*3 || len(appender.resultFloats)%6 != 0 { - t.Fatalf("Expected at least 3 scrapes with 6 samples each, got %d samples", len(appender.resultFloats)) - } + require.GreaterOrEqual(t, len(appender.resultFloats), 6*3, "Expected at least 3 scrapes with 6 samples each.") + require.Zero(t, len(appender.resultFloats)%6, "There is a scrape with missing samples.") // All samples in a scrape must have the same timestamp. var ts int64 for i, s := range appender.resultFloats { @@ -785,9 +776,7 @@ func TestScrapeLoopStop(t *testing.T) { } // All samples from the last scrape must be stale markers. for _, s := range appender.resultFloats[len(appender.resultFloats)-5:] { - if !value.IsStaleNaN(s.f) { - t.Fatalf("Appended last sample not as expected. Wanted: stale NaN Got: %x", math.Float64bits(s.f)) - } + require.True(t, value.IsStaleNaN(s.f), "Appended last sample not as expected. Wanted: stale NaN Got: %x", math.Float64bits(s.f)) } } @@ -843,9 +832,9 @@ func TestScrapeLoopRun(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Cancellation during initial offset failed") + require.FailNow(t, "Cancellation during initial offset failed.") case err := <-errc: - t.Fatalf("Unexpected error: %s", err) + require.FailNow(t, "Unexpected error: %s", err) } // The provided timeout must cause cancellation of the context passed down to the @@ -873,11 +862,9 @@ func TestScrapeLoopRun(t *testing.T) { select { case err := <-errc: - if !errors.Is(err, context.DeadlineExceeded) { - t.Fatalf("Expected timeout error but got: %s", err) - } + require.ErrorIs(t, err, context.DeadlineExceeded) case <-time.After(3 * time.Second): - t.Fatalf("Expected timeout error but got none") + require.FailNow(t, "Expected timeout error but got none.") } // We already caught the timeout error and are certainly in the loop. @@ -890,9 +877,9 @@ func TestScrapeLoopRun(t *testing.T) { case <-signal: // Loop terminated as expected. case err := <-errc: - t.Fatalf("Unexpected error: %s", err) + require.FailNow(t, "Unexpected error: %s", err) case <-time.After(3 * time.Second): - t.Fatalf("Loop did not terminate on context cancellation") + require.FailNow(t, "Loop did not terminate on context cancellation") } } @@ -912,7 +899,7 @@ func TestScrapeLoopForcedErr(t *testing.T) { sl.setForcedError(forcedErr) scraper.scrapeFunc = func(context.Context, io.Writer) error { - t.Fatalf("should not be scraped") + require.FailNow(t, "Should not be scraped.") return nil } @@ -923,18 +910,16 @@ func TestScrapeLoopForcedErr(t *testing.T) { select { case err := <-errc: - if !errors.Is(err, forcedErr) { - t.Fatalf("Expected forced error but got: %s", err) - } + require.ErrorIs(t, err, forcedErr) case <-time.After(3 * time.Second): - t.Fatalf("Expected forced error but got none") + require.FailNow(t, "Expected forced error but got none.") } cancel() select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape not stopped") + require.FailNow(t, "Scrape not stopped.") } } @@ -1141,7 +1126,7 @@ func TestScrapeLoopRunCreatesStaleMarkersOnFailedScrape(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape wasn't stopped.") + require.FailNow(t, "Scrape wasn't stopped.") } // 1 successfully scraped sample, 1 stale marker after first fail, 5 report samples for @@ -1188,7 +1173,7 @@ func TestScrapeLoopRunCreatesStaleMarkersOnParseFailure(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape wasn't stopped.") + require.FailNow(t, "Scrape wasn't stopped.") } // 1 successfully scraped sample, 1 stale marker after first fail, 5 report samples for @@ -1220,19 +1205,15 @@ func TestScrapeLoopCache(t *testing.T) { scraper.scrapeFunc = func(ctx context.Context, w io.Writer) error { switch numScrapes { case 1, 2: - if _, ok := sl.cache.series["metric_a"]; !ok { - t.Errorf("metric_a missing from cache after scrape %d", numScrapes) - } - if _, ok := sl.cache.series["metric_b"]; !ok { - t.Errorf("metric_b missing from cache after scrape %d", numScrapes) - } + _, ok := sl.cache.series["metric_a"] + require.True(t, ok, "metric_a missing from cache after scrape %d", numScrapes) + _, ok = sl.cache.series["metric_b"] + require.True(t, ok, "metric_b missing from cache after scrape %d", numScrapes) case 3: - if _, ok := sl.cache.series["metric_a"]; !ok { - t.Errorf("metric_a missing from cache after scrape %d", numScrapes) - } - if _, ok := sl.cache.series["metric_b"]; ok { - t.Errorf("metric_b present in cache after scrape %d", numScrapes) - } + _, ok := sl.cache.series["metric_a"] + require.True(t, ok, "metric_a missing from cache after scrape %d", numScrapes) + _, ok = sl.cache.series["metric_b"] + require.False(t, ok, "metric_b present in cache after scrape %d", numScrapes) } numScrapes++ @@ -1257,7 +1238,7 @@ func TestScrapeLoopCache(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape wasn't stopped.") + require.FailNow(t, "Scrape wasn't stopped.") } // 1 successfully scraped sample, 1 stale marker after first fail, 5 report samples for @@ -1305,12 +1286,10 @@ func TestScrapeLoopCacheMemoryExhaustionProtection(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape wasn't stopped.") + require.FailNow(t, "Scrape wasn't stopped.") } - if len(sl.cache.series) > 2000 { - t.Fatalf("More than 2000 series cached. Got: %d", len(sl.cache.series)) - } + require.LessOrEqual(t, len(sl.cache.series), 2000, "More than 2000 series cached.") } func TestScrapeLoopAppend(t *testing.T) { @@ -1541,9 +1520,7 @@ func TestScrapeLoopAppendSampleLimit(t *testing.T) { now := time.Now() slApp := sl.appender(context.Background()) total, added, seriesAdded, err := sl.append(app, []byte("metric_a 1\nmetric_b 1\nmetric_c 1\n"), "", now) - if !errors.Is(err, errSampleLimit) { - t.Fatalf("Did not see expected sample limit error: %s", err) - } + require.ErrorIs(t, err, errSampleLimit) require.NoError(t, slApp.Rollback()) require.Equal(t, 3, total) require.Equal(t, 3, added) @@ -1572,9 +1549,7 @@ func TestScrapeLoopAppendSampleLimit(t *testing.T) { now = time.Now() slApp = sl.appender(context.Background()) total, added, seriesAdded, err = sl.append(slApp, []byte("metric_a 1\nmetric_b 1\nmetric_c{deleteme=\"yes\"} 1\nmetric_d 1\nmetric_e 1\nmetric_f 1\nmetric_g 1\nmetric_h{deleteme=\"yes\"} 1\nmetric_i{deleteme=\"yes\"} 1\n"), "", now) - if !errors.Is(err, errSampleLimit) { - t.Fatalf("Did not see expected sample limit error: %s", err) - } + require.ErrorIs(t, err, errSampleLimit) require.NoError(t, slApp.Rollback()) require.Equal(t, 9, total) require.Equal(t, 6, added) @@ -2357,15 +2332,12 @@ func TestTargetScraperScrapeOK(t *testing.T) { http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if protobufParsing { accept := r.Header.Get("Accept") - if !strings.HasPrefix(accept, "application/vnd.google.protobuf;") { - t.Errorf("Expected Accept header to prefer application/vnd.google.protobuf, got %q", accept) - } + require.True(t, strings.HasPrefix(accept, "application/vnd.google.protobuf;"), + "Expected Accept header to prefer application/vnd.google.protobuf.") } timeout := r.Header.Get("X-Prometheus-Scrape-Timeout-Seconds") - if timeout != expectedTimeout { - t.Errorf("Expected scrape timeout header %q, got %q", expectedTimeout, timeout) - } + require.Equal(t, expectedTimeout, timeout, "Expected scrape timeout header.") w.Header().Set("Content-Type", `text/plain; version=0.0.4`) w.Write([]byte("metric_a 1\nmetric_b 2\n")) @@ -2453,7 +2425,7 @@ func TestTargetScrapeScrapeCancel(t *testing.T) { select { case <-time.After(5 * time.Second): - t.Fatalf("Scrape function did not return unexpectedly") + require.FailNow(t, "Scrape function did not return unexpectedly.") case err := <-errc: require.NoError(t, err) } @@ -3053,7 +3025,7 @@ func TestScrapeReportSingleAppender(t *testing.T) { select { case <-signal: case <-time.After(5 * time.Second): - t.Fatalf("Scrape wasn't stopped.") + require.FailNow(t, "Scrape wasn't stopped.") } } diff --git a/scrape/target_test.go b/scrape/target_test.go index dac502a80e..6e87ce71d9 100644 --- a/scrape/target_test.go +++ b/scrape/target_test.go @@ -77,9 +77,7 @@ func TestTargetOffset(t *testing.T) { buckets := make([]int, interval/bucketSize) for _, offset := range offsets { - if offset < 0 || offset >= interval { - t.Fatalf("Offset %v out of bounds", offset) - } + require.InDelta(t, time.Duration(0), offset, float64(interval), "Offset %v out of bounds.", offset) bucket := offset / bucketSize buckets[bucket]++ @@ -98,9 +96,7 @@ func TestTargetOffset(t *testing.T) { diff = -diff } - if float64(diff)/float64(avg) > tolerance { - t.Fatalf("Bucket out of tolerance bounds") - } + require.LessOrEqual(t, float64(diff)/float64(avg), tolerance, "Bucket out of tolerance bounds.") } } @@ -150,9 +146,7 @@ func TestNewHTTPBearerToken(t *testing.T) { func(w http.ResponseWriter, r *http.Request) { expected := "Bearer 1234" received := r.Header.Get("Authorization") - if expected != received { - t.Fatalf("Authorization header was not set correctly: expected '%v', got '%v'", expected, received) - } + require.Equal(t, expected, received, "Authorization header was not set correctly.") }, ), ) @@ -162,13 +156,9 @@ func TestNewHTTPBearerToken(t *testing.T) { BearerToken: "1234", } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } func TestNewHTTPBearerTokenFile(t *testing.T) { @@ -177,9 +167,7 @@ func TestNewHTTPBearerTokenFile(t *testing.T) { func(w http.ResponseWriter, r *http.Request) { expected := "Bearer 12345" received := r.Header.Get("Authorization") - if expected != received { - t.Fatalf("Authorization header was not set correctly: expected '%v', got '%v'", expected, received) - } + require.Equal(t, expected, received, "Authorization header was not set correctly.") }, ), ) @@ -189,13 +177,9 @@ func TestNewHTTPBearerTokenFile(t *testing.T) { BearerTokenFile: "testdata/bearertoken.txt", } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } func TestNewHTTPBasicAuth(t *testing.T) { @@ -203,9 +187,9 @@ func TestNewHTTPBasicAuth(t *testing.T) { http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { username, password, ok := r.BasicAuth() - if !(ok && username == "user" && password == "password123") { - t.Fatalf("Basic authorization header was not set correctly: expected '%v:%v', got '%v:%v'", "user", "password123", username, password) - } + require.True(t, ok, "Basic authorization header was not set correctly.") + require.Equal(t, "user", username) + require.Equal(t, "password123", password) }, ), ) @@ -218,13 +202,9 @@ func TestNewHTTPBasicAuth(t *testing.T) { }, } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } func TestNewHTTPCACert(t *testing.T) { @@ -246,13 +226,9 @@ func TestNewHTTPCACert(t *testing.T) { }, } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } func TestNewHTTPClientCert(t *testing.T) { @@ -279,13 +255,9 @@ func TestNewHTTPClientCert(t *testing.T) { }, } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } func TestNewHTTPWithServerName(t *testing.T) { @@ -308,13 +280,9 @@ func TestNewHTTPWithServerName(t *testing.T) { }, } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) } func TestNewHTTPWithBadServerName(t *testing.T) { @@ -337,31 +305,23 @@ func TestNewHTTPWithBadServerName(t *testing.T) { }, } c, err := config_util.NewClientFromConfig(cfg, "test") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) _, err = c.Get(server.URL) - if err == nil { - t.Fatal("Expected error, got nil.") - } + require.Error(t, err) } func newTLSConfig(certName string, t *testing.T) *tls.Config { tlsConfig := &tls.Config{} caCertPool := x509.NewCertPool() caCert, err := os.ReadFile(caCertPath) - if err != nil { - t.Fatalf("Couldn't set up TLS server: %v", err) - } + require.NoError(t, err, "Couldn't read CA cert.") caCertPool.AppendCertsFromPEM(caCert) tlsConfig.RootCAs = caCertPool tlsConfig.ServerName = "127.0.0.1" certPath := fmt.Sprintf("testdata/%s.cer", certName) keyPath := fmt.Sprintf("testdata/%s.key", certName) cert, err := tls.LoadX509KeyPair(certPath, keyPath) - if err != nil { - t.Errorf("Unable to use specified server cert (%s) & key (%v): %s", certPath, keyPath, err) - } + require.NoError(t, err, "Unable to use specified server cert (%s) & key (%v).", certPath, keyPath) tlsConfig.Certificates = []tls.Certificate{cert} return tlsConfig } @@ -375,9 +335,7 @@ func TestNewClientWithBadTLSConfig(t *testing.T) { }, } _, err := config_util.NewClientFromConfig(cfg, "test") - if err == nil { - t.Fatalf("Expected error, got nil.") - } + require.Error(t, err) } func TestTargetsFromGroup(t *testing.T) { @@ -389,15 +347,9 @@ func TestTargetsFromGroup(t *testing.T) { } lb := labels.NewBuilder(labels.EmptyLabels()) targets, failures := TargetsFromGroup(&targetgroup.Group{Targets: []model.LabelSet{{}, {model.AddressLabel: "localhost:9090"}}}, &cfg, false, nil, lb) - if len(targets) != 1 { - t.Fatalf("Expected 1 target, got %v", len(targets)) - } - if len(failures) != 1 { - t.Fatalf("Expected 1 failure, got %v", len(failures)) - } - if failures[0].Error() != expectedError { - t.Fatalf("Expected error %s, got %s", expectedError, failures[0]) - } + require.Len(t, targets, 1) + require.Len(t, failures, 1) + require.EqualError(t, failures[0], expectedError) } func BenchmarkTargetsFromGroup(b *testing.B) { From 1a47c7d59bef3c8aac2be0b5d65517ea1ae1ddeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Szulik?= Date: Wed, 15 Sep 2021 14:22:37 +0200 Subject: [PATCH 5/6] Refactor lexer tests to use testify. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Szulik --- promql/parser/lex_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/promql/parser/lex_test.go b/promql/parser/lex_test.go index 23c9dfbee0..4a29351b06 100644 --- a/promql/parser/lex_test.go +++ b/promql/parser/lex_test.go @@ -815,16 +815,10 @@ func TestLexer(t *testing.T) { hasError = true } } - if !hasError { - t.Logf("%d: input %q", i, test.input) - require.Fail(t, "expected lexing error but did not fail") - } + require.True(t, hasError, "%d: input %q, expected lexing error but did not fail", i, test.input) continue } - if lastItem.Typ == ERROR { - t.Logf("%d: input %q", i, test.input) - require.Fail(t, "unexpected lexing error at position %d: %s", lastItem.Pos, lastItem) - } + require.NotEqual(t, ERROR, lastItem.Typ, "%d: input %q, unexpected lexing error at position %d: %s", i, test.input, lastItem.Pos, lastItem) eofItem := Item{EOF, posrange.Pos(len(test.input)), ""} require.Equal(t, lastItem, eofItem, "%d: input %q", i, test.input) From 16e68c01e4bf0df3da3a02a214f0076bed911d4d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 1 Feb 2024 14:18:01 +0000 Subject: [PATCH 6/6] tests: remove err from message when testify prints it already For instance `require.NoError` will print the unexpected error; we don't need to include it in the message. Signed-off-by: Bryan Boreham --- cmd/prometheus/main_unix_test.go | 2 +- tsdb/db_test.go | 2 +- tsdb/fileutil/flock_test.go | 12 ++++++------ tsdb/querier_test.go | 2 +- tsdb/wlog/reader_test.go | 7 +++---- web/api/v1/api_test.go | 12 ++++++------ 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/cmd/prometheus/main_unix_test.go b/cmd/prometheus/main_unix_test.go index 59c5fe2bce..2011fb123f 100644 --- a/cmd/prometheus/main_unix_test.go +++ b/cmd/prometheus/main_unix_test.go @@ -73,6 +73,6 @@ Loop: require.Error(t, err, "prometheus didn't shutdown gracefully after sending the Interrupt signal") // TODO - find a better way to detect when the process didn't exit as expected! if stoppedErr != nil { - require.EqualError(t, stoppedErr, "signal: interrupt", "prometheus exited with an unexpected error: %v", stoppedErr) + require.EqualError(t, stoppedErr, "signal: interrupt", "prometheus exit") } } diff --git a/tsdb/db_test.go b/tsdb/db_test.go index fb36bbb868..70b12d487a 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -3480,7 +3480,7 @@ func testQuerierShouldNotPanicIfHeadChunkIsTruncatedWhileReadingQueriedChunks(t // when the iterator tries to fetch an head chunk which has been offloaded because // of the head compaction in the meanwhile. if firstErr != nil { - require.ErrorContains(t, firstErr, "cannot populate chunk", "unexpected error: %s", firstErr.Error()) + require.ErrorContains(t, firstErr, "cannot populate chunk") } } diff --git a/tsdb/fileutil/flock_test.go b/tsdb/fileutil/flock_test.go index 240e0954b1..7aff789a26 100644 --- a/tsdb/fileutil/flock_test.go +++ b/tsdb/fileutil/flock_test.go @@ -33,12 +33,12 @@ func TestLocking(t *testing.T) { require.Error(t, err, "File %q unexpectedly exists.", fileName) lock, existed, err := Flock(fileName) - require.NoError(t, err, "Error locking file %q: %s", fileName, err) + require.NoError(t, err, "Error locking file %q", fileName) require.False(t, existed, "File %q reported as existing during locking.", fileName) // File must now exist. _, err = os.Stat(fileName) - require.NoError(t, err, "Could not stat file %q expected to exist: %s", fileName, err) + require.NoError(t, err, "Could not stat file %q expected to exist", fileName) // Try to lock again. lockedAgain, existed, err := Flock(fileName) @@ -47,17 +47,17 @@ func TestLocking(t *testing.T) { require.True(t, existed, "Existing file %q not recognized.", fileName) err = lock.Release() - require.NoError(t, err, "Error releasing lock for file %q: %s", fileName, err) + require.NoError(t, err, "Error releasing lock for file %q", fileName) // File must still exist. _, err = os.Stat(fileName) - require.NoError(t, err, "Could not stat file %q expected to exist: %s", fileName, err) + require.NoError(t, err, "Could not stat file %q expected to exist", fileName) // Lock existing file. lock, existed, err = Flock(fileName) - require.NoError(t, err, "Error locking file %q: %s", fileName, err) + require.NoError(t, err, "Error locking file %q", fileName) require.True(t, existed, "Existing file %q not recognized.", fileName) err = lock.Release() - require.NoError(t, err, "Error releasing lock for file %q: %s", fileName, err) + require.NoError(t, err, "Error releasing lock for file %q", fileName) } diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index 6e1a19ee4b..93704809b9 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -3084,7 +3084,7 @@ func TestClose(t *testing.T) { createBlock(t, dir, genSeries(1, 1, 10, 20)) db, err := Open(dir, nil, nil, DefaultOptions(), nil) - require.NoError(t, err, "Opening test storage failed: %s", err) + require.NoError(t, err, "Opening test storage failed: %s") defer func() { require.NoError(t, db.Close()) }() diff --git a/tsdb/wlog/reader_test.go b/tsdb/wlog/reader_test.go index 1546a97527..484eff3664 100644 --- a/tsdb/wlog/reader_test.go +++ b/tsdb/wlog/reader_test.go @@ -186,10 +186,9 @@ func TestReader(t *testing.T) { require.Equal(t, c.exp[j], rec, "Bytes within record did not match expected Bytes") } if !c.fail { - require.NoError(t, r.Err(), "unexpected error: %s", r.Err()) - } - if c.fail { - require.Error(t, r.Err(), "expected error but got none") + require.NoError(t, r.Err()) + } else { + require.Error(t, r.Err()) } }) } diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 97a4c6220d..eceadb20ae 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -295,7 +295,7 @@ func (m *rulesRetrieverMock) CreateRuleGroups() { } recordingExpr, err := parser.ParseExpr(`vector(1)`) - require.NoError(m.testing, err, "unable to parse alert expression: %s", err) + require.NoError(m.testing, err, "unable to parse alert expression") recordingRule := rules.NewRecordingRule("recording-rule-1", recordingExpr, labels.Labels{}) r = append(r, recordingRule) @@ -3238,10 +3238,10 @@ func TestRespondError(t *testing.T) { defer s.Close() resp, err := http.Get(s.URL) - require.NoError(t, err, "Error on test request: %s", err) + require.NoError(t, err, "Error on test request") body, err := io.ReadAll(resp.Body) defer resp.Body.Close() - require.NoError(t, err, "Error reading response body: %s", err) + require.NoError(t, err, "Error reading response body") want, have := http.StatusServiceUnavailable, resp.StatusCode require.Equal(t, want, have, "Return code %d expected in error response but got %d", want, have) h := resp.Header.Get("Content-Type") @@ -3249,7 +3249,7 @@ func TestRespondError(t *testing.T) { var res Response err = json.Unmarshal(body, &res) - require.NoError(t, err, "Error unmarshaling JSON body: %s", err) + require.NoError(t, err, "Error unmarshaling JSON body") exp := &Response{ Status: statusError, @@ -3379,7 +3379,7 @@ func TestParseTime(t *testing.T) { for _, test := range tests { ts, err := parseTime(test.input) if !test.fail { - require.NoError(t, err, "Unexpected error for %q: %s", test.input, err) + require.NoError(t, err, "Unexpected error for %q", test.input) require.NotNil(t, ts) require.True(t, ts.Equal(test.result), "Expected time %v for input %q but got %v", test.result, test.input, ts) continue @@ -3429,7 +3429,7 @@ func TestParseDuration(t *testing.T) { for _, test := range tests { d, err := parseDuration(test.input) if !test.fail { - require.NoError(t, err, "Unexpected error for %q: %s", test.input, err) + require.NoError(t, err, "Unexpected error for %q", test.input) require.Equal(t, test.result, d, "Expected duration %v for input %q but got %v", test.result, test.input, d) continue }