gotest.tools
gotest.tools copied to clipboard
gotestyourself
assert: use type constraints (generics)
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 assert and assert/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.
I wonder if this should be a v4, or do you reckon there's no breaking changes for 1.18+?
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 func(t assert.TestingT, x any, msg...any). That seems unlikely, but it's possible.
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 options ...cmp.Options parameter to DeepEqual isn't changing. I think those should be ok, unless I'm missing something.
Ahh, if I understand correctly you mean when the args passed to DeepEqual are different types, but a Transformer changes the type. That is possible. It seems strange that someone would do that, since they can transform the arguments before passing them to DeepEqual. I wonder how common it is to use transformers in this way. I don't think I've ever used one myself. I only use ignore and compare.
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
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
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 👼🏼
