diff --git a/src/cmd/tools/dtest/docker/harness/carbon_test.go b/src/cmd/tools/dtest/docker/harness/carbon_test.go index 199a770a43..6bc35a9ee7 100644 --- a/src/cmd/tools/dtest/docker/harness/carbon_test.go +++ b/src/cmd/tools/dtest/docker/harness/carbon_test.go @@ -31,12 +31,16 @@ import ( "github.com/stretchr/testify/assert" ) -func findVerifier(expected string) resources.GoalStateVerifier { - return func(s string, err error) error { +func findVerifier(expected string) resources.ResponseVerifier { + return func(status int, s string, err error) error { if err != nil { return err } + if status/100 != 2 { + return fmt.Errorf("expected 200 status code, got %v", status) + } + if s == expected { return nil } @@ -45,17 +49,21 @@ func findVerifier(expected string) resources.GoalStateVerifier { } } -func renderVerifier(metric string, v float64) resources.GoalStateVerifier { +func renderVerifier(v float64) resources.ResponseVerifier { type graphiteRender struct { Target string `json:"target"` Datapoints [][]float64 `json:"datapoints"` } - return func(s string, err error) error { + return func(status int, s string, err error) error { if err != nil { return err } + if status/100 != 2 { + return fmt.Errorf("expected 200 status code, got %v", status) + } + var render []graphiteRender if err := json.Unmarshal([]byte(s), &render); err != nil { return err @@ -103,7 +111,7 @@ func TestCarbon(t *testing.T) { read := func(metric string, expected float64) { assert.NoError(t, coord.RunQuery( - renderVerifier(metric, expected), + renderVerifier(expected), graphiteQuery(metric, timestamp))) } diff --git a/src/cmd/tools/dtest/docker/harness/query_api_test.go b/src/cmd/tools/dtest/docker/harness/query_api_test.go new file mode 100644 index 0000000000..289f5cff55 --- /dev/null +++ b/src/cmd/tools/dtest/docker/harness/query_api_test.go @@ -0,0 +1,131 @@ +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package harness + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/m3db/m3/src/cmd/tools/dtest/docker/harness/resources" +) + +type urlTest struct { + name string + url string +} + +func TestInvalidInstantQueryReturns400(t *testing.T) { + coord := singleDBNodeDockerResources.Coordinator() + + instantQueryBadRequestTest := []urlTest{ + // FAILING issue #2: invalid or missing query string should result in 400 + // {"missing query", queryURL("query", "")}, + // {"invalid query", queryURL("query", "@!")}, + {"invalid time", queryURL("time", "INVALID")}, + {"invalid timeout", queryURL("timeout", "INVALID")}, + } + + for _, tt := range instantQueryBadRequestTest { + t.Run(tt.name, func(t *testing.T) { + assert.NoError(t, coord.RunQuery(verifyStatus(400), tt.url), "for query '%v'", tt.url) + }) + } +} + +func TestInvalidRangeQueryReturns400(t *testing.T) { + coord := singleDBNodeDockerResources.Coordinator() + + queryBadRequestTest := []urlTest{ + {"missing query", queryRangeURL("query", "")}, + // FAILING issue #2: invalid query string should result in 400 + // {"invalid query", queryRangeURL("query", "@!")}, + {"missing start", queryRangeURL("start", "")}, + {"invalid start", queryRangeURL("start", "INVALID")}, + {"missing end", queryRangeURL("end", "")}, + {"invalid end", queryRangeURL("end", "INVALID")}, + {"missing step", queryRangeURL("step", "")}, + {"invalid step", queryRangeURL("step", "INVALID")}, + {"invalid timeout", queryRangeURL("timeout", "INVALID")}, + } + + for _, tt := range queryBadRequestTest { + t.Run(tt.name, func(t *testing.T) { + assert.NoError(t, coord.RunQuery(verifyStatus(400), tt.url), "for query '%v'", tt.url) + }) + } +} + +func queryURL(key, value string) string { + params := map[string]string{ + "query": "foo", + } + + if value == "" { + delete(params, key) + } else { + params[key] = value + } + + return "api/v1/query?" + queryString(params) +} + +func queryRangeURL(key, value string) string { + params := map[string]string{ + "query": "foo", + "start": "now", + "end": "now", + "step": "1000", + } + + if value == "" { + delete(params, key) + } else { + params[key] = value + } + + return "api/v1/query_range?" + queryString(params) +} + +func queryString(params map[string]string) string { + p := make([]string, 0) + for k, v := range params { + p = append(p, fmt.Sprintf("%v=%v", k, v)) + } + + return strings.Join(p, "&") +} + +func verifyStatus(expectedStatus int) resources.ResponseVerifier { + return func(status int, resp string, err error) error { + if err != nil { + return err + } + + if status != expectedStatus { + return fmt.Errorf("expeceted %v status code, got %v", expectedStatus, status) + } + + return nil + } +} diff --git a/src/cmd/tools/dtest/docker/harness/resources/coordinator.go b/src/cmd/tools/dtest/docker/harness/resources/coordinator.go index 0333519ecc..d2d320a446 100644 --- a/src/cmd/tools/dtest/docker/harness/resources/coordinator.go +++ b/src/cmd/tools/dtest/docker/harness/resources/coordinator.go @@ -54,6 +54,9 @@ var ( } ) +// ResponseVerifier is a function that checks if the query response is valid. +type ResponseVerifier func(int, string, error) error + // Coordinator is a wrapper for a coordinator. It provides a wrapper on HTTP // endpoints that expose cluster management APIs as well as read and write // endpoints for series data. @@ -64,7 +67,7 @@ type Coordinator interface { // WriteCarbon writes a carbon metric datapoint at a given time. WriteCarbon(port int, metric string, v float64, t time.Time) error // RunQuery runs the given query with a given verification function. - RunQuery(verifier GoalStateVerifier, query string) error + RunQuery(verifier ResponseVerifier, query string) error } // Admin is a wrapper for admin functions. @@ -341,7 +344,7 @@ func makePostRequest(logger *zap.Logger, url string, body proto.Message) (*http. } func (c *coordinator) query( - verifier GoalStateVerifier, query string, + verifier ResponseVerifier, query string, ) error { if c.resource.closed { return errClosed @@ -357,20 +360,14 @@ func (c *coordinator) query( return err } - defer resp.Body.Close() - if resp.StatusCode/100 != 2 { - logger.Error("status code not 2xx", - zap.Int("status code", resp.StatusCode), - zap.String("status", resp.Status)) - return fmt.Errorf("status code %d", resp.StatusCode) - } defer resp.Body.Close() b, err := ioutil.ReadAll(resp.Body) - return verifier(string(b), err) + + return verifier(resp.StatusCode, string(b), err) } func (c *coordinator) RunQuery( - verifier GoalStateVerifier, query string, + verifier ResponseVerifier, query string, ) error { if c.resource.closed { return errClosed