Trivial style cleanup in integration tests.

- Use the conventional `r` and `w` short names for http.Request and http.ResponseWriter parameters. - Factor out ghttp.Server declaration. - Push recorderURL down into the only test case that uses it. - Fix missing err check on a call to URL.Parse(). No functional change (apart from the missing err check), just readability.
alphagov · Jan 20, 2024 · a37389c · a37389c
1 parent 88dde29
commit a37389c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 40 deletions.
diff --git a/handlers/redirect_handler.go b/handlers/redirect_handler.go
@@ -31,16 +31,16 @@ func NewRedirectHandler(source, target string, preserve bool, temporary bool) ht
 	return &redirectHandler{target, statusMoved}
 }
 
-func addCacheHeaders(writer http.ResponseWriter) {
-	writer.Header().Set("Expires", time.Now().Add(cacheDuration).Format(time.RFC1123))
-	writer.Header().Set("Cache-Control", fmt.Sprintf("max-age=%d, public", cacheDuration/time.Second))
+func addCacheHeaders(w http.ResponseWriter) {
+	w.Header().Set("Expires", time.Now().Add(cacheDuration).Format(time.RFC1123))
+	w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%d, public", cacheDuration/time.Second))
 }
 
-func addGAQueryParam(target string, request *http.Request) string {
-	if ga := request.URL.Query().Get("_ga"); ga != "" {
+func addGAQueryParam(target string, r *http.Request) string {
+	if ga := r.URL.Query().Get("_ga"); ga != "" {
 		u, err := url.Parse(target)
 		if err != nil {
-			defer logger.NotifySentry(logger.ReportableError{Error: err, Request: request})
+			defer logger.NotifySentry(logger.ReportableError{Error: err, Request: r})
 			return target
 		}
 		values := u.Query()
@@ -56,11 +56,11 @@ type redirectHandler struct {
 	code int
 }
 
-func (handler *redirectHandler) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
-	addCacheHeaders(writer)
+func (handler *redirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	addCacheHeaders(w)
 
-	target := addGAQueryParam(handler.url, request)
-	http.Redirect(writer, request, target, handler.code)
+	target := addGAQueryParam(handler.url, r)
+	http.Redirect(w, r, target, handler.code)
 
 	redirectCountMetric.With(prometheus.Labels{
 		"redirect_code": fmt.Sprintf("%d", handler.code),
@@ -74,14 +74,14 @@ type pathPreservingRedirectHandler struct {
 	code         int
 }
 
-func (handler *pathPreservingRedirectHandler) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
-	target := handler.targetPrefix + strings.TrimPrefix(request.URL.Path, handler.sourcePrefix)
-	if request.URL.RawQuery != "" {
-		target += "?" + request.URL.RawQuery
+func (handler *pathPreservingRedirectHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	target := handler.targetPrefix + strings.TrimPrefix(r.URL.Path, handler.sourcePrefix)
+	if r.URL.RawQuery != "" {
+		target += "?" + r.URL.RawQuery
 	}
 
-	addCacheHeaders(writer)
-	http.Redirect(writer, request, target, handler.code)
+	addCacheHeaders(w)
+	http.Redirect(w, r, target, handler.code)
 
 	redirectCountMetric.With(prometheus.Labels{
 		"redirect_code": fmt.Sprintf("%d", handler.code),

diff --git a/integration_tests/proxy_function_test.go b/integration_tests/proxy_function_test.go
@@ -15,6 +15,7 @@ import (
 )
 
 var _ = Describe("Functioning as a reverse proxy", func() {
+	var recorder *ghttp.Server
 
 	Describe("connecting to the backend", func() {
 		It("should return a 502 if the connection to the backend is refused", func() {
@@ -74,10 +75,7 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 		})
 
 		Describe("response header timeout", func() {
-			var (
-				tarpit1 *httptest.Server
-				tarpit2 *httptest.Server
-			)
+			var tarpit1, tarpit2 *httptest.Server
 
 			BeforeEach(func() {
 				err := startRouter(3167, 3166, []string{"ROUTER_BACKEND_HEADER_TIMEOUT=0.3s"})
@@ -125,14 +123,8 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 	})
 
 	Describe("header handling", func() {
-		var (
-			recorder    *ghttp.Server
-			recorderURL *url.URL
-		)
-
 		BeforeEach(func() {
 			recorder = startRecordingBackend()
-			recorderURL, _ = url.Parse(recorder.URL())
 			addBackend("backend", recorder.URL())
 			addRoute("/foo", NewBackendRoute("backend", "prefix"))
 			reloadRoutes(apiPort)
@@ -161,6 +153,9 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 			})
 			Expect(resp.StatusCode).To(Equal(200))
 
+			recorderURL, err := url.Parse(recorder.URL())
+			Expect(err).NotTo(HaveOccurred())
+
 			Expect(recorder.ReceivedRequests()).To(HaveLen(1))
 			beReq := recorder.ReceivedRequests()[0]
 			Expect(beReq.Host).To(Equal(recorderURL.Host))
@@ -252,10 +247,6 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 	})
 
 	Describe("request verb, path, query and body handling", func() {
-		var (
-			recorder *ghttp.Server
-		)
-
 		BeforeEach(func() {
 			recorder = startRecordingBackend()
 			addBackend("backend", recorder.URL())
@@ -312,10 +303,6 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 	})
 
 	Describe("handling a backend with a non '/' path", func() {
-		var (
-			recorder *ghttp.Server
-		)
-
 		BeforeEach(func() {
 			recorder = startRecordingBackend()
 			addBackend("backend", recorder.URL()+"/something")
@@ -347,10 +334,6 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 	})
 
 	Describe("handling HTTP/1.0 requests", func() {
-		var (
-			recorder *ghttp.Server
-		)
-
 		BeforeEach(func() {
 			recorder = startRecordingBackend()
 			addBackend("backend", recorder.URL())
@@ -384,8 +367,6 @@ var _ = Describe("Functioning as a reverse proxy", func() {
 	})
 
 	Describe("handling requests to a HTTPS backend", func() {
-		var recorder *ghttp.Server
-
 		BeforeEach(func() {
 			err := startRouter(3167, 3166, []string{"ROUTER_TLS_SKIP_VERIFY=1"})
 			Expect(err).NotTo(HaveOccurred())