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>

diff --git a/src/testing/match.go b/src/testing/match.go
index b18c6e7f389ac64bfa10056ac958f69c50003eb9..d97e415765682e1a4b6114e7cf7fd447d875b1e4 100644 (file)
--- a/src/testing/match.go
+++ b/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)
-               for i, s := range filter {
-                       filter[i] = rewrite(s)
-               }
-               // 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)
-                       }
+               impl = splitRegexp(patterns)
+               if err := impl.verify(name, matchString); err != nil {
+                       fmt.Fprintf(os.Stderr, "testing: invalid regexp for %s\n", 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 {
-               if i >= len(m.filter) {
+       ok, partial = m.filter.matches(elem, m.matchFunc)
+       return name, ok, partial
+}
+
+func (m simpleMatch) matches(name []string, matchString func(pat, str string) (bool, error)) (ok, partial bool) {
+       for i, s := range name {
+               if i >= len(m) {
                        break
                }
-               if ok, _ := m.matchFunc(m.filter[i], s); !ok {
-                       return name, false, false
+               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 name, true, len(elem) < len(m.filter)
+       return nil
 }
 
-func splitRegexp(s string) []string {
-       a := make([]string, 0, strings.Count(s, "/"))
+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

diff --git a/src/testing/match_test.go b/src/testing/match_test.go
index 8c09dc660fb1a42b164ae8977ba87b8b61b3f294..9ceadbb31d99282315f3a1f7ecc56a0174744492 100644 (file)
--- a/src/testing/match_test.go
+++ b/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 := regexp.Compile(re); err != nil {
-                                       ok = false
-                               }
+                       if err := a.verify("", regexp.MatchString); err != nil {
+                               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, " | "))
+}