Skip to content

test: use custom filter to make tests less brittle #2544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sozercan merged 5 commits into from
Mar 29, 2023
Merged

test: use custom filter to make tests less brittle #2544

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6876614
test: make tests less brittle
acpana Jan 28, 2023
1cfbd50
revert cmd/gator/test/gatortest_test.go
acpana Feb 1, 2023
63a542b
use a custom cmp.Option
acpana Feb 1, 2023
3e09926
refactor _withTrace test
acpana Mar 28, 2023
f086ea6
include cmpOption in testcase
acpana Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 34 additions & 21 deletions pkg/gator/test/test_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package test

import (
"errors"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -10,6 +9,7 @@ import (
"github.com/open-policy-agent/frameworks/constraint/pkg/types"
"github.com/open-policy-agent/gatekeeper/pkg/gator/fixtures"
"github.com/open-policy-agent/gatekeeper/pkg/target"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/yaml"
)
Expand Down Expand Up @@ -56,12 +56,27 @@ func init() {
}
}

func ignoreGatorResultFields() cmp.Option {
Copy link
Copy Markdown
Contributor Author

@acpana acpana Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main difference here between our custom option and the IgnoreFields option is that the latter relies the existence of a field; ref: https://github.com/google/go-cmp/blob/a97318bf6562f2ed2632c5f985db51b1bc5bdcd0/cmp/cmpopts/struct_filter.go#L177-L179

return cmp.FilterPath(
func(p cmp.Path) bool {
switch p.String() {
// ignore these fields
case "Result.Metadata", "Result.EvaluationMeta", "Result.EnforcementAction", "ViolatingObject":
return true
default:
return false
}
},
cmp.Ignore())
}

func TestTest(t *testing.T) {
tcs := []struct {
name string
inputs []string
want []*GatorResult
err error
name string
inputs []string
want []*GatorResult
cmpOption cmp.Option
err error
}{
{
name: "basic no violation",
Expand All @@ -70,6 +85,7 @@ func TestTest(t *testing.T) {
fixtures.ConstraintAlwaysValidate,
fixtures.Object,
},
cmpOption: ignoreGatorResultFields(),
},
{
name: "basic violation",
Expand Down Expand Up @@ -101,6 +117,7 @@ func TestTest(t *testing.T) {
},
},
},
cmpOption: ignoreGatorResultFields(),
},
{
name: "referential constraint with violation",
Expand All @@ -126,6 +143,7 @@ func TestTest(t *testing.T) {
},
},
},
cmpOption: ignoreGatorResultFields(),
},
{
name: "referential constraint without violation",
Expand Down Expand Up @@ -168,27 +186,26 @@ func TestTest(t *testing.T) {
var objs []*unstructured.Unstructured
for _, input := range tc.inputs {
u, err := readUnstructured([]byte(input))
if err != nil {
t.Fatalf("readUnstructured for input %q: %v", input, err)
}
require.NoError(t, err)
objs = append(objs, u)
}

resps, err := Test(objs, false)
if tc.err != nil {
if err == nil {
t.Errorf("got nil err, want %v", tc.err)
}
if !errors.Is(err, tc.err) {
t.Errorf("got err %q, want %q", err, tc.err)
}
require.ErrorIs(t, err, tc.err)
} else if err != nil {
t.Errorf("got err '%v', want nil", err)
require.NoError(t, err)
}

got := resps.Results()

diff := cmp.Diff(tc.want, got, cmpopts.IgnoreFields(GatorResult{}, "Metadata", "EvaluationMeta", "EnforcementAction", "ViolatingObject"))
var diff string
if tc.cmpOption != nil {
diff = cmp.Diff(tc.want, got, tc.cmpOption)
} else {
diff = cmp.Diff(tc.want, got)
}

if diff != "" {
t.Errorf("diff in GatorResult objects (-want +got):\n%s", diff)
}
Expand Down Expand Up @@ -244,12 +261,8 @@ func Test_Test_withTrace(t *testing.T) {
},
}

diff := cmp.Diff(want, got, cmpopts.IgnoreFields(
diff := cmp.Diff(want, got, ignoreGatorResultFields(), cmpopts.IgnoreFields(
GatorResult{},
"Metadata",
"EvaluationMeta",
"EnforcementAction",
"ViolatingObject",
"Trace", // ignore Trace for now, we will assert non nil further down
))
if diff != "" {
Expand Down