Exclude known edge cases from fuzz tests

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>

Author: harry671003

engine/engine_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/thanos-io/promql-engine/engine"
 	"github.com/thanos-io/promql-engine/execution/model"
 	"github.com/thanos-io/promql-engine/execution/warnings"
+	"github.com/thanos-io/promql-engine/extlabels"
 	"github.com/thanos-io/promql-engine/logicalplan"
 	"github.com/thanos-io/promql-engine/query"
 	"github.com/thanos-io/promql-engine/storage/prometheus"
@@ -6049,10 +6050,23 @@ var (
 			return l.Hash() == r.Hash()
 		}
 
-		if x.Err != nil && y.Err != nil {
-			return cmp.Equal(x.Err.Error(), y.Err.Error())
-		} else if x.Err != nil || y.Err != nil {
-			return false
+		compareErrors := func(l, r error) (stop bool, result bool) {
+			if l == nil && r == nil {
+				return false, true
+			}
+			if l != nil && r != nil {
+				return true, l.Error() == r.Error()
+			}
+			err := l
+			if err == nil {
+				err = r
+			}
+			// Thanos engine handles duplicate label check differently than Prometheus engine.
+			return true, err.Error() == extlabels.ErrDuplicateLabelSet.Error()
+		}
+
+		if stop, result := compareErrors(x.Err, y.Err); stop {
+			return result
 		}
 
 		if !compareAnnotations(x.Warnings, y.Warnings) {

engine/enginefuzz_test.go

@@ -39,15 +39,28 @@ type testCase struct {
 	validateSamples    bool
 }
 
+type testType int
+
+const (
+	testTypeFloat           testType = iota // 0
+	testTypeNativeHistogram                 // 1
+)
+
 // shouldValidateSamples checks if the samples can be compared for the expr.
-// For certain known cases, prometheus engine and thanos engine returns different samples.
+// For certain known cases, Thanos engine returns less samples than Prometheus engine due to optimizations.
 func shouldValidateSamples(expr parser.Expr) bool {
 	valid := true
 
 	parser.Inspect(expr, func(node parser.Node, path []parser.Node) error {
 		switch n := node.(type) {
 		case *parser.Call:
-			if n.Func.Name == "scalar" {
+			switch n.Func.Name {
+			case "scalar":
+				// Optimized to step_invariant in Thanos engine.
+				valid = false
+				return errors.New("error")
+			case "histogram_count", "histogram_sum", "histogram_avg":
+				// Optimized using DetectHistogramStatsOptimizer and will return smaller samples than Prometheus engine.
 				valid = false
 				return errors.New("error")
 			}
@@ -57,6 +70,55 @@ func shouldValidateSamples(expr parser.Expr) bool {
 	return valid
 }
 
+// validateExpr checks if the given expression is valid for fuzz tests.
+// For certain known cases Thanos engine results do not match with Prometheus engine.
+func validateExpr(expr parser.Expr, testType testType) bool {
+	expr = promql.PreprocessExpr(expr, time.Unix(0, 0), time.Unix(0, 0))
+	valid := true
+
+	parser.Inspect(expr, func(node parser.Node, path []parser.Node) error {
+		switch n := node.(type) {
+		case *parser.Call:
+			switch n.Func.Name {
+			case "sort", "sort_desc", "sort_by_label", "sort_by_label_desc":
+				if testType == testTypeNativeHistogram {
+					// Prometheus engine filters out native histograms in nested sort().
+					// Thanos engine implements sorting only at the presentation time and ignores nested sort().
+					// See: https://github.com/thanos-io/promql-engine/pull/595
+					valid = false
+					return errors.New("error")
+				}
+			case "predict_linear":
+				switch t := n.Args[0].(type) {
+				case *parser.StepInvariantExpr:
+					// Thanos engine cannot correctly handle a MatrixSelector wrapped by StepInvariant.
+					// eg: predict_linear({__name__="http_request_duration_seconds"}[5m] @ end(), 0.5)
+					// See: https://github.com/thanos-io/promql-engine/pull/527
+					if _, ok := t.Expr.(*parser.MatrixSelector); ok {
+						valid = false
+						return errors.New("error")
+					}
+				}
+			case "timestamp":
+				if testType == testTypeNativeHistogram {
+					// TODO(johrry): Remove after merging https://github.com/thanos-io/promql-engine/pull/598
+					valid = false
+					return errors.New("error")
+				}
+			case "last_over_time":
+				if testType == testTypeNativeHistogram {
+					// Upstream prometheus bug: https://github.com/prometheus/prometheus/pull/16744
+					// TODO(johrry): Remove after pulling in latest Prometheus
+					valid = false
+					return errors.New("error")
+				}
+			}
+		}
+		return nil
+	})
+	return valid
+}
+
 func FuzzEnginePromQLSmithRangeQuery(f *testing.F) {
 	f.Add(int64(0), uint32(0), uint32(120), uint32(30), 1.0, 1.0, 1.0, 2.0, 30)
 
@@ -114,6 +176,9 @@ func FuzzEnginePromQLSmithRangeQuery(f *testing.F) {
 		for i := range testRuns {
 			for {
 				expr := ps.WalkRangeQuery()
+				if !validateExpr(expr, testTypeFloat) {
+					continue
+				}
 				validateSamples = shouldValidateSamples(expr)
 
 				query = expr.Pretty(0)
@@ -200,8 +265,9 @@ func FuzzEnginePromQLSmithInstantQuery(f *testing.F) {
 		ps := promqlsmith.New(rnd, seriesSet, psOpts...)
 
 		var (
-			q1    promql.Query
-			query string
+			q1              promql.Query
+			query           string
+			validateSamples bool
 		)
 		cases := make([]*testCase, testRuns)
 		for i := range testRuns {
@@ -210,9 +276,10 @@ func FuzzEnginePromQLSmithInstantQuery(f *testing.F) {
 			// Parsing experimental function, like mad_over_time, will lead to a parser.ParseErrors, so we also ignore those.
 			for {
 				expr := ps.WalkInstantQuery()
-				if !shouldValidateSamples(expr) {
+				if !validateExpr(expr, testTypeFloat) {
 					continue
 				}
+				validateSamples = shouldValidateSamples(expr)
 				query = expr.Pretty(0)
 				q1, err = newEngine.NewInstantQuery(context.Background(), storage, qOpts, query, queryTime)
 				if engine.IsUnimplemented(err) || errors.As(err, &parser.ParseErrors{}) {
@@ -243,7 +310,7 @@ func FuzzEnginePromQLSmithInstantQuery(f *testing.F) {
 				loads:           []string{load},
 				start:           queryTime,
 				end:             queryTime,
-				validateSamples: true,
+				validateSamples: validateSamples,
 			}
 		}
 		validateTestCases(t, cases)
@@ -308,7 +375,6 @@ func getMaxNativeHistogramSumLimit(schema int8, noOfBuckets int) float64 {
 }
 
 func FuzzNativeHistogramQuery(f *testing.F) {
-	f.Skip()
 	f.Add(int64(0), uint32(0), uint32(60), uint32(120), int8(0), int8(0), uint64(1), uint64(2), uint64(1))
 
 	f.Fuzz(func(t *testing.T, seed int64, startTS, endTS, intervalSeconds uint32, schema1 int8, schema2 int8, bucketValue1, bucketValue2, bucketValue3 uint64) {
@@ -396,17 +462,21 @@ func FuzzNativeHistogramQuery(f *testing.F) {
 
 		for range testRuns / 2 {
 			var (
-				qInstant     promql.Query
-				qRange       promql.Query
-				instantQuery string
-				rangeQuery   string
+				qInstant                       promql.Query
+				qRange                         promql.Query
+				instantQuery                   string
+				rangeQuery                     string
+				validateSamplesForInstantQuery bool
+				validateSamplesForRangeQuery   bool
 			)
 
 			for {
 				expr := ps.WalkInstantQuery()
-				if !shouldValidateSamples(expr) {
+				if !validateExpr(expr, testTypeNativeHistogram) {
 					continue
 				}
+
+				validateSamplesForInstantQuery = shouldValidateSamples(expr)
 				instantQuery = expr.Pretty(0)
 
 				qInstant, err = newEngine.NewInstantQuery(context.Background(), storage, qOpts, instantQuery, startTime)
@@ -420,9 +490,11 @@ func FuzzNativeHistogramQuery(f *testing.F) {
 
 			for {
 				expr := ps.WalkRangeQuery()
-				if !shouldValidateSamples(expr) {
+				if !validateExpr(expr, testTypeNativeHistogram) {
 					continue
 				}
+
+				validateSamplesForRangeQuery = shouldValidateSamples(expr)
 				rangeQuery = expr.Pretty(0)
 
 				qRange, err = newEngine.NewRangeQuery(context.Background(), storage, qOpts, rangeQuery, startTime, endTime, interval)
@@ -463,7 +535,8 @@ func FuzzNativeHistogramQuery(f *testing.F) {
 				loads:           []string{load},
 				start:           startTime,
 				end:             startTime,
-				validateSamples: true,
+				interval:        0,
+				validateSamples: validateSamplesForInstantQuery,
 			})
 
 			rangeCases = append(rangeCases, &testCase{
@@ -475,7 +548,8 @@ func FuzzNativeHistogramQuery(f *testing.F) {
 				loads:           []string{load},
 				start:           startTime,
 				end:             endTime,
-				validateSamples: true,
+				interval:        interval,
+				validateSamples: validateSamplesForRangeQuery,
 			})
 		}