Skip to content

Commit

Permalink
Merge pull request prometheus#13452 from bboreham/go-cmp
Browse files Browse the repository at this point in the history
Tests: Use DeepEqual replacement using go-cmp, which is more flexible
  • Loading branch information
bboreham committed Feb 12, 2024
2 parents 1efca80 + 12cac5b commit ff6c832
Show file tree
Hide file tree
Showing 31 changed files with 169 additions and 96 deletions.
3 changes: 2 additions & 1 deletion cmd/promtool/backfill_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/prometheus/prometheus/storage"
"github.com/prometheus/prometheus/tsdb"
"github.com/prometheus/prometheus/tsdb/chunkenc"
"github.com/prometheus/prometheus/util/testutil"
)

type backfillSample struct {
Expand Down Expand Up @@ -76,7 +77,7 @@ func testBlocks(t *testing.T, db *tsdb.DB, expectedMinTime, expectedMaxTime, exp
allSamples := queryAllSeries(t, q, expectedMinTime, expectedMaxTime)
sortSamples(allSamples)
sortSamples(expectedSamples)
require.Equal(t, expectedSamples, allSamples, "did not create correct samples")
testutil.RequireEqual(t, expectedSamples, allSamples, "did not create correct samples")

if len(allSamples) > 0 {
require.Equal(t, expectedMinTime, allSamples[0].Timestamp, "timestamp of first sample is not the expected minimum time")
Expand Down
4 changes: 2 additions & 2 deletions cmd/promtool/sd.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ import (
"encoding/json"
"fmt"
"os"
"reflect"
"time"

"github.com/go-kit/log"
"github.com/google/go-cmp/cmp"
"github.com/prometheus/client_golang/prometheus"

"github.com/prometheus/prometheus/config"
Expand Down Expand Up @@ -153,7 +153,7 @@ func getSDCheckResult(targetGroups []*targetgroup.Group, scrapeConfig *config.Sc

duplicateRes := false
for _, sdCheckRes := range sdCheckResults {
if reflect.DeepEqual(sdCheckRes, result) {
if cmp.Equal(sdCheckRes, result, cmp.Comparer(labels.Equal)) {
duplicateRes = true
break
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/promtool/sd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/prometheus/prometheus/discovery/targetgroup"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/model/relabel"
"github.com/prometheus/prometheus/util/testutil"

"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -69,5 +70,5 @@ func TestSDCheckResult(t *testing.T) {
},
}

require.Equal(t, expectedSDCheckResult, getSDCheckResult(targetGroups, scrapeConfig, true))
testutil.RequireEqual(t, expectedSDCheckResult, getSDCheckResult(targetGroups, scrapeConfig, true))
}
6 changes: 3 additions & 3 deletions cmd/promtool/unittest.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ import (
"fmt"
"os"
"path/filepath"
"reflect"
"sort"
"strconv"
"strings"
"time"

"github.com/go-kit/log"
"github.com/google/go-cmp/cmp"
"github.com/grafana/regexp"
"github.com/nsf/jsondiff"
"github.com/prometheus/common/model"
Expand Down Expand Up @@ -340,7 +340,7 @@ func (tg *testGroup) test(evalInterval time.Duration, groupOrderMap map[string]i
sort.Sort(gotAlerts)
sort.Sort(expAlerts)

if !reflect.DeepEqual(expAlerts, gotAlerts) {
if !cmp.Equal(expAlerts, gotAlerts, cmp.Comparer(labels.Equal)) {
var testName string
if tg.TestGroupName != "" {
testName = fmt.Sprintf(" name: %s,\n", tg.TestGroupName)
Expand Down Expand Up @@ -448,7 +448,7 @@ Outer:
sort.Slice(gotSamples, func(i, j int) bool {
return labels.Compare(gotSamples[i].Labels, gotSamples[j].Labels) <= 0
})
if !reflect.DeepEqual(expSamples, gotSamples) {
if !cmp.Equal(expSamples, gotSamples, cmp.Comparer(labels.Equal)) {
errs = append(errs, fmt.Errorf(" expr: %q, time: %s,\n exp: %v\n got: %v", testCase.Expr,
testCase.EvalTime.String(), parsedSamplesString(expSamples), parsedSamplesString(gotSamples)))
}
Expand Down
7 changes: 4 additions & 3 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
"github.com/prometheus/prometheus/discovery/zookeeper"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/model/relabel"
"github.com/prometheus/prometheus/util/testutil"
)

func mustParseURL(u string) *config.URL {
Expand Down Expand Up @@ -2037,16 +2038,16 @@ func TestExpandExternalLabels(t *testing.T) {

c, err := LoadFile("testdata/external_labels.good.yml", false, false, log.NewNopLogger())
require.NoError(t, err)
require.Equal(t, labels.FromStrings("bar", "foo", "baz", "foo${TEST}bar", "foo", "${TEST}", "qux", "foo$${TEST}", "xyz", "foo$$bar"), c.GlobalConfig.ExternalLabels)
testutil.RequireEqual(t, labels.FromStrings("bar", "foo", "baz", "foo${TEST}bar", "foo", "${TEST}", "qux", "foo$${TEST}", "xyz", "foo$$bar"), c.GlobalConfig.ExternalLabels)

c, err = LoadFile("testdata/external_labels.good.yml", false, true, log.NewNopLogger())
require.NoError(t, err)
require.Equal(t, labels.FromStrings("bar", "foo", "baz", "foobar", "foo", "", "qux", "foo${TEST}", "xyz", "foo$bar"), c.GlobalConfig.ExternalLabels)
testutil.RequireEqual(t, labels.FromStrings("bar", "foo", "baz", "foobar", "foo", "", "qux", "foo${TEST}", "xyz", "foo$bar"), c.GlobalConfig.ExternalLabels)

os.Setenv("TEST", "TestValue")
c, err = LoadFile("testdata/external_labels.good.yml", false, true, log.NewNopLogger())
require.NoError(t, err)
require.Equal(t, labels.FromStrings("bar", "foo", "baz", "fooTestValuebar", "foo", "TestValue", "qux", "foo${TEST}", "xyz", "foo$bar"), c.GlobalConfig.ExternalLabels)
testutil.RequireEqual(t, labels.FromStrings("bar", "foo", "baz", "fooTestValuebar", "foo", "TestValue", "qux", "foo${TEST}", "xyz", "foo$bar"), c.GlobalConfig.ExternalLabels)
}

func TestAgentMode(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
github.com/go-zookeeper/zk v1.0.3
github.com/gogo/protobuf v1.3.2
github.com/golang/snappy v0.0.4
github.com/google/go-cmp v0.6.0
github.com/google/pprof v0.0.0-20240117000934-35fc243c5815
github.com/google/uuid v1.5.0
github.com/gophercloud/gophercloud v1.8.0
Expand Down Expand Up @@ -135,7 +136,6 @@ require (
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/gnostic-models v0.6.8 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/go-querystring v1.1.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/s2a-go v0.1.7 // indirect
Expand Down
3 changes: 2 additions & 1 deletion model/relabel/relabel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"gopkg.in/yaml.v2"

"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/util/testutil"
)

func TestRelabel(t *testing.T) {
Expand Down Expand Up @@ -591,7 +592,7 @@ func TestRelabel(t *testing.T) {
res, keep := Process(test.input, test.relabel...)
require.Equal(t, !test.drop, keep)
if keep {
require.Equal(t, test.output, res)
testutil.RequireEqual(t, test.output, res)
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions model/textparse/openmetricsparse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/prometheus/prometheus/model/exemplar"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/util/testutil"
)

func TestOpenMetricsParse(t *testing.T) {
Expand Down Expand Up @@ -268,12 +269,12 @@ foo_total 17.0 1520879607.789 # {id="counter-test"} 5`
require.Equal(t, exp[i].m, string(m))
require.Equal(t, exp[i].t, ts)
require.Equal(t, exp[i].v, v)
require.Equal(t, exp[i].lset, res)
testutil.RequireEqual(t, exp[i].lset, res)
if exp[i].e == nil {
require.False(t, found)
} else {
require.True(t, found)
require.Equal(t, *exp[i].e, e)
testutil.RequireEqual(t, *exp[i].e, e)
}

case EntryType:
Expand Down
3 changes: 2 additions & 1 deletion model/textparse/promparse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/util/testutil"
)

func TestPromParse(t *testing.T) {
Expand Down Expand Up @@ -191,7 +192,7 @@ testmetric{label="\"bar\""} 1`
require.Equal(t, exp[i].m, string(m))
require.Equal(t, exp[i].t, ts)
require.Equal(t, exp[i].v, v)
require.Equal(t, exp[i].lset, res)
testutil.RequireEqual(t, exp[i].lset, res)

case EntryType:
m, typ := p.Type()
Expand Down
9 changes: 5 additions & 4 deletions model/textparse/protobufparse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/prometheus/prometheus/model/exemplar"
"github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/util/testutil"

dto "github.com/prometheus/prometheus/prompb/io/prometheus/client"
)
Expand Down Expand Up @@ -1993,12 +1994,12 @@ func TestProtobufParse(t *testing.T) {
require.Equal(t, int64(0), exp[i].t, "i: %d", i)
}
require.Equal(t, exp[i].v, v, "i: %d", i)
require.Equal(t, exp[i].lset, res, "i: %d", i)
testutil.RequireEqual(t, exp[i].lset, res, "i: %d", i)
if len(exp[i].e) == 0 {
require.False(t, eFound, "i: %d", i)
} else {
require.True(t, eFound, "i: %d", i)
require.Equal(t, exp[i].e[0], e, "i: %d", i)
testutil.RequireEqual(t, exp[i].e[0], e, "i: %d", i)
require.False(t, p.Exemplar(&e), "too many exemplars returned, i: %d", i)
}
if exp[i].ct != 0 {
Expand All @@ -2017,7 +2018,7 @@ func TestProtobufParse(t *testing.T) {
} else {
require.Equal(t, int64(0), exp[i].t, "i: %d", i)
}
require.Equal(t, exp[i].lset, res, "i: %d", i)
testutil.RequireEqual(t, exp[i].lset, res, "i: %d", i)
require.Equal(t, exp[i].m, string(m), "i: %d", i)
if shs != nil {
require.Equal(t, exp[i].shs, shs, "i: %d", i)
Expand All @@ -2026,7 +2027,7 @@ func TestProtobufParse(t *testing.T) {
}
j := 0
for e := (exemplar.Exemplar{}); p.Exemplar(&e); j++ {
require.Equal(t, exp[i].e[j], e, "i: %d", i)
testutil.RequireEqual(t, exp[i].e[j], e, "i: %d", i)
e = exemplar.Exemplar{}
}
require.Len(t, exp[i].e, j, "not enough exemplars found, i: %d", i)
Expand Down
15 changes: 8 additions & 7 deletions promql/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/prometheus/prometheus/util/annotations"
"github.com/prometheus/prometheus/util/stats"
"github.com/prometheus/prometheus/util/teststorage"
"github.com/prometheus/prometheus/util/testutil"
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -1631,7 +1632,7 @@ load 1ms
sort.Sort(expMat)
sort.Sort(res.Value.(Matrix))
}
require.Equal(t, c.result, res.Value, "query %q failed", c.query)
testutil.RequireEqual(t, c.result, res.Value, "query %q failed", c.query)
})
}
}
Expand Down Expand Up @@ -1956,7 +1957,7 @@ func TestSubquerySelector(t *testing.T) {
require.Equal(t, c.Result.Err, res.Err)
mat := res.Value.(Matrix)
sort.Sort(mat)
require.Equal(t, c.Result.Value, mat)
testutil.RequireEqual(t, c.Result.Value, mat)
})
}
})
Expand Down Expand Up @@ -2001,7 +2002,7 @@ load 1m

res := qry.Exec(context.Background())
require.NoError(t, res.Err)
require.Equal(t, expectedResult, res.Value)
testutil.RequireEqual(t, expectedResult, res.Value)
}

type FakeQueryLogger struct {
Expand Down Expand Up @@ -3147,7 +3148,7 @@ func TestRangeQuery(t *testing.T) {

res := qry.Exec(context.Background())
require.NoError(t, res.Err)
require.Equal(t, c.Result, res.Value)
testutil.RequireEqual(t, c.Result, res.Value)
})
}
}
Expand Down Expand Up @@ -4347,7 +4348,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) {
vector, err := res.Vector()
require.NoError(t, err)

require.Equal(t, exp, vector)
testutil.RequireEqual(t, exp, vector)
}

// sum().
Expand Down Expand Up @@ -4605,7 +4606,7 @@ func TestNativeHistogram_SubOperator(t *testing.T) {
}
}

require.Equal(t, exp, vector)
testutil.RequireEqual(t, exp, vector)
}

// - operator.
Expand Down Expand Up @@ -4753,7 +4754,7 @@ func TestNativeHistogram_MulDivOperator(t *testing.T) {
vector, err := res.Vector()
require.NoError(t, err)

require.Equal(t, exp, vector)
testutil.RequireEqual(t, exp, vector)
}

// histogram * scalar.
Expand Down
3 changes: 2 additions & 1 deletion promql/parser/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/util/testutil"

"github.com/prometheus/prometheus/promql/parser/posrange"
)
Expand Down Expand Up @@ -4018,7 +4019,7 @@ func TestParseSeries(t *testing.T) {

if !test.fail {
require.NoError(t, err)
require.Equal(t, test.expectedMetric, metric, "error on input '%s'", test.input)
testutil.RequireEqual(t, test.expectedMetric, metric, "error on input '%s'", test.input)
require.Equal(t, test.expectedValues, vals, "error in input '%s'", test.input)
} else {
require.Error(t, err)
Expand Down
15 changes: 8 additions & 7 deletions rules/alerting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/prometheus/prometheus/promql/parser"
"github.com/prometheus/prometheus/storage"
"github.com/prometheus/prometheus/util/teststorage"
"github.com/prometheus/prometheus/util/testutil"
)

var testEngine = promql.NewEngine(promql.EngineOpts{
Expand Down Expand Up @@ -180,7 +181,7 @@ func TestAlertingRuleLabelsUpdate(t *testing.T) {
}
}

require.Equal(t, result, filteredRes)
testutil.RequireEqual(t, result, filteredRes)
}
evalTime := baseTime.Add(time.Duration(len(results)) * time.Minute)
res, err := rule.Eval(context.TODO(), evalTime, EngineQueryFunc(testEngine, storage), nil, 0)
Expand Down Expand Up @@ -278,7 +279,7 @@ func TestAlertingRuleExternalLabelsInTemplate(t *testing.T) {
}
}

require.Equal(t, result, filteredRes)
testutil.RequireEqual(t, result, filteredRes)
}

func TestAlertingRuleExternalURLInTemplate(t *testing.T) {
Expand Down Expand Up @@ -371,7 +372,7 @@ func TestAlertingRuleExternalURLInTemplate(t *testing.T) {
}
}

require.Equal(t, result, filteredRes)
testutil.RequireEqual(t, result, filteredRes)
}

func TestAlertingRuleEmptyLabelFromTemplate(t *testing.T) {
Expand Down Expand Up @@ -425,7 +426,7 @@ func TestAlertingRuleEmptyLabelFromTemplate(t *testing.T) {
require.Equal(t, "ALERTS_FOR_STATE", smplName)
}
}
require.Equal(t, result, filteredRes)
testutil.RequireEqual(t, result, filteredRes)
}

func TestAlertingRuleQueryInTemplate(t *testing.T) {
Expand Down Expand Up @@ -718,7 +719,7 @@ func TestSendAlertsDontAffectActiveAlerts(t *testing.T) {

// The relabel rule changes a1=1 to a1=bug.
// But the labels with the AlertingRule should not be changed.
require.Equal(t, labels.FromStrings("a1", "1"), rule.active[h].Labels)
testutil.RequireEqual(t, labels.FromStrings("a1", "1"), rule.active[h].Labels)
}

func TestKeepFiringFor(t *testing.T) {
Expand Down Expand Up @@ -823,7 +824,7 @@ func TestKeepFiringFor(t *testing.T) {
}
}

require.Equal(t, result, filteredRes)
testutil.RequireEqual(t, result, filteredRes)
}
evalTime := baseTime.Add(time.Duration(len(results)) * time.Minute)
res, err := rule.Eval(context.TODO(), evalTime, EngineQueryFunc(testEngine, storage), nil, 0)
Expand Down Expand Up @@ -870,7 +871,7 @@ func TestPendingAndKeepFiringFor(t *testing.T) {
for _, smpl := range res {
smplName := smpl.Metric.Get("__name__")
if smplName == "ALERTS" {
require.Equal(t, result, smpl)
testutil.RequireEqual(t, result, smpl)
} else {
// If not 'ALERTS', it has to be 'ALERTS_FOR_STATE'.
require.Equal(t, "ALERTS_FOR_STATE", smplName)
Expand Down
Loading

0 comments on commit ff6c832

Please sign in to comment.