From 9fc01b272bfc4fc8cb9c27e3cfafba1d6c07b9cb Mon Sep 17 00:00:00 2001 From: Matt Mix <36235043+mwmix@users.noreply.github.com> Date: Tue, 5 Aug 2025 08:09:38 -0400 Subject: [PATCH] test: improve coverage on http and metrics (#5712) * chore(tests): Adding in missing tests for CustomRoundTrip struct. * chore(tests): Adding in missing unit test. * chore(tests): Refactored and addressed missing coverage in models.go --- pkg/http/http_test.go | 98 +++++++++++++++++++++++++++++++++++++ pkg/metrics/metrics_test.go | 8 +++ pkg/metrics/models_test.go | 78 ++++++++++++++++++++++++----- 3 files changed, 172 insertions(+), 12 deletions(-) diff --git a/pkg/http/http_test.go b/pkg/http/http_test.go index 1e46e2292..3fc558bd4 100644 --- a/pkg/http/http_test.go +++ b/pkg/http/http_test.go @@ -18,9 +18,12 @@ package http import ( "fmt" + "io" "net/http" + "net/url" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -57,3 +60,98 @@ func TestNewInstrumentedClient(t *testing.T) { _, ok = result2.Transport.(*CustomRoundTripper) require.True(t, ok) } + +func TestCancelRequest(t *testing.T) { + for _, tt := range []struct { + title string + customRoundTripper CustomRoundTripper + request *http.Request + }{ + { + title: "CancelRequest does nothing", + customRoundTripper: CustomRoundTripper{}, + request: &http.Request{}, + }, + } { + t.Run(tt.title, func(t *testing.T) { + tt.customRoundTripper.CancelRequest(tt.request) + }) + } +} + +type mockRoundTripper struct { + response *http.Response + error error +} + +func (mrt mockRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { + return mrt.response, mrt.error +} + +func TestRoundTrip(t *testing.T) { + for _, tt := range []struct { + title string + nextRoundTripper mockRoundTripper + request *http.Request + method string + url string + body io.Reader + + expectError bool + expectedResponse *http.Response + }{ + { + title: "RoundTrip returns no error", + nextRoundTripper: mockRoundTripper{}, + request: &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "HTTPS", + Host: "test.local", + Path: "/path", + }, + Body: nil, + }, + expectError: false, + expectedResponse: nil, + }, + { + title: "RoundTrip extracts status from request", + nextRoundTripper: mockRoundTripper{ + response: &http.Response{ + StatusCode: http.StatusOK, + }, + }, + request: &http.Request{ + Method: http.MethodGet, + URL: &url.URL{ + Scheme: "HTTPS", + Host: "test.local", + Path: "/path", + }, + Body: nil, + }, + expectError: false, + expectedResponse: &http.Response{ + StatusCode: http.StatusOK, + }, + }, + } { + t.Run(tt.title, func(t *testing.T) { + req, err := http.NewRequest(tt.method, tt.url, tt.body) + customRoundTripper := CustomRoundTripper{ + next: tt.nextRoundTripper, + } + + resp, err := customRoundTripper.RoundTrip(req) + + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.expectedResponse, resp) + }) + } +} diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index b7a0aca49..3406577cc 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -72,6 +72,14 @@ func TestMustRegister(t *testing.T) { }, expected: 0, }, + { + name: "skip if metric exists", + metrics: []IMetric{ + NewGaugeWithOpts(prometheus.GaugeOpts{Name: "existing_metric"}), + NewGaugeWithOpts(prometheus.GaugeOpts{Name: "existing_metric"}), + }, + expected: 1, + }, } for _, tt := range tests { diff --git a/pkg/metrics/models_test.go b/pkg/metrics/models_test.go index dc9c7a7db..5c80db58a 100644 --- a/pkg/metrics/models_test.go +++ b/pkg/metrics/models_test.go @@ -113,21 +113,75 @@ func TestGaugeV_SetWithLabels(t *testing.T) { assert.Len(t, m.Label, 2) } -func TestNewBuildInfoCollector(t *testing.T) { - metric := NewGaugeFuncMetric(prometheus.GaugeOpts{ - Namespace: Namespace, - Name: "build_info", - ConstLabels: prometheus.Labels{ - "version": "0.0.1", - "goversion": "1.24", - "arch": "arm64", +func TestNewGaugeFuncMetric(t *testing.T) { + tests := []struct { + name string + metricName string + subSystem string + constLabels prometheus.Labels + expectedFqName string + expectedDescString string + expectedGaugeFuncReturn float64 + }{ + { + name: "NewGaugeFuncMetric returns build_info", + metricName: "build_info", + subSystem: "", + constLabels: prometheus.Labels{ + "version": "0.0.1", + "goversion": "1.24", + "arch": "arm64", + }, + expectedFqName: "external_dns_build_info", + expectedDescString: "version=\"0.0.1\"", + expectedGaugeFuncReturn: 1, }, - }) + { + name: "NewGaugeFuncMetric subsystem alters name", + metricName: "metricName", + subSystem: "subSystem", + constLabels: prometheus.Labels{}, + expectedFqName: "external_dns_subSystem_metricName", + expectedDescString: "", + expectedGaugeFuncReturn: 1, + }, + { + name: "NewGaugeFuncMetric GaugeFunc returns 1", + metricName: "metricName", + subSystem: "", + constLabels: prometheus.Labels{}, + expectedFqName: "external_dns_metricName", + expectedDescString: "", + expectedGaugeFuncReturn: 1, + }, + } - desc := metric.GaugeFunc.Desc() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + metric := NewGaugeFuncMetric(prometheus.GaugeOpts{ + Namespace: Namespace, + Name: tt.metricName, + Subsystem: tt.subSystem, + ConstLabels: tt.constLabels, + }) - assert.Equal(t, "external_dns_build_info", reflect.ValueOf(desc).Elem().FieldByName("fqName").String()) - assert.Contains(t, desc.String(), "version=\"0.0.1\"") + desc := metric.GaugeFunc.Desc() + + assert.Equal(t, tt.expectedFqName, reflect.ValueOf(desc).Elem().FieldByName("fqName").String()) + assert.Contains(t, desc.String(), tt.expectedDescString) + + testRegistry := prometheus.NewRegistry() + err := testRegistry.Register(metric.GaugeFunc) + require.NoError(t, err) + + metricFamily, err := testRegistry.Gather() + require.NoError(t, err) + require.Len(t, metricFamily, 1) + + require.NotNil(t, metricFamily[0].Metric[0].Gauge) + assert.InDelta(t, tt.expectedGaugeFuncReturn, metricFamily[0].Metric[0].GetGauge().GetValue(), 0.0001) + }) + } } func TestSummaryV_SetWithLabels(t *testing.T) {