Refactor test assertions (prometheus#8110)

* Refactor test assertions This pull request gets rid of assert.True where possible to use fine-grained assertions. Signed-off-by: Julien Pivotto <[email protected]>
kieslee · Oct 27, 2020 · 1282d1b · 1282d1b
1 parent 2cbc0f9
commit 1282d1b
Show file tree

Hide file tree

Showing 40 changed files with 200 additions and 203 deletions.
diff --git a/cmd/prometheus/query_log_test.go b/cmd/prometheus/query_log_test.go
@@ -298,7 +298,7 @@ func (p *queryLogTest) run(t *testing.T) {
 	if p.exactQueryCount() {
 		assert.Equal(t, 1, qc)
 	} else {
-		assert.True(t, qc > 0, "no queries logged")
+		assert.Greater(t, qc, 0, "no queries logged")
 	}
 	p.validateLastQuery(t, ql)
 
@@ -324,7 +324,7 @@ func (p *queryLogTest) run(t *testing.T) {
 	if p.exactQueryCount() {
 		assert.Equal(t, qc, len(ql))
 	} else {
-		assert.True(t, len(ql) > qc, "no queries logged")
+		assert.Greater(t, len(ql), qc, "no queries logged")
 	}
 	p.validateLastQuery(t, ql)
 	qc = len(ql)
@@ -355,7 +355,7 @@ func (p *queryLogTest) run(t *testing.T) {
 	if p.exactQueryCount() {
 		assert.Equal(t, qc, len(ql))
 	} else {
-		assert.True(t, len(ql) > qc, "no queries logged")
+		assert.Greater(t, len(ql), qc, "no queries logged")
 	}
 	p.validateLastQuery(t, ql)
 
@@ -368,7 +368,7 @@ func (p *queryLogTest) run(t *testing.T) {
 	if p.exactQueryCount() {
 		assert.Equal(t, 1, qc)
 	} else {
-		assert.True(t, qc > 0, "no queries logged")
+		assert.Greater(t, qc, 0, "no queries logged")
 	}
 }
 

diff --git a/config/config_test.go b/config/config_test.go
@@ -19,7 +19,6 @@ import (
 	"net/url"
 	"path/filepath"
 	"regexp"
-	"strings"
 	"testing"
 	"time"
 
@@ -763,8 +762,8 @@ func TestElideSecrets(t *testing.T) {
 	yamlConfig := string(config)
 
 	matches := secretRe.FindAllStringIndex(yamlConfig, -1)
-	assert.True(t, len(matches) == 10, "wrong number of secret matches found")
-	assert.True(t, !strings.Contains(yamlConfig, "mysecret"),
+	assert.Equal(t, 10, len(matches), "wrong number of secret matches found")
+	assert.NotContains(t, yamlConfig, "mysecret",
 		"yaml marshal reveals authentication credentials.")
 }
 
@@ -1027,7 +1026,7 @@ func TestBadConfigs(t *testing.T) {
 	for _, ee := range expectedErrors {
 		_, err := LoadFile("testdata/" + ee.filename)
 		assert.Error(t, err, "%s", ee.filename)
-		assert.True(t, strings.Contains(err.Error(), ee.errMsg),
+		assert.Contains(t, err.Error(), ee.errMsg,
 			"Expected error for %s to contain %q but got: %s", ee.filename, ee.errMsg, err)
 	}
 }

diff --git a/discovery/consul/consul_test.go b/discovery/consul/consul_test.go
@@ -275,7 +275,7 @@ func checkOneTarget(t *testing.T, tg []*targetgroup.Group) {
 	assert.Equal(t, target.Source, string(target.Labels["__meta_consul_service"]))
 	if target.Source == "test" {
 		// test service should have one node.
-		assert.True(t, len(target.Targets) > 0, "Test service should have one node")
+		assert.Greater(t, len(target.Targets), 0, "Test service should have one node")
 	}
 }
 

diff --git a/discovery/openstack/hypervisor_test.go b/discovery/openstack/hypervisor_test.go
@@ -15,7 +15,6 @@ package openstack
 
 import (
 	"context"
-	"strings"
 	"testing"
 
 	"github.com/prometheus/common/model"
@@ -96,5 +95,5 @@ func TestOpenstackSDHypervisorRefreshWithDoneContext(t *testing.T) {
 	cancel()
 	_, err := hypervisor.refresh(ctx)
 	assert.Error(t, err)
-	assert.True(t, strings.Contains(err.Error(), context.Canceled.Error()), "%q doesn't contain %q", err, context.Canceled)
+	assert.Contains(t, err.Error(), context.Canceled.Error(), "%q doesn't contain %q", err, context.Canceled)
 }
diff --git a/discovery/openstack/instance_test.go b/discovery/openstack/instance_test.go
@@ -16,7 +16,6 @@ package openstack
 import (
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/prometheus/common/model"
@@ -135,5 +134,5 @@ func TestOpenstackSDInstanceRefreshWithDoneContext(t *testing.T) {
 	cancel()
 	_, err := hypervisor.refresh(ctx)
 	assert.Error(t, err)
-	assert.True(t, strings.Contains(err.Error(), context.Canceled.Error()), "%q doesn't contain %q", err, context.Canceled)
+	assert.Contains(t, err.Error(), context.Canceled.Error(), "%q doesn't contain %q", err, context.Canceled)
 }
diff --git a/discovery/triton/triton_test.go b/discovery/triton/triton_test.go
@@ -87,7 +87,7 @@ func TestTritonSDNew(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NotNil(t, td)
 	assert.NotNil(t, td.client)
-	assert.True(t, td.interval != 0, "")
+	assert.NotZero(t, td.interval)
 	assert.NotNil(t, td.sdConfig)
 	assert.Equal(t, conf.Account, td.sdConfig.Account)
 	assert.Equal(t, conf.DNSSuffix, td.sdConfig.DNSSuffix)
@@ -98,15 +98,15 @@ func TestTritonSDNew(t *testing.T) {
 func TestTritonSDNewBadConfig(t *testing.T) {
 	td, err := newTritonDiscovery(badconf)
 	assert.Error(t, err)
-	assert.True(t, td == nil, "")
+	assert.Nil(t, td)
 }
 
 func TestTritonSDNewGroupsConfig(t *testing.T) {
 	td, err := newTritonDiscovery(groupsconf)
 	assert.NoError(t, err)
 	assert.NotNil(t, td)
 	assert.NotNil(t, td.client)
-	assert.True(t, td.interval != 0, "")
+	assert.NotZero(t, td.interval)
 	assert.NotNil(t, td.sdConfig)
 	assert.Equal(t, groupsconf.Account, td.sdConfig.Account)
 	assert.Equal(t, groupsconf.DNSSuffix, td.sdConfig.DNSSuffix)
@@ -120,8 +120,8 @@ func TestTritonSDNewCNConfig(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NotNil(t, td)
 	assert.NotNil(t, td.client)
-	assert.True(t, td.interval != 0, "")
-	assert.NotNil(t, td.sdConfig)
+	assert.NotZero(t, td.interval)
+	assert.NotZero(t, td.sdConfig)
 	assert.Equal(t, cnconf.Role, td.sdConfig.Role)
 	assert.Equal(t, cnconf.Account, td.sdConfig.Account)
 	assert.Equal(t, cnconf.DNSSuffix, td.sdConfig.DNSSuffix)
@@ -131,7 +131,7 @@ func TestTritonSDNewCNConfig(t *testing.T) {
 
 func TestTritonSDRefreshNoTargets(t *testing.T) {
 	tgts := testTritonSDRefresh(t, conf, "{\"containers\":[]}")
-	assert.True(t, tgts == nil, "")
+	assert.Nil(t, tgts)
 }
 
 func TestTritonSDRefreshMultipleTargets(t *testing.T) {
@@ -234,12 +234,12 @@ func testTritonSDRefresh(t *testing.T, c SDConfig, dstr string) []model.LabelSet
 
 	host, strport, err := net.SplitHostPort(u.Host)
 	assert.NoError(t, err)
-	assert.True(t, host != "", "")
-	assert.True(t, strport != "", "")
+	assert.NotEmpty(t, host)
+	assert.NotEmpty(t, strport)
 
 	port, err := strconv.Atoi(strport)
 	assert.NoError(t, err)
-	assert.True(t, port != 0, "")
+	assert.NotZero(t, port)
 
 	td.sdConfig.Port = port
 

diff --git a/notifier/notifier_test.go b/notifier/notifier_test.go
@@ -83,7 +83,7 @@ func TestHandlerNextBatch(t *testing.T) {
 	assert.NoError(t, alertsEqual(expected[0:maxBatchSize], h.nextBatch()))
 	assert.NoError(t, alertsEqual(expected[maxBatchSize:2*maxBatchSize], h.nextBatch()))
 	assert.NoError(t, alertsEqual(expected[2*maxBatchSize:], h.nextBatch()))
-	assert.True(t, len(h.queue) == 0, "Expected queue to be empty but got %d alerts", len(h.queue))
+	assert.Equal(t, 0, len(h.queue), "Expected queue to be empty but got %d alerts", len(h.queue))
 }
 
 func alertsEqual(a, b []*Alert) error {
@@ -201,7 +201,7 @@ func TestHandlerSendAll(t *testing.T) {
 	checkNoErr()
 
 	status2.Store(int32(http.StatusInternalServerError))
-	assert.True(t, !h.sendAll(h.queue...), "all sends succeeded unexpectedly")
+	assert.False(t, h.sendAll(h.queue...), "all sends succeeded unexpectedly")
 	checkNoErr()
 }
 

diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go
@@ -387,10 +387,7 @@ func TestLabels_FromStrings(t *testing.T) {
 
 	assert.Equal(t, expected, labels, "unexpected labelset")
 
-	defer func() { recover() }()
-	FromStrings("aaa", "111", "bbb")
-
-	assert.True(t, false, "did not panic as expected")
+	assert.Panics(t, func() { FromStrings("aaa", "111", "bbb") })
 }
 
 func TestLabels_Compare(t *testing.T) {
@@ -640,8 +637,8 @@ func TestLabels_Hash(t *testing.T) {
 		{Name: "baz", Value: "qux"},
 	}
 	assert.Equal(t, lbls.Hash(), lbls.Hash())
-	assert.True(t, lbls.Hash() != Labels{lbls[1], lbls[0]}.Hash(), "unordered labels match.")
-	assert.True(t, lbls.Hash() != Labels{lbls[0]}.Hash(), "different labels match.")
+	assert.NotEqual(t, lbls.Hash(), Labels{lbls[1], lbls[0]}.Hash(), "unordered labels match.")
+	assert.NotEqual(t, lbls.Hash(), Labels{lbls[0]}.Hash(), "different labels match.")
 }
 
 var benchmarkLabelsResult uint64

diff --git a/pkg/relabel/relabel_test.go b/pkg/relabel/relabel_test.go
@@ -440,7 +440,7 @@ func TestTargetLabelValidity(t *testing.T) {
 		{"foo${bar}foo", true},
 	}
 	for _, test := range tests {
-		assert.True(t, relabelTarget.Match([]byte(test.str)) == test.valid,
+		assert.Equal(t, test.valid, relabelTarget.Match([]byte(test.str)),
 			"Expected %q to be %v", test.str, test.valid)
 	}
 }
diff --git a/pkg/textparse/interface.go b/pkg/textparse/interface.go
@@ -85,12 +85,12 @@ const (
 type MetricType string
 
 const (
-	MetricTypeCounter        = "counter"
-	MetricTypeGauge          = "gauge"
-	MetricTypeHistogram      = "histogram"
-	MetricTypeGaugeHistogram = "gaugehistogram"
-	MetricTypeSummary        = "summary"
-	MetricTypeInfo           = "info"
-	MetricTypeStateset       = "stateset"
-	MetricTypeUnknown        = "unknown"
+	MetricTypeCounter        = MetricType("counter")
+	MetricTypeGauge          = MetricType("gauge")
+	MetricTypeHistogram      = MetricType("histogram")
+	MetricTypeGaugeHistogram = MetricType("gaugehistogram")
+	MetricTypeSummary        = MetricType("summary")
+	MetricTypeInfo           = MetricType("info")
+	MetricTypeStateset       = MetricType("stateset")
+	MetricTypeUnknown        = MetricType("unknown")
 )
diff --git a/promql/engine_test.go b/promql/engine_test.go
@@ -1129,7 +1129,7 @@ func TestQueryLogger_basic(t *testing.T) {
 	assert.Equal(t, 2*l, len(f1.logs))
 
 	// Test that we close the query logger when unsetting it.
-	assert.True(t, !f1.closed, "expected f1 to be open, got closed")
+	assert.False(t, f1.closed, "expected f1 to be open, got closed")
 	engine.SetQueryLogger(nil)
 	assert.True(t, f1.closed, "expected f1 to be closed, got open")
 	queryExec()
@@ -1138,11 +1138,11 @@ func TestQueryLogger_basic(t *testing.T) {
 	f2 := NewFakeQueryLogger()
 	f3 := NewFakeQueryLogger()
 	engine.SetQueryLogger(f2)
-	assert.True(t, !f2.closed, "expected f2 to be open, got closed")
+	assert.False(t, f2.closed, "expected f2 to be open, got closed")
 	queryExec()
 	engine.SetQueryLogger(f3)
 	assert.True(t, f2.closed, "expected f2 to be closed, got open")
-	assert.True(t, !f3.closed, "expected f3 to be open, got closed")
+	assert.False(t, f3.closed, "expected f3 to be open, got closed")
 	queryExec()
 }
 

diff --git a/promql/functions_test.go b/promql/functions_test.go
@@ -15,7 +15,6 @@ package promql
 
 import (
 	"context"
-	"fmt"
 	"testing"
 	"time"
 
@@ -56,19 +55,19 @@ func TestDeriv(t *testing.T) {
 	assert.NoError(t, result.Err)
 
 	vec, _ := result.Vector()
-	assert.True(t, len(vec) == 1, "Expected 1 result, got %d", len(vec))
-	assert.True(t, vec[0].V == 0.0, "Expected 0.0 as value, got %f", vec[0].V)
+	assert.Equal(t, 1, len(vec), "Expected 1 result, got %d", len(vec))
+	assert.Equal(t, 0.0, vec[0].V, "Expected 0.0 as value, got %f", vec[0].V)
 }
 
 func TestFunctionList(t *testing.T) {
 	// Test that Functions and parser.Functions list the same functions.
 	for i := range FunctionCalls {
 		_, ok := parser.Functions[i]
-		assert.True(t, ok, fmt.Sprintf("function %s exists in promql package, but not in parser package", i))
+		assert.True(t, ok, "function %s exists in promql package, but not in parser package", i)
 	}
 
 	for i := range parser.Functions {
 		_, ok := FunctionCalls[i]
-		assert.True(t, ok, (fmt.Sprintf("function %s exists in parser package, but not in promql package", i)))
+		assert.True(t, ok, "function %s exists in parser package, but not in promql package", i)
 	}
 }
diff --git a/promql/parser/parse_test.go b/promql/parser/parse_test.go
@@ -15,7 +15,6 @@ package parser
 
 import (
 	"math"
-	"strings"
 	"testing"
 	"time"
 
@@ -2659,14 +2658,14 @@ func TestParseExpressions(t *testing.T) {
 			expr, err := ParseExpr(test.input)
 
 			// Unexpected errors are always caused by a bug.
-			assert.True(t, err != errUnexpected, "unexpected error occurred")
+			assert.NotEqual(t, err, errUnexpected, "unexpected error occurred")
 
 			if !test.fail {
 				assert.NoError(t, err)
 				assert.Equal(t, test.expected, expr, "error on input '%s'", test.input)
 			} else {
 				assert.Error(t, err)
-				assert.True(t, strings.Contains(err.Error(), test.errMsg), "unexpected error on input '%s', expected '%s', got '%s'", test.input, test.errMsg, err.Error())
+				assert.Contains(t, err.Error(), test.errMsg, "unexpected error on input '%s', expected '%s', got '%s'", test.input, test.errMsg, err.Error())
 
 				errorList, ok := err.(ParseErrors)
 
@@ -2804,7 +2803,7 @@ func TestParseSeries(t *testing.T) {
 		metric, vals, err := ParseSeriesDesc(test.input)
 
 		// Unexpected errors are always caused by a bug.
-		assert.True(t, err != errUnexpected, "unexpected error occurred")
+		assert.NotEqual(t, err, errUnexpected, "unexpected error occurred")
 
 		if !test.fail {
 			assert.NoError(t, err)

diff --git a/promql/test_test.go b/promql/test_test.go
@@ -134,9 +134,9 @@ func TestLazyLoader_WithSamplesTill(t *testing.T) {
 
 					// Get the series for the matcher.
 					ss := querier.Select(false, nil, matchers...)
-					assert.True(t, ss.Next(), "")
+					assert.True(t, ss.Next())
 					storageSeries := ss.At()
-					assert.True(t, !ss.Next(), "Expecting only 1 series")
+					assert.False(t, ss.Next(), "Expecting only 1 series")
 
 					// Convert `storage.Series` to `promql.Series`.
 					got := Series{

diff --git a/rules/alerting_test.go b/rules/alerting_test.go
@@ -15,6 +15,7 @@ package rules
 
 import (
 	"context"
+	"html/template"
 	"testing"
 	"time"
 
@@ -33,16 +34,16 @@ func TestAlertingRuleHTMLSnippet(t *testing.T) {
 	assert.NoError(t, err)
 	rule := NewAlertingRule("testrule", expr, 0, labels.FromStrings("html", "<b>BOLD</b>"), labels.FromStrings("html", "<b>BOLD</b>"), nil, false, nil)
 
-	const want = `alert: <a href="/test/prefix/graph?g0.expr=ALERTS%7Balertname%3D%22testrule%22%7D&g0.tab=1">testrule</a>
+	const want = template.HTML(`alert: <a href="/test/prefix/graph?g0.expr=ALERTS%7Balertname%3D%22testrule%22%7D&g0.tab=1">testrule</a>
 expr: <a href="/test/prefix/graph?g0.expr=foo%7Bhtml%3D%22%3Cb%3EBOLD%3Cb%3E%22%7D&g0.tab=1">foo{html=&#34;&lt;b&gt;BOLD&lt;b&gt;&#34;}</a>
 labels:
   html: '&lt;b&gt;BOLD&lt;/b&gt;'
 annotations:
   html: '&lt;b&gt;BOLD&lt;/b&gt;'
-`
+`)
 
 	got := rule.HTMLSnippet("/test/prefix")
-	assert.True(t, want == got, "incorrect HTML snippet; want:\n\n|%v|\n\ngot:\n\n|%v|", want, got)
+	assert.Equal(t, want, got, "incorrect HTML snippet; want:\n\n|%v|\n\ngot:\n\n|%v|", want, got)
 }
 
 func TestAlertingRuleState(t *testing.T) {
@@ -81,7 +82,7 @@ func TestAlertingRuleState(t *testing.T) {
 		rule := NewAlertingRule(test.name, nil, 0, nil, nil, nil, true, nil)
 		rule.active = test.active
 		got := rule.State()
-		assert.True(t, test.want == got, "test case %d unexpected AlertState, want:%d got:%d", i, test.want, got)
+		assert.Equal(t, test.want, got, "test case %d unexpected AlertState, want:%d got:%d", i, test.want, got)
 	}
 }