Skip to content

Commit f447422

Browse files
committed
testutil compareMetricFamilies: make less error-prone
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: #1351 Signed-off-by: leonnicolas <[email protected]>
1 parent 36b9f46 commit f447422

File tree

2 files changed

+95
-46
lines changed

2 files changed

+95
-46
lines changed

prometheus/testutil/testutil.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,9 @@ func compareMetricFamilies(got, expected []*dto.MetricFamily, metricNames ...str
277277
if metricNames != nil {
278278
got = filterMetrics(got, metricNames)
279279
expected = filterMetrics(expected, metricNames)
280+
if len(metricNames) > len(got) {
281+
return fmt.Errorf("expected metrics name not found")
282+
}
280283
}
281284

282285
return compare(got, expected)

prometheus/testutil/testutil_test.go

Lines changed: 92 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -328,27 +328,46 @@ func TestMetricNotFound(t *testing.T) {
328328
}
329329

330330
func TestScrapeAndCompare(t *testing.T) {
331-
const expected = `
331+
scenarios := map[string]struct {
332+
want string
333+
metricNames []string
334+
errPrefix string
335+
fail bool
336+
}{
337+
"empty metric Names": {
338+
want: `
332339
# HELP some_total A value that represents a counter.
333340
# TYPE some_total counter
334341
335342
some_total{ label1 = "value1" } 1
336-
`
343+
`,
344+
metricNames: []string{},
345+
},
346+
"one metric": {
347+
want: `
348+
# HELP some_total A value that represents a counter.
349+
# TYPE some_total counter
337350
338-
expectedReader := strings.NewReader(expected)
351+
some_total{ label1 = "value1" } 1
352+
`,
353+
metricNames: []string{"some_total"},
354+
},
355+
"multiple expected": {
356+
want: `
357+
# HELP some_total A value that represents a counter.
358+
# TYPE some_total counter
339359
340-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
341-
fmt.Fprintln(w, expected)
342-
}))
343-
defer ts.Close()
360+
some_total{ label1 = "value1" } 1
344361
345-
if err := ScrapeAndCompare(ts.URL, expectedReader, "some_total"); err != nil {
346-
t.Errorf("unexpected scraping result:\n%s", err)
347-
}
348-
}
362+
# HELP some_total2 A value that represents a counter.
363+
# TYPE some_total2 counter
349364
350-
func TestScrapeAndCompareWithMultipleExpected(t *testing.T) {
351-
const expected = `
365+
some_total2{ label2 = "value2" } 1
366+
`,
367+
metricNames: []string{"some_total2"},
368+
},
369+
"expected metric name is not scraped": {
370+
want: `
352371
# HELP some_total A value that represents a counter.
353372
# TYPE some_total counter
354373
@@ -358,53 +377,80 @@ func TestScrapeAndCompareWithMultipleExpected(t *testing.T) {
358377
# TYPE some_total2 counter
359378
360379
some_total2{ label2 = "value2" } 1
361-
`
362-
363-
expectedReader := strings.NewReader(expected)
380+
`,
381+
metricNames: []string{"some_total3"},
382+
errPrefix: "expected metrics name not found",
383+
fail: true,
384+
},
385+
"one of multiple expected metric names is not scraped": {
386+
want: `
387+
# HELP some_total A value that represents a counter.
388+
# TYPE some_total counter
364389
365-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
366-
fmt.Fprintln(w, expected)
367-
}))
368-
defer ts.Close()
390+
some_total{ label1 = "value1" } 1
369391
370-
if err := ScrapeAndCompare(ts.URL, expectedReader, "some_total2"); err != nil {
371-
t.Errorf("unexpected scraping result:\n%s", err)
372-
}
373-
}
392+
# HELP some_total2 A value that represents a counter.
393+
# TYPE some_total2 counter
374394
375-
func TestScrapeAndCompareFetchingFail(t *testing.T) {
376-
err := ScrapeAndCompare("some_url", strings.NewReader("some expectation"), "some_total")
377-
if err == nil {
378-
t.Errorf("expected an error but got nil")
395+
some_total2{ label2 = "value2" } 1
396+
`,
397+
metricNames: []string{"some_total1", "some_total3"},
398+
errPrefix: "expected metrics name not found",
399+
fail: true,
400+
},
379401
}
380-
if !strings.HasPrefix(err.Error(), "scraping metrics failed") {
381-
t.Errorf("unexpected error happened: %s", err)
402+
for name, scenario := range scenarios {
403+
t.Run(name, func(t *testing.T) {
404+
expectedReader := strings.NewReader(scenario.want)
405+
406+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
407+
fmt.Fprintln(w, scenario.want)
408+
}))
409+
defer ts.Close()
410+
if err := ScrapeAndCompare(ts.URL, expectedReader, scenario.metricNames...); err != nil {
411+
if !scenario.fail || !strings.HasPrefix(err.Error(), scenario.errPrefix) {
412+
t.Errorf("unexpected error happened: %s", err)
413+
}
414+
} else if scenario.fail {
415+
t.Errorf("expected an error but got nil")
416+
}
417+
})
382418
}
383-
}
384419

385-
func TestScrapeAndCompareBadStatusCode(t *testing.T) {
386-
const expected = `
420+
t.Run("fetching fail", func(t *testing.T) {
421+
err := ScrapeAndCompare("some_url", strings.NewReader("some expectation"), "some_total")
422+
if err == nil {
423+
t.Errorf("expected an error but got nil")
424+
}
425+
if !strings.HasPrefix(err.Error(), "scraping metrics failed") {
426+
t.Errorf("unexpected error happened: %s", err)
427+
}
428+
})
429+
430+
t.Run("bad status code", func(t *testing.T) {
431+
const expected = `
387432
# HELP some_total A value that represents a counter.
388433
# TYPE some_total counter
389434
390435
some_total{ label1 = "value1" } 1
391436
`
392437

393-
expectedReader := strings.NewReader(expected)
438+
expectedReader := strings.NewReader(expected)
394439

395-
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
396-
w.WriteHeader(http.StatusBadGateway)
397-
fmt.Fprintln(w, expected)
398-
}))
399-
defer ts.Close()
440+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
441+
w.WriteHeader(http.StatusBadGateway)
442+
fmt.Fprintln(w, expected)
443+
}))
444+
defer ts.Close()
400445

401-
err := ScrapeAndCompare(ts.URL, expectedReader, "some_total")
402-
if err == nil {
403-
t.Errorf("expected an error but got nil")
404-
}
405-
if !strings.HasPrefix(err.Error(), "the scraping target returned a status code other than 200") {
406-
t.Errorf("unexpected error happened: %s", err)
407-
}
446+
err := ScrapeAndCompare(ts.URL, expectedReader, "some_total")
447+
if err == nil {
448+
t.Errorf("expected an error but got nil")
449+
}
450+
if !strings.HasPrefix(err.Error(), "the scraping target returned a status code other than 200") {
451+
t.Errorf("unexpected error happened: %s", err)
452+
}
453+
})
408454
}
409455

410456
func TestCollectAndCount(t *testing.T) {

0 commit comments

Comments
 (0)