From aa0e9fb4f6485c64131a046995f12223320de2bf Mon Sep 17 00:00:00 2001 From: Mariana Franco Date: Thu, 30 Sep 2021 16:59:51 -0700 Subject: [PATCH 1/3] Adding unit test for parsing errors on frontend split queries Signed-off-by: Mariana Franco --- .../queryrange/split_by_interval_test.go | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pkg/querier/queryrange/split_by_interval_test.go b/pkg/querier/queryrange/split_by_interval_test.go index 828077263e8..69e5bd0129c 100644 --- a/pkg/querier/queryrange/split_by_interval_test.go +++ b/pkg/querier/queryrange/split_by_interval_test.go @@ -2,6 +2,7 @@ package queryrange import ( "context" + "github.com/weaveworks/common/httpgrpc" "io/ioutil" "net/http" "net/http/httptest" @@ -349,3 +350,31 @@ func Test_evaluateAtModifier(t *testing.T) { }) } } + +func Test_evaluateAtModifier_Error(t *testing.T) { + const ( + start, end = int64(1546300800), int64(1646300800) + ) + for _, tt := range []struct { + in string + }{ + { + // parse error: missing unit character in duration + "http_requests_total[5] @ 10.001", + }, + { + // parse error: @ modifier must be preceded by an instant vector selector or range vector selector or a subquery + "sum(http_requests_total[5m]) @ 10.001", + }, + } { + tt := tt + t.Run(tt.in, func(t *testing.T) { + t.Parallel() + _, err := evaluateAtModifierFunction(tt.in, start, end) + require.Error(t, err) + httpResp, ok := httpgrpc.HTTPResponseFromError(err) + require.True(t, ok, "returned error is not an httpgrpc response") + require.Equal(t, http.StatusBadRequest, int(httpResp.Code)) + }) + } +} From 39a3956d1de565e3a18747c5d15325eee8f01f8b Mon Sep 17 00:00:00 2001 From: Mariana Franco Date: Thu, 30 Sep 2021 17:35:51 -0700 Subject: [PATCH 2/3] Fix lint errors Signed-off-by: Mariana Franco --- pkg/querier/queryrange/split_by_interval_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/querier/queryrange/split_by_interval_test.go b/pkg/querier/queryrange/split_by_interval_test.go index 69e5bd0129c..c5c71abdacf 100644 --- a/pkg/querier/queryrange/split_by_interval_test.go +++ b/pkg/querier/queryrange/split_by_interval_test.go @@ -2,7 +2,6 @@ package queryrange import ( "context" - "github.com/weaveworks/common/httpgrpc" "io/ioutil" "net/http" "net/http/httptest" @@ -11,6 +10,8 @@ import ( "testing" "time" + "github.com/weaveworks/common/httpgrpc" + "github.com/prometheus/prometheus/promql/parser" "github.com/stretchr/testify/require" "github.com/weaveworks/common/middleware" From e01d967a079deee9fedb5494be876559fbd3eea5 Mon Sep 17 00:00:00 2001 From: Mariana Franco Date: Fri, 1 Oct 2021 16:04:32 -0700 Subject: [PATCH 3/3] Merging error test cases with success ones Signed-off-by: Mariana Franco --- .../queryrange/split_by_interval_test.go | 65 +++++++++---------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/pkg/querier/queryrange/split_by_interval_test.go b/pkg/querier/queryrange/split_by_interval_test.go index c5c71abdacf..081d049ac7a 100644 --- a/pkg/querier/queryrange/split_by_interval_test.go +++ b/pkg/querier/queryrange/split_by_interval_test.go @@ -306,13 +306,23 @@ func Test_evaluateAtModifier(t *testing.T) { start, end = int64(1546300800), int64(1646300800) ) for _, tt := range []struct { - in, expected string + in, expected string + expectedErrorCode int }{ - {"topk(5, rate(http_requests_total[1h] @ start()))", "topk(5, rate(http_requests_total[1h] @ 1546300.800))"}, - {"topk(5, rate(http_requests_total[1h] @ 0))", "topk(5, rate(http_requests_total[1h] @ 0.000))"}, - {"http_requests_total[1h] @ 10.001", "http_requests_total[1h] @ 10.001"}, { - `min_over_time( + in: "topk(5, rate(http_requests_total[1h] @ start()))", + expected: "topk(5, rate(http_requests_total[1h] @ 1546300.800))", + }, + { + in: "topk(5, rate(http_requests_total[1h] @ 0))", + expected: "topk(5, rate(http_requests_total[1h] @ 0.000))", + }, + { + in: "http_requests_total[1h] @ 10.001", + expected: "http_requests_total[1h] @ 10.001", + }, + { + in: `min_over_time( sum by(cluster) ( rate(http_requests_total[5m] @ end()) )[10m:] @@ -325,7 +335,7 @@ func Test_evaluateAtModifier(t *testing.T) { [5m:1m]) [2m:]) [10m:])`, - `min_over_time( + expected: `min_over_time( sum by(cluster) ( rate(http_requests_total[5m] @ 1646300.800) )[10m:] @@ -339,43 +349,32 @@ func Test_evaluateAtModifier(t *testing.T) { [2m:]) [10m:])`, }, - } { - tt := tt - t.Run(tt.in, func(t *testing.T) { - t.Parallel() - expectedExpr, err := parser.ParseExpr(tt.expected) - require.NoError(t, err) - out, err := evaluateAtModifierFunction(tt.in, start, end) - require.NoError(t, err) - require.Equal(t, expectedExpr.String(), out) - }) - } -} - -func Test_evaluateAtModifier_Error(t *testing.T) { - const ( - start, end = int64(1546300800), int64(1646300800) - ) - for _, tt := range []struct { - in string - }{ { // parse error: missing unit character in duration - "http_requests_total[5] @ 10.001", + in: "http_requests_total[5] @ 10.001", + expectedErrorCode: http.StatusBadRequest, }, { // parse error: @ modifier must be preceded by an instant vector selector or range vector selector or a subquery - "sum(http_requests_total[5m]) @ 10.001", + in: "sum(http_requests_total[5m]) @ 10.001", + expectedErrorCode: http.StatusBadRequest, }, } { tt := tt t.Run(tt.in, func(t *testing.T) { t.Parallel() - _, err := evaluateAtModifierFunction(tt.in, start, end) - require.Error(t, err) - httpResp, ok := httpgrpc.HTTPResponseFromError(err) - require.True(t, ok, "returned error is not an httpgrpc response") - require.Equal(t, http.StatusBadRequest, int(httpResp.Code)) + out, err := evaluateAtModifierFunction(tt.in, start, end) + if tt.expectedErrorCode != 0 { + require.Error(t, err) + httpResp, ok := httpgrpc.HTTPResponseFromError(err) + require.True(t, ok, "returned error is not an httpgrpc response") + require.Equal(t, tt.expectedErrorCode, int(httpResp.Code)) + } else { + require.NoError(t, err) + expectedExpr, err := parser.ParseExpr(tt.expected) + require.NoError(t, err) + require.Equal(t, expectedExpr.String(), out) + } }) } }