From e3882629bae4ef34cd8ac541951ac5eebab80170 Mon Sep 17 00:00:00 2001 From: Alex Dzyoba Date: Thu, 10 Oct 2019 03:06:53 +0300 Subject: [PATCH] promql: Move tests to testutil (#6103) * promql: Move tests to testutil Signed-off-by: Alex Dzyoba * promql: Match error type via errors.As in tests Signed-off-by: Alex Dzyoba * promql: Remove unused `expectedList` func from lex_test.go Signed-off-by: Alex Dzyoba --- promql/engine_test.go | 136 ++++++++++++------------------------ promql/lex_test.go | 24 ++----- promql/parse_test.go | 83 ++++++---------------- promql/printer_test.go | 12 ++-- promql/promql_test.go | 17 +++-- promql/query_logger_test.go | 14 ++-- 6 files changed, 88 insertions(+), 198 deletions(-) diff --git a/promql/engine_test.go b/promql/engine_test.go index 3ca4f92fbb..65da6b2983 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -15,12 +15,11 @@ package promql import ( "context" - "reflect" + "errors" "testing" "time" "github.com/go-kit/kit/log" - "github.com/pkg/errors" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/storage" @@ -104,14 +103,14 @@ func TestQueryTimeout(t *testing.T) { }) res := query.Exec(ctx) - if res.Err == nil { - t.Fatalf("expected timeout error but got none") - } - if _, ok := res.Err.(ErrQueryTimeout); res.Err != nil && !ok { - t.Fatalf("expected timeout error but got: %s", res.Err) - } + testutil.NotOk(t, res.Err, "expected timeout error but got none") + + var e ErrQueryTimeout + testutil.Assert(t, errors.As(res.Err, &e), "expected timeout error but got: %s", res.Err) } +const errQueryCanceled = ErrQueryCanceled("test statement execution") + func TestQueryCancel(t *testing.T) { opts := EngineOpts{ Logger: nil, @@ -146,12 +145,8 @@ func TestQueryCancel(t *testing.T) { block <- struct{}{} <-processing - if res.Err == nil { - t.Fatalf("expected cancellation error for query1 but got none") - } - if ee := ErrQueryCanceled("test statement execution"); res.Err != ee { - t.Fatalf("expected error %q, got %q", ee, res.Err) - } + testutil.NotOk(t, res.Err, "expected cancellation error for query1 but got none") + testutil.Equals(t, res.Err, errQueryCanceled) // Canceling a query before starting it must have no effect. query2 := engine.newTestQuery(func(ctx context.Context) error { @@ -160,9 +155,7 @@ func TestQueryCancel(t *testing.T) { query2.Cancel() res = query2.Exec(ctx) - if res.Err != nil { - t.Fatalf("unexpected error on executing query2: %s", res.Err) - } + testutil.Ok(t, res.Err) } // errQuerier implements storage.Querier which always returns error. @@ -203,28 +196,18 @@ func TestQueryError(t *testing.T) { defer cancelCtx() vectorQuery, err := engine.NewInstantQuery(queryable, "foo", time.Unix(1, 0)) - if err != nil { - t.Fatalf("unexpected error creating query: %q", err) - } + testutil.Ok(t, err) + res := vectorQuery.Exec(ctx) - if res.Err == nil { - t.Fatalf("expected error on failed select but got none") - } - if res.Err != errStorage { - t.Fatalf("expected error %q, got %q", errStorage, res.Err) - } + testutil.NotOk(t, res.Err, "expected error on failed select but got none") + testutil.Equals(t, res.Err, errStorage) matrixQuery, err := engine.NewInstantQuery(queryable, "foo[1m]", time.Unix(1, 0)) - if err != nil { - t.Fatalf("unexpected error creating query: %q", err) - } + testutil.Ok(t, err) + res = matrixQuery.Exec(ctx) - if res.Err == nil { - t.Fatalf("expected error on failed select but got none") - } - if res.Err != errStorage { - t.Fatalf("expected error %q, got %q", errStorage, res.Err) - } + testutil.NotOk(t, res.Err, "expected error on failed select but got none") + testutil.Equals(t, res.Err, errStorage) } // paramCheckerQuerier implements storage.Querier which checks the start and end times @@ -427,12 +410,8 @@ func TestEngineShutdown(t *testing.T) { block <- struct{}{} <-processing - if res.Err == nil { - t.Fatalf("expected error on shutdown during query but got none") - } - if ee := ErrQueryCanceled("test statement execution"); res.Err != ee { - t.Fatalf("expected error %q, got %q", ee, res.Err) - } + testutil.NotOk(t, res.Err, "expected error on shutdown during query but got none") + testutil.Equals(t, res.Err, errQueryCanceled) query2 := engine.newTestQuery(func(context.Context) error { t.Fatalf("reached query execution unexpectedly") @@ -442,12 +421,10 @@ func TestEngineShutdown(t *testing.T) { // The second query is started after the engine shut down. It must // be canceled immediately. res2 := query2.Exec(ctx) - if res2.Err == nil { - t.Fatalf("expected error on querying with canceled context but got none") - } - if _, ok := res2.Err.(ErrQueryCanceled); !ok { - t.Fatalf("expected cancellation error, got %q", res2.Err) - } + testutil.NotOk(t, res2.Err, "expected error on querying with canceled context but got none") + + var e ErrQueryCanceled + testutil.Assert(t, errors.As(res2.Err, &e), "expected cancellation error but got: %s", res2.Err) } func TestEngineEvalStmtTimestamps(t *testing.T) { @@ -455,15 +432,11 @@ func TestEngineEvalStmtTimestamps(t *testing.T) { load 10s metric 1 2 `) - if err != nil { - t.Fatalf("unexpected error creating test: %q", err) - } + testutil.Ok(t, err) defer test.Close() err = test.Run() - if err != nil { - t.Fatalf("unexpected error initializing test: %q", err) - } + testutil.Ok(t, err) cases := []struct { Query string @@ -540,20 +513,16 @@ load 10s } else { qry, err = test.QueryEngine().NewRangeQuery(test.Queryable(), c.Query, c.Start, c.End, c.Interval) } - if err != nil { - t.Fatalf("unexpected error creating query: %q", err) - } + testutil.Ok(t, err) + res := qry.Exec(test.Context()) if c.ShouldError { testutil.NotOk(t, res.Err, "expected error for the query %q", c.Query) continue } - if res.Err != nil { - t.Fatalf("unexpected error running query: %q", res.Err) - } - if !reflect.DeepEqual(res.Value, c.Result) { - t.Fatalf("unexpected result for query %q: got %q wanted %q", c.Query, res.Value.String(), c.Result.String()) - } + + testutil.Ok(t, res.Err) + testutil.Equals(t, res.Value, c.Result) } } @@ -563,16 +532,11 @@ func TestMaxQuerySamples(t *testing.T) { load 10s metric 1 2 `) - - if err != nil { - t.Fatalf("unexpected error creating test: %q", err) - } + testutil.Ok(t, err) defer test.Close() err = test.Run() - if err != nil { - t.Fatalf("unexpected error initializing test: %q", err) - } + testutil.Ok(t, err) cases := []struct { Query string @@ -772,16 +736,11 @@ load 10s } else { qry, err = engine.NewRangeQuery(test.Queryable(), c.Query, c.Start, c.End, c.Interval) } - if err != nil { - t.Fatalf("unexpected error creating query: %q", err) - } + testutil.Ok(t, err) + res := qry.Exec(test.Context()) - if res.Err != nil && res.Err != c.Result.Err { - t.Fatalf("unexpected error running query: %q, expected to get result: %q", res.Err, c.Result.Value) - } - if !reflect.DeepEqual(res.Value, c.Result.Value) { - t.Fatalf("unexpected result for query %q: got %q wanted %q", c.Query, res.Value.String(), c.Result.String()) - } + testutil.Equals(t, res.Err, c.Result.Err) + testutil.Equals(t, res.Value, c.Result.Value) } } @@ -1048,16 +1007,12 @@ func TestSubquerySelector(t *testing.T) { SetDefaultEvaluationInterval(1 * time.Minute) for _, tst := range tests { test, err := NewTest(t, tst.loadString) + testutil.Ok(t, err) - if err != nil { - t.Fatalf("unexpected error creating test: %q", err) - } defer test.Close() err = test.Run() - if err != nil { - t.Fatalf("unexpected error initializing test: %q", err) - } + testutil.Ok(t, err) engine := test.QueryEngine() for _, c := range tst.cases { @@ -1065,16 +1020,11 @@ func TestSubquerySelector(t *testing.T) { var qry Query qry, err = engine.NewInstantQuery(test.Queryable(), c.Query, c.Start) - if err != nil { - t.Fatalf("unexpected error creating query: %q", err) - } + testutil.Ok(t, err) + res := qry.Exec(test.Context()) - if res.Err != nil && res.Err != c.Result.Err { - t.Fatalf("unexpected error running query: %q, expected to get result: %q", res.Err, c.Result.Value) - } - if !reflect.DeepEqual(res.Value, c.Result.Value) { - t.Fatalf("unexpected result for query %q: got %q wanted %q", c.Query, res.Value.String(), c.Result.String()) - } + testutil.Equals(t, res.Err, c.Result.Err) + testutil.Equals(t, res.Value, c.Result.Value) } } } diff --git a/promql/lex_test.go b/promql/lex_test.go index f9ca711702..ef7231ad13 100644 --- a/promql/lex_test.go +++ b/promql/lex_test.go @@ -14,9 +14,9 @@ package promql import ( - "fmt" - "reflect" "testing" + + "github.com/prometheus/prometheus/util/testutil" ) type testCase struct { @@ -688,24 +688,12 @@ func TestLexer(t *testing.T) { t.Fatalf("unexpected lexing error at position %d: %s", lastItem.pos, lastItem) } - if !reflect.DeepEqual(lastItem, item{ItemEOF, Pos(len(test.input)), ""}) { - t.Logf("%d: input %q", i, test.input) - t.Fatalf("lexing error: expected output to end with EOF item.\ngot:\n%s", expectedList(out)) - } + eofItem := item{ItemEOF, Pos(len(test.input)), ""} + testutil.Equals(t, lastItem, eofItem, "%d: input %q", i, test.input) + out = out[:len(out)-1] - if !reflect.DeepEqual(out, test.expected) { - t.Logf("%d: input %q", i, test.input) - t.Fatalf("lexing mismatch:\nexpected:\n%s\ngot:\n%s", expectedList(test.expected), expectedList(out)) - } + testutil.Equals(t, out, test.expected, "%d: input %q", i, test.input) } }) } } - -func expectedList(exp []item) string { - s := "" - for _, it := range exp { - s += fmt.Sprintf("\t%#v\n", it) - } - return s -} diff --git a/promql/parse_test.go b/promql/parse_test.go index f4c27c272c..f303eb4089 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -15,7 +15,6 @@ package promql import ( "math" - "reflect" "strings" "testing" "time" @@ -1590,26 +1589,14 @@ func TestParseExpressions(t *testing.T) { expr, err := ParseExpr(test.input) // Unexpected errors are always caused by a bug. - if err == errUnexpected { - t.Fatalf("unexpected error occurred") - } + testutil.Assert(t, err != errUnexpected, "unexpected error occurred") - if !test.fail && err != nil { - t.Errorf("error in input '%s'", test.input) - t.Fatalf("could not parse: %s", err) - } - - if test.fail && err != nil { - if !strings.Contains(err.Error(), test.errMsg) { - t.Errorf("unexpected error on input '%s'", test.input) - t.Fatalf("expected error to contain %q but got %q", test.errMsg, err) - } - continue - } - - if !reflect.DeepEqual(expr, test.expected) { - t.Errorf("error on input '%s'", test.input) - t.Fatalf("no match\n\nexpected:\n%s\ngot: \n%s\n", Tree(test.expected), Tree(expr)) + if !test.fail { + testutil.Ok(t, err) + testutil.Equals(t, expr, test.expected, "error on input '%s'", test.input) + } else { + testutil.NotOk(t, err) + testutil.Assert(t, strings.Contains(err.Error(), test.errMsg), "unexpected error on input '%s'", test.input) } } } @@ -1617,21 +1604,11 @@ func TestParseExpressions(t *testing.T) { // NaN has no equality. Thus, we need a separate test for it. func TestNaNExpression(t *testing.T) { expr, err := ParseExpr("NaN") - if err != nil { - t.Errorf("error on input 'NaN'") - t.Fatalf("could not parse: %s", err) - } + testutil.Ok(t, err) nl, ok := expr.(*NumberLiteral) - if !ok { - t.Errorf("error on input 'NaN'") - t.Fatalf("expected number literal but got %T", expr) - } - - if !math.IsNaN(float64(nl.Val)) { - t.Errorf("error on input 'NaN'") - t.Fatalf("expected 'NaN' in number literal but got %v", nl.Val) - } + testutil.Assert(t, ok, "expected number literal but got %T", expr) + testutil.Assert(t, math.IsNaN(float64(nl.Val)), "expected 'NaN' in number literal but got %v", nl.Val) } func mustLabelMatcher(mt labels.MatchType, name, val string) *labels.Matcher { @@ -1746,29 +1723,14 @@ func TestParseSeries(t *testing.T) { metric, vals, err := parseSeriesDesc(test.input) // Unexpected errors are always caused by a bug. - if err == errUnexpected { - t.Fatalf("unexpected error occurred") - } + testutil.Assert(t, err != errUnexpected, "unexpected error occurred") - if test.fail { - if err != nil { - continue - } - t.Errorf("error in input: \n\n%s\n", test.input) - t.Fatalf("failure expected, but passed") + if !test.fail { + testutil.Ok(t, err) + testutil.Equals(t, test.expectedMetric, metric, "error on input '%s'", test.input) + testutil.Equals(t, test.expectedValues, vals, "error in input '%s'", test.input) } else { - if err != nil { - t.Errorf("error in input: \n\n%s\n", test.input) - t.Fatalf("could not parse: %s", err) - } - } - - testutil.Equals(t, test.expectedMetric, metric) - testutil.Equals(t, test.expectedValues, vals) - - if !reflect.DeepEqual(vals, test.expectedValues) || !reflect.DeepEqual(metric, test.expectedMetric) { - t.Errorf("error in input: \n\n%s\n", test.input) - t.Fatalf("no match\n\nexpected:\n%s %s\ngot: \n%s %s\n", test.expectedMetric, test.expectedValues, metric, vals) + testutil.NotOk(t, err) } } } @@ -1778,13 +1740,10 @@ func TestRecoverParserRuntime(t *testing.T) { var err error defer func() { - if err != errUnexpected { - t.Fatalf("wrong error message: %q, expected %q", err, errUnexpected) - } + testutil.Equals(t, err, errUnexpected) - if _, ok := <-p.lex.items; ok { - t.Fatalf("lex.items was not closed") - } + _, ok := <-p.lex.items + testutil.Assert(t, !ok, "lex.items was not closed") }() defer p.recover(&err) // Cause a runtime panic. @@ -1800,9 +1759,7 @@ func TestRecoverParserError(t *testing.T) { e := errors.New("custom error") defer func() { - if err.Error() != e.Error() { - t.Fatalf("wrong error message: %q, expected %q", err, e) - } + testutil.Equals(t, err.Error(), e.Error()) }() defer p.recover(&err) diff --git a/promql/printer_test.go b/promql/printer_test.go index 27c7b5363a..e9785a0472 100644 --- a/promql/printer_test.go +++ b/promql/printer_test.go @@ -15,6 +15,8 @@ package promql import ( "testing" + + "github.com/prometheus/prometheus/util/testutil" ) func TestExprString(t *testing.T) { @@ -88,15 +90,13 @@ func TestExprString(t *testing.T) { for _, test := range inputs { expr, err := ParseExpr(test.in) - if err != nil { - t.Fatalf("parsing error for %q: %s", test.in, err) - } + testutil.Ok(t, err) + exp := test.in if test.out != "" { exp = test.out } - if expr.String() != exp { - t.Fatalf("expected %q to be returned as:\n%s\ngot:\n%s\n", test.in, exp, expr.String()) - } + + testutil.Equals(t, expr.String(), exp) } } diff --git a/promql/promql_test.go b/promql/promql_test.go index 77f8aa31b7..bb90d9f535 100644 --- a/promql/promql_test.go +++ b/promql/promql_test.go @@ -16,22 +16,21 @@ package promql import ( "path/filepath" "testing" + + "github.com/prometheus/prometheus/util/testutil" ) func TestEvaluations(t *testing.T) { files, err := filepath.Glob("testdata/*.test") - if err != nil { - t.Fatal(err) - } + testutil.Ok(t, err) + for _, fn := range files { test, err := newTestFromFile(t, fn) - if err != nil { - t.Errorf("error creating test for %s: %s", fn, err) - } + testutil.Ok(t, err) + err = test.Run() - if err != nil { - t.Errorf("error running test %s: %s", fn, err) - } + testutil.Ok(t, err) + test.Close() } } diff --git a/promql/query_logger_test.go b/promql/query_logger_test.go index 27e5b66446..3b77b0e698 100644 --- a/promql/query_logger_test.go +++ b/promql/query_logger_test.go @@ -18,6 +18,8 @@ import ( "os" "regexp" "testing" + + "github.com/prometheus/prometheus/util/testutil" ) func TestQueryLogging(t *testing.T) { @@ -106,24 +108,18 @@ func TestIndexReuse(t *testing.T) { func TestMMapFile(t *testing.T) { file, err := ioutil.TempFile("", "mmapedFile") - if err != nil { - t.Fatalf("Couldn't create temp test file. %s", err) - } + testutil.Ok(t, err) filename := file.Name() defer os.Remove(filename) fileAsBytes, err := getMMapedFile(filename, 2, nil) - if err != nil { - t.Fatalf("Couldn't create test mmaped file") - } + testutil.Ok(t, err) copy(fileAsBytes, "ab") f, err := os.Open(filename) - if err != nil { - t.Fatalf("Couldn't open test mmaped file") - } + testutil.Ok(t, err) bytes := make([]byte, 4) n, err := f.Read(bytes)