Repository: prometheus/prometheus
File: promql/engine_test.go (Go)

Code before:
	ctx, cancelCtx := context.WithCancel(context.Background())
	ctx = promql.NewOriginContext(ctx, map[string]interface{}{"foo": "bar"})
	defer cancelCtx()
	testErr := errors.New("failure")
	query := engine.NewTestQuery(func(_ context.Context) error {
		return testErr
	})

	res := query.Exec(ctx)
	require.Error(t, res.Err, "query should have failed")

	logLines := getLogLines(t, ql1File)
	require.Contains(t, logLines[0], "error", testErr)
	require.Contains(t, logLines[0], "params", map[string]interface{}{"query": "test statement"})
}

func TestPreprocessAndWrapWithStepInvariantExpr(t *testing.T) {
	startTime := time.Unix(1000, 0)
	endTime := time.Unix(9999, 0)
	testCases := []struct {
		input      string      // The input to be parsed.
		expected   parser.Expr // The expected expression AST.
		outputTest bool
	}{
		{
			input: "123.4567",
			expected: &parser.NumberLiteral{
				Val:      123.4567,
				PosRange: posrange.PositionRange{Start: 0, End: 8},
			},
		},
		{
			input: `"foo"`,
			expected: &parser.StringLiteral{
				Val:      "foo",
				PosRange: posrange.PositionRange{Start: 0, End: 5},
			},
		},
		{
			input: "foo * bar",
			expected: &parser.BinaryExpr{
				Op: parser.MUL,
				LHS: &parser.VectorSelector{
					Name: "foo",
					LabelMatchers: []*labels.Matcher{
						parser.MustLabelMatcher(labels.MatchEqual, "__name__", "foo"),
					},
					PosRange: posrange.PositionRange{
						Start: 0,
						End:   3,
					},

Review comment (refactor):
I think there is still one more missing unwrap ...

This can be verified by adding in test cases for the following;

* "(<number>)"
* `("<string>")`
* `("")`

Currently these would not be unwrapped to the NumberLiteral and StringLiteral.

I think in the PreprocessExpr() it is safe to do a unwrapParenExpr(&expr) before passing into the preprocessExprHelper and this will then allow the above test cases to pass.

The current flow of handling these cases in the *parser.ParenExpr in the preprocessExprHelper() does not actually strip the unnecessary parent expr.

Try adding these test cases in;

`
		{
			input: "(123.4567)",
			expected: &parser.NumberLiteral{
				Val:      123.4567,
				PosRange: posrange.PositionRange{Start: 1, End: 9},
			},
		},
		{
			input: `("foo")`,
			expected: &parser.StringLiteral{
				Val:      "foo",
				PosRange: posrange.PositionRange{Start: 1, End: 6},
			},
		},
		{
			input: `("")`,
			expected: &parser.StringLiteral{
				Val:      "",
				PosRange: posrange.PositionRange{Start: 1, End: 3},
			},
		}` 


Code after:
	ctx, cancelCtx := context.WithCancel(context.Background())
	ctx = promql.NewOriginContext(ctx, map[string]interface{}{"foo": "bar"})
	defer cancelCtx()
	testErr := errors.New("failure")
	query := engine.NewTestQuery(func(context.Context) error {
		return testErr
	})

	res := query.Exec(ctx)
	require.Error(t, res.Err, "query should have failed")

	logLines := getLogLines(t, ql1File)
	require.Contains(t, logLines[0], "error", testErr)
	require.Contains(t, logLines[0], "params", map[string]interface{}{"query": "test statement"})
}

func TestPreprocessAndWrapWithStepInvariantExpr(t *testing.T) {
	startTime := time.Unix(1000, 0)
	endTime := time.Unix(9999, 0)
	testCases := []struct {
		input      string      // The input to be parsed.
		expected   parser.Expr // The expected expression AST.
		outputTest bool
	}{
		{
			input: "123.4567",
			expected: &parser.NumberLiteral{
				Val:      123.4567,
				PosRange: posrange.PositionRange{Start: 0, End: 8},
			},
		},
		{
			input: `"foo"`,
			expected: &parser.StringLiteral{
				Val:      "foo",
				PosRange: posrange.PositionRange{Start: 0, End: 5},
			},
		},
		{
			input: "foo * bar",
			expected: &parser.BinaryExpr{
				Op: parser.MUL,
				LHS: &parser.VectorSelector{
					Name: "foo",
					LabelMatchers: []*labels.Matcher{
						parser.MustLabelMatcher(labels.MatchEqual, "__name__", "foo"),
					},
					PosRange: posrange.PositionRange{
						Start: 0,
						End:   3,
					},