Repository: gotestyourself/gotestsum
Pull request #155 (2020-10-22 03:08:41)

testjson: create TestName
These small methods on a `type TestName string` make the `IsSubTest` conditional a lot more explicit.


Patch:
diff --git a/cmd/rerunfails.go b/cmd/rerunfails.go
index 5ec7b0e..74378a8 100644
--- a/cmd/rerunfails.go
+++ b/cmd/rerunfails.go
@@ -123,12 +123,12 @@ func (r *failureRecorder) count() int {
 	return len(r.failures)
 }
 
-func goTestRunFlagForTestCase(name string) string {
-	root, sub := testjson.SplitTestName(name)
-	if sub == "" {
-		return "-test.run=^" + name + "$"
+func goTestRunFlagForTestCase(test testjson.TestName) string {
+	if test.IsSubTest() {
+		root, sub := test.Split()
+		return "-test.run=^" + root + "$/^" + sub + "$"
 	}
-	return "-test.run=^" + root + "$/^" + sub + "$"
+	return "-test.run=^" + test.Name() + "$"
 }
 
 func writeRerunFailsReport(opts *options, exec *testjson.Execution) error {
@@ -144,7 +144,7 @@ func writeRerunFailsReport(opts *options, exec *testjson.Execution) error {
 	names := []string{}
 	results := map[string]testCaseCounts{}
 	for _, failure := range exec.Failed() {
-		name := failure.Package + "." + failure.Test
+		name := failure.Package + "." + failure.Test.Name()
 		if _, ok := results[name]; ok {
 			continue
 		}
diff --git a/cmd/tool/slowest/ast.go b/cmd/tool/slowest/ast.go
index c4bcca5..9c43707 100644
--- a/cmd/tool/slowest/ast.go
+++ b/cmd/tool/slowest/ast.go
@@ -124,22 +124,18 @@ func testNamesByPkgName(tcs []testjson.TestCase) ([]string, map[string]set) {
 	var pkgs []string
 	index := make(map[string]set)
 	for _, tc := range tcs {
-		if isSubTest(tc.Test) {
+		if tc.Test.IsSubTest() {
 			continue
 		}
 		if len(index[tc.Package]) == 0 {
 			pkgs = append(pkgs, tc.Package)
 			index[tc.Package] = make(map[string]struct{})
 		}
-		index[tc.Package][tc.Test] = struct{}{}
+		index[tc.Package][tc.Test.Name()] = struct{}{}
 	}
 	return pkgs, index
 }
 
-func isSubTest(name string) bool {
-	return strings.Contains(name, "/")
-}
-
 func errPkgLoad(pkg *packages.Package) error {
 	buf := new(strings.Builder)
 	for _, err := range pkg.Errors {
diff --git a/cmd/tool/slowest/slowest.go b/cmd/tool/slowest/slowest.go
index 5068fac..8a0fa88 100644
--- a/cmd/tool/slowest/slowest.go
+++ b/cmd/tool/slowest/slowest.go
@@ -168,7 +168,7 @@ func aggregateTestCases(cases []testjson.TestCase) []testjson.TestCase {
 	}
 	pkg := cases[0].Package
 	// nolint: prealloc // size is not predictable
-	m := make(map[string][]time.Duration)
+	m := make(map[testjson.TestName][]time.Duration)
 	for _, tc := range cases {
 		m[tc.Test] = append(m[tc.Test], tc.Elapsed)
 	}
diff --git a/internal/junitxml/report.go b/internal/junitxml/report.go
index a0368bc..1b50e81 100644
--- a/internal/junitxml/report.go
+++ b/internal/junitxml/report.go
@@ -180,7 +180,7 @@ func packageTestCases(pkg *testjson.Package, formatClassname FormatFunc) []JUnit
 func newJUnitTestCase(tc testjson.TestCase, formatClassname FormatFunc) JUnitTestCase {
 	return JUnitTestCase{
 		Classname: formatClassname(tc.Package),
-		Name:      tc.Test,
+		Name:      tc.Test.Name(),
 		Time:      formatDurationAsSeconds(tc.Elapsed),
 	}
 }


Test patch:
diff --git a/cmd/rerunfails_test.go b/cmd/rerunfails_test.go
index 1b8e410..ff21af9 100644
--- a/cmd/rerunfails_test.go
+++ b/cmd/rerunfails_test.go
@@ -42,7 +42,7 @@ func TestGoTestRunFlagFromTestCases(t *testing.T) {
 		expected string
 	}
 	fn := func(t *testing.T, tc testCase) {
-		actual := goTestRunFlagForTestCase(tc.input)
+		actual := goTestRunFlagForTestCase(testjson.TestName(tc.input))
 		assert.Equal(t, actual, tc.expected)
 	}
 
diff --git a/cmd/tool/slowest/slowest_test.go b/cmd/tool/slowest/slowest_test.go
index 2ef86ce..a412a9a 100644
--- a/cmd/tool/slowest/slowest_test.go
+++ b/cmd/tool/slowest/slowest_test.go
@@ -40,7 +40,7 @@ func TestAggregateTestCases(t *testing.T) {
 	}
 	assert.DeepEqual(t, actual, expected,
 		cmpopts.SortSlices(func(x, y testjson.TestCase) bool {
-			return strings.Compare(x.Test, y.Test) == -1
+			return strings.Compare(x.Test.Name(), y.Test.Name()) == -1
 		}),
 		cmpopts.IgnoreUnexported(testjson.TestCase{}))
 }
diff --git a/testjson/execution.go b/testjson/execution.go
index 5ce1989..f7ba22e 100644
--- a/testjson/execution.go
+++ b/testjson/execution.go
@@ -121,7 +121,7 @@ func (p *Package) TestCases() []TestCase {
 // provides a little more safety if that ever changes.
 func (p *Package) LastFailedByName(name string) TestCase {
 	for i := len(p.Failed) - 1; i >= 0; i-- {
-		if p.Failed[i].Test == name {
+		if p.Failed[i].Test.Name() == name {
 			return p.Failed[i]
 		}
 	}
@@ -143,13 +143,12 @@ func (p *Package) Output(id int) string {
 // then all output for every subtest under the root test is returned.
 // See https://github.com/golang/go/issues/29755.
 func (p *Package) OutputLines(tc TestCase) []string {
-	_, sub := SplitTestName(tc.Test)
 	lines := p.output[tc.ID]
 
 	// If this is a subtest, or a root test case with subtest failures the
 	// subtest failure output should contain the relevant lines, so we don't need
 	// extra lines.
-	if sub != "" || tc.hasSubTestFailed {
+	if tc.Test.IsSubTest() || tc.hasSubTestFailed {
 		return lines
 	}
 
@@ -166,15 +165,26 @@ func (p *Package) addOutput(id int, output string) {
 	p.output[id] = append(p.output[id], output)
 }
 
-// SplitTestName into root test name and any subtest names.
-func SplitTestName(name string) (root, sub string) {
-	parts := strings.SplitN(name, "/", 2)
+type TestName string
+
+func (n TestName) Split() (root string, sub string) {
+	parts := strings.SplitN(string(n), "/", 2)
 	if len(parts) < 2 {
-		return name, ""
+		return string(n), ""
 	}
 	return parts[0], parts[1]
 }
 
+// IsSubTest returns true if the name indicates the test is a subtest run using
+// t.Run().
+func (n TestName) IsSubTest() bool {
+	return strings.Contains(string(n), "/")
+}
+
+func (n TestName) Name() string {
+	return string(n)
+}
+
 func (p *Package) removeOutput(id int) {
 	delete(p.output, id)
 
@@ -216,7 +226,7 @@ func (p *Package) end() []TestEvent {
 		result = append(result, TestEvent{
 			Action:  ActionFail,
 			Package: tc.Package,
-			Test:    tc.Test,
+			Test:    tc.Test.Name(),
 			Elapsed: float64(neverFinished),
 		})
 		delete(p.running, k)
@@ -232,7 +242,7 @@ type TestCase struct {
 	// test case.
 	ID      int
 	Package string
-	Test    string
+	Test    TestName
 	Elapsed time.Duration
 	// RunID from the ScanConfig which produced this test case.
 	RunID int
@@ -287,8 +297,7 @@ func (e *Execution) addPackageEvent(pkg *Package, event TestEvent) {
 }
 
 func (p *Package) addTestEvent(event TestEvent) {
-	tc := p.running[event.Test]
-	root, subTest := SplitTestName(event.Test)
+	root, _ := TestName(event.Test).Split()
 
 	switch event.Action {
 	case ActionRun:
@@ -297,24 +306,27 @@ func (p *Package) addTestEvent(event TestEvent) {
 		p.Total++
 		tc := TestCase{
 			Package: event.Package,
-			Test:    event.Test,
+			Test:    TestName(event.Test),
 			ID:      p.Total,
 			RunID:   event.RunID,
 		}
 		p.running[event.Test] = tc
 
-		if subTest != "" {
+		if tc.Test.IsSubTest() {
 			rootID := p.running[root].ID
 			p.subTests[rootID] = append(p.subTests[rootID], tc.ID)
 		}
 		return
 	case ActionOutput, ActionBench:
+		tc := p.running[event.Test]
 		p.addOutput(tc.ID, event.Output)
 		return
 	case ActionPause, ActionCont:
 		return
 	}
 
+	// the event.Action must be one of the three test end events
+	tc := p.running[event.Test]
 	delete(p.running, event.Test)
 	tc.Elapsed = elapsedDuration(event.Elapsed)
 
@@ -323,7 +335,7 @@ func (p *Package) addTestEvent(event TestEvent) {
 		p.Failed = append(p.Failed, tc)
 
 		// If this is a subtest, mark the root test as having a failed subtest
-		if subTest != "" {
+		if tc.Test.IsSubTest() {
 			rootTestCase := p.running[root]
 			rootTestCase.hasSubTestFailed = true
 			p.running[root] = rootTestCase
@@ -337,7 +349,7 @@ func (p *Package) addTestEvent(event TestEvent) {
 		// Do not immediately remove output for subtests, to work around a bug
 		// in 'go test' where output is attributed to the wrong sub test.
 		// github.com/golang/go/issues/29755.
-		if subTest != "" {
+		if tc.Test.IsSubTest() {
 			return
 		}
 
diff --git a/testjson/summary.go b/testjson/summary.go
index 0424b6e..9a1ce89 100644
--- a/testjson/summary.go
+++ b/testjson/summary.go
@@ -178,7 +178,7 @@ func writeTestCaseSummary(out io.Writer, execution executionSummary, conf testCa
 			formatRunID(tc.RunID),
 			FormatDurationAsSeconds(tc.Elapsed, 2))
 		for _, line := range execution.OutputLines(tc) {
-			if isFramingLine(line) || conf.filter(tc.Test, line) {
+			if isFramingLine(line) || conf.filter(tc.Test.Name(), line) {
 				continue
 			}
 			fmt.Fprint(out, line)