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:
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:
- Fix our internal mocks to implement
Helper() (not a breaking change)
- Implement
Helper() on type assert.CollectT (not a breaking change)
- Extend
assert.TestingT, require.TestingT, mock.TestingT to add Helper() (limited breaking change)
- Replace the calls to
Helper()
Cc: @fefe982 @boyan-soubachov @brackendawson
Since Go 1.9 The
testing.T/testing.B/testing.Fand more generally thetesting.TBinterface all have anHelper()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:This works fine, but:
testing.TB) that supportsHelper(), so the guardIt would be easier if we could just do:
However our current definition of
assert.TestingTis:I propose to add the
Helper()method toassert.TestingT.This can be considered a breaking change of the API contract. However:
testing.TB.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 upgradetestify.assert.Testing.Tin user code may not implementHelper(). I think this is reasonable to force them to be fixed.testing.TBhas been extended to includeHelper()Steps required to accomplish this:
Helper()(not a breaking change)Helper()on typeassert.CollectT(not a breaking change)assert.TestingT,require.TestingT,mock.TestingTto addHelper()(limited breaking change)Helper()Cc: @fefe982 @boyan-soubachov @brackendawson