diff --git a/src/testing/benchmark.go b/src/testing/benchmark.go index 3293ccd0..e7d98033 100644 --- a/src/testing/benchmark.go +++ b/src/testing/benchmark.go @@ -365,7 +365,7 @@ func runBenchmarks(matchString func(pat, str string) (bool, error), benchmarks [ return true } ctx := &benchContext{ - match: newMatcher(matchString, *matchBenchmarks, "-test.bench"), + match: newMatcher(matchString, *matchBenchmarks, "-test.bench", ""), } var bs []InternalBenchmark for _, Benchmark := range benchmarks { diff --git a/src/testing/match.go b/src/testing/match.go index 2ba8c5fe..1762b265 100644 --- a/src/testing/match.go +++ b/src/testing/match.go @@ -14,41 +14,73 @@ import ( // matcher sanitizes, uniques, and filters names of subtests and subbenchmarks. type matcher struct { - filter []string + filter filterMatch + skip filterMatch matchFunc func(pat, str string) (bool, error) - mu sync.Mutex - subNames map[string]int64 + mu sync.Mutex + + // subNames is used to deduplicate subtest names. + // Each key is the subtest name joined to the deduplicated name of the parent test. + // Each value is the count of the number of occurrences of the given subtest name + // already seen. + subNames map[string]int32 } +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 { +func allMatcher() *matcher { + return newMatcher(nil, "", "", "") +} + +func newMatcher(matchString func(pat, str string) (bool, error), patterns, name, skips string) *matcher { if isBaremetal { - // Probably not enough ram to load regexp, substitute something simpler. matchString = fakeMatchString } - var filter []string - if patterns != "" { + var filter, skip filterMatch + if patterns == "" { + filter = simpleMatch{} // always partial true + } else { filter = splitRegexp(patterns) - for i, s := range filter { - filter[i] = rewrite(s) + if err := filter.verify(name, matchString); err != nil { + 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) - } + } + if skips == "" { + skip = alternationMatch{} // always false + } else { + skip = splitRegexp(skips) + if err := skip.verify("-test.skip", matchString); err != nil { + fmt.Fprintf(os.Stderr, "testing: invalid regexp for %v\n", err) + os.Exit(1) } } return &matcher{ filter: filter, + skip: skip, matchFunc: matchString, - subNames: map[string]int64{}, + subNames: map[string]int32{}, } } @@ -65,22 +97,83 @@ func (m *matcher) fullName(c *common, subname string) (name string, ok, partial matchMutex.Lock() defer matchMutex.Unlock() - // We check the full array of paths each time to allow for the case that - // a pattern contains a '/'. + // 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) { - break - } - if ok, _ := m.matchFunc(m.filter[i], s); !ok { - return name, false, false - } + + // filter must match. + // accept partial match that may produce full match later. + ok, partial = m.filter.matches(elem, m.matchFunc) + if !ok { + return name, false, false } - return name, true, len(elem) < len(m.filter) + + // skip must not match. + // ignore partial match so we can get to more precise match later. + skip, partialSkip := m.skip.matches(elem, m.matchFunc) + if skip && !partialSkip { + return name, false, false + } + + return name, ok, partial } -func splitRegexp(s string) []string { - a := make([]string, 0, strings.Count(s, "/")) +// clearSubNames clears the matcher's internal state, potentially freeing +// memory. After this is called, T.Name may return the same strings as it did +// for earlier subtests. +func (m *matcher) clearSubNames() { + m.mu.Lock() + defer m.mu.Unlock() + for key := range m.subNames { + delete(m.subNames, key) + } +} + +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, _ := 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); { @@ -108,33 +201,90 @@ 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 // with one or more counts, if necessary. func (m *matcher) unique(parent, subname string) string { - name := fmt.Sprintf("%s/%s", parent, subname) - empty := subname == "" - for { - next, exists := m.subNames[name] - if !empty && !exists { - m.subNames[name] = 1 // next count is 1 - return name - } - // Name was already used. We increment with the count and append a - // string with the count. - m.subNames[name] = next + 1 + base := parent + "/" + subname - // Add a count to guarantee uniqueness. - name = fmt.Sprintf("%s#%02d", name, next) - empty = false + for { + n := m.subNames[base] + if n < 0 { + panic("subtest count overflow") + } + m.subNames[base] = n + 1 + + if n == 0 && subname != "" { + prefix, nn := parseSubtestNumber(base) + if len(prefix) < len(base) && nn < m.subNames[prefix] { + // This test is explicitly named like "parent/subname#NN", + // and #NN was already used for the NNth occurrence of "parent/subname". + // Loop to add a disambiguating suffix. + continue + } + return base + } + + name := fmt.Sprintf("%s#%02d", base, n) + if m.subNames[name] != 0 { + // This is the nth occurrence of base, but the name "parent/subname#NN" + // collides with the first occurrence of a subtest *explicitly* named + // "parent/subname#NN". Try the next number. + continue + } + + return name } } +// parseSubtestNumber splits a subtest name into a "#%02d"-formatted int32 +// suffix (if present), and a prefix preceding that suffix (always). +func parseSubtestNumber(s string) (prefix string, nn int32) { + i := strings.LastIndex(s, "#") + if i < 0 { + return s, 0 + } + + prefix, suffix := s[:i], s[i+1:] + if len(suffix) < 2 || (len(suffix) > 2 && suffix[0] == '0') { + // Even if suffix is numeric, it is not a possible output of a "%02" format + // string: it has either too few digits or too many leading zeroes. + return s, 0 + } + if suffix == "00" { + if !strings.HasSuffix(prefix, "/") { + // We only use "#00" as a suffix for subtests named with the empty + // string — it isn't a valid suffix if the subtest name is non-empty. + return s, 0 + } + } + + n, err := strconv.ParseInt(suffix, 10, 32) + if err != nil || n < 0 { + return s, 0 + } + return prefix, int32(n) +} + // rewrite rewrites a subname to having only printable characters and no white // space. func rewrite(s string) string { @@ -171,14 +321,3 @@ func isSpace(r rune) bool { } return false } - -// A fake regexp matcher. -// Inflexible, but saves 50KB of flash and 50KB of RAM per -size full, -// and lets tests pass on cortex-m. -func fakeMatchString(pat, str string) (bool, error) { - if pat == ".*" { - return true, nil - } - matched := strings.Contains(str, pat) - return matched, nil -} diff --git a/src/testing/match_test.go b/src/testing/match_test.go index 8c09dc66..9ef5ceb3 100644 --- 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) @@ -86,50 +89,75 @@ func TestSplitRegexp(t *T) { func TestMatcher(t *T) { testCases := []struct { pattern string + skip string parent, sub string ok bool partial bool }{ // Behavior without subtests. - {"", "", "TestFoo", true, false}, - {"TestFoo", "", "TestFoo", true, false}, - {"TestFoo/", "", "TestFoo", true, true}, - {"TestFoo/bar/baz", "", "TestFoo", true, true}, - {"TestFoo", "", "TestBar", false, false}, - {"TestFoo/", "", "TestBar", false, false}, - {"TestFoo/bar/baz", "", "TestBar/bar/baz", false, false}, + {"", "", "", "TestFoo", true, false}, + {"TestFoo", "", "", "TestFoo", true, false}, + {"TestFoo/", "", "", "TestFoo", true, true}, + {"TestFoo/bar/baz", "", "", "TestFoo", true, true}, + {"TestFoo", "", "", "TestBar", false, false}, + {"TestFoo/", "", "", "TestBar", false, false}, + {"TestFoo/bar/baz", "", "", "TestBar/bar/baz", false, false}, + {"", "TestBar", "", "TestFoo", true, false}, + {"", "TestBar", "", "TestBar", false, false}, + + // Skipping a non-existent test doesn't change anything. + {"", "TestFoo/skipped", "", "TestFoo", true, false}, + {"TestFoo", "TestFoo/skipped", "", "TestFoo", true, false}, + {"TestFoo/", "TestFoo/skipped", "", "TestFoo", true, true}, + {"TestFoo/bar/baz", "TestFoo/skipped", "", "TestFoo", true, true}, + {"TestFoo", "TestFoo/skipped", "", "TestBar", false, false}, + {"TestFoo/", "TestFoo/skipped", "", "TestBar", false, false}, + {"TestFoo/bar/baz", "TestFoo/skipped", "", "TestBar/bar/baz", false, false}, // with subtests - {"", "TestFoo", "x", true, false}, - {"TestFoo", "TestFoo", "x", true, false}, - {"TestFoo/", "TestFoo", "x", true, false}, - {"TestFoo/bar/baz", "TestFoo", "bar", true, true}, + {"", "", "TestFoo", "x", true, false}, + {"TestFoo", "", "TestFoo", "x", true, false}, + {"TestFoo/", "", "TestFoo", "x", true, false}, + {"TestFoo/bar/baz", "", "TestFoo", "bar", true, true}, + + {"", "TestFoo/skipped", "TestFoo", "x", true, false}, + {"TestFoo", "TestFoo/skipped", "TestFoo", "x", true, false}, + {"TestFoo", "TestFoo/skipped", "TestFoo", "skipped", false, false}, + {"TestFoo/", "TestFoo/skipped", "TestFoo", "x", true, false}, + {"TestFoo/bar/baz", "TestFoo/skipped", "TestFoo", "bar", true, true}, + // Subtest with a '/' in its name still allows for copy and pasted names // to match. - {"TestFoo/bar/baz", "TestFoo", "bar/baz", true, false}, - {"TestFoo/bar/baz", "TestFoo/bar", "baz", true, false}, - {"TestFoo/bar/baz", "TestFoo", "x", false, false}, - {"TestFoo", "TestBar", "x", false, false}, - {"TestFoo/", "TestBar", "x", false, false}, - {"TestFoo/bar/baz", "TestBar", "x/bar/baz", false, false}, + {"TestFoo/bar/baz", "", "TestFoo", "bar/baz", true, false}, + {"TestFoo/bar/baz", "TestFoo/bar/baz", "TestFoo", "bar/baz", false, false}, + {"TestFoo/bar/baz", "TestFoo/bar/baz/skip", "TestFoo", "bar/baz", true, false}, + {"TestFoo/bar/baz", "", "TestFoo/bar", "baz", true, false}, + {"TestFoo/bar/baz", "", "TestFoo", "x", false, false}, + {"TestFoo", "", "TestBar", "x", false, false}, + {"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}, - {"./", "TestFoo", "x", true, false}, - {"./.", "TestFoo", "x", true, false}, - {"/bar/baz", "TestFoo", "bar", true, true}, - {"/bar/baz", "TestFoo", "bar/baz", true, false}, - {"//baz", "TestFoo", "bar/baz", true, false}, - {"//", "TestFoo", "bar/baz", true, false}, - {"/bar/baz", "TestFoo/bar", "baz", true, false}, - {"//foo", "TestFoo", "bar/baz", false, false}, - {"/bar/baz", "TestFoo", "x", false, false}, - {"/bar/baz", "TestBar", "x/bar/baz", false, false}, + {"", "", "TestFoo", "x", true, false}, + {"/", "", "TestFoo", "x", true, false}, + {"./", "", "TestFoo", "x", true, false}, + {"./.", "", "TestFoo", "x", true, false}, + {"/bar/baz", "", "TestFoo", "bar", true, true}, + {"/bar/baz", "", "TestFoo", "bar/baz", true, false}, + {"//baz", "", "TestFoo", "bar/baz", true, false}, + {"//", "", "TestFoo", "bar/baz", true, false}, + {"/bar/baz", "", "TestFoo/bar", "baz", true, false}, + {"//foo", "", "TestFoo", "bar/baz", false, false}, + {"/bar/baz", "", "TestFoo", "x", false, false}, + {"/bar/baz", "", "TestBar", "x/bar/baz", false, false}, } for _, tc := range testCases { - m := newMatcher(regexp.MatchString, tc.pattern, "-test.run") + m := newMatcher(regexp.MatchString, tc.pattern, "-test.run", tc.skip) parent := &common{name: tc.parent} if tc.parent != "" { @@ -142,45 +170,90 @@ func TestMatcher(t *T) { } } -func TestNaming(t *T) { - m := newMatcher(regexp.MatchString, "", "") +var namingTestCases = []struct{ name, want string }{ + // Uniqueness + {"", "x/#00"}, + {"", "x/#01"}, + {"#0", "x/#0"}, // Doesn't conflict with #00 because the number of digits differs. + {"#00", "x/#00#01"}, // Conflicts with implicit #00 (used above), so add a suffix. + {"#", "x/#"}, + {"#", "x/##01"}, + {"t", "x/t"}, + {"t", "x/t#01"}, + {"t", "x/t#02"}, + {"t#00", "x/t#00"}, // Explicit "#00" doesn't conflict with the unsuffixed first subtest. + + {"a#01", "x/a#01"}, // user has subtest with this name. + {"a", "x/a"}, // doesn't conflict with this name. + {"a", "x/a#02"}, // This string is claimed now, so resume + {"a", "x/a#03"}, // with counting. + {"a#02", "x/a#02#01"}, // We already used a#02 once, so add a suffix. + + {"b#00", "x/b#00"}, + {"b", "x/b"}, // Implicit 0 doesn't conflict with explicit "#00". + {"b", "x/b#01"}, + {"b#9223372036854775807", "x/b#9223372036854775807"}, // MaxInt64 + {"b", "x/b#02"}, + {"b", "x/b#03"}, + + // Sanitizing + {"A:1 B:2", "x/A:1_B:2"}, + {"s\t\r\u00a0", "x/s___"}, + {"\x01", `x/\x01`}, + {"\U0010ffff", `x/\U0010ffff`}, +} + +func TestNaming(t *T) { + m := newMatcher(regexp.MatchString, "", "", "") parent := &common{name: "x", level: 1} // top-level test. - // Rig the matcher with some preloaded values. - m.subNames["x/b"] = 1000 - - testCases := []struct { - name, want string - }{ - // Uniqueness - {"", "x/#00"}, - {"", "x/#01"}, - - {"t", "x/t"}, - {"t", "x/t#01"}, - {"t", "x/t#02"}, - - {"a#01", "x/a#01"}, // user has subtest with this name. - {"a", "x/a"}, // doesn't conflict with this name. - {"a", "x/a#01#01"}, // conflict, add disambiguating string. - {"a", "x/a#02"}, // This string is claimed now, so resume - {"a", "x/a#03"}, // with counting. - {"a#02", "x/a#02#01"}, - - {"b", "x/b#1000"}, // rigged, see above - {"b", "x/b#1001"}, - - // // Sanitizing - {"A:1 B:2", "x/A:1_B:2"}, - {"s\t\r\u00a0", "x/s___"}, - {"\x01", `x/\x01`}, - {"\U0010ffff", `x/\U0010ffff`}, - } - - for i, tc := range testCases { + for i, tc := range namingTestCases { if got, _, _ := m.fullName(parent, tc.name); got != tc.want { t.Errorf("%d:%s: got %q; want %q", i, tc.name, got, tc.want) } } } + +func FuzzNaming(f *F) { + for _, tc := range namingTestCases { + f.Add(tc.name) + } + parent := &common{name: "x", level: 1} + var m *matcher + var seen map[string]string + reset := func() { + m = allMatcher() + seen = make(map[string]string) + } + reset() + + f.Fuzz(func(t *T, subname string) { + if len(subname) > 10 { + // Long names attract the OOM killer. + t.Skip() + } + name := m.unique(parent.name, subname) + if !strings.Contains(name, "/"+subname) { + t.Errorf("name %q does not contain subname %q", name, subname) + } + if prev, ok := seen[name]; ok { + t.Errorf("name %q generated by both %q and %q", name, prev, subname) + } + if len(seen) > 1e6 { + // Free up memory. + reset() + } + seen[name] = subname + }) +} + +// 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, " | ")) +} diff --git a/src/testing/testing.go b/src/testing/testing.go index 26cd53eb..6d7d6184 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -499,7 +499,7 @@ func (m *M) Run() (code int) { func runTests(matchString func(pat, str string) (bool, error), tests []InternalTest) (ran, ok bool) { ok = true - ctx := newTestContext(newMatcher(matchString, flagRunRegexp, "-test.run")) + ctx := newTestContext(newMatcher(matchString, flagRunRegexp, "-test.run", "")) t := &T{ common: common{ output: &logger{logToStdout: flagVerbose}, @@ -567,3 +567,14 @@ func MainStart(deps interface{}, tests []InternalTest, benchmarks []InternalBenc deps: deps.(testDeps), } } + +// A fake regexp matcher. +// Inflexible, but saves 50KB of flash and 50KB of RAM per -size full, +// and lets tests pass on cortex-m. +func fakeMatchString(pat, str string) (bool, error) { + if pat == ".*" { + return true, nil + } + matched := strings.Contains(str, pat) + return matched, nil +}