Use t.Cleanup (added in go1.14) to call Finish automatically

[prashantv]

Requested feature

Currently, any test that uses gomock often has this snippet at the top:

ctrl := gomock.NewController(t)
defer ctrl.Finish()

The caller has to remember to defer ctrl.Finish(). This could be avoided if the gomock library supported go1.14's t.Cleanup functionality.

Why the feature is needed
It reduces likelihood of the caller forgetting to call Finish, and reduces noise in tests.

Proposed solution
(Likely in a go1.14+-only file)

• Make Finish OK to call multiple times.
• Add a new constructor that takes testing.TB and calls Finish in a cleanup function

[codyoss]

I think this is a great idea. I would just want to play around and make sure it behaves well in things like subtests and TestMains.

This change can be made without adding a new constructor. An unexported inferface could be declared:

type cleanuper interface {
	Cleanup(f func())
}

Then in NewController you can just use a cast and call if the method is there:

func NewController(t TestReporter) {
        ...
	if c, ok := t.(cleanuper); ok {
		c.Cleanup(func() { fmt.Println("I cleaned up something") })
	}
        ...
}

Also, this should not need build version guards then which is always an added benefit in my mind.
This would need to be documented but then the surface of the API does not need to change.

[prashantv]

Agree, I was considering the same thing. It does have a small behaviour change in the case that the user was never calling Finish intentionally, since now Finish will be called automatically in go1.14.

That does seem like an arcane use-case, so I'm +1 on reusing the existing API (less changes, and doesn't require changes to existing test code to benefit).

[cvgw]

Do we consider using gomock without calling Controller.Finish a valid use case? I would assume no, that it is a mistake when Finish is not called.

This seems like a great change; I forget to call Finish all the time!