Use testutil in rules subpackage (#3278)

* Use testutil in rules subpackage

* Fix manager test

* Use testutil in rules subpackage

* Fix manager test

* Fix rebase

* Change to testutil for applyConfig tests

rules/alerting_test.go

@@ -18,13 +18,12 @@ import (
 
 	"github.com/prometheus/prometheus/pkg/labels"
 	"github.com/prometheus/prometheus/promql"
+	"github.com/prometheus/prometheus/util/testutil"
 )
 
 func TestAlertingRuleHTMLSnippet(t *testing.T) {
 	expr, err := promql.ParseExpr(`foo{html="<b>BOLD<b>"}`)
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatal(err)
-	}
 	rule := NewAlertingRule("testrule", expr, 0, labels.FromStrings("html", "<b>BOLD</b>"), labels.FromStrings("html", "<b>BOLD</b>"), nil)
 
 	const want = `alert: <a href="/test/prefix/graph?g0.expr=ALERTS%7Balertname%3D%22testrule%22%7D&g0.tab=1">testrule</a>
@@ -36,7 +35,5 @@ annotations:
 `
 
 	got := rule.HTMLSnippet("/test/prefix")
-	if got != want {
+	testutil.Assert(t, want == got, "incorrect HTML snippet; want:\n\n|%v|\n\ngot:\n\n|%v|", want, got)
-		t.Fatalf("incorrect HTML snippet; want:\n\n|%v|\n\ngot:\n\n|%v|", want, got)
-	}
 }

rules/manager_test.go

