Skip to content
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.

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

smart diff to testutil.GatherAndCompare #998

Merged
merged 9 commits into from
Apr 13, 2022
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ module github.com/prometheus/client_golang
require (
github.com/beorn7/perks v1.0.1
github.com/cespare/xxhash/v2 v2.1.2
github.com/davecgh/go-spew v1.1.1
github.com/golang/protobuf v1.5.2
github.com/json-iterator/go v1.1.12
github.com/pmezard/go-difflib v1.0.0
Copy link
Member

Choose a reason for hiding this comment

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

I don't want to be a pain but this module is THIS PACKAGE IS NO LONGER MAINTAINED. Not the best, since all users depending on our module will have to download it too.

I wonder if it makes sense to just copy the code for now with the required code (with test) over. Looks reasonably small. There is not benefit in depending on module that does not allow merging new changes.

Or we could find different package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙃that was my question previously #998 (comment)

Ideally, I would prefer an alternative to the package but I couldn't find a good replacement. So maybe coping small piece of code and maintaining it over here ?!!!

Copy link
Member

Choose a reason for hiding this comment

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

Yea, I think copy might be better

github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.32.1
github.com/prometheus/procfs v0.7.3
Expand Down
70 changes: 63 additions & 7 deletions prometheus/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ import (
"bytes"
"fmt"
"io"
"reflect"

"github.com/davecgh/go-spew/spew"
"github.com/pmezard/go-difflib/difflib"
"github.com/prometheus/common/expfmt"

dto "github.com/prometheus/client_model/go"
Expand Down Expand Up @@ -211,18 +214,71 @@ func compare(got, want []*dto.MetricFamily) error {
return fmt.Errorf("encoding expected metrics failed: %s", err)
}
}
if diffErr := diff(wantBuf, gotBuf); diffErr != "" {
return fmt.Errorf(diffErr)
}
return nil
}

if wantBuf.String() != gotBuf.String() {
return fmt.Errorf(`
metric output does not match expectation; want:
// diff returns a diff of both values as long as both are of the same type and
// are a struct, map, slice, array or string. Otherwise it returns an empty string.
func diff(expected interface{}, actual interface{}) string {
if expected == nil || actual == nil {
return ""
}

%s
got:
et, ek := typeAndKind(expected)
at, _ := typeAndKind(actual)
if et != at {
return ""
}

%s`, wantBuf.String(), gotBuf.String())
if ek != reflect.Struct && ek != reflect.Map && ek != reflect.Slice && ek != reflect.Array && ek != reflect.String {
return ""
}

var e, a string
c := spew.ConfigState{
Indent: " ",
DisablePointerAddresses: true,
DisableCapacities: true,
SortKeys: true,
}
return nil
if et != reflect.TypeOf("") {
e = c.Sdump(expected)
a = c.Sdump(actual)
} else {
e = reflect.ValueOf(expected).String()
a = reflect.ValueOf(actual).String()
}

diff, _ := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{
A: difflib.SplitLines(e),
B: difflib.SplitLines(a),
FromFile: "metric output does not match expectation; want",
FromDate: "",
ToFile: "got:",
ToDate: "",
Context: 1,
})

if diff == "" {
return ""
}

return "\n\nDiff:\n" + diff
}

// typeAndKind returns the type and kind of the given interface{}
func typeAndKind(v interface{}) (reflect.Type, reflect.Kind) {
t := reflect.TypeOf(v)
k := t.Kind()

if k == reflect.Ptr {
t = t.Elem()
k = t.Kind()
}
return t, k
}

func filterMetrics(metrics []*dto.MetricFamily, names []string) []*dto.MetricFamily {
Expand Down
21 changes: 11 additions & 10 deletions prometheus/testutil/testutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,17 +284,18 @@ func TestMetricNotFound(t *testing.T) {
`

expectedError := `
metric output does not match expectation; want:

# HELP some_other_metric A value that represents a counter.
# TYPE some_other_metric counter
some_other_metric{label1="value1"} 1

got:

# HELP some_total A value that represents a counter.
# TYPE some_total counter
some_total{label1="value1"} 1
Diff:
--- metric output does not match expectation; want
+++ got:
@@ -1,4 +1,4 @@
-(bytes.Buffer) # HELP some_other_metric A value that represents a counter.
-# TYPE some_other_metric counter
-some_other_metric{label1="value1"} 1
+(bytes.Buffer) # HELP some_total A value that represents a counter.
+# TYPE some_total counter
+some_total{label1="value1"} 1

`

err := CollectAndCompare(c, strings.NewReader(metadata+expected))
Expand Down