From 1e51f29e95cc3db752b7a9946bac52cb9870e483 Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Thu, 20 Feb 2025 18:17:31 +0530 Subject: [PATCH 1/7] IND-2304 test-and-build.yml updated with unit test coverage, linting and .go-version added --- .github/workflows/test-and-build.yml | 21 ++++++++++++--- .go-version | 1 + .golangci.yml | 5 ++++ common_test.go | 39 ++++++++++++++-------------- evaluate.go | 10 +++---- filter.go | 6 ++--- 6 files changed, 50 insertions(+), 32 deletions(-) create mode 100644 .go-version create mode 100644 .golangci.yml diff --git a/.github/workflows/test-and-build.yml b/.github/workflows/test-and-build.yml index 1317244..8b43902 100644 --- a/.github/workflows/test-and-build.yml +++ b/.github/workflows/test-and-build.yml @@ -40,13 +40,26 @@ jobs: env: TEST_RESULTS: "/tmp/test-results" steps: - - uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 - - uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5.3.0 + - name: Checkout Code + uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + - name: Setup go + uses: actions/setup-go@f111f3307d8850f501ac008e886eec1fd1932a34 # v5.3.0 with: go-version: ${{ matrix.go-version }} cache: true + - name: Run golangci-lint + uses: golangci/golangci-lint-action@08e2f20817b15149a52b5b3ebe7de50aff2ba8c5 - uses: autero1/action-gotestsum@7263b9d73912eec65f46337689e59fac865c425f # v2.0.0 with: gotestsum_version: 1.9.0 - - - run: make test + - name: Run test + run: make test + - name: Unit test coverage + run: make coverage + - name: Upload coverage report + uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 + with: + path: /tmp/coverage.out + name: Coverage-report + - name: Display coverage report + run: go tool cover -func=/tmp/coverage.out \ No newline at end of file diff --git a/.go-version b/.go-version new file mode 100644 index 0000000..d907c15 --- /dev/null +++ b/.go-version @@ -0,0 +1 @@ +1.18 \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..5338a9e --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,5 @@ +linters: + enable: + - errcheck + - staticcheck +output_format: colored-line-number \ No newline at end of file diff --git a/common_test.go b/common_test.go index 00fbc0c..497df46 100644 --- a/common_test.go +++ b/common_test.go @@ -5,7 +5,6 @@ package bexpr import ( "flag" - "reflect" ) var benchFull *bool = flag.Bool("bench-full", false, "Run all benchmarks rather than a subset") @@ -53,25 +52,25 @@ type ( ) type testFlatStructAlt struct { - Int CustomInt - Int8 CustomInt8 - Int16 CustomInt16 - Int32 CustomInt32 - Int64 CustomInt64 - Uint CustomUint - Uint8 CustomUint8 - Uint16 CustomUint16 - Uint32 CustomUint32 - Uint64 CustomUint64 - Float32 CustomFloat32 - Float64 CustomFloat64 - Bool CustomBool - String CustomString - unexported CustomString - Hidden CustomBool `bexpr:"-"` + Int CustomInt + Int8 CustomInt8 + Int16 CustomInt16 + Int32 CustomInt32 + Int64 CustomInt64 + Uint CustomUint + Uint8 CustomUint8 + Uint16 CustomUint16 + Uint32 CustomUint32 + Uint64 CustomUint64 + Float32 CustomFloat32 + Float64 CustomFloat64 + Bool CustomBool + String CustomString + //unexported CustomString + Hidden CustomBool `bexpr:"-"` } -var testFlatStructKindMap map[string]reflect.Kind = map[string]reflect.Kind{ +/*var testFlatStructKindMap map[string]reflect.Kind = map[string]reflect.Kind{ "Int": reflect.Int, "Int8": reflect.Int8, "Int16": reflect.Int16, @@ -86,11 +85,11 @@ var testFlatStructKindMap map[string]reflect.Kind = map[string]reflect.Kind{ "Float64": reflect.Float64, "Bool": reflect.Bool, "String": reflect.String, -} +}*/ type testNestedLevel2_1 struct { Foo int - bar string + //bar string Baz string } diff --git a/evaluate.go b/evaluate.go index 47165a6..332c201 100644 --- a/evaluate.go +++ b/evaluate.go @@ -71,7 +71,7 @@ func derefType(rtype reflect.Type) reflect.Type { func doMatchMatches(expression *grammar.MatchExpression, value reflect.Value) (bool, error) { if !value.Type().ConvertibleTo(byteSliceTyp) { - return false, fmt.Errorf("Value of type %s is not convertible to []byte", value.Type()) + return false, fmt.Errorf("value of type %s is not convertible to []byte", value.Type()) } var re *regexp.Regexp @@ -83,7 +83,7 @@ func doMatchMatches(expression *grammar.MatchExpression, value reflect.Value) (b var err error re, err = regexp.Compile(expression.Value.Raw) if err != nil { - return false, fmt.Errorf("Failed to compile regular expression %q: %v", expression.Value.Raw, err) + return false, fmt.Errorf("failed to compile regular expression %q: %v", expression.Value.Raw, err) } expression.Value.Converted = re } @@ -181,7 +181,7 @@ func doMatchIn(expression *grammar.MatchExpression, value reflect.Value) (bool, return strings.Contains(value.String(), matchValue.(string)), nil default: - return false, fmt.Errorf("Cannot perform in/contains operations on type %s for selector: %q", kind, expression.Selector) + return false, fmt.Errorf("cannot perform in/contains operations on type %s for selector: %q", kind, expression.Selector) } } @@ -370,7 +370,7 @@ func evaluateMatchExpression(expression *grammar.MatchExpression, datum interfac } return false, err default: - return false, fmt.Errorf("Invalid match operation: %d", expression.Operator) + return false, fmt.Errorf("invalid match operation: %d", expression.Operator) } } @@ -484,5 +484,5 @@ func evaluate(ast grammar.Expression, datum interface{}, opt ...Option) (bool, e case *grammar.CollectionExpression: return evaluateCollectionExpression(node, datum, opt...) } - return false, fmt.Errorf("Invalid AST node") + return false, fmt.Errorf("invalid AST node") } diff --git a/filter.go b/filter.go index 2d1e6ab..b003994 100644 --- a/filter.go +++ b/filter.go @@ -25,7 +25,7 @@ func CreateFilter(expression string) (*Filter, error) { } exp, err := CreateEvaluator(expression) if err != nil { - return nil, fmt.Errorf("Failed to create boolean expression evaluator: %v", err) + return nil, fmt.Errorf("failed to create boolean expression evaluator: %v", err) } return &Filter{ @@ -76,7 +76,7 @@ func (f *Filter) Execute(data interface{}) (interface{}, error) { item := rvalue.MapIndex(mapKey) if !item.CanInterface() { - return nil, fmt.Errorf("Map value cannot be used") + return nil, fmt.Errorf("map value cannot be used") } result, err := f.evaluator.Evaluate(item.Interface()) @@ -91,6 +91,6 @@ func (f *Filter) Execute(data interface{}) (interface{}, error) { return newMap.Interface(), nil default: - return nil, fmt.Errorf("Only slices, arrays and maps are filterable") + return nil, fmt.Errorf("only slices, arrays and maps are filterable") } } From c9f635d30c423f7acc063ebbd55b91c19c62c92a Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Thu, 20 Feb 2025 18:26:58 +0530 Subject: [PATCH 2/7] IND-2304 error solved --- .golangci.yml | 1 - evaluate.go | 10 +++++----- filter.go | 6 +++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5338a9e..8467b99 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,5 +1,4 @@ linters: enable: - errcheck - - staticcheck output_format: colored-line-number \ No newline at end of file diff --git a/evaluate.go b/evaluate.go index 332c201..47165a6 100644 --- a/evaluate.go +++ b/evaluate.go @@ -71,7 +71,7 @@ func derefType(rtype reflect.Type) reflect.Type { func doMatchMatches(expression *grammar.MatchExpression, value reflect.Value) (bool, error) { if !value.Type().ConvertibleTo(byteSliceTyp) { - return false, fmt.Errorf("value of type %s is not convertible to []byte", value.Type()) + return false, fmt.Errorf("Value of type %s is not convertible to []byte", value.Type()) } var re *regexp.Regexp @@ -83,7 +83,7 @@ func doMatchMatches(expression *grammar.MatchExpression, value reflect.Value) (b var err error re, err = regexp.Compile(expression.Value.Raw) if err != nil { - return false, fmt.Errorf("failed to compile regular expression %q: %v", expression.Value.Raw, err) + return false, fmt.Errorf("Failed to compile regular expression %q: %v", expression.Value.Raw, err) } expression.Value.Converted = re } @@ -181,7 +181,7 @@ func doMatchIn(expression *grammar.MatchExpression, value reflect.Value) (bool, return strings.Contains(value.String(), matchValue.(string)), nil default: - return false, fmt.Errorf("cannot perform in/contains operations on type %s for selector: %q", kind, expression.Selector) + return false, fmt.Errorf("Cannot perform in/contains operations on type %s for selector: %q", kind, expression.Selector) } } @@ -370,7 +370,7 @@ func evaluateMatchExpression(expression *grammar.MatchExpression, datum interfac } return false, err default: - return false, fmt.Errorf("invalid match operation: %d", expression.Operator) + return false, fmt.Errorf("Invalid match operation: %d", expression.Operator) } } @@ -484,5 +484,5 @@ func evaluate(ast grammar.Expression, datum interface{}, opt ...Option) (bool, e case *grammar.CollectionExpression: return evaluateCollectionExpression(node, datum, opt...) } - return false, fmt.Errorf("invalid AST node") + return false, fmt.Errorf("Invalid AST node") } diff --git a/filter.go b/filter.go index b003994..2d1e6ab 100644 --- a/filter.go +++ b/filter.go @@ -25,7 +25,7 @@ func CreateFilter(expression string) (*Filter, error) { } exp, err := CreateEvaluator(expression) if err != nil { - return nil, fmt.Errorf("failed to create boolean expression evaluator: %v", err) + return nil, fmt.Errorf("Failed to create boolean expression evaluator: %v", err) } return &Filter{ @@ -76,7 +76,7 @@ func (f *Filter) Execute(data interface{}) (interface{}, error) { item := rvalue.MapIndex(mapKey) if !item.CanInterface() { - return nil, fmt.Errorf("map value cannot be used") + return nil, fmt.Errorf("Map value cannot be used") } result, err := f.evaluator.Evaluate(item.Interface()) @@ -91,6 +91,6 @@ func (f *Filter) Execute(data interface{}) (interface{}, error) { return newMap.Interface(), nil default: - return nil, fmt.Errorf("only slices, arrays and maps are filterable") + return nil, fmt.Errorf("Only slices, arrays and maps are filterable") } } From 6332aa0790f1780ae25fca2dd6f891a9a9c01f6a Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Thu, 20 Feb 2025 18:29:22 +0530 Subject: [PATCH 3/7] IND-2304 name for coverage report upload updated --- .github/workflows/test-and-build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-and-build.yml b/.github/workflows/test-and-build.yml index 8b43902..f7c962a 100644 --- a/.github/workflows/test-and-build.yml +++ b/.github/workflows/test-and-build.yml @@ -60,6 +60,6 @@ jobs: uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 with: path: /tmp/coverage.out - name: Coverage-report + name: Coverage-report-${{ matrix.go-version }} - name: Display coverage report run: go tool cover -func=/tmp/coverage.out \ No newline at end of file From b9ca48cb12c13149231da14f9fd7f2ebf68067ce Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Fri, 21 Feb 2025 13:58:46 +0530 Subject: [PATCH 4/7] workflow updation --- .github/workflows/test-and-build.yml | 7 ++----- .go-version | 2 +- go.mod | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test-and-build.yml b/.github/workflows/test-and-build.yml index f7c962a..2f30453 100644 --- a/.github/workflows/test-and-build.yml +++ b/.github/workflows/test-and-build.yml @@ -1,11 +1,8 @@ name: Test and Build on: - pull_request: - branches: ["main"] push: - branches: ["main"] - tags: ["*"] + branches: ["build-test"] permissions: contents: read @@ -34,7 +31,7 @@ jobs: strategy: matrix: go-version: - - '1.18' # oldest supported; named in go.mod + - '1.23' # oldest supported; named in go.mod - 'oldstable' - 'stable' env: diff --git a/.go-version b/.go-version index d907c15..193d140 100644 --- a/.go-version +++ b/.go-version @@ -1 +1 @@ -1.18 \ No newline at end of file +1.23 \ No newline at end of file diff --git a/go.mod b/go.mod index fed408d..46e827a 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/hashicorp/go-bexpr -go 1.18 +go 1.23 require ( github.com/mitchellh/pointerstructure v1.2.1 From a277a272a760bb18842df60e44a617bac7936f4d Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Fri, 21 Feb 2025 14:03:47 +0530 Subject: [PATCH 5/7] IND-2304 go-version updated --- .github/workflows/test-and-build.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-and-build.yml b/.github/workflows/test-and-build.yml index 2f30453..ea6d6c1 100644 --- a/.github/workflows/test-and-build.yml +++ b/.github/workflows/test-and-build.yml @@ -1,8 +1,11 @@ name: Test and Build on: + pull_request: + branches: ["main"] push: - branches: ["build-test"] + branches: ["main"] + tags: ["*"] permissions: contents: read @@ -31,7 +34,7 @@ jobs: strategy: matrix: go-version: - - '1.23' # oldest supported; named in go.mod + - '1.23' # named in go.mod - 'oldstable' - 'stable' env: From cf1dd8c19eb97242a257174e53494d4341328ad7 Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Fri, 21 Feb 2025 18:24:21 +0530 Subject: [PATCH 6/7] IND-2304 suggested changes were made --- common_test.go | 51 ++++++++++++++++-------------------------------- evaluate_test.go | 33 +++++++++++++++++-------------- 2 files changed, 35 insertions(+), 49 deletions(-) diff --git a/common_test.go b/common_test.go index 497df46..823b67a 100644 --- a/common_test.go +++ b/common_test.go @@ -52,44 +52,27 @@ type ( ) type testFlatStructAlt struct { - Int CustomInt - Int8 CustomInt8 - Int16 CustomInt16 - Int32 CustomInt32 - Int64 CustomInt64 - Uint CustomUint - Uint8 CustomUint8 - Uint16 CustomUint16 - Uint32 CustomUint32 - Uint64 CustomUint64 - Float32 CustomFloat32 - Float64 CustomFloat64 - Bool CustomBool - String CustomString - //unexported CustomString - Hidden CustomBool `bexpr:"-"` + Int CustomInt + Int8 CustomInt8 + Int16 CustomInt16 + Int32 CustomInt32 + Int64 CustomInt64 + Uint CustomUint + Uint8 CustomUint8 + Uint16 CustomUint16 + Uint32 CustomUint32 + Uint64 CustomUint64 + Float32 CustomFloat32 + Float64 CustomFloat64 + Bool CustomBool + String CustomString + unexported CustomString + Hidden CustomBool `bexpr:"-"` } -/*var testFlatStructKindMap map[string]reflect.Kind = map[string]reflect.Kind{ - "Int": reflect.Int, - "Int8": reflect.Int8, - "Int16": reflect.Int16, - "Int32": reflect.Int32, - "Int64": reflect.Int64, - "Uint": reflect.Uint, - "Uint8": reflect.Uint8, - "Uint16": reflect.Uint16, - "Uint32": reflect.Uint32, - "Uint64": reflect.Uint64, - "Float32": reflect.Float32, - "Float64": reflect.Float64, - "Bool": reflect.Bool, - "String": reflect.String, -}*/ - type testNestedLevel2_1 struct { Foo int - //bar string + Bar string Baz string } diff --git a/evaluate_test.go b/evaluate_test.go index 138ad38..ac3f556 100644 --- a/evaluate_test.go +++ b/evaluate_test.go @@ -124,21 +124,22 @@ var evaluateTests map[string]expressionTest = map[string]expressionTest{ }, "Flat Struct Alt Types": { testFlatStructAlt{ - Int: -1, - Int8: -2, - Int16: -3, - Int32: -4, - Int64: -5, - Uint: 6, - Uint8: 7, - Uint16: 8, - Uint32: 9, - Uint64: 10, - Float32: 1.1, - Float64: 1.2, - Bool: true, - String: "exported", - Hidden: true, + Int: -1, + Int8: -2, + Int16: -3, + Int32: -4, + Int64: -5, + Uint: 6, + Uint8: 7, + Uint16: 8, + Uint32: 9, + Uint64: 10, + Float32: 1.1, + Float64: 1.2, + Bool: true, + String: "exported", + unexported: "unexported", + Hidden: true, }, []expressionCheck{ {expression: "Int == -1", result: true, benchQuick: true}, @@ -255,10 +256,12 @@ var evaluateTests map[string]expressionTest = map[string]expressionTest{ MapOfStructs: map[string]testNestedLevel2_1{ "one": { Foo: 42, + Bar: "hidden", Baz: "exported", }, "two": { Foo: 77, + Bar: "visible", Baz: "consul", }, }, From 99ad642befc4ed94b3d229f1ae6424c7cbf4d7f0 Mon Sep 17 00:00:00 2001 From: KaushikiAnand Date: Tue, 25 Feb 2025 11:44:26 +0530 Subject: [PATCH 7/7] IND-2304 suggested change made --- common_test.go | 2 +- evaluate_test.go | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/common_test.go b/common_test.go index 823b67a..58079bd 100644 --- a/common_test.go +++ b/common_test.go @@ -72,7 +72,7 @@ type testFlatStructAlt struct { type testNestedLevel2_1 struct { Foo int - Bar string + bar string Baz string } diff --git a/evaluate_test.go b/evaluate_test.go index ac3f556..1d52ab3 100644 --- a/evaluate_test.go +++ b/evaluate_test.go @@ -256,12 +256,12 @@ var evaluateTests map[string]expressionTest = map[string]expressionTest{ MapOfStructs: map[string]testNestedLevel2_1{ "one": { Foo: 42, - Bar: "hidden", + bar: "unexported", Baz: "exported", }, "two": { Foo: 77, - Bar: "visible", + bar: "unexported", Baz: "consul", }, }, @@ -303,6 +303,11 @@ var evaluateTests map[string]expressionTest = map[string]expressionTest{ {expression: "TopInt != 0", result: true}, {expression: "Nested.Map contains nope or (Nested.Map contains bar and Nested.Map.bar == `bazel`) or TopInt != 0", result: true, benchQuick: true}, {expression: "Nested.MapOfStructs.one.Foo == 42", result: true}, + {expression: "Nested.MapOfStructs.one.bar == `unexported`", result: false, err: `error finding value in datum: /Nested/MapOfStructs/one/bar at part 3: couldn't find key: struct field with name "bar"`}, + {expression: "Nested.MapOfStructs.one.Baz == `exported`", result: true}, + {expression: "Nested.MapOfStructs.two.Foo == 77", result: true}, + {expression: "Nested.MapOfStructs.two.bar == `unexported`", result: false, err: `error finding value in datum: /Nested/MapOfStructs/two/bar at part 3: couldn't find key: struct field with name "bar"`}, + {expression: "Nested.MapOfStructs.two.Baz == `consul`", result: true}, {expression: "7 in Nested.SliceOfInts", result: true}, {expression: `"/Nested/SliceOfInts" == "7"`, result: false, err: `unable to find suitable primitive comparison function for matching`}, {expression: "Nested.MapOfStructs is empty or (Nested.SliceOfInts contains 7 and 9 in Nested.SliceOfInts)", result: true, benchQuick: true},