From d4bbb2be3bc57f6193315b709471f1c78fe55514 Mon Sep 17 00:00:00 2001 From: Lucas Hinderberger Date: Thu, 6 Jun 2024 17:29:23 +0200 Subject: [PATCH] rewriting old path_utils based on new path syntax With the new, simplified path syntax, we can replace the old path_utils with a simpler, more efficient new version. --- pkg/lib/util/file.go | 11 +-- pkg/lib/util/file_test.go | 127 --------------------------------- pkg/lib/util/path_util.go | 82 ++++++++++++--------- pkg/lib/util/path_util_test.go | 83 +++++++++++++++++++++ 4 files changed, 133 insertions(+), 170 deletions(-) create mode 100644 pkg/lib/util/path_util_test.go 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..cc7e469f 100644 --- a/pkg/lib/util/path_util.go +++ b/pkg/lib/util/path_util.go @@ -3,55 +3,67 @@ 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 - } +var pathSpecRegex *regexp.Regexp = regexp.MustCompile(`^([0-9]*)@([^"]+)$`) - return IsParallelPathSpec(pathSpec) -} +// 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 IsParallelPathSpec(pathSpec string) bool { - n, _ := GetParallelPathSpec(pathSpec) - return n > 0 + // Path matches the literal path, e.g. foo.json in "@foo.json" + Path string } -func GetParallelPathSpec(pathSpec string) (parallelRepititions int, parsedPath string) { - regex := *regexp.MustCompile(`^\"{0,1}p(\d+)@(.+)\"{0,1}$`) - res := regex.FindAllStringSubmatch(pathSpec, -1) +// 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) { + // Remove outer quotes, if present + if len(s) >= 2 && s[0] == '"' { + if s[len(s)-1] != '"' { + // path spec must have matching quotes, if quotes are present + return PathSpec{}, false + } - if len(res) != 1 { - return 0, "" + s = s[1 : len(s)-1] } - if len(res[0]) != 3 { - return 0, "" + + // Parse + matches := pathSpecRegex.FindStringSubmatch(s) + if matches == nil { + return PathSpec{}, false + } + + 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 +} + +// IsPathSpec is a wrapper around ParsePathSpec that discards the parsed PathSpec. +// It's useful for chaining within boolean expressions. +func IsPathSpec(s string) bool { + _, ok := ParsePathSpec(s) + return ok } diff --git a/pkg/lib/util/path_util_test.go b/pkg/lib/util/path_util_test.go new file mode 100644 index 00000000..ee0bce79 --- /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) + }) + } + }) +}