refactor(promql-compliance): remove expect error support (#727)

Author: milindsrivastava1997

promql-compliance/ARCHITECTURE.md

@@ -88,10 +88,8 @@ For each query, the runner can perform four related checks:
 - Reference parity: Prometheus range-at-t versus Prometheus instant-at-t.
 - Test parity: ASAPQuery range-at-t versus ASAPQuery instant-at-t.
 
-The report passes only if every configured comparison passes. An unexpected
-HTTP/query error from either target fails the comparison, even if both targets
-fail. The exception is a query explicitly marked `expect_error: true`, where
-both targets must return an error.
+The report passes only if every configured comparison passes. An HTTP/query
+error from either target fails the comparison, even if both targets fail.
 
 Equal successful empty results are still equal results; use a dataset and
 probe query that should contain samples when testing ingestion readiness.

promql-compliance/HOW_TO.md

@@ -146,27 +146,6 @@ queries:
 If tolerance is omitted, values are compared exactly. A tolerance should be
 small and justified; a broad tolerance can hide a correctness bug.
 
-## Test expected failures
-
-Use this only when the query is deliberately expected to fail on both
-targets:
-
-```yaml
-- name: intentionally-unsupported
-  expr: unsupported_expression
-  instant_offsets_seconds: [600]
-  expect_error: true
-```
-
-For ordinary queries, an error from either target makes the comparison fail.
-Two matching errors do not accidentally pass. With `expect_error: true`, both
-targets must return errors; a success from either target fails the comparison.
-
-This also makes the suite useful for finding unsupported ASAPQuery queries:
-leave `expect_error` unset for a query that Prometheus supports. A Prometheus
-success paired with an ASAPQuery error produces `passed: false` and records the
-ASAPQuery error in `testError`.
-
 ## Run with DEBUG logging
 
 The default Compose stack uses `INFO`. Temporarily change the query engine

promql-compliance/runner/compare.go

@@ -72,7 +72,7 @@ func CompareQuery(ctx context.Context, reference, test QueryAPI, query QueryCase
 		}
 		referenceRange, _, referenceRangeErr = reference.QueryRange(ctx, query.Expr, rng)
 		testRange, _, testRangeErr = test.QueryRange(ctx, query.Expr, rng)
-		rangeOutcome := responseComparison(referenceRange, testRange, referenceRangeErr, testRangeErr, effective, query.ExpectError)
+		rangeOutcome := responseComparison(referenceRange, testRange, referenceRangeErr, testRangeErr, effective)
 		report.Range = &rangeOutcome
 		report.Passed = report.Passed && rangeOutcome.Passed
 	}
@@ -81,15 +81,15 @@ func CompareQuery(ctx context.Context, reference, test QueryAPI, query QueryCase
 	for index, instantTime := range instantTimes {
 		referenceInstant, _, referenceErr := reference.Query(ctx, query.Expr, instantTime)
 		testInstant, _, testErr := test.Query(ctx, query.Expr, instantTime)
-		outcome := responseComparison(referenceInstant, testInstant, referenceErr, testErr, effective, query.ExpectError)
+		outcome := responseComparison(referenceInstant, testInstant, referenceErr, testErr, effective)
 		report.Instant = append(report.Instant, InstantComparison{
 			OffsetSeconds: query.InstantOffsetsSeconds[index],
 			Time:          instantTime,
 			Comparison:    outcome,
 		})
 		report.Passed = report.Passed && outcome.Passed
 
-		if query.Range == nil || referenceRangeErr != nil || testRangeErr != nil || referenceErr != nil || testErr != nil || query.ExpectError {
+		if query.Range == nil || referenceRangeErr != nil || testRangeErr != nil || referenceErr != nil || testErr != nil {
 			continue
 		}
 		referenceParity := parityComparison(referenceRange, referenceInstant, instantTime, effective)
@@ -124,18 +124,14 @@ func (q QueryCase) RangeAt(base time.Time) (clientv1.Range, error) {
 	}, nil
 }
 
-func responseComparison(reference, test model.Value, referenceErr, testErr error, tolerance ComparisonPolicy, expectError bool) ComparisonOutcome {
+func responseComparison(reference, test model.Value, referenceErr, testErr error, tolerance ComparisonPolicy) ComparisonOutcome {
 	outcome := ComparisonOutcome{}
 	if referenceErr != nil {
 		outcome.ReferenceError = referenceErr.Error()
 	}
 	if testErr != nil {
 		outcome.TestError = testErr.Error()
 	}
-	if expectError {
-		outcome.Passed = referenceErr != nil && testErr != nil
-		return outcome
-	}
 	if referenceErr != nil || testErr != nil {
 		outcome.Passed = false
 		return outcome

promql-compliance/runner/compare_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestCompareQueryRejectsUnexpectedSharedErrors(t *testing.T) {
 	err := errors.New("query failed")
-	outcome := responseComparison(nil, nil, err, err, ComparisonPolicy{}, false)
+	outcome := responseComparison(nil, nil, err, err, ComparisonPolicy{})
 
 	if outcome.Passed {
 		t.Fatal("comparison passed even though both targets failed unexpectedly")
@@ -22,15 +22,6 @@ func TestCompareQueryRejectsUnexpectedSharedErrors(t *testing.T) {
 	}
 }
 
-func TestCompareQueryAcceptsSharedExpectedErrors(t *testing.T) {
-	err := errors.New("query failed")
-	outcome := responseComparison(nil, nil, err, err, ComparisonPolicy{}, true)
-
-	if !outcome.Passed {
-		t.Fatal("comparison rejected matching expected errors")
-	}
-}
-
 type fakeTarget struct {
 	rangeValue  model.Value
 	instantByMS map[int64]model.Value

promql-compliance/runner/config.go

@@ -24,7 +24,6 @@ type QueryCase struct {
 	InstantOffsetsSeconds []float64         `yaml:"instant_offsets_seconds" json:"instantOffsetsSeconds"`
 	Range                 *RangeSpec        `yaml:"range" json:"range"`
 	Comparison            *ComparisonPolicy `yaml:"comparison" json:"comparison"`
-	ExpectError           bool              `yaml:"expect_error" json:"expectError"`
 }
 
 type RangeSpec struct {