Apply suggestions from code review

- remove if nil check
- use two nested loops instead of map
- use new function `hasMetricByName` for readability

Co-authored-by: Bartlomiej Plotka <[email protected]>
Signed-off-by: leonnicolas <[email protected]>

Author: leonnicolas

prometheus/testutil/testutil.go

@@ -278,16 +278,9 @@ func compareMetricFamilies(got, expected []*dto.MetricFamily, metricNames ...str
 		got = filterMetrics(got, metricNames)
 		expected = filterMetrics(expected, metricNames)
 		if len(metricNames) > len(got) {
-			h := make(map[string]struct{})
-			for _, mf := range got {
-				if mf == nil {
-					continue
-				}
-				h[mf.GetName()] = struct{}{}
-			}
 			var missingMetricNames []string
 			for _, name := range metricNames {
-				if _, ok := h[name]; !ok {
+				if ok := hasMetricByName(got, name); !ok {
 					missingMetricNames = append(missingMetricNames, name)
 				}
 			}
@@ -334,3 +327,12 @@ 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
+}

prometheus/testutil/testutil_test.go

@@ -331,8 +331,8 @@ func TestScrapeAndCompare(t *testing.T) {
 	scenarios := map[string]struct {
 		want        string
 		metricNames []string
-		errPrefix   string
-		fail        bool
+		// expectedErrPrefix if empty, means no fail is expected for the comparison.
+		expectedErrPrefix string
 	}{
 		"empty metric Names": {
 			want: `
@@ -378,9 +378,8 @@ func TestScrapeAndCompare(t *testing.T) {
 
 		some_total2{ label2 = "value2" } 1
 	`,
-			metricNames: []string{"some_total3"},
-			errPrefix:   "expected metric name(s) not found",
-			fail:        true,
+			metricNames:       []string{"some_total3"},
+			expectedErrPrefix: "expected metric name(s) not found",
 		},
 		"one of multiple expected metric names is not scraped": {
 			want: `
@@ -394,9 +393,8 @@ func TestScrapeAndCompare(t *testing.T) {
 
 		some_total2{ label2 = "value2" } 1
 	`,
-			metricNames: []string{"some_total1", "some_total3"},
-			errPrefix:   "expected metric name(s) not found",
-			fail:        true,
+			metricNames:       []string{"some_total1", "some_total3"},
+			expectedErrPrefix: "expected metric name(s) not found",
 		},
 	}
 	for name, scenario := range scenarios {
@@ -408,10 +406,10 @@ func TestScrapeAndCompare(t *testing.T) {
 			}))
 			defer ts.Close()
 			if err := ScrapeAndCompare(ts.URL, expectedReader, scenario.metricNames...); err != nil {
-				if !scenario.fail || !strings.HasPrefix(err.Error(), scenario.errPrefix) {
+				if scenario.expectedErrPrefix == "" || !strings.HasPrefix(err.Error(), scenario.expectedErrPrefix) {
 					t.Errorf("unexpected error happened: %s", err)
 				}
-			} else if scenario.fail {
+			} else if scenario.expectedErrPrefix != "" {
 				t.Errorf("expected an error but got nil")
 			}
 		})