fix(test): use httptest.newRequest instead of (incorrect) http.newRequest

httptest.newRequest is suppose to be used for testing request handing, http.newRequest is for preparing outgoing requests, this allows to fix cache key selection and increase test coverage

api_test.go

@@ -836,7 +836,7 @@ func TestVerifyAllGroups(t *testing.T) {
 		t.Logf("Testing API using mock files from Alertmanager %s", version)
 		mockAlerts(version)
 		r := ginTestEngine()
-		req, _ := http.NewRequest("GET", "/alerts.json", nil)
+		req := httptest.NewRequest("GET", "/alerts.json", nil)
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != http.StatusOK {

autocomplete.go

@@ -19,11 +19,6 @@ func knownLabelNames(c *gin.Context) {
 	start := time.Now()
 
 	cacheKey := c.Request.RequestURI
-	if cacheKey == "" {
-		// FIXME c.Request.RequestURI is empty when running tests for some reason
-		// needs checking, below acts as a workaround
-		cacheKey = c.Request.URL.RawQuery
-	}
 
 	data, found := apiCache.Get(cacheKey)
 	if found {
@@ -67,11 +62,6 @@ func knownLabelValues(c *gin.Context) {
 	start := time.Now()
 
 	cacheKey := c.Request.RequestURI
-	if cacheKey == "" {
-		// FIXME c.Request.RequestURI is empty when running tests for some reason
-		// needs checking, below acts as a workaround
-		cacheKey = c.Request.URL.RawQuery
-	}
 
 	data, found := apiCache.Get(cacheKey)
 	if found {

autocomplete_test.go

@@ -41,32 +41,36 @@ func TestKnownLabelNames(t *testing.T) {
 		mockAlerts(version)
 		r := ginTestEngine()
 
-		req, _ := http.NewRequest("GET", "/labelNames.json", nil)
-		resp := httptest.NewRecorder()
-		r.ServeHTTP(resp, req)
-		if resp.Code != http.StatusOK {
-			t.Errorf("Invalid status code for request without any query: %d", resp.Code)
-		}
-
-		for _, testCase := range labelTests {
-			url := fmt.Sprintf("/labelNames.json?term=%s", testCase.Term)
-			req, _ := http.NewRequest("GET", url, nil)
+		// repeat test a few times to test cached responses
+		for i := 1; i <= 3; i++ {
+			req := httptest.NewRequest("GET", "/labelNames.json", nil)
 			resp := httptest.NewRecorder()
 			r.ServeHTTP(resp, req)
 
 			if resp.Code != http.StatusOK {
-				t.Errorf("GET %s returned status %d", url, resp.Code)
+				t.Errorf("Invalid status code for request without any query: %d", resp.Code)
 			}
 
-			ur := []string{}
-			err := json.Unmarshal(resp.Body.Bytes(), &ur)
-			if err != nil {
-				t.Errorf("Failed to unmarshal response: %s", err)
-			}
+			for _, testCase := range labelTests {
+				url := fmt.Sprintf("/labelNames.json?term=%s", testCase.Term)
+				req := httptest.NewRequest("GET", url, nil)
+				resp := httptest.NewRecorder()
+				r.ServeHTTP(resp, req)
 
-			if len(ur) != len(testCase.Results) {
-				t.Errorf("Invalid number of label names for %s, got %d, expected %d", url, len(ur), len(testCase.Results))
-				t.Errorf("Results: %s", ur)
+				if resp.Code != http.StatusOK {
+					t.Errorf("GET %s returned status %d", url, resp.Code)
+				}
+
+				ur := []string{}
+				err := json.Unmarshal(resp.Body.Bytes(), &ur)
+				if err != nil {
+					t.Errorf("Failed to unmarshal response: %s", err)
+				}
+
+				if len(ur) != len(testCase.Results) {
+					t.Errorf("Invalid number of label names for %s, got %d, expected %d", url, len(ur), len(testCase.Results))
+					t.Errorf("Results: %s", ur)
+				}
 			}
 		}
 	}
@@ -99,32 +103,35 @@ func TestKnownLabelValues(t *testing.T) {
 		mockAlerts(version)
 		r := ginTestEngine()
 
-		req, _ := http.NewRequest("GET", "/labelValues.json", nil)
-		resp := httptest.NewRecorder()
-		r.ServeHTTP(resp, req)
-		if resp.Code != http.StatusBadRequest {
-			t.Errorf("Invalid status code for request without any query: %d", resp.Code)
-		}
-
-		for _, testCase := range valueTests {
-			url := fmt.Sprintf("/labelValues.json?name=%s", testCase.Name)
-			req, _ := http.NewRequest("GET", url, nil)
+		// repeat test a few times to test cached responses
+		for i := 1; i <= 3; i++ {
+			req := httptest.NewRequest("GET", "/labelValues.json", nil)
 			resp := httptest.NewRecorder()
 			r.ServeHTTP(resp, req)
-
-			if resp.Code != http.StatusOK {
-				t.Errorf("GET %s returned status %d", url, resp.Code)
+			if resp.Code != http.StatusBadRequest {
+				t.Errorf("Invalid status code for request without any query: %d", resp.Code)
 			}
 
-			ur := []string{}
-			err := json.Unmarshal(resp.Body.Bytes(), &ur)
-			if err != nil {
-				t.Errorf("Failed to unmarshal response: %s", err)
-			}
+			for _, testCase := range valueTests {
+				url := fmt.Sprintf("/labelValues.json?name=%s", testCase.Name)
+				req := httptest.NewRequest("GET", url, nil)
+				resp := httptest.NewRecorder()
+				r.ServeHTTP(resp, req)
 
-			if len(ur) != len(testCase.Results) {
-				t.Errorf("Invalid number of label values for %s, got %d, expected %d", url, len(ur), len(testCase.Results))
-				t.Errorf("Results: %s", ur)
+				if resp.Code != http.StatusOK {
+					t.Errorf("GET %s returned status %d", url, resp.Code)
+				}
+
+				ur := []string{}
+				err := json.Unmarshal(resp.Body.Bytes(), &ur)
+				if err != nil {
+					t.Errorf("Failed to unmarshal response: %s", err)
+				}
+
+				if len(ur) != len(testCase.Results) {
+					t.Errorf("Invalid number of label values for %s, got %d, expected %d", url, len(ur), len(testCase.Results))
+					t.Errorf("Results: %s", ur)
+				}
 			}
 		}
 	}

proxy_test.go

@@ -108,7 +108,7 @@ func TestProxy(t *testing.T) {
 		if testCase.upstreamURI != "" {
 			httpmock.RegisterResponder(testCase.method, testCase.upstreamURI, httpmock.NewStringResponder(testCase.code, testCase.response))
 		}
-		req, _ := http.NewRequest(testCase.method, testCase.localPath, nil)
+		req := httptest.NewRequest(testCase.method, testCase.localPath, nil)
 		resp := newCloseNotifyingRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != testCase.code {
@@ -215,7 +215,7 @@ func TestProxyHeaders(t *testing.T) {
 			return httpmock.NewStringResponse(testCase.code, "ok"), nil
 		})
 
-		req, _ := http.NewRequest(testCase.method, testCase.localPath, nil)
+		req := httptest.NewRequest(testCase.method, testCase.localPath, nil)
 		resp := newCloseNotifyingRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != testCase.code {

views.go

@@ -222,11 +222,6 @@ func autocomplete(c *gin.Context) {
 	start := time.Now()
 
 	cacheKey := c.Request.RequestURI
-	if cacheKey == "" {
-		// FIXME c.Request.RequestURI is empty when running tests for some reason
-		// needs checking, below acts as a workaround
-		cacheKey = c.Request.URL.RawQuery
-	}
 
 	data, found := apiCache.Get(cacheKey)
 	if found {

views_test.go

@@ -50,7 +50,7 @@ func ginTestEngine() *gin.Engine {
 func TestIndex(t *testing.T) {
 	mockConfig()
 	r := ginTestEngine()
-	req, _ := http.NewRequest("GET", "/", nil)
+	req := httptest.NewRequest("GET", "/", nil)
 	resp := httptest.NewRecorder()
 	r.ServeHTTP(resp, req)
 	if resp.Code != http.StatusOK {
@@ -63,7 +63,7 @@ func TestIndexPrefix(t *testing.T) {
 	defer os.Unsetenv("LISTEN_PREFIX")
 	mockConfig()
 	r := ginTestEngine()
-	req, _ := http.NewRequest("GET", "/prefix/", nil)
+	req := httptest.NewRequest("GET", "/prefix/", nil)
 	resp := httptest.NewRecorder()
 	r.ServeHTTP(resp, req)
 	if resp.Code != http.StatusOK {
@@ -90,7 +90,7 @@ func TestAlerts(t *testing.T) {
 		t.Logf("Testing alerts using mock files from Alertmanager %s", version)
 		mockAlerts(version)
 		r := ginTestEngine()
-		req, _ := http.NewRequest("GET", "/alerts.json?q=@receiver=by-cluster-service&q=alertname=HTTP_Probe_Failed&q=instance=web1", nil)
+		req := httptest.NewRequest("GET", "/alerts.json?q=@receiver=by-cluster-service&q=alertname=HTTP_Probe_Failed&q=instance=web1", nil)
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != http.StatusOK {
@@ -160,7 +160,7 @@ func TestValidateAllAlerts(t *testing.T) {
 		t.Logf("Validating alerts.json response using mock files from Alertmanager %s", version)
 		mockAlerts(version)
 		r := ginTestEngine()
-		req, _ := http.NewRequest("GET", "/alerts.json?q=alertname=HTTP_Probe_Failed&q=instance=web1", nil)
+		req := httptest.NewRequest("GET", "/alerts.json?q=alertname=HTTP_Probe_Failed&q=instance=web1", nil)
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != http.StatusOK {
@@ -327,7 +327,7 @@ func TestAutocomplete(t *testing.T) {
 		mockAlerts(version)
 		r := ginTestEngine()
 
-		req, _ := http.NewRequest("GET", "/autocomplete.json", nil)
+		req := httptest.NewRequest("GET", "/autocomplete.json", nil)
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != http.StatusBadRequest {
@@ -336,7 +336,7 @@ func TestAutocomplete(t *testing.T) {
 
 		for _, acTest := range acTests {
 			url := fmt.Sprintf("/autocomplete.json?term=%s", acTest.Term)
-			req, _ := http.NewRequest("GET", url, nil)
+			req := httptest.NewRequest("GET", url, nil)
 			resp := httptest.NewRecorder()
 			r.ServeHTTP(resp, req)
 
@@ -395,7 +395,7 @@ func TestStaticFiles(t *testing.T) {
 	mockConfig()
 	r := ginTestEngine()
 	for _, staticFileTest := range staticFileTests {
-		req, _ := http.NewRequest("GET", staticFileTest.path, nil)
+		req := httptest.NewRequest("GET", staticFileTest.path, nil)
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != staticFileTest.code {
@@ -433,7 +433,7 @@ func TestStaticFilesPrefix(t *testing.T) {
 	mockConfig()
 	r := ginTestEngine()
 	for _, staticFileTest := range staticFilePrefixTests {
-		req, _ := http.NewRequest("GET", staticFileTest.path, nil)
+		req := httptest.NewRequest("GET", staticFileTest.path, nil)
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
 		if resp.Code != staticFileTest.code {
@@ -447,7 +447,7 @@ func TestGzipMiddleware(t *testing.T) {
 	r := ginTestEngine()
 	paths := []string{"/", "/alerts.json", "/autocomplete.json", "/metrics"}
 	for _, path := range paths {
-		req, _ := http.NewRequest("GET", path, nil)
+		req := httptest.NewRequest("GET", path, nil)
 		req.Header.Set("Accept-Encoding", "gzip")
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)
@@ -470,7 +470,7 @@ func TestGzipMiddlewareWithoutAcceptEncoding(t *testing.T) {
 	r := ginTestEngine()
 	paths := []string{"/", "/alerts.json", "/autocomplete.json", "/metrics"}
 	for _, path := range paths {
-		req, _ := http.NewRequest("GET", path, nil)
+		req := httptest.NewRequest("GET", path, nil)
 		req.Header.Set("Accept-Encoding", "") // ensure that we don't pass anything up
 		resp := httptest.NewRecorder()
 		r.ServeHTTP(resp, req)