diff --git a/pkg/lib/util/file.go b/pkg/lib/util/file.go index d760e7b4..efe99cf3 100644 --- a/pkg/lib/util/file.go +++ b/pkg/lib/util/file.go @@ -17,14 +17,9 @@ var c = &http.Client{ // OpenFileOrUrl opens either a local file or gives the resp.Body from a remote file func OpenFileOrUrl(path, rootDir string) (string, io.ReadCloser, error) { - if strings.HasPrefix(path, "@") { - path = path[1:] - } else if strings.HasPrefix(path, "p@") { - // p@ -> parallel tests - path = path[2:] - } else if IsParallelPathSpec(path) { - // pN@ -> N parallel repetitions of tests - _, path = GetParallelPathSpec(path) + pathSpec, ok := ParsePathSpec(path) + if ok { + path = pathSpec.Path } if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") { diff --git a/pkg/lib/util/file_test.go b/pkg/lib/util/file_test.go index 2bb96001..21f4f333 100644 --- a/pkg/lib/util/file_test.go +++ b/pkg/lib/util/file_test.go @@ -27,133 +27,6 @@ type testOpenFileStruct struct { expHash [16]byte } -func TestGetParallelPathSpec(t *testing.T) { - - tests := []testParallelPathSpecStruct{ - { - pathSpec: "\"", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "[]", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "{}", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "p", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "@", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "1@", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "x@", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "p@", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "@path", - expIsPath: true, - expIsParallel: false, - }, - { - pathSpec: "1@a", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "x@a", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "p1@", - expIsPath: false, - expIsParallel: false, - }, - { - pathSpec: "p1@path", - expIsPath: true, - expIsParallel: true, - expPath: "path", - expParallelRepititions: 1, - }, - { - pathSpec: "p10@path", - expIsPath: true, - expIsParallel: true, - expPath: "path", - expParallelRepititions: 10, - }, - { - pathSpec: "p01@path", - expIsPath: true, - expIsParallel: true, - expPath: "path", - expParallelRepititions: 1, - }, - { - pathSpec: "@path", - expIsPath: true, - expIsParallel: false, - }, - { - pathSpec: "@../path", - expIsPath: true, - expIsParallel: false, - }, - } - - for _, v := range tests { - t.Run(fmt.Sprintf("pathSpec:'%s'", v.pathSpec), func(t *testing.T) { - isPathSpec := IsPathSpec(v.pathSpec) - isParallelPathSpec := IsParallelPathSpec(v.pathSpec) - if isPathSpec != v.expIsPath { - t.Errorf("IsPathSpec: Got %v != %v Exp", isPathSpec, v.expIsPath) - } - if isParallelPathSpec != v.expIsParallel { - t.Errorf("IsParallelPathSpec: Got %v != %v Exp", isParallelPathSpec, v.expIsParallel) - } - - if v.expIsPath { - // the path must also be recognized as a path in case it has trailing quotes - if !IsPathSpec(fmt.Sprintf("\"%s\"", v.pathSpec)) { - t.Errorf("IsPathSpec (with trailing \"): Got %v != %v Exp", isPathSpec, v.expIsPath) - } - } - - if v.expIsParallel { - parallelRepititions, path := GetParallelPathSpec(v.pathSpec) - if parallelRepititions != v.expParallelRepititions { - t.Errorf("ParallelRepititions: Got '%d' != '%d' Exp", parallelRepititions, v.expParallelRepititions) - } - if path != v.expPath { - t.Errorf("Path: Got '%s' != '%s' Exp", path, v.expPath) - } - } - }) - } -} - func TestOpenFileOrUrl(t *testing.T) { filesystem.Fs = afero.NewMemMapFs() ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/lib/util/path_util.go b/pkg/lib/util/path_util.go index 02031598..3c94fd94 100644 --- a/pkg/lib/util/path_util.go +++ b/pkg/lib/util/path_util.go @@ -3,55 +3,51 @@ package util import ( "regexp" "strconv" - "strings" ) /* throughout this file we assume 'manifestDir' to be an absolute path */ -func IsPathSpec(pathSpec string) bool { - if len(pathSpec) < 3 { - return false - } - if strings.HasPrefix(pathSpec, "@") { - return true - } - if strings.HasPrefix(pathSpec, "p@") { - return true - } - // pathSpec could have trailing quotes - if strings.HasPrefix(pathSpec, "\"@") { - return true - } - if strings.HasPrefix(pathSpec, "\"p@") { - return true - } +// TODO: Add unit tests for this file - return IsParallelPathSpec(pathSpec) -} +var pathSpecRegex *regexp.Regexp = regexp.MustCompile(`^"?([0-9]*)@([^"]+)"?$`) -func IsParallelPathSpec(pathSpec string) bool { - n, _ := GetParallelPathSpec(pathSpec) - return n > 0 -} +// PathSpec is a path specifier for including tests within manifests. +type PathSpec struct { + // Repetitions matches the number of repetitions specified + // in a path spec like "5@foo.json" + Repetitions int -func GetParallelPathSpec(pathSpec string) (parallelRepititions int, parsedPath string) { - regex := *regexp.MustCompile(`^\"{0,1}p(\d+)@(.+)\"{0,1}$`) - res := regex.FindAllStringSubmatch(pathSpec, -1) + // Path matches the literal path, e.g. foo.json in "@foo.json" + Path string +} - if len(res) != 1 { - return 0, "" +// ParsePathSpec tries to parse the given string into a PathSpec. +// +// It returns a boolean result that indicates if parsing was successful +// (i.e. if s is a valid path specifier). +func ParsePathSpec(s string) (PathSpec, bool) { + matches := pathSpecRegex.FindStringSubmatch(s) + if matches == nil { + return PathSpec{}, false } - if len(res[0]) != 3 { - return 0, "" + + spec := PathSpec{ + Repetitions: 1, + Path: matches[2], // can't be empty, or else it wouldn't match the regex } - parsedPath = res[0][2] - parallelRepititions, err := strconv.Atoi(res[0][1]) - if err != nil { - return 0, parsedPath + // Determine number of repetitions, if supplied + if matches[1] != "" { + reps, err := strconv.Atoi(matches[1]) + if err != nil { + // matches[1] is all digits, so there must be something seriously wrong + panic("error Atoi-ing all-decimal regex match") + } + + spec.Repetitions = reps } - return parallelRepititions, parsedPath + return spec, true } diff --git a/pkg/lib/util/path_util_test.go b/pkg/lib/util/path_util_test.go new file mode 100644 index 00000000..f6185308 --- /dev/null +++ b/pkg/lib/util/path_util_test.go @@ -0,0 +1,83 @@ +package util + +import( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParsePathSpec(t *testing.T) { + t.Run("happy path", func(t *testing.T) { + testCases := []struct{ + s string + expected PathSpec + }{ + { + s: "@foo.json", + expected: PathSpec{ + Repetitions: 1, + Path: "foo.json", + }, + }, + { + s: "5@bar.json", + expected: PathSpec{ + Repetitions: 5, + Path: "bar.json", + }, + }, + { + s: "123@baz.json", + expected: PathSpec{ + Repetitions: 123, + Path: "baz.json", + }, + }, + } + + for i := range testCases { + testCase := testCases[i] + + t.Run(testCase.s, func(t *testing.T) { + actual, ok := ParsePathSpec(testCase.s) + require.True(t, ok) + require.Equal(t, testCase.expected, actual) + }) + + t.Run(testCase.s + " quoted", func(t *testing.T) { + s := `"` + testCase.s + `"` + + actual, ok := ParsePathSpec(s) + require.True(t, ok) + require.Equal(t, testCase.expected, actual) + }) + } + }) + + t.Run("invalid path specs are detected", func(t *testing.T) { + testCases := []string{ + "", // empty + `"@foo.json`, `@foo.json"`, // superfluous quotes + `foo@bar.baz`, `1.23@foo.json`, // non-digit repetitions + `p@old.syntax`, `p5@old.syntax`, `p123@old.syntax`, // old syntax + } + + for _, testCase := range testCases { + s := testCase + + t.Run(s, func(t *testing.T) { + actual, ok := ParsePathSpec(s) + require.False(t, ok) + require.Zero(t, actual) + }) + + t.Run(s + " quoted", func(t *testing.T) { + sq := `"` + s + `"` + + actual, ok := ParsePathSpec(sq) + require.False(t, ok) + require.Zero(t, actual) + }) + } + }) +}