Skip to content

fix: make gator output relative paths #2443

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
maxsmythe merged 3 commits into from
Dec 10, 2022
Merged

fix: make gator output relative paths #2443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edd3059
fix: make gator output relative paths
acpana Dec 9, 2022
1e3f41b
Merge branch 'master' into acpana/gator-relative-paths
acpana Dec 9, 2022
e3243c1
Merge branch 'master' into acpana/gator-relative-paths
acpana Dec 9, 2022
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
3 changes: 2 additions & 1 deletion cmd/gator/verify/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,11 @@ func runE(cmd *cobra.Command, args []string) error {
if strings.HasSuffix(originalPath, "/...") {
recursive = true
targetPath = strings.TrimSuffix(targetPath, "...")
originalPath = strings.TrimSuffix(originalPath, "...")
Copy link
Copy Markdown
Contributor Author

@acpana acpana Dec 9, 2022

Choose a reason for hiding this comment

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

in this file, right above lines 68-79 see how we create a fileSystem .

Since we pass that in and the absolute path, it's hard to print the results with the input path, given that we can traverse a directory.

Another way to tackle this problem, where we don't pass in the InputPath would be to push down the fileSystem/ absolute path creation to the point where we walk the directory or where we try to read a file. But I think that approach would require a more sweeping refactor.

}
targetPath = strings.Trim(targetPath, "/")

suites, err := gator.ReadSuites(fileSystem, targetPath, recursive)
suites, err := gator.ReadSuites(fileSystem, targetPath, originalPath, recursive)
if err != nil {
return fmt.Errorf("listing test files: %w", err)
}
Expand Down
27 changes: 22 additions & 5 deletions pkg/gator/read_suites.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/fs"
"path"
"sort"
"strings"

"gopkg.in/yaml.v3"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -48,7 +49,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) ([]*Suite, error) {
func ReadSuites(f fs.FS, target, originalPath string, recursive bool) ([]*Suite, error) {
if f == nil {
return nil, ErrNoFileSystem
}
Expand Down Expand Up @@ -84,11 +85,13 @@ func ReadSuites(f fs.FS, target string, recursive bool) ([]*Suite, error) {
return nil, err
}

return readSuites(f, files)
return readSuites(f, files, originalPath)
}

// readSuites reads the passed set of files into Suites on the given filesystem.
func readSuites(f fs.FS, files []string) ([]*Suite, error) {
// originalPath argument is used to construct paths relative to the original input
// path from the traversed file system walks.
func readSuites(f fs.FS, files []string, originalPath string) ([]*Suite, error) {
var suites []*Suite
for _, file := range files {
suite, err := readSuite(f, file)
Expand All @@ -97,14 +100,28 @@ func readSuites(f fs.FS, files []string) ([]*Suite, error) {
}

if suite != nil {
suite.Path = file
suite.AbsolutePath = file

// trim any prefixes like "/" or "./" in order for the
// .Cut call below to actually work with the absolute path
// contained in the file var.
cutPath := strings.TrimPrefix(originalPath, "/")
cutPath = strings.TrimPrefix(cutPath, "./")

_, after, found := strings.Cut(file, cutPath)
if !found {
return nil, fmt.Errorf("could not find %s in %s", cutPath, file)
}

suite.InputPath = originalPath + after

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[i].AbsolutePath < suites[j].AbsolutePath
})

return suites, nil
Expand Down
82 changes: 56 additions & 26 deletions pkg/gator/read_suites_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ func TestReadSuites(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
target string
recursive bool
fileSystem fs.FS
want []*Suite
wantErr error
name string
target string
originalPath string
recursive bool
fileSystem fs.FS
want []*Suite
wantErr error
}{
{
name: "no filesystem",
Expand Down Expand Up @@ -70,7 +71,23 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: []*Suite{{Path: "test.yaml"}},
want: []*Suite{{AbsolutePath: "test.yaml"}},
wantErr: nil,
},
{
name: "single target relative path",
target: "test.yaml",
originalPath: "./test.yaml",
recursive: false,
fileSystem: fstest.MapFS{
"test.yaml": &fstest.MapFile{
Data: []byte(`
kind: Suite
apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: []*Suite{{AbsolutePath: "test.yaml", InputPath: "./test.yaml"}},
wantErr: nil,
},
{
Expand Down Expand Up @@ -145,7 +162,7 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: []*Suite{{Path: "tests/test.yaml"}},
want: []*Suite{{AbsolutePath: "tests/test.yaml"}},
wantErr: nil,
},
{
Expand All @@ -163,7 +180,7 @@ apiVersion: test.gatekeeper.sh/v1alpha1
Data: []byte(`some data`),
},
},
want: []*Suite{{Path: "tests/test.yaml"}},
want: []*Suite{{AbsolutePath: "tests/test.yaml"}},
wantErr: nil,
},
{
Expand All @@ -190,13 +207,14 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: []*Suite{{Path: "tests/test.yaml"}},
want: []*Suite{{AbsolutePath: "tests/test.yaml"}},
wantErr: nil,
},
{
name: "recursive directory with subdirectory",
target: "tests",
recursive: true,
name: "recursive directory with subdirectory",
target: "tests",
originalPath: "./tests",
recursive: true,
fileSystem: fstest.MapFS{
"tests/annotations/test.yaml": &fstest.MapFile{
Data: []byte(`
Expand All @@ -218,9 +236,9 @@ apiVersion: test.gatekeeper.sh/v1alpha1
},
},
want: []*Suite{
{Path: "tests/annotations/test.yaml"},
{Path: "tests/labels/test.yaml"},
{Path: "tests/test.yaml"},
{AbsolutePath: "tests/annotations/test.yaml", InputPath: "./tests/annotations/test.yaml"},
{AbsolutePath: "tests/labels/test.yaml", InputPath: "./tests/labels/test.yaml"},
{AbsolutePath: "tests/test.yaml", InputPath: "./tests/test.yaml"},
},
wantErr: nil,
},
Expand All @@ -236,7 +254,7 @@ apiVersion: test.gatekeeper.sh/v1alpha1
`),
},
},
want: []*Suite{{Path: "tests/labels.yaml/test.yaml"}},
want: []*Suite{{AbsolutePath: "tests/labels.yaml/test.yaml"}},
wantErr: nil,
},
{
Expand Down Expand Up @@ -276,7 +294,7 @@ tests:
},
},
want: []*Suite{{
Path: "test.yaml",
AbsolutePath: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -316,7 +334,7 @@ tests:
},
},
want: []*Suite{{
Path: "test.yaml",
AbsolutePath: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -357,7 +375,7 @@ tests:
},
},
want: []*Suite{{
Path: "test.yaml",
AbsolutePath: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -398,8 +416,8 @@ tests:
},
},
want: []*Suite{{
Path: "test.yaml",
Skip: true,
AbsolutePath: "test.yaml",
Skip: true,
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -439,7 +457,7 @@ tests:
},
},
want: []*Suite{{
Path: "test.yaml",
AbsolutePath: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -474,7 +492,7 @@ tests:
},
},
want: []*Suite{{
Path: "test.yaml",
AbsolutePath: "test.yaml",
Tests: []Test{{
Template: "template.yaml",
Constraint: "constraint.yaml",
Expand Down Expand Up @@ -506,13 +524,25 @@ tests: {}
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

got, gotErr := ReadSuites(tc.fileSystem, tc.target, tc.recursive)
var (
got []*Suite
gotErr error
ignore cmp.Option
)
// for tests that don't mimic a relative path test mode
if tc.originalPath == "" {
got, gotErr = ReadSuites(tc.fileSystem, tc.target, tc.target, tc.recursive)
ignore = cmpopts.IgnoreFields(Suite{}, "InputPath")
} else {
got, gotErr = ReadSuites(tc.fileSystem, tc.target, tc.originalPath, tc.recursive)
}

if !errors.Is(gotErr, tc.wantErr) {
t.Fatalf("got error %v, want error %v",
gotErr, tc.wantErr)
}

if diff := cmp.Diff(tc.want, got, cmpopts.EquateEmpty(), cmpopts.IgnoreUnexported(Assertion{})); diff != "" {
if diff := cmp.Diff(tc.want, got, cmpopts.EquateEmpty(), cmpopts.IgnoreUnexported(Assertion{}), ignore); diff != "" {
t.Error(diff)
}
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/gator/result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ func (d Duration) String() string {

// SuiteResult is the Result of running a Suite of tests.
type SuiteResult struct {
// Path is the absolute path to the file which defines Suite.
// Path is the path to the file which defines Suite.
// This may be absolute or relative.
Path string

// Error is the error which stopped the Suite from executing.
Expand Down
6 changes: 3 additions & 3 deletions pkg/gator/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,15 @@ func (r *Runner) Run(ctx context.Context, filter Filter, s *Suite) SuiteResult {

if s.Skip {
return SuiteResult{
Path: s.Path,
Path: s.InputPath,
Skipped: true,
}
}

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

return SuiteResult{
Path: s.Path,
Path: s.InputPath,
Error: err,
Runtime: Duration(time.Since(start)),
TestResults: results,
Expand Down
7 changes: 5 additions & 2 deletions pkg/gator/suite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ type Suite struct {
// each.
Tests []Test `json:"tests"`

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

// InputPath is the filepath passed by clients using this Suite.
InputPath string `json:"-"`

// Skip, if true, skips this Suite.
Skip bool `json:"skip"`
Expand Down