Go 1.20.x undocumented change: t.Run panics when called in t.Cleanup

[Samuel Nelson]

I am a maintainer of a library that was calling `t.Run` inside of `t.Cleanup`, in order to call test functions after everything (the test functions and their corresponding setup/teardown functions) had been registered. Go 1.20.x changed that so that `t.Run` cannot be called inside of `t.Cleanup`, and it's not mentioned anywhere in the release notes or the `testing` package docs. The package in question: https://pkg.go.dev/github.com/poy/onpar/v2. Here's where we register the `Run` calls during `Cleanup`: https://github.com/poy/onpar/blob/v2.0.1/onpar.go#L70-L72

I don't particularly _mind_ the library breaking (clearly we were relying on unsupported functionality), but this feels like something that should have been mentioned - at least in the release notes, maybe also in the `testing` docs. Is this something I should bring up in a github issue or something?

-Sam

[Ian Lance Taylor]

On Thu, Mar 9, 2023 at 8:11 AM Samuel Nelson <valczir....@gmail.com> wrote:
>
> I am a maintainer of a library that was calling `t.Run` inside of `t.Cleanup`, in order to call test functions after everything (the test functions and their corresponding setup/teardown functions) had been registered. Go 1.20.x changed that so that `t.Run` cannot be called inside of `t.Cleanup`, and it's not mentioned anywhere in the release notes or the `testing` package docs. The package in question: https://pkg.go.dev/github.com/poy/onpar/v2. Here's where we register the `Run` calls during `Cleanup`: https://github.com/poy/onpar/blob/v2.0.1/onpar.go#L70-L72
>
> I don't particularly _mind_ the library breaking (clearly we were relying on unsupported functionality), but this feels like something that should have been mentioned - at least in the release notes, maybe also in the `testing` docs. Is this something I should bring up in a github issue or something?

Thanks, I saw the comment in the GitHub issue, and I've sent
https://go.dev/cl/475215 to update the release notes.

Cleanup is documented to run when all subtests complete, so it's kind
of contradictory to start a new subtest in a Cleanup. I'm not sure we
need to document it there. But I don't feel strongly about it.

Ian