testing: add matching of subtest

Allows passing regexps per subtest to --test.run and --test.bench

Note that the documentation explicitly states that the split regular
expressions match the correpsonding parts (path components) of
the bench/test identifier. This is intended and slightly different
from the i'th RE matching the subtest/subbench at the respective
level.  Picking this semantics allows guaranteeing that a test or
benchmark identifier as printed by go test can be passed verbatim
(possibly quoted) to, respectively, -run or -bench: subtests and
subbenches might have a '/' in their name, causing a misaligment if
their ID is passed to -run or -bench as is.
This semantics has other benefits, but this is the main motivation.

Fixes golang.go#15126

Change-Id: If72e6d3f54db1df6bc2729ac6edc7ab3c740e7c3
Reviewed-on: https://go-review.googlesource.com/19122
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

2 files changed, 179 insertions, 10 deletions

diff --git a/src/testing/match.go b/src/testing/match.go
index d0c52142ba..7751035760 100644
--- a/src/testing/match.go
+++ b/src/testing/match.go
@@ -8,29 +8,40 @@ import (
 	"fmt"
 	"os"
 	"strconv"
+	"strings"
 	"sync"
 )
 
 // matcher sanitizes, uniques, and filters names of subtests and subbenchmarks.
 type matcher struct {
-	filter    string
+	filter    []string
 	matchFunc func(pat, str string) (bool, error)
 
 	mu       sync.Mutex
 	subNames map[string]int64
 }
 
-// TODO: fix test_main to avoid race and improve caching.
+// 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), pattern, name string) *matcher {
-	// Verify filters before doing any processing.
-	if _, err := matchString(pattern, "non-empty"); err != nil {
-		fmt.Fprintf(os.Stderr, "testing: invalid regexp for %s: %s\n", name, err)
-		os.Exit(1)
+func newMatcher(matchString func(pat, str string) (bool, error), patterns, name string) *matcher {
+	var filter []string
+	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)
+			}
+		}
 	}
 	return &matcher{
-		filter:    pattern,
+		filter:    filter,
 		matchFunc: matchString,
 		subNames:  map[string]int64{},
 	}
