Revert "testutil compareMetricFamilies: make less error-prone (#1424)"

This reverts commit c7c7509.
prometheus · Oct 14, 2024 · 584a7ce · 584a7ce
1 parent 05fcde9
commit 584a7ce
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 108 deletions.
diff --git a/prometheus/testutil/testutil.go b/prometheus/testutil/testutil.go
@@ -277,15 +277,6 @@ func compareMetricFamilies(got, expected []*dto.MetricFamily, metricNames ...str
 	if metricNames != nil {
 		got = filterMetrics(got, metricNames)
 		expected = filterMetrics(expected, metricNames)
-		if len(metricNames) > len(got) {
-			var missingMetricNames []string
-			for _, name := range metricNames {
-				if ok := hasMetricByName(got, name); !ok {
-					missingMetricNames = append(missingMetricNames, name)
-				}
-			}
-			return fmt.Errorf("expected metric name(s) not found: %v", missingMetricNames)
-		}
 	}
 
 	return compare(got, expected)
@@ -327,12 +318,3 @@ func filterMetrics(metrics []*dto.MetricFamily, names []string) []*dto.MetricFam
 	}
 	return filtered
 }
-
-func hasMetricByName(metrics []*dto.MetricFamily, name string) bool {
-	for _, mf := range metrics {
-		if mf.GetName() == name {
-			return true
-		}
-	}
-	return false
-}
diff --git a/prometheus/testutil/testutil_test.go b/prometheus/testutil/testutil_test.go
@@ -328,46 +328,27 @@ func TestMetricNotFound(t *testing.T) {
 }
 
 func TestScrapeAndCompare(t *testing.T) {
-	scenarios := map[string]struct {
-		want        string
-		metricNames []string
-		// expectedErr if empty, means no fail is expected for the comparison.
-		expectedErr string
-	}{
-		"empty metric Names": {
-			want: `
+	const expected = `
 		# 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
+	`
 
-		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
+	expectedReader := strings.NewReader(expected)
 
-		some_total{ label1 = "value1" } 1
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, expected)
+	}))
+	defer ts.Close()
 
-		# HELP some_total2 A value that represents a counter.
-		# TYPE some_total2 counter
+	if err := ScrapeAndCompare(ts.URL, expectedReader, "some_total"); err != nil {
+		t.Errorf("unexpected scraping result:\n%s", err)
+	}
+}
 
-		some_total2{ label2 = "value2" } 1
-	`,
-			metricNames: []string{"some_total2"},
-		},
-		"expected metric name is not scraped": {
-			want: `
+func TestScrapeAndCompareWithMultipleExpected(t *testing.T) {
+	const expected = `
 		# HELP some_total A value that represents a counter.
 		# TYPE some_total counter
 
@@ -377,78 +358,53 @@ func TestScrapeAndCompare(t *testing.T) {
 		# TYPE some_total2 counter
 
 		some_total2{ label2 = "value2" } 1
-	`,
-			metricNames: []string{"some_total3"},
-			expectedErr: "expected metric name(s) not found: [some_total3]",
-		},
-		"one of multiple expected metric names is not scraped": {
-			want: `
-		# HELP some_total A value that represents a counter.
-		# TYPE some_total counter
+	`
 
-		some_total{ label1 = "value1" } 1
+	expectedReader := strings.NewReader(expected)
 
-		# HELP some_total2 A value that represents a counter.
-		# TYPE some_total2 counter
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, expected)
+	}))
+	defer ts.Close()
 
-		some_total2{ label2 = "value2" } 1
-	`,
-			metricNames: []string{"some_total1", "some_total3"},
-			expectedErr: "expected metric name(s) not found: [some_total1 some_total3]",
-		},
-	}
-	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.expectedErr == "" || err.Error() != scenario.expectedErr {
-					t.Errorf("unexpected error happened: %s", err)
-				}
-			} else if scenario.expectedErr != "" {
-				t.Errorf("expected an error but got nil")
-			}
-		})
+	if err := ScrapeAndCompare(ts.URL, expectedReader, "some_total2"); err != nil {
+		t.Errorf("unexpected scraping result:\n%s", err)
 	}
+}
 
-	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)
-		}
-	})
+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")
+	}
+	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 = `
+func TestScrapeAndCompareBadStatusCode(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) {