You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
ifh, 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 typeTestingTinterface {
Errorf(formatstring, 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)
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.
Uh oh!
There was an error while loading. Please reload this page.
Since Go 1.9 The
testing.T
/testing.B
/testing.F
and more generally thetesting.TB
interface 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.TestingT
is: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.T
in user code may not implementHelper()
. I think this is reasonable to force them to be fixed.testing.TB
has 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.TestingT
to addHelper()
(limited breaking change)Helper()
Cc: @fefe982 @boyan-soubachov @brackendawson
The text was updated successfully, but these errors were encountered: