assert: use type constraints (generics) #255
Conversation
| @@ -197,7 +199,7 @@ func NilError(t TestingT, err error, msgAndArgs ...interface{}) { | |||
| // called from the goroutine running the test function, not from other | |||
| // goroutines created during the test. Use Check with cmp.Equal from other | |||
| // goroutines. | |||
| func Equal(t TestingT, x, y interface{}, msgAndArgs ...interface{}) { | |||
| func Equal[ANY any](t TestingT, x, y ANY, msgAndArgs ...any) { | |||
There was a problem hiding this comment.
I wonder if this one could/should be the comparable constraint?
There was a problem hiding this comment.
Good question! I did attempt to use comparable here while working on this. It works for some types, but a few tests failed. Currently Equal works with pointers and interfaces like error. Changing to comparable caused the build to fail because error is not comparable. So I had to use any.
| @@ -100,13 +100,15 @@ import ( | |||
|
|
|||
| // BoolOrComparison can be a bool, cmp.Comparison, or error. See Assert for | |||
| // details about how this type is used. | |||
| type BoolOrComparison interface{} | |||
| type BoolOrComparison interface { | |||
| bool | func() (bool, string) | ~func() cmp.Result | |||
There was a problem hiding this comment.
This might need to include error. I believe it was previously possible to use assert.Assert(t, err).
|
I wonder if this should be a |
|
That's the big question for sure. My hope is that it can be backwards compatible for 1.18+. The function signatures have changed, but they aren't methods on a type. I believe the only way this could break existing code is if the functions are assigned to a variable with an explicit type of Are there other cases where these changes might break existing code? |
|
I suppose if the tests are currently passing, then the types should already match. Although - what about go-cmp transforms? |
|
The |
|
I think the transform options allow different types to compare equal |
|
Ahh, if I understand correctly you mean when the args passed to |
|
I used that feature once to ignore differences between numeric types, but that was for numbers nestled within a polymorphic structure - but never for two args at the top level |
|
I'm still not sure if we need a v4 for this. I wrote up #257 with some other changes we might want to make for a v4 release. I think if we decide to increment the major version we should try to include other breaking changes at the same time. cc @vdemeester as well, to get your thoughts |
This was always the intent. With generics we can express the intent in a way the compiler understands.
dnephin
force-pushed
the
assert-generic-args
branch
from
cee4049
to
d39110e
Compare
I would tend to agree that it could be a good time to do a v4 and include other breaking changes there 👼🏼. So, I would go the v4 ways at least 👼🏼 |
Closes #249
Once merged, this change would require the use of go1.18+ to import
gotest.tools/v3.This PR adds type constraints to the functions in
assertandassert/cmp. By using type constraints the compiler (and also any IDE or editor that uses gopls) can warn the user if they are using different types as the arguments to comparison functions.Also use a type constraint for
BoolOrComparison. This type was always intended to be a union type, and now it's possible to express that.