From 1442a30aaba6144c118ca4b1b89484e4b9779aea Mon Sep 17 00:00:00 2001 From: Chris Banks Date: Sun, 21 Jan 2024 21:56:22 +0000 Subject: [PATCH] Prefer `HaveHTTPStatus()` over `StatusCode.To(Equal())`. This also gets rid of change-detectors on error response bodies (which we don't care about, because the edge caches replace them with prettier ones anyway). https://onsi.github.io/gomega/#working-with-http-responses Generated with: ```sh g grep -l resp.StatusCode integration_tests | xargs gsed -i \ 's/Expect(resp.StatusCode)\.To(Equal(\([0-9]\{3\}\)))/Expect(resp).To(HaveHTTPStatus(\1))/' ``` --- integration_tests/disabled_routes_test.go | 4 +- integration_tests/metrics_test.go | 2 +- integration_tests/proxy_function_test.go | 48 +++++++++++------------ integration_tests/redirect_test.go | 10 ++--- integration_tests/reload_api_test.go | 14 +++---- integration_tests/route_loading_test.go | 6 +-- integration_tests/route_selection_test.go | 22 +++++------ integration_tests/router_support.go | 2 +- 8 files changed, 54 insertions(+), 54 deletions(-) diff --git a/integration_tests/disabled_routes_test.go b/integration_tests/disabled_routes_test.go index 7c1ae267..5da6f413 100644 --- a/integration_tests/disabled_routes_test.go +++ b/integration_tests/disabled_routes_test.go @@ -16,12 +16,12 @@ var _ = Describe("marking routes as disabled", func() { It("should return a 503 to the client", func() { resp := routerRequest(routerPort, "/unavailable") - Expect(resp.StatusCode).To(Equal(503)) + Expect(resp).To(HaveHTTPStatus(503)) }) It("should continue to route other requests", func() { resp := routerRequest(routerPort, "/something-live") - Expect(resp.StatusCode).To(Equal(301)) + Expect(resp).To(HaveHTTPStatus(301)) Expect(resp.Header.Get("Location")).To(Equal("/somewhere-else")) }) }) diff --git a/integration_tests/metrics_test.go b/integration_tests/metrics_test.go index ba4812e9..09a83ed6 100644 --- a/integration_tests/metrics_test.go +++ b/integration_tests/metrics_test.go @@ -11,7 +11,7 @@ var _ = Describe("/metrics API endpoint", func() { BeforeEach(func() { resp := doRequest(newRequest("GET", routerURL(apiPort, "/metrics"))) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) responseBody = readBody(resp) }) diff --git a/integration_tests/proxy_function_test.go b/integration_tests/proxy_function_test.go index 6c6de1d6..28e57aef 100644 --- a/integration_tests/proxy_function_test.go +++ b/integration_tests/proxy_function_test.go @@ -27,7 +27,7 @@ var _ = Describe("As a reverse proxy", func() { req.Header.Set("X-Varnish", "12345678") resp := doRequest(req) - Expect(resp.StatusCode).To(Equal(502)) + Expect(resp).To(HaveHTTPStatus(502)) logDetails := lastRouterErrorLogEntry() Expect(logDetails.Fields).To(Equal(map[string]interface{}{ @@ -58,7 +58,7 @@ var _ = Describe("As a reverse proxy", func() { resp := doRequest(req) duration := time.Since(start) - Expect(resp.StatusCode).To(Equal(504)) + Expect(resp).To(HaveHTTPStatus(504)) Expect(duration).To(BeNumerically("~", 320*time.Millisecond, 20*time.Millisecond)) logDetails := lastRouterErrorLogEntry() @@ -98,7 +98,7 @@ var _ = Describe("As a reverse proxy", func() { req := newRequest(http.MethodGet, routerURL(3167, "/tarpit1")) req.Header.Set("X-Varnish", "12341112") resp := doRequest(req) - Expect(resp.StatusCode).To(Equal(504)) + Expect(resp).To(HaveHTTPStatus(504)) logDetails := lastRouterErrorLogEntry() tarpitURL, _ := url.Parse(tarpit1.URL) @@ -115,7 +115,7 @@ var _ = Describe("As a reverse proxy", func() { It("should still return the response if the body takes longer than the header timeout", func() { resp := routerRequest(3167, "/tarpit2") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(readBody(resp)).To(Equal("Tarpit\n")) }) }) @@ -140,7 +140,7 @@ var _ = Describe("As a reverse proxy", func() { "Foo": "bar", "User-Agent": "Router test suite 2.7182", }) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -152,7 +152,7 @@ var _ = Describe("As a reverse proxy", func() { resp := routerRequestWithHeaders(routerPort, "/foo", map[string]string{ "Host": "www.example.com", }) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -162,7 +162,7 @@ var _ = Describe("As a reverse proxy", func() { It("should not add a default User-Agent if there isn't one in the request", func() { // Most http libraries add a default User-Agent header. resp := routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -172,7 +172,7 @@ var _ = Describe("As a reverse proxy", func() { It("should add the client IP to X-Forwarded-For", func() { resp := routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -181,7 +181,7 @@ var _ = Describe("As a reverse proxy", func() { resp = routerRequestWithHeaders(routerPort, "/foo", map[string]string{ "X-Forwarded-For": "10.9.8.7", }) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(2)) beReq = recorder.ReceivedRequests()[1] @@ -193,7 +193,7 @@ var _ = Describe("As a reverse proxy", func() { It("should add itself to the Via request header for an HTTP/1.1 request", func() { resp := routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -202,7 +202,7 @@ var _ = Describe("As a reverse proxy", func() { resp = routerRequestWithHeaders(routerPort, "/foo", map[string]string{ "Via": "1.0 fred, 1.1 barney", }) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(2)) beReq = recorder.ReceivedRequests()[1] @@ -212,7 +212,7 @@ var _ = Describe("As a reverse proxy", func() { It("should add itself to the Via request header for an HTTP/1.0 request", func() { req := newRequest(http.MethodGet, routerURL(routerPort, "/foo")) resp := doHTTP10Request(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -222,7 +222,7 @@ var _ = Describe("As a reverse proxy", func() { "Via": "1.0 fred, 1.1 barney", }) resp = doHTTP10Request(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(2)) beReq = recorder.ReceivedRequests()[1] @@ -231,14 +231,14 @@ var _ = Describe("As a reverse proxy", func() { It("should add itself to the Via response heaver", func() { resp := routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(resp.Header.Get("Via")).To(Equal("1.1 router")) recorder.AppendHandlers(ghttp.RespondWith(200, "body", http.Header{ "Via": []string{"1.0 fred, 1.1 barney"}, })) resp = routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(resp.Header.Get("Via")).To(Equal("1.0 fred, 1.1 barney, 1.1 router")) }) }) @@ -264,11 +264,11 @@ var _ = Describe("As a reverse proxy", func() { req := newRequest("POST", routerURL(routerPort, "/foo")) resp := doRequest(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) req = newRequest("DELETE", routerURL(routerPort, "/foo/bar/baz.json")) resp = doRequest(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(2)) }) @@ -276,7 +276,7 @@ var _ = Describe("As a reverse proxy", func() { It("should pass through the query string unmodified", func() { recorder.AppendHandlers(ghttp.VerifyRequest("GET", "/foo/bar", "baz=qux")) resp := routerRequest(routerPort, "/foo/bar?baz=qux") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) }) @@ -292,7 +292,7 @@ var _ = Describe("As a reverse proxy", func() { req := newRequest("POST", routerURL(routerPort, "/foo")) req.Body = io.NopCloser(strings.NewReader("I am the request body. Woohoo!")) resp := doRequest(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) }) @@ -312,7 +312,7 @@ var _ = Describe("As a reverse proxy", func() { It("should merge the 2 paths", func() { resp := routerRequest(routerPort, "/foo/bar") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -321,7 +321,7 @@ var _ = Describe("As a reverse proxy", func() { It("should preserve the request query string", func() { resp := routerRequest(routerPort, "/foo/bar?baz=qux") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -344,7 +344,7 @@ var _ = Describe("As a reverse proxy", func() { It("should work with incoming HTTP/1.1 requests", func() { req := newRequest("GET", routerURL(routerPort, "/foo")) resp := doHTTP10Request(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -354,7 +354,7 @@ var _ = Describe("As a reverse proxy", func() { It("should proxy to the backend as HTTP/1.1 requests", func() { req := newRequest("GET", routerURL(routerPort, "/foo")) resp := doHTTP10Request(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] @@ -382,7 +382,7 @@ var _ = Describe("As a reverse proxy", func() { It("should correctly reverse proxy to a HTTPS backend", func() { req := newRequest("GET", routerURL(3167, "/foo")) resp := doRequest(req) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) beReq := recorder.ReceivedRequests()[0] diff --git a/integration_tests/redirect_test.go b/integration_tests/redirect_test.go index 91474c21..3ccae7b8 100644 --- a/integration_tests/redirect_test.go +++ b/integration_tests/redirect_test.go @@ -21,12 +21,12 @@ var _ = Describe("Redirection", func() { It("should redirect permanently by default", func() { resp := routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(301)) + Expect(resp).To(HaveHTTPStatus(301)) }) It("should redirect temporarily when asked to", func() { resp := routerRequest(routerPort, "/foo-temp") - Expect(resp.StatusCode).To(Equal(302)) + Expect(resp).To(HaveHTTPStatus(302)) }) It("should contain the redirect location", func() { @@ -78,13 +78,13 @@ var _ = Describe("Redirection", func() { It("should redirect permanently to the destination", func() { resp := routerRequest(routerPort, "/foo") - Expect(resp.StatusCode).To(Equal(301)) + Expect(resp).To(HaveHTTPStatus(301)) Expect(resp.Header.Get("Location")).To(Equal("/bar")) }) It("should redirect temporarily to the destination when asked to", func() { resp := routerRequest(routerPort, "/foo-temp") - Expect(resp.StatusCode).To(Equal(302)) + Expect(resp).To(HaveHTTPStatus(302)) Expect(resp.Header.Get("Location")).To(Equal("/bar-temp")) }) @@ -126,7 +126,7 @@ var _ = Describe("Redirection", func() { reloadRoutes(apiPort) resp := routerRequest(routerPort, "/foo bar/something") - Expect(resp.StatusCode).To(Equal(301)) + Expect(resp).To(HaveHTTPStatus(301)) Expect(resp.Header.Get("Location")).To(Equal("/bar%20baz/something")) }) }) diff --git a/integration_tests/reload_api_test.go b/integration_tests/reload_api_test.go index 0b52cd17..3947f85a 100644 --- a/integration_tests/reload_api_test.go +++ b/integration_tests/reload_api_test.go @@ -12,23 +12,23 @@ var _ = Describe("reload API endpoint", func() { Describe("request handling", func() { It("should return 202 for POST /reload", func() { resp := doRequest(newRequest("POST", routerURL(apiPort, "/reload"))) - Expect(resp.StatusCode).To(Equal(202)) + Expect(resp).To(HaveHTTPStatus(202)) Expect(readBody(resp)).To(Equal("Reload queued")) }) It("should return 404 for POST /foo", func() { resp := doRequest(newRequest("POST", routerURL(apiPort, "/foo"))) - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) It("should return 404 for POST /reload/foo", func() { resp := doRequest(newRequest("POST", routerURL(apiPort, "/reload/foo"))) - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) It("should return 405 for GET /reload", func() { resp := doRequest(newRequest("GET", routerURL(apiPort, "/reload"))) - Expect(resp.StatusCode).To(Equal(405)) + Expect(resp).To(HaveHTTPStatus(405)) Expect(resp.Header.Get("Allow")).To(Equal("POST")) }) @@ -50,13 +50,13 @@ var _ = Describe("reload API endpoint", func() { Describe("healthcheck", func() { It("should return HTTP 200 OK on GET", func() { resp := doRequest(newRequest("GET", routerURL(apiPort, "/healthcheck"))) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(readBody(resp)).To(Equal("OK")) }) It("should return HTTP 405 Method Not Allowed on POST", func() { resp := doRequest(newRequest("POST", routerURL(apiPort, "/healthcheck"))) - Expect(resp.StatusCode).To(Equal(405)) + Expect(resp).To(HaveHTTPStatus(405)) Expect(resp.Header.Get("Allow")).To(Equal("GET")) }) }) @@ -69,7 +69,7 @@ var _ = Describe("reload API endpoint", func() { reloadRoutes(apiPort) resp := doRequest(newRequest("GET", routerURL(apiPort, "/memory-stats"))) - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) var data map[string]interface{} readJSONBody(resp, &data) diff --git a/integration_tests/route_loading_test.go b/integration_tests/route_loading_test.go index 827c1deb..417d0cd0 100644 --- a/integration_tests/route_loading_test.go +++ b/integration_tests/route_loading_test.go @@ -36,7 +36,7 @@ var _ = Describe("loading routes from the db", func() { It("should skip the invalid route", func() { resp := routerRequest(routerPort, "/bar") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) It("should continue to load other routes", func() { @@ -59,7 +59,7 @@ var _ = Describe("loading routes from the db", func() { It("should skip the invalid route", func() { resp := routerRequest(routerPort, "/bar") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) It("should continue to load other routes", func() { @@ -99,7 +99,7 @@ var _ = Describe("loading routes from the db", func() { It("should send requests to the backend_url provided in the env var", func() { resp := routerRequest(routerPort, "/oof") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(readBody(resp)).To(Equal("backend 3")) }) }) diff --git a/integration_tests/route_selection_test.go b/integration_tests/route_selection_test.go index 4b60e44b..579d388b 100644 --- a/integration_tests/route_selection_test.go +++ b/integration_tests/route_selection_test.go @@ -44,18 +44,18 @@ var _ = Describe("Route selection", func() { It("should 404 for children of the exact route", func() { resp := routerRequest(routerPort, "/foo/bar") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) It("should 404 for non-matching requests", func() { resp := routerRequest(routerPort, "/wibble") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) resp = routerRequest(routerPort, "/") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) resp = routerRequest(routerPort, "/foo.json") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) }) @@ -104,13 +104,13 @@ var _ = Describe("Route selection", func() { It("should 404 for non-matching requests", func() { resp := routerRequest(routerPort, "/wibble") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) resp = routerRequest(routerPort, "/") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) resp = routerRequest(routerPort, "/foo.json") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) }) @@ -296,7 +296,7 @@ var _ = Describe("Route selection", func() { Expect(readBody(resp)).To(Equal("other backend")) resp = routerRequest(routerPort, "/bar") - Expect(resp.StatusCode).To(Equal(404)) + Expect(resp).To(HaveHTTPStatus(404)) }) It("should handle a prefix route at the root level", func() { @@ -341,14 +341,14 @@ var _ = Describe("Route selection", func() { It("should not be redirected by our recorder backend", func() { resp := routerRequest(routerPort, "/foo/bar/baz//qux") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) Expect(recorder.ReceivedRequests()[0].URL.Path).To(Equal("/foo/bar/baz//qux")) }) It("should collapse double slashes when looking up route, but pass request as-is", func() { resp := routerRequest(routerPort, "/foo//bar") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) Expect(recorder.ReceivedRequests()[0].URL.Path).To(Equal("/foo//bar")) }) @@ -370,7 +370,7 @@ var _ = Describe("Route selection", func() { reloadRoutes(apiPort) resp := routerRequest(routerPort, "/foo bar") - Expect(resp.StatusCode).To(Equal(200)) + Expect(resp).To(HaveHTTPStatus(200)) Expect(recorder.ReceivedRequests()).To(HaveLen(1)) Expect(recorder.ReceivedRequests()[0].RequestURI).To(Equal("/foo%20bar")) }) diff --git a/integration_tests/router_support.go b/integration_tests/router_support.go index 1165df06..46193114 100644 --- a/integration_tests/router_support.go +++ b/integration_tests/router_support.go @@ -36,7 +36,7 @@ func reloadRoutes(port int) { resp, err := http.DefaultClient.Do(req) Expect(err).NotTo(HaveOccurred()) - Expect(resp.StatusCode).To(Equal(202)) + Expect(resp).To(HaveHTTPStatus(202)) resp.Body.Close() // Now that reloading is done asynchronously, we need a small sleep to ensure // it has actually been performed.