Fixup style and missing test

This are stylistic changes and missing test for invalid json in the
details column.

cs_integration_tests/http_request_helpers.go

@@ -16,11 +16,11 @@ import (
 )
 
 func routerRequest(port int, path string) *http.Response {
-	return doRequest(newRequest("GET", routerURL(port, path)))
+	return doRequest(newRequest(http.MethodGet, routerURL(port, path)))
 }
 
 func routerRequestWithHeaders(port int, path string, headers map[string]string) *http.Response {
-	return doRequest(newRequestWithHeaders("GET", routerURL(port, path), headers))
+	return doRequest(newRequestWithHeaders(http.MethodGet, routerURL(port, path), headers))
 }
 
 func newRequest(method, url string) *http.Request {
@@ -55,7 +55,7 @@ func doHTTP10Request(req *http.Request) *http.Response {
 	defer conn.Close()
 
 	if req.Method == "" {
-		req.Method = "GET"
+		req.Method = http.MethodGet
 	}
 	req.Proto = "HTTP/1.0"
 	req.ProtoMinor = 0

cs_integration_tests/integration_test.go

@@ -18,8 +18,7 @@ func TestEverything(t *testing.T) {
 
 var _ = BeforeSuite(func() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
-	var err error
-	err = setupTempLogfile()
+	err := setupTempLogfile()
 	if err != nil {
 		Fail(err.Error())
 	}
@@ -38,12 +37,10 @@ var _ = BeforeSuite(func() {
 		backendEnvVars = append(backendEnvVars, envVar)
 	}
 
-	err = startRouter(routerPort, apiPort, backendEnvVars)
-	if err != nil {
+	if err := startRouter(routerPort, apiPort, backendEnvVars); err != nil {
 		Fail(err.Error())
 	}
-	err = initRouteHelper()
-	if err != nil {
+	if err := initRouteHelper(); err != nil {
 		Fail(err.Error())
 	}
 })

cs_integration_tests/metrics_test.go

@@ -7,16 +7,10 @@ import (
 
 var _ = Describe("/metrics API endpoint", func() {
 	Context("response body", func() {
-		var responseBody string
-
-		BeforeEach(func() {
+		It("should contain at least one metric", func() {
 			resp := doRequest(newRequest("GET", routerURL(apiPort, "/metrics")))
 			Expect(resp.StatusCode).To(Equal(200))
-			responseBody = readBody(resp)
-		})
-
-		It("should contain at least one metric", func() {
-			Expect(responseBody).To(ContainSubstring("router_"))
+			Expect(readBody(resp)).To(ContainSubstring("router_"))
 		})
 	})
 })

lib/load_routes.go

@@ -163,15 +163,7 @@ func (rt *Router) PeriodicCSRouteUpdates() {
 
 func (rt *Router) waitForReload() {
 	for range rt.CsReloadChan {
-		func() {
-			defer func() {
-				if r := recover(); r != nil {
-					logWarn(r)
-				}
-			}()
-
-			rt.reloadCsRoutes(rt.pool)
-		}()
+		rt.reloadCsRoutes(rt.pool)
 	}
 }
 

lib/routes.go

@@ -20,6 +20,9 @@ type CsRoute struct {
 	Details      *string
 }
 
+// returns the backend which should be used for this route
+// if the route is a gone route, but has an explaination in the details field,
+// then route to the backend, or by default to government-frontend
 func (route *CsRoute) backend() *string {
 	if route.SchemaName != nil && *route.SchemaName == "gone" && !route.gone() {
 		if route.BackendID != nil {
@@ -46,18 +49,24 @@ func (route *CsRoute) handlerType() string {
 func (route *CsRoute) gone() bool {
 	if route.SchemaName != nil && *route.SchemaName == "gone" {
 		if route.Details == nil {
+			// if the details field is empty, use a standard gone route
 			return true
 		}
 
+		// deserialise the details field, which should be JSON
 		var detailsMap map[string]interface{}
 		if err := json.Unmarshal([]byte(*route.Details), &detailsMap); err != nil {
-			return false
+			// if the details field is not valid JSON, use a standard gone route
+			return true
 		}
+		// check if keys in the details map are not empty
 		for _, value := range detailsMap {
 			if value != nil && value != "" {
+				// not a standard gone route
 				return false
 			}
 		}
+		// if the details field is empty, use a standard gone route
 		return true
 	}
 	return false

lib/routes_test.go

@@ -86,6 +86,15 @@ var _ = Describe("CsRoute", func() {
 			})
 		})
 
+		Context("when schema is 'gone' and details is invalid json", func() {
+			It("should return true", func() {
+				route.SchemaName = stringPtr("gone")
+				details := "{invalid}"
+				route.Details = &details
+				Expect(route.gone()).To(BeTrue())
+			})
+		})
+
 		Context("when schema is not 'gone'", func() {
 			It("should return false", func() {
 				Expect(route.gone()).To(BeFalse())