As one data point, this morning I fixed a bug where a test did t.Cleanup(func() { os.Remove(tmpFile) }), and we weren't actually closing the file properly before the remove call. On Windows, this led to the os.Remove silently failing, since you can't remove a file while it's still open. We never noticed due to the lack of error checking, which I attribute to laziness, since doing the proper error check is five lines instead of one, like the last example above.

Instead we could have written t.CleanupErr(func() error { return os.Remove(tmpFile) }) from the start, which is just three more words.

Happy to send a CL if this gets approved - the implementation is trivial. This is purely about reducing verbosity on the caller side.

Yet another example is chaining of multiple cleanup steps while checking errors. Right now, I might write:

t.Cleanup(func() {
    if err := step1(); err != nil {
        t.Error(err)
    }
    if err := step2(arg); err != nil {
        t.Error(err)
    }
})

Whereas now I could write:

t.CleanupErr(func() error { return step2(arg) })
t.CleanupErr(step1)

Devil's advocate: Isn't the obvious missing API here t.AssertNilErr? If we don't want to add that (and I think we don't), why not? Isn't this a step on the slippery slope to t.AssertNilErr?

Context: I wrote my own be.NilErr helper.

Sorry, I don't see how that's an obvious conclusion, or how it relates to this proposal.

If there were t.AssertNilErr, you would write this instead:

t.Cleanup(func() {
    t.AssertNilErr(os.Chdir(orig))
})

This feels a little bit too much like fixing one spot where writing if err != nil { t.Error(err) } is tedious so it ends up being skipped, leading to test mistakes, but there are lots of places where that is tedious to write, so a lot of people use testify etc. to get out of writing it.

Personally, I disagree - I would rather solve the issue of if err != nil generally for all Go code, like with the former try proposal, and not just for test funcs taking testing.TB. AssertNilErr feels like a band-aid with a rather narrow scope.

If general error handling were to make TB.CleanupErr entirely unnecessary, I'd happily retract this proposal. For now it's hard to say; there isn't active work around it, as far as I'm aware.

FWIW, with @dsnet's try package, you could do:

defer try.F(t.Fatal)

t.Cleanup(func() {
    try.E(os.Chdir(orig))
})

Much moreso than the rest of the standard library, the testing.T API seems to prioritize concisely expressing common situations vs. aiming for orthogonality. There are already numerous ways to make a test fail even though t.Fail is all that is strictly needed, and I assume that's to help test code be relatively concise to avoid distracting from what's actually being tested.

I agree with the assessment that most times I have cleanup to do in a test it's something fallible, like deleting some temporary files or terminating a child process, and so I would agree that this seems like a common enough situation to deserve a concise way of writing it, and that having a concise way to write it is likely to help authors write correct tests that will fail when their cleanup steps fail.

Since the concept of cleanup is relatively new the codebases I spend most time in haven't universally adopted it yet, but there is some existing use of defer statements for similar purposes and those tend to end up ignoring errors too in my experience, as test authors prioritize being concise. I think the addition of something like this proposal would motivate a more proactive retrofitting of the new API instead of defer in those codebases, therefore making the test cases more robust without a significant increase in verbosity.