accept async functions with testing.TB variant instead of Gomega#224
Merged
nunnatsa merged 1 commit intonunnatsa:mainfrom Feb 4, 2026
Merged
accept async functions with testing.TB variant instead of Gomega#224nunnatsa merged 1 commit intonunnatsa:mainfrom
nunnatsa merged 1 commit intonunnatsa:mainfrom
Conversation
pohly
force-pushed
the
assertion-wrappers
branch
from
19ae601 to
8a7cb7c
Compare
pohly
force-pushed
the
assertion-wrappers
branch
from
8a7cb7c to
191627f
Compare
Collaborator
Pull Request Test Coverage Report for Build 21637444724Details
💛 - Coveralls |
nunnatsa
reviewed
Jan 31, 2026
Comment on lines
+45
to
+74
| // implementsTB checks if the argument type implements any of the methods in testing.TB which | ||
| // can be used to report test failures. Such a type is a potential alternative to a Gomega | ||
| // parameter in some Gomega wrappers. | ||
| func implementsTB(t gotypes.Type) bool { | ||
| for _, interfaceType := range tbInterfaces { | ||
| if gotypes.Implements(t, interfaceType) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| var tbInterfaces = []*gotypes.Interface{ | ||
| tbInterface("Error", false), | ||
| } | ||
|
|
||
| func tbInterface(name string, printf bool) *gotypes.Interface { | ||
| var params []*gotypes.Var | ||
| if printf { | ||
| params = append(params, gotypes.NewVar(0, nil, "", gotypes.Typ[gotypes.String])) | ||
| } | ||
| params = append(params, gotypes.NewVar(0, nil, "", gotypes.NewSlice(gotypes.Universe.Lookup("any").Type()))) | ||
| signature := gotypes.NewSignatureType(nil, nil, nil, | ||
| gotypes.NewTuple(params...), | ||
| gotypes.NewTuple(), | ||
| true, | ||
| ) | ||
| method := gotypes.NewFunc(0, nil, name, signature) | ||
| return gotypes.NewInterfaceType([]*gotypes.Func{method}, nil) | ||
| } |
Owner
There was a problem hiding this comment.
WDYT about moving this code to the internal/typecheck pakcage?
Contributor
Author
There was a problem hiding this comment.
Okay. I used the opportunity to add the alternative error reporting functions and a unit test.
pohly
force-pushed
the
assertion-wrappers
branch
2 times, most recently
from
c1bf483 to
4d95fcc
Compare
pohly
commented
Feb 2, 2026
In Kubernetes' ktesting package the following assertion is valid:
tCtx.Eventually(func(tCtx TContext) { /* maybe fail via tCtx */ }).Should(Succeed())
That works because tCtx.Eventually wraps the function such that failures
reported via tCtx.Error/Errorf/Fatal/Fatalf are visible to the matcher.
Without this change, the "Success matcher only support a single error value, or
function with Gomega as its first parameter" warning is triggered.
Instead of building in specific support for ktesting, the approach is to check
for a first parameter which has any (not necessarily all!) of the testing.TB
methods for reporting failures. That is considered a valid alternative to
a Gomega instance.
There's now a potential false negative in the linter when passing such a
function directly to gomega.Eventually/Consistently. That'll fail at runtime
because gomega cannot call it, so such a false negative isn't too bad (fails
reliably and obviously).
The warning message doesn't get changed because it would be confusing for most
users to call out some vague alternative (vague because the linter has no
details) that most users won't be able to use.
pohly
force-pushed
the
assertion-wrappers
branch
from
4d95fcc to
fcfeb70
Compare
nunnatsa
approved these changes
Feb 4, 2026
pohly
added a commit
to pohly/kubernetes
that referenced
this pull request
Mar 9, 2026
Among other updates and features this brings nunnatsa/ginkgolinter#224 into Kubernetes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In Kubernetes' ktesting package the following assertion is valid:
That works because
tCtx.Eventuallywraps the function such that failures reported viatCtx.Error/Errorf/Fatal/Fatalfare visible to the matcher.Without this change, the "Success matcher only support a single error value, or function with Gomega as its first parameter" warning is triggered.
Instead of building in specific support for ktesting, the approach is to check for a first parameter which has any (not necessarily all!) of the
testing.TBmethods for reporting failures. That is considered a valid alternative to Gomega.There's now a potential false negative in the linter when passing such a function directly to gomega.Eventually/Consistently. That'll fail at runtime because gomega cannot call it, so such a false negative isn't too bad (fails reliably and obviously).
The warning message doesn't get changed because it would be confusing for most users to call out some vague alternative (vague because the linter has no details) that most users won't be able to use.
Fixes # (issue)
kubernetes/kubernetes#135664 (comment)
Type of change
How Has This Been Tested?
Checklist:
make goimports@nunnatsa