From 129ffb7e1765ad0449875d6df17e95af8155f29f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Tue, 21 Aug 2018 00:29:35 +0100 Subject: [PATCH] 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 | 2 +- autocomplete.go | 10 ----- autocomplete_test.go | 87 ++++++++++++++++++++++++-------------------- proxy_test.go | 4 +- views.go | 5 --- views_test.go | 20 +++++----- 6 files changed, 60 insertions(+), 68 deletions(-) diff --git a/api_test.go b/api_test.go index 1444e4232..6d1ce0e8c 100644 --- a/api_test.go +++ b/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 { diff --git a/autocomplete.go b/autocomplete.go index 969258d64..84d53356c 100644 --- a/autocomplete.go +++ b/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 { diff --git a/autocomplete_test.go b/autocomplete_test.go index a11f021c0..4c32fca93 100644 --- a/autocomplete_test.go +++ b/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) + } } } } diff --git a/proxy_test.go b/proxy_test.go index 951cc5360..f983c9323 100644 --- a/proxy_test.go +++ b/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 { diff --git a/views.go b/views.go index b625163a3..e01d32b70 100644 --- a/views.go +++ b/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 { diff --git a/views_test.go b/views_test.go index 557afcd7c..c592ab12c 100644 --- a/views_test.go +++ b/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)