testing: increase alternation precedence

Updates handling of go test flags -run and -bench to give alternation
precendence over the / delimiter. Currently, `A/B|C/D` is effectively
`A/(B|C)/D` - with this change, it changes to effectively `(A/B)|(C/D)`.

Fixes #39904

Change-Id: Iebe5efd8d91c72eed6351bd63b4689b0fcb0ed0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/343883
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Than McIntosh <thanm@google.com>

src/testing/match.go

@@ -14,34 +14,45 @@ import (
 
 // matcher sanitizes, uniques, and filters names of subtests and subbenchmarks.
 type matcher struct {
-	filter    []string
+	filter    filterMatch
 	matchFunc func(pat, str string) (bool, error)
 
 	mu       sync.Mutex
 	subNames map[string]int64
 }
 
+type filterMatch interface {
+	// matches checks the name against the receiver's pattern strings using the
+	// given match function.
+	matches(name []string, matchString func(pat, str string) (bool, error)) (ok, partial bool)
+
+	// verify checks that the receiver's pattern strings are valid filters by
+	// calling the given match function.
+	verify(name string, matchString func(pat, str string) (bool, error)) error
+}
+
+// simpleMatch matches a test name if all of the pattern strings match in
+// sequence.
+type simpleMatch []string
+
+// alternationMatch matches a test name if one of the alternations match.
+type alternationMatch []filterMatch
+
 // TODO: fix test_main to avoid race and improve caching, also allowing to
 // eliminate this Mutex.
 var matchMutex sync.Mutex
 
 func newMatcher(matchString func(pat, str string) (bool, error), patterns, name string) *matcher {
-	var filter []string
+	var impl filterMatch
 	if patterns != "" {
-		filter = splitRegexp(patterns)
+		impl = splitRegexp(patterns)
-		for i, s := range filter {
+		if err := impl.verify(name, matchString); err != nil {
-			filter[i] = rewrite(s)
+			fmt.Fprintf(os.Stderr, "testing: invalid regexp for %s\n", err)
-		}
+			os.Exit(1)
-		// Verify filters before doing any processing.
-		for i, s := range filter {
-			if _, err := matchString(s, "non-empty"); err != nil {
-				fmt.Fprintf(os.Stderr, "testing: invalid regexp for element %d of %s (%q): %s\n", i, name, s, err)
-				os.Exit(1)
-			}
 		}
 	}
 	return &matcher{
-		filter:    filter,
+		filter:    impl,
 		matchFunc: matchString,
 		subNames:  map[string]int64{},
 	}
@@ -60,22 +71,63 @@ func (m *matcher) fullName(c *common, subname string) (name string, ok, partial
 	matchMutex.Lock()
 	defer matchMutex.Unlock()
 
+	if m.filter == nil {
+		return name, true, false
+	}
+
 	// We check the full array of paths each time to allow for the case that
 	// a pattern contains a '/'.
 	elem := strings.Split(name, "/")
-	for i, s := range elem {
+	ok, partial = m.filter.matches(elem, m.matchFunc)
-		if i >= len(m.filter) {
+	return name, ok, partial
-			break
-		}
-		if ok, _ := m.matchFunc(m.filter[i], s); !ok {
-			return name, false, false
-		}
-	}
-	return name, true, len(elem) < len(m.filter)
 }
 
-func splitRegexp(s string) []string {
+func (m simpleMatch) matches(name []string, matchString func(pat, str string) (bool, error)) (ok, partial bool) {
-	a := make([]string, 0, strings.Count(s, "/"))
+	for i, s := range name {
+		if i >= len(m) {
+			break
+		}
+		if ok, _ := matchString(m[i], s); !ok {
+			return false, false
+		}
+	}
+	return true, len(name) < len(m)
+}
+
+func (m simpleMatch) verify(name string, matchString func(pat, str string) (bool, error)) error {
+	for i, s := range m {
+		m[i] = rewrite(s)
+	}
+	// Verify filters before doing any processing.
+	for i, s := range m {
+		if _, err := matchString(s, "non-empty"); err != nil {
+			return fmt.Errorf("element %d of %s (%q): %s", i, name, s, err)
+		}
+	}
+	return nil
+}
+
+func (m alternationMatch) matches(name []string, matchString func(pat, str string) (bool, error)) (ok, partial bool) {
+	for _, m := range m {
+		if ok, partial = m.matches(name, matchString); ok {
+			return ok, partial
+		}
+	}
+	return false, false
+}
+
+func (m alternationMatch) verify(name string, matchString func(pat, str string) (bool, error)) error {
+	for i, m := range m {
+		if err := m.verify(name, matchString); err != nil {
+			return fmt.Errorf("alternation %d of %s", i, err)
+		}
+	}
+	return nil
+}
+
+func splitRegexp(s string) filterMatch {
+	a := make(simpleMatch, 0, strings.Count(s, "/"))
+	b := make(alternationMatch, 0, strings.Count(s, "|"))
 	cs := 0
 	cp := 0
 	for i := 0; i < len(s); {
@@ -103,10 +155,24 @@ func splitRegexp(s string) []string {
 				i = 0
 				continue
 			}
+		case '|':
+			if cs == 0 && cp == 0 {
+				a = append(a, s[:i])
+				s = s[i+1:]
+				i = 0
+				b = append(b, a)
+				a = make(simpleMatch, 0, len(a))
+				continue
+			}
 		}
 		i++
 	}
-	return append(a, s)
+
+	a = append(a, s)
+	if len(b) == 0 {
+		return a
+	}
+	return append(b, a)
 }
 
 // unique creates a unique name for the given parent and subname by affixing it

src/testing/match_test.go

@@ -5,8 +5,10 @@
 package testing
 
 import (
+	"fmt"
 	"reflect"
 	"regexp"
+	"strings"
 	"unicode"
 )
 
@@ -25,10 +27,11 @@ func TestIsSpace(t *T) {
 }
 
 func TestSplitRegexp(t *T) {
-	res := func(s ...string) []string { return s }
+	res := func(s ...string) filterMatch { return simpleMatch(s) }
+	alt := func(m ...filterMatch) filterMatch { return alternationMatch(m) }
 	testCases := []struct {
 		pattern string
-		result  []string
+		result  filterMatch
 	}{
 		// Correct patterns
 		// If a regexp pattern is correct, all split regexps need to be correct
@@ -49,6 +52,8 @@ func TestSplitRegexp(t *T) {
 		{`([)/][(])`, res(`([)/][(])`)},
 		{"[(]/[)]", res("[(]", "[)]")},
 
+		{"A/B|C/D", alt(res("A", "B"), res("C", "D"))},
+
 		// Faulty patterns
 		// Errors in original should produce at least one faulty regexp in results.
 		{")/", res(")/")},
@@ -71,10 +76,8 @@ func TestSplitRegexp(t *T) {
 		// needs to have an error as well.
 		if _, err := regexp.Compile(tc.pattern); err != nil {
 			ok := true
-			for _, re := range a {
+			if err := a.verify("", regexp.MatchString); err != nil {
-				if _, err := regexp.Compile(re); err != nil {
+				ok = false
-					ok = false
-				}
 			}
 			if ok {
 				t.Errorf("%s: expected error in any of %q", tc.pattern, a)
@@ -113,6 +116,10 @@ func TestMatcher(t *T) {
 		{"TestFoo/", "TestBar", "x", false, false},
 		{"TestFoo/bar/baz", "TestBar", "x/bar/baz", false, false},
 
+		{"A/B|C/D", "TestA", "B", true, false},
+		{"A/B|C/D", "TestC", "D", true, false},
+		{"A/B|C/D", "TestA", "C", false, false},
+
 		// subtests only
 		{"", "TestFoo", "x", true, false},
 		{"/", "TestFoo", "x", true, false},
@@ -184,3 +191,13 @@ func TestNaming(t *T) {
 		}
 	}
 }
+
+// GoString returns a string that is more readable than the default, which makes
+// it easier to read test errors.
+func (m alternationMatch) GoString() string {
+	s := make([]string, len(m))
+	for i, m := range m {
+		s[i] = fmt.Sprintf("%#v", m)
+	}
+	return fmt.Sprintf("(%s)", strings.Join(s, " | "))
+}