Skip to content

Commit

Permalink
Prefer HaveHTTPStatus() over StatusCode.To(Equal()).
Browse files Browse the repository at this point in the history 
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))/'
```
  • Loading branch information
@sengi
sengi committed Jan 22, 2024
1 parent 6fc41a4 commit 7b67e02
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 54 deletions.
4 changes: 2 additions & 2 deletions integration_tests/disabled_routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
})
})
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down
48 changes: 24 additions & 24 deletions integration_tests/proxy_function_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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"))
})
})
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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"))
})
})
Expand All @@ -264,19 +264,19 @@ 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))
})

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))
})
Expand All @@ -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))
})
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
10 changes: 5 additions & 5 deletions integration_tests/redirect_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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"))
})

Expand Down Expand Up @@ -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"))
})
})
Expand Down
14 changes: 7 additions & 7 deletions integration_tests/reload_api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
})

Expand All @@ -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"))
})
})
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions integration_tests/route_loading_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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"))
})
})
Expand Down
Loading

0 comments on commit 7b67e02

Please sign in to comment.