Re-running of failed tests does not handle multiple levels of subtests

[brycekahle]

If you have multiple levels of nesting for subtests, the re-running of failed tests does not handle re-running properly and will end up running all the levels in-between the root and the subtest.

Example JSON output:

{"Package": "pkg", "Action": "run"}
{"Package": "pkg", "Test": "TestParent", "Action": "run"}
{"Package": "pkg", "Test": "TestParent/TestNested", "Action": "run"}
{"Package": "pkg", "Test": "TestParent/TestNested/TestOne", "Action": "run"}
{"Package": "pkg", "Test": "TestParent/TestNested/TestOne", "Action": "pass"}
{"Package": "pkg", "Test": "TestParent/TestNested/TestTwo", "Action": "run"}
{"Package": "pkg", "Test": "TestParent/TestNested/TestTwo", "Action": "fail"}
{"Package": "pkg", "Test": "TestParent/TestNested", "Action": "fail"}
{"Package": "pkg", "Test": "TestParent", "Action": "fail"}
{"Package": "pkg", "Action": "fail"}

This should end up running:

go test -json -test.run=^TestParent$/^TestNested$/^TestTwo$ pkg

but it runs:

go test -json -test.run=^TestParent$/^TestNested/TestTwo$ pkg
go test -json -test.run=^TestParent$/^TestNested$ pkg

Notice there are two errors here:

1. The regexp for the first line should be ^TestParent$/^TestNested$/^TestTwo$ (missing $ and ^ around second /)
2. It should not run TestParent/TestNested.

I do not know what is causing error number 1, but I tracked the cause for number 2 down to the hasSubTestFailed being incorrect for the in-between levels. I added a test case that fails if added to testjson/execution_test.go:

func TestScanOutput_WithNestedSubTestFailures(t *testing.T) {
	source := []byte(`{"Package": "pkg", "Action": "run"}
	{"Package": "pkg", "Test": "TestParent", "Action": "run"}
	{"Package": "pkg", "Test": "TestParent/TestNested", "Action": "run"}
	{"Package": "pkg", "Test": "TestParent/TestNested/TestOne", "Action": "run"}
	{"Package": "pkg", "Test": "TestParent/TestNested/TestOne", "Action": "pass"}
	{"Package": "pkg", "Test": "TestParent/TestNested/TestTwo", "Action": "run"}
	{"Package": "pkg", "Test": "TestParent/TestNested/TestTwo", "Action": "fail"}
	{"Package": "pkg", "Test": "TestParent/TestNested", "Action": "fail"}
	{"Package": "pkg", "Test": "TestParent", "Action": "fail"}
	{"Package": "pkg", "Action": "fail"}`)

	handler := &captureHandler{}
	cfg := ScanConfig{
		Stdout:  bytes.NewReader(source),
		Handler: handler,
	}
	exec, err := ScanTestOutput(cfg)
	assert.NilError(t, err)
	pkg := exec.Package("pkg")
	for _, tc := range pkg.TestCases() {
		switch string(tc.Test) {
		case "TestParent/TestNested", "TestParent":
			assert.Equal(t, tc.hasSubTestFailed, true, "test %s should have failing subtests", tc.Test)
		default:
			assert.Equal(t, tc.hasSubTestFailed, false, "test %s should not have failing subtests", tc.Test)
		}
	}
}

=== RUN   TestScanOutput_WithNestedSubTestFailures
    execution_test.go:262: assertion failed: false (tc.hasSubTestFailed bool) != true (true bool): test TestParent/TestNested should have failing subtests
--- FAIL: TestScanOutput_WithNestedSubTestFailures (0.00s)
FAIL

[dnephin]

Thank you for the bug report! The missing ^ and $ around extra path segments definitely sounds like a bug. That should be easy to fix by splitting on all / and adding the necessary characters. The code for that is here:

gotestsum/cmd/rerunfails.go

Line 141 in 049fc26

return "-test.run=^" + regexp.QuoteMeta(root) + "$/^" + regexp.QuoteMeta(sub) + "$"

The second issue it a bit more nuanced and more difficult to solve. There's no way to run a Go sub test without running its parent. The sub tests are defined dynamically in code, so you can't discover them without running their parent. In your example TestParent/TestNested is actually run twice (once for each call to go test). If there were 3 failing tests it would have run 4 times, etc. The problem in this case is that the extra call to go test actually re-runs all its sub tests as well. This is definitely a bug.

When I first wrote this there were limitations with the go test -run flag that required running all tests individually (golang/go#39904). I think now that golang/go@025308f is done (and has been in all Go versions since go1.18), this can be fixed to re-run all the tests in a package at once. That should remove most of the extra calls to parent tests.

I think FilterFailedUnique using hasSubTestFailed was probably a mistake. Instead of hasSubTestFailed, I think FilterFailedUnique needs to filter out any test names that are substrings of other test names. It should be able to do this by looking at the list of all the failed tests in the package. Once those test names are filtered to just the most unique set, then a regex to run all of them should be correct, and should minimize re-running of the intermediary sub tests.

[brycekahle]

I opened #342 to address the regex generation bug

[brycekahle]

I took a crack at fixing the problem in FilterFailedUnique in #343

[dnephin]

I created #347 to track a future improvement of running all the tests in a package at once, instead of each test individually. I believe this issue should be fixed now so I'm going to close it.