Skip to content

smart diff to testutil.GatherAndCompare #998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 13, 2022
Merged

smart diff to testutil.GatherAndCompare #998

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
94022f3
added smart diff to testutil.GatherAndCompare
sourikghosh Mar 7, 2022
066306b
v2 testuitls added for better diff
sourikghosh Mar 17, 2022
b33c848
replaced CollectAndCompare to CollectAndCompareV2 in testutil_test
sourikghosh Mar 17, 2022
cf6a226
renamed methods from v2 to withT
sourikghosh Mar 17, 2022
7729983
replaced testify with custom diff func
sourikghosh Mar 18, 2022
fc11393
Merge branch 'main' into better-diff-for-test
sourikghosh Mar 31, 2022
39fc465
difflib GetUnifiedDiffString added with test
sourikghosh Mar 31, 2022
b33c45f
license meta data added to file
sourikghosh Mar 31, 2022
b890818
moved difflib to internal
sourikghosh Apr 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.32.1
github.com/prometheus/procfs v0.7.3
github.com/stretchr/testify v1.4.0
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9
google.golang.org/protobuf v1.26.0
)
Expand Down
21 changes: 12 additions & 9 deletions prometheus/testutil/testutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ import (
"bytes"
"fmt"
"io"
"testing"

"github.com/prometheus/common/expfmt"
"github.com/stretchr/testify/require"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm.. is there a way to avoid this package? For a reason we wanted to make sure it's not used, so we are not leaking it.

Since this is not testing code, every project that imports client_golang needs to import testify.... I think we want to copy the diff function and maintain here.

See how we did that here https://github.com/efficientgo/tools/blob/main/core/pkg/testutil/testutil.go#L101

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree. I think I was a little hasty to review this, and I haven't noticed testify is not there already 🙈

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess because it's already a transitive dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I agree too, this makes sense
but the above diff function has a dep on
https://github.com/efficientgo/tools/blob/fe763185946be83b20da626605319733bd7f97cb/core/pkg/testutil/testutil.go#L16 which is not maintained should I transfer the module implementation over here or just simply import it.


dto "github.com/prometheus/client_model/go"

Expand Down Expand Up @@ -154,27 +156,27 @@ func GatherAndCount(g prometheus.Gatherer, metricNames ...string) (int, error) {
// CollectAndCompare registers the provided Collector with a newly created
// pedantic Registry. It then calls GatherAndCompare with that Registry and with
// the provided metricNames.
func CollectAndCompare(c prometheus.Collector, expected io.Reader, metricNames ...string) error {
func CollectAndCompare(t *testing.T, c prometheus.Collector, expected io.Reader, shouldFail bool, metricNames ...string) error {
reg := prometheus.NewPedanticRegistry()
if err := reg.Register(c); err != nil {
return fmt.Errorf("registering collector failed: %s", err)
}
return GatherAndCompare(reg, expected, metricNames...)
return GatherAndCompare(t, reg, expected, shouldFail, metricNames...)
}

// GatherAndCompare gathers all metrics from the provided Gatherer and compares
// it to an expected output read from the provided Reader in the Prometheus text
// exposition format. If any metricNames are provided, only metrics with those
// names are compared.
func GatherAndCompare(g prometheus.Gatherer, expected io.Reader, metricNames ...string) error {
return TransactionalGatherAndCompare(prometheus.ToTransactionalGatherer(g), expected, metricNames...)
func GatherAndCompare(t *testing.T, g prometheus.Gatherer, expected io.Reader, shouldFail bool, metricNames ...string) error {
return TransactionalGatherAndCompare(t, prometheus.ToTransactionalGatherer(g), expected, shouldFail, metricNames...)
}

// TransactionalGatherAndCompare gathers all metrics from the provided Gatherer and compares
// it to an expected output read from the provided Reader in the Prometheus text
// exposition format. If any metricNames are provided, only metrics with those
// names are compared.
func TransactionalGatherAndCompare(g prometheus.TransactionalGatherer, expected io.Reader, metricNames ...string) error {
func TransactionalGatherAndCompare(t *testing.T, g prometheus.TransactionalGatherer, expected io.Reader, shouldFail bool, metricNames ...string) error {
got, done, err := g.Gather()
defer done()
if err != nil {
Expand All @@ -190,14 +192,14 @@ func TransactionalGatherAndCompare(g prometheus.TransactionalGatherer, expected
}
want := internal.NormalizeMetricFamilies(wantRaw)

return compare(got, want)
return compare(t, got, want, shouldFail)
}

// compare encodes both provided slices of metric families into the text format,
// compares their string message, and returns an error if they do not match.
// The error contains the encoded text of both the desired and the actual
// result.
func compare(got, want []*dto.MetricFamily) error {
func compare(t *testing.T, got, want []*dto.MetricFamily, shouldFail bool) error {
var gotBuf, wantBuf bytes.Buffer
enc := expfmt.NewEncoder(&gotBuf, expfmt.FmtText)
for _, mf := range got {
Expand All @@ -212,16 +214,17 @@ func compare(got, want []*dto.MetricFamily) error {
}
}

if wantBuf.String() != gotBuf.String() {
if shouldFail && wantBuf.String() != gotBuf.String() {
return fmt.Errorf(`
metric output does not match expectation; want:

%s
got:

%s`, wantBuf.String(), gotBuf.String())

}

require.Equal(t, wantBuf.String(), gotBuf.String())
return nil
}

Expand Down
15 changes: 7 additions & 8 deletions prometheus/testutil/testutil_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"testing"

"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/require"
)

type untypedCollector struct{}
Expand Down Expand Up @@ -138,7 +139,7 @@ func TestCollectAndCompare(t *testing.T) {
some_total{ label1 = "value1" } 1
`

if err := CollectAndCompare(c, strings.NewReader(metadata+expected), "some_total"); err != nil {
if err := CollectAndCompare(t, c, strings.NewReader(metadata+expected), false, "some_total"); err != nil {
t.Errorf("unexpected collecting result:\n%s", err)
}
}
Expand All @@ -160,7 +161,7 @@ func TestCollectAndCompareNoLabel(t *testing.T) {
some_total 1
`

if err := CollectAndCompare(c, strings.NewReader(metadata+expected), "some_total"); err != nil {
if err := CollectAndCompare(t, c, strings.NewReader(metadata+expected), false, "some_total"); err != nil {
t.Errorf("unexpected collecting result:\n%s", err)
}
}
Expand Down Expand Up @@ -232,7 +233,7 @@ func TestCollectAndCompareHistogram(t *testing.T) {
}

t.Run(input.name, func(t *testing.T) {
if err := CollectAndCompare(input.c, strings.NewReader(input.metadata+input.expect)); err != nil {
if err := CollectAndCompare(t, input.c, strings.NewReader(input.metadata+input.expect), false); err != nil {
t.Errorf("unexpected collecting result:\n%s", err)
}
})
Expand All @@ -259,7 +260,7 @@ func TestNoMetricFilter(t *testing.T) {
some_total{label1="value1"} 1
`

if err := CollectAndCompare(c, strings.NewReader(metadata+expected)); err != nil {
if err := CollectAndCompare(t, c, strings.NewReader(metadata+expected), false); err != nil {
t.Errorf("unexpected collecting result:\n%s", err)
}
}
Expand Down Expand Up @@ -297,14 +298,12 @@ got:
some_total{label1="value1"} 1
`

err := CollectAndCompare(c, strings.NewReader(metadata+expected))
err := CollectAndCompare(t, c, strings.NewReader(metadata+expected), true)
if err == nil {
t.Error("Expected error, got no error.")
}

if err.Error() != expectedError {
t.Errorf("Expected\n%#+v\nGot:\n%#+v", expectedError, err.Error())
}
require.EqualErrorf(t, err, expectedError, "Expected\n%#+v\nGot:\n%#+v", expectedError, err.Error())
}

func TestCollectAndCount(t *testing.T) {
Expand Down