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

@@ -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://") {

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) {

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
 }

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)
+			})
+		}
+	})
+}