Skip to content

Commit

Permalink
fixed test by asserting the matchers without FastRegexMatcher
Browse files Browse the repository at this point in the history
Signed-off-by: Vladyslav Diachenko <vlad.diachenko@grafana.com>
  • Loading branch information
vlad-diachenko committed Aug 6, 2024
1 parent cb9089f commit 08c6cc0
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 67 deletions.
30 changes: 13 additions & 17 deletions clients/pkg/logentry/logql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,20 @@ func TestParse(t *testing.T) {
ast, err := ParseExpr(tc.in)
require.Equal(t, tc.err, err)

if tc.exp == nil {
require.Nil(t, ast)
} else {
require.NotNil(t, ast)

// TODO this test is just comparing the matchers, I don't have a smart idea on how to compare the whole expression.
// Maybe we can use the visitor pattern? Alternatively, we may try to use reflection to manipulate the matchers
// to make them comparable with deep equal.
// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare them without FastRegexMatchers.
require.Equal(t, removeFastRegexMatcher(tc.exp), removeFastRegexMatcher(ast))
})
}
}

// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
expectedMatchers := tc.exp.Matchers()
actualMatchers := ast.Matchers()
require.Len(t, actualMatchers, len(expectedMatchers))
for i, expected := range expectedMatchers {
require.Equal(t, expected.String(), actualMatchers[i].String())
}
func removeFastRegexMatcher(exp Expr) Expr {
if typed, ok := exp.(*matchersExpr); ok {
for i, matcher := range typed.matchers {
if matcher.Type == labels.MatchNotRegexp || matcher.Type == labels.MatchRegexp {
typed.matchers[i] = &labels.Matcher{Type: matcher.Type, Name: matcher.Name, Value: matcher.Value}
}
})
}
}
return exp
}
11 changes: 3 additions & 8 deletions pkg/logql/matchers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

"github.com/prometheus/prometheus/model/labels"
"github.com/stretchr/testify/require"

"github.com/grafana/loki/v3/pkg/logql/syntax"
)

func Test_match(t *testing.T) {
Expand Down Expand Up @@ -51,16 +53,9 @@ func Test_match(t *testing.T) {
if tt.wantErr {
require.Error(t, err)
} else {
// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
require.Len(t, got, len(tt.want))

for i, expectedMatchers := range tt.want {
require.Len(t, got[i], len(expectedMatchers))

for j, expectedMatcher := range expectedMatchers {
require.Equal(t, expectedMatcher.String(), got[i][j].String())
}
syntax.AssertMatchers(t, expectedMatchers, got[i])
}
}
})
Expand Down
14 changes: 2 additions & 12 deletions pkg/logql/syntax/ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,7 @@ func Test_SampleExpr_String(t *testing.T) {
expr2, err := ParseExpr(expr.String())
require.Nil(t, err)

// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
require.Equal(t, expr.String(), expr2.String())
AssertExpressions(t, expr, expr2)
})
}
}
Expand Down Expand Up @@ -595,15 +593,7 @@ func Test_FilterMatcher(t *testing.T) {
t.Parallel()
expr, err := ParseLogSelector(tt.q, true)
assert.Nil(t, err)

// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
actualMatchers := expr.Matchers()
require.Len(t, actualMatchers, len(tt.expectedMatchers))
for i, expected := range tt.expectedMatchers {
assert.Equal(t, expected.String(), actualMatchers[i].String())
}

AssertMatchers(t, tt.expectedMatchers, expr.Matchers())
p, err := expr.Pipeline()
assert.Nil(t, err)
if tt.lines == nil {
Expand Down
18 changes: 2 additions & 16 deletions pkg/logql/syntax/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3239,16 +3239,7 @@ func TestParse(t *testing.T) {
t.Run(tc.in, func(t *testing.T) {
ast, err := ParseExpr(tc.in)
require.Equal(t, tc.err, err)

if tc.exp != nil {
require.NotNil(t, ast)

// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
require.Equal(t, tc.exp.String(), ast.String())
} else {
require.Nil(t, ast)
}
AssertExpressions(t, tc.exp, ast)
})
}
}
Expand Down Expand Up @@ -3298,12 +3289,7 @@ func TestParseMatchers(t *testing.T) {
if tt.want == nil {
require.Nil(t, got)
} else {
// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
require.Len(t, got, len(tt.want))
for i, expected := range tt.want {
require.Equal(t, expected.String(), got[i].String())
}
AssertMatchers(t, tt.want, got)
}
})
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/logql/syntax/serialize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ func TestJSONSerializationParseTestCases(t *testing.T) {

t.Log(buf.String())

// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
require.Equal(t, tc.exp.String(), actual.String())
AssertExpressions(t, tc.exp, actual)
})
}
}
Expand Down
75 changes: 75 additions & 0 deletions pkg/logql/syntax/test_utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package syntax

import (
"testing"

"github.com/prometheus/prometheus/model/labels"
"github.com/stretchr/testify/require"

"github.com/grafana/loki/v3/pkg/logql/log"
)

