From fec522f128c277927771839df94cc9397f3153e0 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Tue, 11 Feb 2025 22:39:58 +0100 Subject: [PATCH] frontend: fix tests using httptest.ResponseRecorder Signed-off-by: Simon Pasquier --- frontend/pkg/frontend/middleware_body.go | 1 + frontend/pkg/frontend/middleware_body_test.go | 46 ++++++++----------- .../pkg/frontend/middleware_validatestatic.go | 1 + .../middleware_validatestatic_test.go | 27 ++++------- .../middleware_validatesubscription_test.go | 38 +++++++-------- go.work.sum | 7 ++- 6 files changed, 50 insertions(+), 70 deletions(-) diff --git a/frontend/pkg/frontend/middleware_body.go b/frontend/pkg/frontend/middleware_body.go index 85b0cc285..1c88d3c57 100644 --- a/frontend/pkg/frontend/middleware_body.go +++ b/frontend/pkg/frontend/middleware_body.go @@ -13,6 +13,7 @@ import ( const megabyte int64 = (1 << 20) +// MiddlewareBody ensures that the request's body doesn't exceed the maximum size of 4MB. func MiddlewareBody(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { switch r.Method { case http.MethodPatch, http.MethodPost, http.MethodPut: diff --git a/frontend/pkg/frontend/middleware_body_test.go b/frontend/pkg/frontend/middleware_body_test.go index b4f3ef16a..a856e433e 100644 --- a/frontend/pkg/frontend/middleware_body_test.go +++ b/frontend/pkg/frontend/middleware_body_test.go @@ -6,10 +6,13 @@ package frontend import ( "bytes" "encoding/json" + "io" "net/http" "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" + "github.com/Azure/ARO-HCP/internal/api/arm" ) @@ -80,9 +83,7 @@ func TestMiddlewareBody(t *testing.T) { writer := httptest.NewRecorder() request, err := http.NewRequest(method, "", bytes.NewReader(tt.body)) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) request.Header = tt.header next := func(w http.ResponseWriter, r *http.Request) { @@ -92,36 +93,29 @@ func TestMiddlewareBody(t *testing.T) { MiddlewareBody(writer, request, next) - if tt.wantErr == "" { - if writer.Code != http.StatusOK { - t.Error(writer.Code) - } + res := writer.Result() + b, err := io.ReadAll(res.Body) + assert.NoError(t, err) - if writer.Body.String() != "" { - t.Error(writer.Body.String()) - } + if tt.wantErr == "" { + assert.Equal(t, http.StatusOK, res.StatusCode) + assert.Empty(t, string(b)) if method != http.MethodGet { body, err := BodyFromContext(request.Context()) - if err != nil { - t.Fatal(err) - } - if !bytes.Equal(body, tt.body) { - t.Error(string(body)) - } - } - } else { - var cloudErr *arm.CloudError - err = json.Unmarshal(writer.Body.Bytes(), &cloudErr) - if err != nil { - t.Fatal(err) + assert.NoError(t, err) + assert.Equal(t, string(tt.body), string(body)) } - cloudErr.StatusCode = writer.Code - if tt.wantErr != cloudErr.Error() { - t.Error(cloudErr) - } + return } + + var cloudErr *arm.CloudError + err = json.Unmarshal(b, &cloudErr) + assert.NoError(t, err) + + cloudErr.StatusCode = res.StatusCode + assert.Equal(t, tt.wantErr, cloudErr.Error()) }) } } diff --git a/frontend/pkg/frontend/middleware_validatestatic.go b/frontend/pkg/frontend/middleware_validatestatic.go index 26510804e..ae1a05ade 100644 --- a/frontend/pkg/frontend/middleware_validatestatic.go +++ b/frontend/pkg/frontend/middleware_validatestatic.go @@ -19,6 +19,7 @@ import ( var rxHCPOpenShiftClusterResourceName = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{2,53}$`) var rxNodePoolResourceName = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{2,14}$`) +// MiddlewareValidateStatic ensures that the URL path parses to a valid resource ID. func MiddlewareValidateStatic(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { // To conform with "OAPI012: Resource IDs must not be case sensitive" // we need to use the original, non-lowercased resource ID components diff --git a/frontend/pkg/frontend/middleware_validatestatic_test.go b/frontend/pkg/frontend/middleware_validatestatic_test.go index 6d34b2390..0f3cb5e1d 100644 --- a/frontend/pkg/frontend/middleware_validatestatic_test.go +++ b/frontend/pkg/frontend/middleware_validatestatic_test.go @@ -2,12 +2,12 @@ package frontend import ( "encoding/json" - "io" "net/http" "net/http/httptest" - "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/Azure/ARO-HCP/internal/api/arm" ) @@ -111,29 +111,18 @@ func TestMiddlewareValidateStatic(t *testing.T) { // Execute the middleware MiddlewareValidateStatic(w, req, nextHandler) + res := w.Result() + // Check the response status code - if status := w.Code; status != tc.expectedStatusCode { - t.Errorf("handler returned wrong status code: got %v want %v", - status, tc.expectedStatusCode) - } + assert.Equal(t, tc.expectedStatusCode, res.StatusCode) if tc.expectedStatusCode != http.StatusOK { - var resp CloudErrorContainer - body, err := io.ReadAll(http.MaxBytesReader(w, w.Result().Body, 4*megabyte)) - if err != nil { - t.Fatalf("failed to read response body: %v", err) - } - err = json.Unmarshal(body, &resp) - if err != nil { - t.Fatalf("failed to unmarshal response body: %v", err) - } + err := json.NewDecoder(res.Body).Decode(&resp) + assert.NoError(t, err) // Check if the error message contains the expected text - if !strings.Contains(resp.Error.Message, tc.expectedBody) { - t.Errorf("handler returned unexpected body: got %v want %v", - resp.Error.Message, tc.expectedBody) - } + assert.Contains(t, tc.expectedBody, resp.Error.Message) } }) } diff --git a/frontend/pkg/frontend/middleware_validatesubscription_test.go b/frontend/pkg/frontend/middleware_validatesubscription_test.go index dbdd230e5..c5389b016 100644 --- a/frontend/pkg/frontend/middleware_validatesubscription_test.go +++ b/frontend/pkg/frontend/middleware_validatesubscription_test.go @@ -6,13 +6,12 @@ package frontend import ( "encoding/json" "fmt" - "io" "log/slog" "net/http" "net/http/httptest" "testing" - "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" "github.com/Azure/ARO-HCP/internal/api/arm" @@ -186,9 +185,7 @@ func TestMiddlewareValidateSubscription(t *testing.T) { writer := httptest.NewRecorder() request, err := http.NewRequest(tt.httpMethod, tt.requestPath, nil) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) // Add a logger to the context so parsing errors will be logged. ctx := request.Context() @@ -207,18 +204,19 @@ func TestMiddlewareValidateSubscription(t *testing.T) { MiddlewareValidateSubscriptionState(writer, request, next) + res := writer.Result() if tt.expectedError != nil { - var actualError *arm.CloudError - body, _ := io.ReadAll(http.MaxBytesReader(writer, writer.Result().Body, 4*megabyte)) - _ = json.Unmarshal(body, &actualError) - if (writer.Result().StatusCode != tt.expectedError.StatusCode) || actualError.Code != tt.expectedError.Code || actualError.Message != tt.expectedError.Message { - t.Errorf("unexpected CloudError, wanted %v, got %v", tt.expectedError, actualError) - } - } else { - if doc.Subscription.State != tt.expectedState { - t.Error(cmp.Diff(doc.Subscription.State, tt.expectedState)) - } + var actualError arm.CloudError + err = json.NewDecoder(res.Body).Decode(&actualError) + assert.NoError(t, err) + + assert.Equal(t, tt.expectedError.StatusCode, res.StatusCode) + assert.Equal(t, tt.expectedError.Code, actualError.Code) + assert.Equal(t, tt.expectedError.Message, actualError.Message) + return } + + assert.Equal(t, tt.expectedState, doc.Subscription.State) }) } @@ -226,9 +224,7 @@ func TestMiddlewareValidateSubscription(t *testing.T) { writer := httptest.NewRecorder() request, err := http.NewRequest(http.MethodGet, defaultRequestPath, nil) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, err) request.SetPathValue(PathSegmentSubscriptionID, subscriptionId) ctx := request.Context() @@ -237,8 +233,8 @@ func TestMiddlewareValidateSubscription(t *testing.T) { next := func(w http.ResponseWriter, r *http.Request) {} MiddlewareValidateSubscriptionState(writer, request, next) - if writer.Code != http.StatusInternalServerError { - t.Errorf("expected status code %d, got %d", http.StatusInternalServerError, writer.Code) - } + + res := writer.Result() + assert.Equal(t, http.StatusInternalServerError, res.StatusCode) }) } diff --git a/go.work.sum b/go.work.sum index a9fb021ce..59931950b 100644 --- a/go.work.sum +++ b/go.work.sum @@ -934,7 +934,6 @@ github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/cyphar/filepath-securejoin v0.2.5 h1:6iR5tXJ/e6tJZzzdMc1km3Sa7RRIVBKAK32O2s7AYfo= github.com/cyphar/filepath-securejoin v0.2.5/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= -github.com/cyphar/filepath-securejoin v0.3.4/go.mod h1:8s/MCNJREmFK0H02MF6Ihv1nakJe4L/w3WZLHNkvlYM= github.com/danieljoos/wincred v1.2.0 h1:ozqKHaLK0W/ii4KVbbvluM91W2H3Sh0BncbUNPS7jLE= github.com/danieljoos/wincred v1.2.0/go.mod h1:FzQLLMKBFdvu+osBrnFODiv32YGwCfx0SkRa/eYHgec= github.com/danieljoos/wincred v1.2.1 h1:dl9cBrupW8+r5250DYkYxocLeZ1Y4vB1kxgtjxw8GQs= @@ -1729,7 +1728,6 @@ github.com/soheilhy/cmux v0.1.5/go.mod h1:T7TcVDs9LWfQgPlPsdngu6I6QIoyIFZDDC6sNE github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= -github.com/spf13/cast v1.7.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v0.0.6/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE= github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= @@ -2170,6 +2168,7 @@ golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE= golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= golang.org/x/net v0.28.0/go.mod h1:yqtgsTWOOnlGLG9GFRrK3++bGOUEkNBoHZc8MEDWPNg= golang.org/x/net v0.30.0/go.mod h1:2wGyMJ5iFasEhkwi13ChkO/t1ECNC4X4eBKkVFyYFlU= +golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -2282,6 +2281,7 @@ golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2 h1:IRJeR9r1pYWsHKTRe/IInb7lYvbBVIqOgsX/u0mbOWY= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457 h1:zf5N6UOrA487eEFacMePxjXAJctxKmyjKUsjA11Uzuk= @@ -2317,6 +2317,7 @@ golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -2628,7 +2629,6 @@ honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= k8s.io/api v0.32.0/go.mod h1:4LEwHZEf6Q/cG96F3dqR965sYOfmPM7rq81BLgsE0p0= k8s.io/apiextensions-apiserver v0.32.0/go.mod h1:86hblMvN5yxMvZrZFX2OhIHAuFIMJIZ19bTvzkP+Fmw= -k8s.io/apiextensions-apiserver v0.32.1/go.mod h1:sxWIGuGiYov7Io1fAS2X06NjMIk5CbRHc2StSmbaQto= k8s.io/apimachinery v0.32.0/go.mod h1:GpHVgxoKlTxClKcteaeuF1Ul/lDVb74KpZcxcmLDElE= k8s.io/apiserver v0.31.1 h1:Sars5ejQDCRBY5f7R3QFHdqN3s61nhkpaX8/k1iEw1c= k8s.io/apiserver v0.31.1/go.mod h1:lzDhpeToamVZJmmFlaLwdYZwd7zB+WYRYIboqA1kGxM= @@ -2680,7 +2680,6 @@ sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 h1:2770sDpzrjjsA sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0 h1:CPT0ExVicCzcpeN4baWEV2ko2Z/AsiZgEdwgcfwLgMo= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= -sigs.k8s.io/controller-runtime v0.20.1/go.mod h1:BrP3w158MwvB3ZbNpaAcIKkHQ7YGpYnzpoSTZ8E14WU= sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3/go.mod h1:18nIHnGi6636UCz6m8i4DhaJ65T6EruyzmoQqI2BVDo= sigs.k8s.io/kustomize/api v0.17.2 h1:E7/Fjk7V5fboiuijoZHgs4aHuexi5Y2loXlVOAVAG5g= sigs.k8s.io/kustomize/api v0.17.2/go.mod h1:UWTz9Ct+MvoeQsHcJ5e+vziRRkwimm3HytpZgIYqye0=