@@ -49,14 +60,54 @@ func (m *matcher) fullName(c *common, subname string) (name string, ok bool) {
 	matchMutex.Lock()
 	defer matchMutex.Unlock()
 
-	if c != nil && c.level == 0 {
-		if matched, _ := m.matchFunc(m.filter, subname); !matched {
+	// We check the full array of paths each time to allow for the case that
+	// a pattern contains a '/'.
+	for i, s := range strings.Split(name, "/") {
+		if i >= len(m.filter) {
+			break
+		}
+		if ok, _ := m.matchFunc(m.filter[i], s); !ok {
 			return name, false
 		}
 	}
 	return name, true
 }
 
+func splitRegexp(s string) []string {
+	a := make([]string, 0, strings.Count(s, "/"))
+	cs := 0
+	cp := 0
+	for i := 0; i < len(s); {
+		switch s[i] {
+		case '[':
+			cs++
+		case ']':
+			if cs--; cs < 0 { // An unmatched ']' is legal.
+				cs = 0
+			}
+		case '(':
+			if cs == 0 {
+				cp++
+			}
+		case ')':
+			if cs == 0 {
+				cp--
+			}
+		case '\\':
+			i++
+		case '/':
+			if cs == 0 && cp == 0 {
+				a = append(a, s[:i])
+				s = s[i+1:]
+				i = 0
+				continue
+			}
+		}
+		i++
+	}
+	return append(a, s)
+}
+
 // unique creates a unique name for the given parent and subname by affixing it
 // with one ore more counts, if necessary.
 func (m *matcher) unique(parent, subname string) string {
diff --git a/src/testing/match_test.go b/src/testing/match_test.go
index 68f3e9e867..d19036c72d 100644
--- a/src/testing/match_test.go
+++ b/src/testing/match_test.go
@@ -5,6 +5,7 @@
 package testing
 
 import (
+	"reflect"
 	"regexp"
 	"unicode"
 )
@@ -23,6 +24,123 @@ func TestIsSpace(t *T) {
 	}
 }
 
+func TestSplitRegexp(t *T) {
+	res := func(s ...string) []string { return s }
+	testCases := []struct {
+		pattern string
+		result  []string
+	}{
+		// Correct patterns
+		// If a regexp pattern is correct, all split regexps need to be correct
+		// as well.
+		{"", res("")},
+		{"/", res("", "")},
+		{"//", res("", "", "")},
+		{"A", res("A")},
+		{"A/B", res("A", "B")},
+		{"A/B/", res("A", "B", "")},
+		{"/A/B/", res("", "A", "B", "")},
+		{"[A]/(B)", res("[A]", "(B)")},
+		{"[/]/[/]", res("[/]", "[/]")},
+		{"[/]/[:/]", res("[/]", "[:/]")},
+		{"/]", res("", "]")},
+		{"]/", res("]", "")},
+		{"]/[/]", res("]", "[/]")},
+		{`([)/][(])`, res(`([)/][(])`)},
+		{"[(]/[)]", res("[(]", "[)]")},
+
+		// Faulty patterns
+		// Errors in original should produce at least one faulty regexp in results.
+		{")/", res(")/")},
+		{")/(/)", res(")/(", ")")},
+		{"a[/)b", res("a[/)b")},
+		{"(/]", res("(/]")},
+		{"(/", res("(/")},
+		{"[/]/[/", res("[/]", "[/")},
+		{`\p{/}`, res(`\p{`, "}")},
+		{`\p/`, res(`\p`, "")},
+		{`[[:/:]]`, res(`[[:/:]]`)},
+	}
+	for _, tc := range testCases {
+		a := splitRegexp(tc.pattern)
+		if !reflect.DeepEqual(a, tc.result) {
+			t.Errorf("splitRegexp(%q) = %#v; want %#v", tc.pattern, a, tc.result)
+		}
+
+		// If there is any error in the pattern, one of the returned subpatterns
+		// 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 ok {
+				t.Errorf("%s: expected error in any of %q", tc.pattern, a)
+			}
+		}
+	}
+}
+
+func TestMatcher(t *T) {
+	testCases := []struct {
+		pattern     string
+		parent, sub string
+		ok          bool
+	}{
+		// Behavior without subtests.
+		{"", "", "TestFoo", true},
+		{"TestFoo", "", "TestFoo", true},
+		{"TestFoo/", "", "TestFoo", true},
+		{"TestFoo/bar/baz", "", "TestFoo", true},
+		{"TestFoo", "", "TestBar", false},
+		{"TestFoo/", "", "TestBar", false},
+		{"TestFoo/bar/baz", "", "TestBar/bar/baz", false},
+
+		// with subtests
+		{"", "TestFoo", "x", true},
+		{"TestFoo", "TestFoo", "x", true},
+		{"TestFoo/", "TestFoo", "x", true},
+		{"TestFoo/bar/baz", "TestFoo", "bar", true},
+		// Subtest with a '/' in its name still allows for copy and pasted names
+		// to match.
+		{"TestFoo/bar/baz", "TestFoo", "bar/baz", true},
+		{"TestFoo/bar/baz", "TestFoo/bar", "baz", true},
+		{"TestFoo/bar/baz", "TestFoo", "x", false},
+		{"TestFoo", "TestBar", "x", false},
+		{"TestFoo/", "TestBar", "x", false},
+		{"TestFoo/bar/baz", "TestBar", "x/bar/baz", false},
+
+		// subtests only
+		{"", "TestFoo", "x", true},
+		{"/", "TestFoo", "x", true},
+		{"./", "TestFoo", "x", true},
+		{"./.", "TestFoo", "x", true},
+		{"/bar/baz", "TestFoo", "bar", true},
+		{"/bar/baz", "TestFoo", "bar/baz", true},
+		{"//baz", "TestFoo", "bar/baz", true},
+		{"//", "TestFoo", "bar/baz", true},
+		{"/bar/baz", "TestFoo/bar", "baz", true},
+		{"//foo", "TestFoo", "bar/baz", false},
+		{"/bar/baz", "TestFoo", "x", false},
+		{"/bar/baz", "TestBar", "x/bar/baz", false},
+	}
+
+	for _, tc := range testCases {
+		m := newMatcher(regexp.MatchString, tc.pattern, "-test.run")
+
+		parent := &common{name: tc.parent}
+		if tc.parent != "" {
+			parent.level = 1
+		}
+		if n, ok := m.fullName(parent, tc.sub); ok != tc.ok {
+			t.Errorf("pattern: %q, parent: %q, sub %q: got %v; want %v",
+				tc.pattern, tc.parent, tc.sub, ok, tc.ok, n)
+		}
+	}
+}
+
 func TestNaming(t *T) {
 	m := newMatcher(regexp.MatchString, "", "")