proposal: testing: add (*testing.T).PassNow method about go HOT 6 OPEN

I think tt.assertRegexpCompileError(t, err) needlessly increases the complexity of the test code to begin with. TBH, I had to read the code a bunch to understand it. The solution I would have done is tests := []struct { shouldFail bool and then

re, err := regexp.Compile(tt.regexp)
if tt.shouldFail {
  require.Error(t, err)
  return
}
require.NoError(t, err)
match := re.MatchString("some string")
require.True(t, match)

Using a shouldFail boolean is much simpler and easier to read than having a complicated helper closure. There are times when using a closure can clean up some gnarly testing code, but checking if err should be nil or non-nil isn't one of them.

from go.

The idea that

re, err := regexp.Compile(tt.regexp)
tt.assertRegexpCompileError(t, err)
match := re.MatchString("some string")
require.True(t, match)

might stop the test with a PASS result during assertRegexpCompileError is surprising at the least. You always have to think maybe code won't continue executing due to a panic or other kind of failure, but a success?, that's something special.

The idea that an explicit return is worse because it "increases the number of possible code paths (cyclomatic complexity)" seems backward to me. The code path exists either way. The return makes it visible. That's better.

We're very unlikely to encourage this kind of hidden successful control flow by adding t.PassNow.

If you really insisted, you could always define something like

package passnow

var errPassNow = errors.New("pass now!")

func AllowPassNow() {
    if e := recover(); e != nil && e != ErrPassNow {
        panic(e)
    }
}

func PassNow() {
    panic(errPassNow)
}

and then write the tests like

t.Run(tt.name, func(t *testing.T) {
	defer passnow.AllowPassNow()
	re, err := regexp.Compile(tt.regexp)
	tt.assertRegexpCompileError(t, err)
	match := re.MatchString("some string")
	require.True(t, match)
})

where calling passnow.PassNow() will panic out to the recover in AllowPassNow.

Then at least each test depending on this hidden control flow is explicitly opting in.

from go.

Similar Issues

• proposal: testing: verify failure (rather than skip) #28488
• proposal: testing: add conditional-error helpers to T and B #45650
• proposal: provide TestResult.Test function, analogous to BenchmarkResult.Benchmark #25889
• Proposal: testing.MustRun #22353
• proposal: testing: Collect test failure logs on the testing.common struct and expose collected logs via a new method on testing.TB #61860

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

from go.

re, err := regexp.Compile(tt.regexp)
tt.assertRegexpCompileError(t, err)
if err != nil {
	return
}

from go.

@earthboundkid I mentioned return in the description as a potential alternative already.
But as described, this always adds additional conditions, which increases the number of possible code paths (cyclomatic complexity) and with this, it basically raises the question "who tests the test code". It makes the test code needlessly harder to reason about (of course not a single if err != nil, real world cases I observed can be way more involved). Therefore I think it is advisable to keep the cyclomatic complexity of the test code it self to 1.

from go.

@rsc thanks for your proposed passnow package. I will consider this idea. I guess it boils down to unsafe versus panic.

I understand, that there is very little to no interest to explore this idea so I will save the time to try to counter the arguments.

It is sad, but I guess, I have to accept this.

from go.