assert: add Helper() in TestingT interface

[dolmen]

trafficstars

Since Go 1.9 The testing.T/testing.B/testing.F and more generally the testing.TB interface all have an Helper() method that must be used by test helpers to hide them in the caller stack in order to properly report test errors at the location where the user cares.

In testify MRs #554, #1026 added the call to Helper() in every testify function. Here is the code:

 if h, ok := t.(tHelper); ok {
	h.Helper()
}

This works fine, but:

• this code is verbose
• every standard use of testify is done with a testing context (testing.TB) that supports Helper(), so the guard
• the interface guard make every testify function slower than necessary

It would be easier if we could just do:

t.Helper()

However our current definition of assert.TestingT is:

// TestingT is an interface wrapper around *testing.T                                                                                                                                                
type TestingT interface {                                                                                                                                                                            
        Errorf(format string, args ...interface{})                                                                                                                                                   
}

I propose to add the Helper() method to assert.TestingT.

This can be considered a breaking change of the API contract. However:

• this will not affect any user code that calls the testify methods with values implementing testing.TB.
• there is a risk that this could affect helpers built around testify functions, however those wrappers should also call Helper() and if they didn't we can safely consider this is a good thing if they have to be fixed when the user will upgrade testify.
• some mocks of assert.Testing.T in user code may not implement Helper(). I think this is reasonable to force them to be fixed.
• this is quite similar as when interface testing.TB has been extended to include Helper()

Steps required to accomplish this:

1. Fix our internal mocks to implement Helper() (not a breaking change)
2. Implement Helper() on type assert.CollectT (not a breaking change)
3. Extend assert.TestingT, require.TestingT, mock.TestingT to add Helper() (limited breaking change)
4. Replace the calls to Helper()

Cc: @fefe982 @boyan-soubachov @brackendawson

[dolmen]

#262 is the last time we attempted extending assert.TestingT to add FailNow() and this was reverted by #263. That was 9 years ago.

[dolmen]

Cc: @jasdel because of your comment https://github.com/stretchr/testify/pull/262#issuecomment-170071174

However, project github.com/gucumber/gucumber is dead (replaced by github.com/cucumber/gherkin/go/v26

[brackendawson]

I've been thinking about this one in the shower for the past week. The advangages here are cosmetic in our implementation and extremely minor perormance gains, the reflection most assertions make will be far slower than this type assertion.

While I acknowledge that there should be no modules out there which would be broken by this change, abiding by the compatability promise is critically important for working in the Go module ecosystem to prevent broken builds by incompatible inderect dependencies. I think we should live with this legacy.

[dolmen]

@brackendawson Thanks for the review.

I agree that this will not bring significant benefits at the cost of a breaking change. So we must not do it. Closing as not planned.