@@ -17,7 +17,6 @@ import (
 	"context"
 	"fmt"
 	"math"
-	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -40,19 +39,14 @@ func TestAlertingRule(t *testing.T) {
 			http_requests{job="app-server", instance="0", group="canary", severity="overwrite-me"}	75 85  95 105 105  95  85
 			http_requests{job="app-server", instance="1", group="canary", severity="overwrite-me"}	80 90 100 110 120 130 140
 	`)
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatal(err)
-	}
 	defer suite.Close()
 
-	if err := suite.Run(); err != nil {
+	err = suite.Run()
-		t.Fatal(err)
+	testutil.Ok(t, err)
-	}
 
 	expr, err := promql.ParseExpr(`http_requests{group="canary", job="app-server"} < 100`)
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatalf("Unable to parse alert expression: %s", err)
-	}
 
 	rule := NewAlertingRule(
 		"HTTPRequestRateLow",
@@ -112,19 +106,14 @@ func TestAlertingRule(t *testing.T) {
 		evalTime := baseTime.Add(test.time)
 
 		res, err := rule.Eval(suite.Context(), evalTime, suite.QueryEngine(), nil)
-		if err != nil {
+		testutil.Ok(t, err)
-			t.Fatalf("Error during alerting rule evaluation: %s", err)
-		}
 
 		actual := strings.Split(res.String(), "\n")
 		expected := annotateWithTime(test.result, evalTime)
 		if actual[0] == "" {
 			actual = []string{}
 		}
-
+		testutil.Assert(t, len(expected) == len(actual), "%d. Number of samples in expected and actual output don't match (%d vs. %d)", i, len(expected), len(actual))
-		if len(actual) != len(expected) {
-			t.Errorf("%d. Number of samples in expected and actual output don't match (%d vs. %d)", i, len(expected), len(actual))
-		}
 
 		for j, expectedSample := range expected {
 			found := false
@@ -133,20 +122,11 @@ func TestAlertingRule(t *testing.T) {
 					found = true
 				}
 			}
-			if !found {
+			testutil.Assert(t, found, "%d.%d. Couldn't find expected sample in output: '%v'", i, j, expectedSample)
-				t.Errorf("%d.%d. Couldn't find expected sample in output: '%v'", i, j, expectedSample)
-			}
-		}
-
-		if t.Failed() {
-			t.Errorf("%d. Expected and actual outputs don't match:", i)
-			t.Fatalf("Expected:\n%v\n----\nActual:\n%v", strings.Join(expected, "\n"), strings.Join(actual, "\n"))
 		}
 
 		for _, aa := range rule.ActiveAlerts() {
-			if v := aa.Labels.Get(model.MetricNameLabel); v != "" {
+			testutil.Assert(t, aa.Labels.Get(model.MetricNameLabel) == "", "%s label set on active alert: %s", model.MetricNameLabel, aa.Labels)
-				t.Fatalf("%s label set on active alert: %s", model.MetricNameLabel, aa.Labels)
-			}
 		}
 	}
 }
@@ -171,9 +151,7 @@ func TestStaleness(t *testing.T) {
 	}
 
 	expr, err := promql.ParseExpr("a + 1")
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatal(err)
-	}
 	rule := NewRecordingRule("a_plus_one", expr, labels.Labels{})
 	group := NewGroup("default", "", time.Second, []Rule{rule}, opts)
 
@@ -182,9 +160,9 @@ func TestStaleness(t *testing.T) {
 	app.Add(labels.FromStrings(model.MetricNameLabel, "a"), 0, 1)
 	app.Add(labels.FromStrings(model.MetricNameLabel, "a"), 1000, 2)
 	app.Add(labels.FromStrings(model.MetricNameLabel, "a"), 2000, math.Float64frombits(value.StaleNaN))
-	if err = app.Commit(); err != nil {
+
-		t.Fatal(err)
+	err = app.Commit()
-	}
+	testutil.Ok(t, err)
 
 	// Execute 3 times, 1 second apart.
 	group.Eval(time.Unix(0, 0))
@@ -192,33 +170,23 @@ func TestStaleness(t *testing.T) {
 	group.Eval(time.Unix(2, 0))
 
 	querier, err := storage.Querier(context.Background(), 0, 2000)
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatal(err)
-	}
 	defer querier.Close()
 	matcher, _ := labels.NewMatcher(labels.MatchEqual, model.MetricNameLabel, "a_plus_one")
 	samples, err := readSeriesSet(querier.Select(matcher))
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatal(err)
-	}
-
 	metric := labels.FromStrings(model.MetricNameLabel, "a_plus_one").String()
 	metricSample, ok := samples[metric]
-	if !ok {
+
-		t.Fatalf("Series %s not returned.", metric)
+	testutil.Assert(t, ok, "Series %s not returned.", metric)
-	}
+	testutil.Assert(t, value.IsStaleNaN(metricSample[2].V), "Appended second sample not as expected. Wanted: stale NaN Got: %x", math.Float64bits(metricSample[2].V))
-	if !value.IsStaleNaN(metricSample[2].V) {
-		t.Fatalf("Appended second sample not as expected. Wanted: stale NaN Got: %x", math.Float64bits(metricSample[2].V))
-	}
 	metricSample[2].V = 42 // reflect.DeepEqual cannot handle NaN.
 
 	want := map[string][]promql.Point{
 		metric: []promql.Point{{0, 2}, {1000, 3}, {2000, 42}},
 	}
 
-	if !reflect.DeepEqual(want, samples) {
+	testutil.Equals(t, want, samples)
-		t.Fatalf("Returned samples not as expected. Wanted: %+v Got: %+v", want, samples)
-	}
 }
 
 // Convert a SeriesSet into a form useable with reflect.DeepEqual.
@@ -280,12 +248,8 @@ func TestCopyState(t *testing.T) {
 		map[string]labels.Labels{"r1": nil},
 		nil,
 	}
-	if !reflect.DeepEqual(want, newGroup.seriesInPreviousEval) {
+	testutil.Equals(t, want, newGroup.seriesInPreviousEval)
-		t.Fatalf("seriesInPreviousEval not as expected. Wanted: %+v Got: %+v", want, newGroup.seriesInPreviousEval)
+	testutil.Equals(t, oldGroup.rules[0], newGroup.rules[3])
-	}
-	if !reflect.DeepEqual(oldGroup.rules[0], newGroup.rules[3]) {
-		t.Fatalf("Active alerts not as expected. Wanted: %+v Got: %+v", oldGroup.rules[0], oldGroup.rules[3])
-	}
 }
 
 func TestApplyConfig(t *testing.T) {
@@ -298,9 +262,7 @@ func TestApplyConfig(t *testing.T) {
 		},
 	}
 	conf, err := config.LoadFile("../config/testdata/conf.good.yml")
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatalf(err.Error())
-	}
 	ruleManager := NewManager(&ManagerOptions{
 		Appendable:  nil,
 		Notifier:    nil,
@@ -310,24 +272,19 @@ func TestApplyConfig(t *testing.T) {
 	})
 	ruleManager.Run()
 
-	if err := ruleManager.ApplyConfig(conf); err != nil {
+	err = ruleManager.ApplyConfig(conf)
-		t.Fatalf(err.Error())
+	testutil.Ok(t, err)
-	}
 	for _, g := range ruleManager.groups {
 		g.seriesInPreviousEval = []map[string]labels.Labels{
 			expected,
 		}
 	}
 
-	if err := ruleManager.ApplyConfig(conf); err != nil {
+	err = ruleManager.ApplyConfig(conf)
-		t.Fatalf(err.Error())
+	testutil.Ok(t, err)
-	}
 	for _, g := range ruleManager.groups {
 		for _, actual := range g.seriesInPreviousEval {
-
+			testutil.Equals(t, expected, actual)
-			if !reflect.DeepEqual(expected, actual) {
-				t.Fatalf("Rule groups state lost after config reload. Expected: %+v Got: %+v", expected, actual)
-			}
 		}
 	}
 }

rules/recording_test.go

@@ -15,7 +15,6 @@ package rules
 
 import (
 	"context"
-	"reflect"
 	"testing"
 	"time"
 
@@ -64,20 +63,14 @@ func TestRuleEval(t *testing.T) {
 	for _, test := range suite {
 		rule := NewRecordingRule(test.name, test.expr, test.labels)
 		result, err := rule.Eval(ctx, now, engine, nil)
-		if err != nil {
+		testutil.Ok(t, err)
-			t.Fatalf("Error evaluating %s", test.name)
+		testutil.Equals(t, result, test.result)
-		}
-		if !reflect.DeepEqual(result, test.result) {
-			t.Fatalf("Error: expected %q, got %q", test.result, result)
-		}
 	}
 }
 
 func TestRecordingRuleHTMLSnippet(t *testing.T) {
 	expr, err := promql.ParseExpr(`foo{html="<b>BOLD<b>"}`)
-	if err != nil {
+	testutil.Ok(t, err)
-		t.Fatal(err)
-	}
 	rule := NewRecordingRule("testrule", expr, labels.FromStrings("html", "<b>BOLD</b>"))
 
 	const want = `record: <a href="/test/prefix/graph?g0.expr=testrule&g0.tab=1">testrule</a>
@@ -87,7 +80,5 @@ labels:
 `
 
 	got := rule.HTMLSnippet("/test/prefix")
-	if got != want {
+	testutil.Assert(t, want == got, "incorrect HTML snippet; want:\n\n%s\n\ngot:\n\n%s", want, got)
-		t.Fatalf("incorrect HTML snippet; want:\n\n%s\n\ngot:\n\n%s", want, got)
-	}
 }