// AssertExpressions function removes FastRegexMatchers from all Regexp matchers to allow simple objects comparison.
// See removeFastRegexMatcherFromExpr function for the details.
func AssertExpressions(t *testing.T, expected, actual Expr) {
require.Equal(t, removeFastRegexMatcherFromExpr(expected), removeFastRegexMatcherFromExpr(actual))
}

// AssertMatchers function removes FastRegexMatchers from all Regexp matchers to allow simple objects comparison.
func AssertMatchers(t *testing.T, expected, actual []*labels.Matcher) {
require.Equal(t, RemoveFastRegexMatchers(expected), RemoveFastRegexMatchers(actual))
}

// RemoveFastRegexMatchers iterates over the matchers and recreates the matchers
// without *FastRegexMatcher, because Prometheus labels matcher sets a new instance each time it's created,
// and it prevents simple object assertions.
func RemoveFastRegexMatchers(matchers []*labels.Matcher) []*labels.Matcher {
result := make([]*labels.Matcher, 0, len(matchers))
for _, matcher := range matchers {
if matcher.Type == labels.MatchNotRegexp || matcher.Type == labels.MatchRegexp {
matcher = &labels.Matcher{Type: matcher.Type, Name: matcher.Name, Value: matcher.Value}
}
result = append(result, matcher)
}
return result
}

func removeFastRegexMatcherFromExpr(expr Expr) Expr {
if expr == nil {
return nil
}
expr.Walk(func(e Expr) {
switch typed := e.(type) {
case *MatchersExpr:
typed.Mts = RemoveFastRegexMatchers(typed.Mts)
case *LabelFilterExpr:
typed.LabelFilterer = removeFastRegexMatcherFromLabelFilterer(typed.LabelFilterer)
case *LogRange:
if typed.Unwrap == nil {
return
}
cleaned := make([]log.LabelFilterer, 0, len(typed.Unwrap.PostFilters))
for _, filter := range typed.Unwrap.PostFilters {
cleaned = append(cleaned, removeFastRegexMatcherFromLabelFilterer(filter))
}
typed.Unwrap.PostFilters = cleaned
default:
return
}
})
return expr
}

func removeFastRegexMatcherFromLabelFilterer(filterer log.LabelFilterer) log.LabelFilterer {
if filterer == nil {
return nil
}
switch typed := filterer.(type) {
case *log.LineFilterLabelFilter:
typed.Matcher = RemoveFastRegexMatchers([]*labels.Matcher{typed.Matcher})[0]
case *log.BinaryLabelFilter:
typed.Left = removeFastRegexMatcherFromLabelFilterer(typed.Left)
typed.Right = removeFastRegexMatcherFromLabelFilterer(typed.Left)
}
return filterer
}
19 changes: 8 additions & 11 deletions pkg/loki/runtime_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/prometheus/prometheus/model/labels"
"github.com/stretchr/testify/require"

"github.com/grafana/loki/v3/pkg/logql/syntax"
"github.com/grafana/loki/v3/pkg/runtime"
"github.com/grafana/loki/v3/pkg/validation"
)
Expand Down Expand Up @@ -48,8 +49,6 @@ overrides:
require.Equal(t, 2*30*24*time.Hour, overrides.RetentionPeriod("29")) // overrides
require.Equal(t, []validation.StreamRetention(nil), overrides.StreamRetention("1"))

// Prometheus label matchers are not comparable with a deep equal because of the internal
// fast regexp implementation. For this reason, we compare their string representation.
actual := overrides.StreamRetention("29")
expected := []validation.StreamRetention{
{Period: model.Duration(48 * time.Hour), Priority: 10, Selector: `{app="foo"}`, Matchers: []*labels.Matcher{
Expand All @@ -61,17 +60,15 @@ overrides:
}},
}

require.Len(t, actual, len(expected))
for i, expectedStreamRetention := range expected {
require.Equal(t, expectedStreamRetention.Selector, actual[i].Selector)
require.Equal(t, expectedStreamRetention.Period, actual[i].Period)
require.Equal(t, expectedStreamRetention.Priority, actual[i].Priority)
require.Equal(t, removeFastRegexMatcher(expected), removeFastRegexMatcher(actual))
}

require.Len(t, actual[i].Matchers, len(expectedStreamRetention.Matchers))
for j, expectedMatcher := range expectedStreamRetention.Matchers {
require.Equal(t, actual[i].Matchers[j].String(), expectedMatcher.String())
}
func removeFastRegexMatcher(configs []validation.StreamRetention) []validation.StreamRetention {
result := make([]validation.StreamRetention, 0, len(configs))
for _, config := range configs {
config.Matchers = syntax.RemoveFastRegexMatchers(config.Matchers)
}
return result
}

func Test_ValidateRules(t *testing.T) {
Expand Down

0 comments on commit 08c6cc0

Please sign in to comment.