Get rid of varargs in test helpers.

Unnecessary abstraction in test helpers can easily undermine the value of the tests. Get rid of unnecessary logic and just be explicit. Most of this was generated with some sed-foo.
alphagov · Aug 19, 2023 · 19982fb · 19982fb
1 parent 10d476a
commit 19982fb
Show file tree

Hide file tree

Showing 13 changed files with 187 additions and 195 deletions.
diff --git a/integration_tests/disabled_routes_test.go b/integration_tests/disabled_routes_test.go
@@ -11,16 +11,16 @@ var _ = Describe("marking routes as disabled", func() {
 		BeforeEach(func() {
 			addRoute("/unavailable", Route{Handler: "gone", Disabled: true})
 			addRoute("/something-live", NewRedirectRoute("/somewhere-else"))
-			reloadRoutes()
+			reloadRoutes(apiPort)
 		})
 
 		It("should return a 503 to the client", func() {
-			resp := routerRequest("/unavailable")
+			resp := routerRequest(routerPort, "/unavailable")
 			Expect(resp.StatusCode).To(Equal(503))
 		})
 
 		It("should continue to route other requests", func() {
-			resp := routerRequest("/something-live")
+			resp := routerRequest(routerPort, "/something-live")
 			Expect(resp.StatusCode).To(Equal(301))
 			Expect(resp.Header.Get("Location")).To(Equal("/somewhere-else"))
 		})

diff --git a/integration_tests/error_handling_test.go b/integration_tests/error_handling_test.go
@@ -11,31 +11,31 @@ var _ = Describe("error handling", func() {
 
 	Describe("handling an empty routing table", func() {
 		BeforeEach(func() {
-			reloadRoutes()
+			reloadRoutes(apiPort)
 		})
 
 		It("should return a 503 error to the client", func() {
-			resp := routerRequest("/")
+			resp := routerRequest(routerPort, "/")
 			Expect(resp.StatusCode).To(Equal(503))
 
-			resp = routerRequest("/foo")
+			resp = routerRequest(routerPort, "/foo")
 			Expect(resp.StatusCode).To(Equal(503))
 		})
 	})
 
 	Describe("handling a panic", func() {
 		BeforeEach(func() {
 			addRoute("/boom", Route{Handler: "boom"})
-			reloadRoutes()
+			reloadRoutes(apiPort)
 		})
 
 		It("should return a 500 error to the client", func() {
-			resp := routerRequest("/boom")
+			resp := routerRequest(routerPort, "/boom")
 			Expect(resp.StatusCode).To(Equal(500))
 		})
 
 		It("should log the fact", func() {
-			routerRequest("/boom")
+			routerRequest(routerPort, "/boom")
 
 			logDetails := lastRouterErrorLogEntry()
 			Expect(logDetails.Fields).To(Equal(map[string]interface{}{

diff --git a/integration_tests/gone_test.go b/integration_tests/gone_test.go
@@ -10,25 +10,25 @@ var _ = Describe("Gone routes", func() {
 	BeforeEach(func() {
 		addRoute("/foo", NewGoneRoute())
 		addRoute("/bar", NewGoneRoute("prefix"))
-		reloadRoutes()
+		reloadRoutes(apiPort)
 	})
 
 	It("should support an exact gone route", func() {
-		resp := routerRequest("/foo")
+		resp := routerRequest(routerPort, "/foo")
 		Expect(resp.StatusCode).To(Equal(410))
 		Expect(readBody(resp)).To(Equal("410 Gone\n"))
 
-		resp = routerRequest("/foo/bar")
+		resp = routerRequest(routerPort, "/foo/bar")
 		Expect(resp.StatusCode).To(Equal(404))
 		Expect(readBody(resp)).To(Equal("404 page not found\n"))
 	})
 
 	It("should support a prefix gone route", func() {
-		resp := routerRequest("/bar")
+		resp := routerRequest(routerPort, "/bar")
 		Expect(resp.StatusCode).To(Equal(410))
 		Expect(readBody(resp)).To(Equal("410 Gone\n"))
 
-		resp = routerRequest("/bar/baz")
+		resp = routerRequest(routerPort, "/bar/baz")
 		Expect(resp.StatusCode).To(Equal(410))
 		Expect(readBody(resp)).To(Equal("410 Gone\n"))
 	})

diff --git a/integration_tests/http_request_helpers.go b/integration_tests/http_request_helpers.go
@@ -15,12 +15,12 @@ import (
 	// revive:enable:dot-imports
 )
 
-func routerRequest(path string, optionalPort ...int) *http.Response {
-	return doRequest(newRequest("GET", routerURL(path, optionalPort...)))
+func routerRequest(port int, path string) *http.Response {
+	return doRequest(newRequest("GET", routerURL(port, path)))
 }
 
-func routerRequestWithHeaders(path string, headers map[string]string, optionalPort ...int) *http.Response {
-	return doRequest(newRequestWithHeaders("GET", routerURL(path, optionalPort...), headers))
+func routerRequestWithHeaders(port int, path string, headers map[string]string) *http.Response {
+	return doRequest(newRequestWithHeaders("GET", routerURL(port, path), headers))
 }
 
 func newRequest(method, url string) *http.Request {

diff --git a/integration_tests/integration_test.go b/integration_tests/integration_test.go
@@ -20,7 +20,7 @@ var _ = BeforeSuite(func() {
 	if err != nil {
 		Fail(err.Error())
 	}
-	err = startRouter(3169, 3168, nil)
+	err = startRouter(routerPort, apiPort, nil)
 	if err != nil {
 		Fail(err.Error())
 	}
@@ -35,6 +35,6 @@ var _ = BeforeEach(func() {
 })
 
 var _ = AfterSuite(func() {
-	stopRouter(3169)
+	stopRouter(routerPort)
 	cleanupTempLogfile()
 })
diff --git a/integration_tests/metrics_test.go b/integration_tests/metrics_test.go
@@ -10,7 +10,7 @@ var _ = Describe("/metrics API endpoint", func() {
 		var responseBody string
 
 		BeforeEach(func() {
-			resp := doRequest(newRequest("GET", routerAPIURL("/metrics")))
+			resp := doRequest(newRequest("GET", routerURL(apiPort, "/metrics")))
 			Expect(resp.StatusCode).To(Equal(200))
 			responseBody = readBody(resp)
 		})

diff --git a/integration_tests/performance_test.go b/integration_tests/performance_test.go
@@ -26,7 +26,7 @@ var _ = Describe("Performance", func() {
 			addBackend("backend-2", backend2.URL)
 			addRoute("/one", NewBackendRoute("backend-1"))
 			addRoute("/two", NewBackendRoute("backend-2"))
-			reloadRoutes()
+			reloadRoutes(apiPort)
 		})
 		AfterEach(func() {
 			backend1.Close()
@@ -48,7 +48,7 @@ var _ = Describe("Performance", func() {
 					case <-stopCh:
 						return
 					case <-ticker.C:
-						reloadRoutes()
+						reloadRoutes(apiPort)
 					}
 				}()
 
@@ -62,9 +62,9 @@ var _ = Describe("Performance", func() {
 				defer slowBackend.Close()
 				addBackend("backend-slow", slowBackend.URL)
 				addRoute("/slow", NewBackendRoute("backend-slow"))
-				reloadRoutes()
+				reloadRoutes(apiPort)
 
-				_, gen := generateLoad([]string{routerURL("/slow")}, 50)
+				_, gen := generateLoad([]string{routerURL(routerPort, "/slow")}, 50)
 				defer gen.Stop()
 
 				assertPerformantRouter(backend1, backend2, 50)
@@ -75,9 +75,9 @@ var _ = Describe("Performance", func() {
 			It("Router should not cause errors or much latency", func() {
 				addBackend("backend-down", "http://127.0.0.1:3162/")
 				addRoute("/down", NewBackendRoute("backend-down"))
-				reloadRoutes()
+				reloadRoutes(apiPort)
 
-				_, gen := generateLoad([]string{routerURL("/down")}, 50)
+				_, gen := generateLoad([]string{routerURL(routerPort, "/down")}, 50)
 				defer gen.Stop()
 
 				assertPerformantRouter(backend1, backend2, 50)
@@ -102,7 +102,7 @@ var _ = Describe("Performance", func() {
 			addBackend("backend-2", backend2.URL)
 			addRoute("/one", NewBackendRoute("backend-1"))
 			addRoute("/two", NewBackendRoute("backend-2"))
-			reloadRoutes()
+			reloadRoutes(apiPort)
 		})
 		AfterEach(func() {
 			backend1.Close()
@@ -117,7 +117,7 @@ var _ = Describe("Performance", func() {
 
 func assertPerformantRouter(backend1, backend2 *httptest.Server, rps int) {
 	directResultsCh, _ := generateLoad([]string{backend1.URL + "/one", backend2.URL + "/two"}, rps)
-	routerResultsCh, _ := generateLoad([]string{routerURL("/one"), routerURL("/two")}, rps)
+	routerResultsCh, _ := generateLoad([]string{routerURL(routerPort, "/one"), routerURL(routerPort, "/two")}, rps)
 
 	directResults := <-directResultsCh
 	routerResults := <-routerResultsCh