Subtest with parenthesis in name fail to rerun

[noBlubb]

Hey all,

we observed that gotestsum with --rerun-fails would sometimes not report (deterministically) failing subtests. Looking into it, the subtests were detected as to be repeated but not picked up. This seems to cause gotestsum to assume no fail => success and mark the repeated attempt as a success:

> gotestsum --format standard-verbose --rerun-fails --packages ./ourpackagesgoeshere -- ./ourpackagesgoeshere -run TestWithSubs

=== RUN   TestWithSubs
=== RUN   TestWithSubs/should_pass
--- PASS: TestWithSubs/should_pass (0.00s)
=== RUN   TestWithSubs/should_(fail)
--- FAIL: TestWithSubs/should_(fail) (0.00s)
--- FAIL: TestWithSubs (0.00s)
FAIL
FAIL    ourpackagesgoeshere 0.270s

DONE 3 tests, 2 failures in 2.578s

=== RUN   TestWithSubs
--- PASS: TestWithSubs (0.00s)
testing: warning: no tests to run
PASS
ok      ourpackagesgoeshere 0.265s [no tests to run]

=== Failed
=== FAIL: ourpackagesgoeshere TestWithSubs/should_(fail) (0.00s)

=== FAIL: ourpackagesgoeshere TestWithSubs (0.00s)

DONE 2 runs, 4 tests, 2 failures in 3.673s

Looking at the exit code we get:

> echo $?
0

Which is unexpected, as the TestWithSubs looks like this:

func TestWithSubs(t *testing.T) {
    tests := []struct {
        name string
        fail bool
    }{
        {
            name: "should pass",
            fail: false,
        },
        {
            name: "should (fail)",
            fail: true,
        },
    }
    for _, test := range tests {
        test := test
        t.Run(test.name, func(t *testing.T) {
            if test.fail {
                t.Fail()
            }
        })
    }
}

Which will causes should (fail) to always fail and therefore should report a non-zero exit code.

Trying to reproduce the issue I first named the failing case should fail but gotestsum handled this as expected, reran the case and finally quit with a non-zero exit code:

=== RUN   TestWithSubs
=== RUN   TestWithSubs/should_pass
--- PASS: TestWithSubs/should_pass (0.00s)
=== RUN   TestWithSubs/should_fail
--- FAIL: TestWithSubs/should_fail (0.00s)
--- FAIL: TestWithSubs (0.00s)
FAIL
FAIL    ourpackagesgoeshere 0.321s

DONE 3 tests, 2 failures in 1.669s

=== RUN   TestWithSubs
=== RUN   TestWithSubs/should_fail
--- FAIL: TestWithSubs/should_fail (0.00s)
--- FAIL: TestWithSubs (0.00s)
FAIL
FAIL    ourpackagesgoeshere 0.269s

DONE 2 runs, 5 tests, 4 failures in 2.757s

=== RUN   TestWithSubs
=== RUN   TestWithSubs/should_fail
--- FAIL: TestWithSubs/should_fail (0.00s)
--- FAIL: TestWithSubs (0.00s)
FAIL
FAIL    ourpackagesgoeshere 0.269s

=== Failed
=== FAIL: ourpackagesgoeshere TestWithSubs/should_fail (0.00s)

=== FAIL: ourpackagesgoeshere TestWithSubs (0.00s)

=== FAIL: ourpackagesgoeshere TestWithSubs/should_fail (re-run 1) (0.00s)

=== FAIL: ourpackagesgoeshere TestWithSubs (re-run 1) (0.00s)

=== FAIL: ourpackagesgoeshere TestWithSubs/should_fail (re-run 2) (0.00s)

=== FAIL: ourpackagesgoeshere TestWithSubs (re-run 2) (0.00s)

DONE 3 runs, 7 tests, 6 failures in 3.847s

With

> echo $?
1

It was only when I tried to include some other characters that I discovered that this breaks gotestsum's ability to rerun a subtest, e.g. as is the case for TestWithSubs/should_(fail). Let me know if this is intended and tests should not use those names but it does seem to work fine with go test. Maybe a good opportunity to contribute something to gotestsum from my side if I get around to it.

[noBlubb]

In my local testing #338 does fix this issue and repeat the TestWithSubs/should_(fail) as expected.

[noBlubb]

@dnephin Awesome, thank you for being so responsive! I expect our use-case is a bit special in that we parameterize subtests and put those parameters in parentheses as part of the subtest name but I can see others affected by this bug. Do you already have plans for the next release?

[dnephin]

New release: https://github.com/gotestyourself/gotestsum/releases/tag/v1.10.1