Skip to content

Commit

Permalink
testutil compareMetricFamilies: make less error-prone
Browse files Browse the repository at this point in the history
The functions `GatherAndCompare`, `ScrapeAndCompare` and others that use
`compareMetricFamilies` under the hood can return no error if
`metricNames` includes none of the names found in the scraped/gathered
results. To avoid false Positves (an error being the negative case), we
can return an error if there is is at least one name in `metricNames`
that is not in the filtered results.

Fixes: prometheus#1351

Signed-off-by: leonnicolas <leonloechner@gmx.de>
  • Loading branch information
leonnicolas committed Jan 4, 2024
1 parent 046e320 commit 7163ac9
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 46 deletions.
3 changes: 3 additions & 0 deletions prometheus/testutil/testutil.go
Expand Up @@ -254,6 +254,9 @@ func compareMetricFamilies(got, expected []*dto.MetricFamily, metricNames ...str
if metricNames != nil {
got = filterMetrics(got, metricNames)
expected = filterMetrics(expected, metricNames)
if len(metricNames) > len(got) {
return fmt.Errorf("expected metrics name not found")
}
}

return compare(got, expected)
Expand Down
138 changes: 92 additions & 46 deletions prometheus/testutil/testutil_test.go
Expand Up @@ -332,27 +332,46 @@ Diff:
}

func TestScrapeAndCompare(t *testing.T) {
const expected = `
scenarios := map[string]struct {
want string
metricNames []string
errPrefix string
fail bool
}{
"empty metric Names": {
want: `
# HELP some_total A value that represents a counter.
# TYPE some_total counter
some_total{ label1 = "value1" } 1
`
`,
metricNames: []string{},
},
"one metric": {
want: `
# HELP some_total A value that represents a counter.
# TYPE some_total counter
expectedReader := strings.NewReader(expected)
some_total{ label1 = "value1" } 1
`,
metricNames: []string{"some_total"},
},
"multiple expected": {
want: `
# HELP some_total A value that represents a counter.
# TYPE some_total counter
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, expected)
}))
defer ts.Close()
some_total{ label1 = "value1" } 1
if err := ScrapeAndCompare(ts.URL, expectedReader, "some_total"); err != nil {
t.Errorf("unexpected scraping result:\n%s", err)
}
}
# HELP some_total2 A value that represents a counter.
# TYPE some_total2 counter
func TestScrapeAndCompareWithMultipleExpected(t *testing.T) {
const expected = `
some_total2{ label2 = "value2" } 1
`,
metricNames: []string{"some_total2"},
},
"expected metric name is not scraped": {
want: `
# HELP some_total A value that represents a counter.
# TYPE some_total counter
Expand All @@ -362,53 +381,80 @@ func TestScrapeAndCompareWithMultipleExpected(t *testing.T) {
# TYPE some_total2 counter
some_total2{ label2 = "value2" } 1
`

expectedReader := strings.NewReader(expected)
`,
metricNames: []string{"some_total3"},
errPrefix: "expected metrics name not found",
fail: true,
},
"one of multiple expected metric names is not scraped": {
want: `
# HELP some_total A value that represents a counter.
# TYPE some_total counter
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, expected)
}))
defer ts.Close()
some_total{ label1 = "value1" } 1
if err := ScrapeAndCompare(ts.URL, expectedReader, "some_total2"); err != nil {
t.Errorf("unexpected scraping result:\n%s", err)
}
}
# HELP some_total2 A value that represents a counter.
# TYPE some_total2 counter
func TestScrapeAndCompareFetchingFail(t *testing.T) {
err := ScrapeAndCompare("some_url", strings.NewReader("some expectation"), "some_total")
if err == nil {
t.Errorf("expected an error but got nil")
some_total2{ label2 = "value2" } 1
`,
metricNames: []string{"some_total1", "some_total3"},
errPrefix: "expected metrics name not found",
fail: true,
},
}
if !strings.HasPrefix(err.Error(), "scraping metrics failed") {
t.Errorf("unexpected error happened: %s", err)
for name, scenario := range scenarios {
t.Run(name, func(t *testing.T) {
expectedReader := strings.NewReader(scenario.want)

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, scenario.want)
}))
defer ts.Close()
if err := ScrapeAndCompare(ts.URL, expectedReader, scenario.metricNames...); err != nil {
if !scenario.fail || !strings.HasPrefix(err.Error(), scenario.errPrefix) {
t.Errorf("unexpected error happened: %s", err)
}
} else if scenario.fail {
t.Errorf("expected an error but got nil")
}
})
}
}

func TestScrapeAndCompareBadStatusCode(t *testing.T) {
const expected = `
t.Run("fetching fail", func(t *testing.T) {
err := ScrapeAndCompare("some_url", strings.NewReader("some expectation"), "some_total")
if err == nil {
t.Errorf("expected an error but got nil")
}
if !strings.HasPrefix(err.Error(), "scraping metrics failed") {
t.Errorf("unexpected error happened: %s", err)
}
})

t.Run("bad status code", func(t *testing.T) {
const expected = `
# HELP some_total A value that represents a counter.
# TYPE some_total counter
some_total{ label1 = "value1" } 1
`

expectedReader := strings.NewReader(expected)
expectedReader := strings.NewReader(expected)

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadGateway)
fmt.Fprintln(w, expected)
}))
defer ts.Close()
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadGateway)
fmt.Fprintln(w, expected)
}))
defer ts.Close()

err := ScrapeAndCompare(ts.URL, expectedReader, "some_total")
if err == nil {
t.Errorf("expected an error but got nil")
}
if !strings.HasPrefix(err.Error(), "the scraping target returned a status code other than 200") {
t.Errorf("unexpected error happened: %s", err)
}
err := ScrapeAndCompare(ts.URL, expectedReader, "some_total")
if err == nil {
t.Errorf("expected an error but got nil")
}
if !strings.HasPrefix(err.Error(), "the scraping target returned a status code other than 200") {
t.Errorf("unexpected error happened: %s", err)
}
})
}

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

0 comments on commit 7163ac9

Please sign in to comment.