Skip to content

Commit

Permalink
Make file traversal deterministic (#1723)
Browse files Browse the repository at this point in the history 
* Make file traversal deterministic

Signed-off-by: Will Beason <willbeason@google.com>

* Remove redundant path parameter

Since it is part of suite, we don't need to pass the Suite's path as a
separate parameter.

Signed-off-by: Will Beason <willbeason@google.com>
  • Loading branch information
Will Beason (he/him) committed Dec 9, 2021
1 parent 2362f7a commit bdbbdfb
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 31 deletions.
6 changes: 3 additions & 3 deletions cmd/gator/test/test.go
View file
Expand Up @@ -101,7 +101,7 @@ func runE(cmd *cobra.Command, args []string) error {
return runSuites(cmd.Context(), fileSystem, suites, filter)
}

func runSuites(ctx context.Context, fileSystem fs.FS, suites map[string]*gktest.Suite, filter gktest.Filter) error {
func runSuites(ctx context.Context, fileSystem fs.FS, suites []*gktest.Suite, filter gktest.Filter) error {
isFailure := false

runner, err := gktest.NewRunner(fileSystem, gktest.NewOPAClient)
Expand All @@ -112,8 +112,8 @@ func runSuites(ctx context.Context, fileSystem fs.FS, suites map[string]*gktest.
results := make([]gktest.SuiteResult, len(suites))
i := 0

for path, suite := range suites {
suiteResult := runner.Run(ctx, filter, path, suite)
for _, suite := range suites {
suiteResult := runner.Run(ctx, filter, suite)
for _, testResult := range suiteResult.TestResults {
if testResult.Error != nil {
isFailure = true
Expand Down
16 changes: 12 additions & 4 deletions pkg/gktest/read_suites.go
View file
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/fs"
"path"
"sort"

"gopkg.in/yaml.v3"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -47,7 +48,7 @@ const (
// Returns an error if:
// - path is a file that does not define a Suite
// - any matched files containing Suites are not parseable.
func ReadSuites(f fs.FS, target string, recursive bool) (map[string]*Suite, error) {
func ReadSuites(f fs.FS, target string, recursive bool) ([]*Suite, error) {
if f == nil {
return nil, ErrNoFileSystem
}
Expand Down Expand Up @@ -87,18 +88,25 @@ func ReadSuites(f fs.FS, target string, recursive bool) (map[string]*Suite, erro
}

// readSuites reads the passed set of files into Suites on the given filesystem.
func readSuites(f fs.FS, files []string) (map[string]*Suite, error) {
suites := make(map[string]*Suite)
func readSuites(f fs.FS, files []string) ([]*Suite, error) {
var suites []*Suite
for _, file := range files {
suite, err := readSuite(f, file)
if err != nil {
return nil, err
}

if suite != nil {
suites[file] = suite
suite.Path = file
suites = append(suites, suite)
}
}

// Ensure Suites are returned in a deterministic order.
sort.Slice(suites, func(i, j int) bool {
return suites[i].Path < suites[j].Path
})

return suites, nil
}

Expand Down
37 changes: 20 additions & 17 deletions pkg/gktest/read_suites_test.go
View file
Expand Up @@ -19,7 +19,7 @@ func TestReadSuites(t *testing.T) {
target string
recursive bool
fileSystem fs.FS
want map[string]*Suite
want []*Suite
wantErr error
}{
{
Expand Down Expand Up @@ -70,7 +70,7 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: map[string]*Suite{"test.yaml": {}},
want: []*Suite{{Path: "test.yaml"}},
wantErr: nil,
},
{
Expand Down Expand Up @@ -145,7 +145,7 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: map[string]*Suite{"tests/test.yaml": {}},
want: []*Suite{{Path: "tests/test.yaml"}},
wantErr: nil,
},
{
Expand All @@ -163,21 +163,21 @@ apiVersion: test.gatekeeper.sh/v1alpha1
Data: []byte(`some data`),
},
},
want: map[string]*Suite{"tests/test.yaml": {}},
want: []*Suite{{Path: "tests/test.yaml"}},
wantErr: nil,
},
{
name: "non-recursive directory with subdirectory",
target: "tests",
recursive: false,
fileSystem: fstest.MapFS{
"tests/labels/test.yaml": &fstest.MapFile{
"tests/annotations/test.yaml": &fstest.MapFile{
Data: []byte(`
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
"tests/annotations/test.yaml": &fstest.MapFile{
"tests/labels/test.yaml": &fstest.MapFile{
Data: []byte(`
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
Expand All @@ -190,21 +190,21 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: map[string]*Suite{"tests/test.yaml": {}},
want: []*Suite{{Path: "tests/test.yaml"}},
wantErr: nil,
},
{
name: "recursive directory with subdirectory",
target: "tests",
recursive: true,
fileSystem: fstest.MapFS{
"tests/labels/test.yaml": &fstest.MapFile{
"tests/annotations/test.yaml": &fstest.MapFile{
Data: []byte(`
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
"tests/annotations/test.yaml": &fstest.MapFile{
"tests/labels/test.yaml": &fstest.MapFile{
Data: []byte(`
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
Expand All @@ -217,10 +217,10 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: map[string]*Suite{
"tests/labels/test.yaml": {},
"tests/annotations/test.yaml": {},
"tests/test.yaml": {},
want: []*Suite{
{Path: "tests/annotations/test.yaml"},
{Path: "tests/labels/test.yaml"},
{Path: "tests/test.yaml"},
},
wantErr: nil,
},
Expand All @@ -236,7 +236,7 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: map[string]*Suite{"tests/labels.yaml/test.yaml": {}},
want: []*Suite{{Path: "tests/labels.yaml/test.yaml"}},
wantErr: nil,
},
{
Expand Down Expand Up @@ -275,7 +275,8 @@ tests:
`),
},
},
want: map[string]*Suite{"test.yaml": {
want: []*Suite{{
Path: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -314,7 +315,8 @@ tests:
`),
},
},
want: map[string]*Suite{"test.yaml": {
want: []*Suite{{
Path: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -348,7 +350,8 @@ tests:
`),
},
},
want: map[string]*Suite{"test.yaml": {
want: []*Suite{{
Path: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down
6 changes: 3 additions & 3 deletions pkg/gktest/runner.go
View file
Expand Up @@ -41,13 +41,13 @@ func NewRunner(filesystem fs.FS, newClient func() (Client, error)) (*Runner, err
}

// Run executes all Tests in the Suite and returns the results.
func (r *Runner) Run(ctx context.Context, filter Filter, suitePath string, s *Suite) SuiteResult {
func (r *Runner) Run(ctx context.Context, filter Filter, s *Suite) SuiteResult {
start := time.Now()

results, err := r.runTests(ctx, filter, suitePath, s.Tests)
results, err := r.runTests(ctx, filter, s.Path, s.Tests)

return SuiteResult{
Path: suitePath,
Path: s.Path,
Error: err,
Runtime: Duration(time.Since(start)),
TestResults: results,
Expand Down
2 changes: 1 addition & 1 deletion pkg/gktest/runner_integer_test.go
View file
Expand Up @@ -291,7 +291,7 @@ func TestRunner_Run_Integer(t *testing.T) {
}},
}

result := runner.Run(ctx, &nilFilter{}, "", suite)
result := runner.Run(ctx, &nilFilter{}, suite)
if !result.IsFailure() {
return
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/gktest/runner_test.go
View file
Expand Up @@ -980,7 +980,7 @@ func TestRunner_Run(t *testing.T) {
t.Fatal(err)
}

got := runner.Run(ctx, filter, "", &tc.suite)
got := runner.Run(ctx, filter, &tc.suite)

if diff := cmp.Diff(tc.want, got, cmpopts.EquateErrors(), cmpopts.EquateEmpty(),
cmpopts.IgnoreFields(SuiteResult{}, "Runtime"), cmpopts.IgnoreFields(TestResult{}, "Runtime"), cmpopts.IgnoreFields(CaseResult{}, "Runtime"),
Expand Down Expand Up @@ -1010,7 +1010,7 @@ func TestRunner_Run_ClientError(t *testing.T) {
suite := &Suite{
Tests: []Test{{}},
}
got := runner.Run(ctx, &nilFilter{}, "", suite)
got := runner.Run(ctx, &nilFilter{}, suite)

if diff := cmp.Diff(want, got, cmpopts.EquateErrors(), cmpopts.EquateEmpty(),
cmpopts.IgnoreFields(SuiteResult{}, "Runtime"), cmpopts.IgnoreFields(TestResult{}, "Runtime"), cmpopts.IgnoreFields(CaseResult{}, "Runtime"),
Expand Down Expand Up @@ -1349,7 +1349,7 @@ func TestRunner_RunCase(t *testing.T) {
t.Fatal(err)
}

got := runner.Run(ctx, &nilFilter{}, "", suite)
got := runner.Run(ctx, &nilFilter{}, suite)

want := SuiteResult{
TestResults: []TestResult{{
Expand Down
3 changes: 3 additions & 0 deletions pkg/gktest/suite.go
View file
Expand Up @@ -11,6 +11,9 @@ type Suite struct {
// Tests is a list of Template&Constraint pairs, with tests to run on
// each.
Tests []Test `json:"tests"`

// Path is the filepath of this Suite on disk.
Path string `json:"-"`
}

// Test defines a Template&Constraint pair to instantiate, and Cases to
Expand Down

0 comments on commit bdbbdfb

Please sign in to comment.