Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using generics #1147

Open
borosr opened this issue Jan 15, 2022 · 19 comments
Open

Using generics #1147

borosr opened this issue Jan 15, 2022 · 19 comments

Comments

@borosr
Copy link

borosr commented Jan 15, 2022

Are you planning to update project to use generics and also support go 1.18?

@glesica
Copy link
Collaborator

glesica commented Jan 15, 2022

I'm not sure how much value we would get from generics. I suppose some of the assertions would benefit. But I doubt any of the maintainers is going to put much effort into it any time soon. Pull requests or further discussion are both always welcome, of course!

@brackendawson
Copy link
Collaborator

Testify already supports generics. Is there a specific test case you've been unable to write?

@jan-xyz
Copy link

jan-xyz commented Mar 18, 2022

I think the assertions would benefit from it. It reduces turn-around time if the compiler already tells you that expected and actual are different types and incompatible instead of waiting for the test result in some cases.

@borosr
Copy link
Author

borosr commented Mar 19, 2022

I agree with @jan-xyz, sometimes it would be helpful if the compiler would prevent me to do assertions between different types.

@jan-xyz
Copy link

jan-xyz commented Mar 20, 2022

@brackendawson I noticed that you down voted my reply, I'd be interested to hear your reasoning for it? Do you think it's not useful and if so why?

In the meantime, I played around with go generics today and implemented some assertions from testify how I imagine them to be.

@pellared
Copy link

I think this feature would require releasing a v2 of testify.

Side note: For sake of "more type-safe" assertions and evaluating a slightly different design, I started experimenting here fluentassert/verify#26. I would appreciate any feedback.

@brackendawson
Copy link
Collaborator

brackendawson commented Mar 22, 2022

@brackendawson I noticed that you down voted my reply, I'd be interested to hear your reasoning for it? Do you think it's not useful and if so why?

The main reason is that it would require all users of testify to be on Go 1.18 and to have >= 1.18 in their go.mod's go directive, as pellared says it would have to be a v2 release. But there are other things I'd like to see in a v2 release (like a suite package that can handle parallel subtests) that I wouldn't like to restrict to only those on 1.18, at least for now. Maybe this situation is different in a couple of years.

On the actual use of generics for some assertions where testify currently uses interface{}, is it actually better? Errors where the assertion is not the right one to use will be compile time rather than runtime (good, I think) but the message is out of testify's control and it might not be clear what the user needs to do (maybe not good).

And also on your func Equal[T comparable](t *testing.T, expected, actual T, msgAndArgs ...any), testify can currently compare some types which aren't comparable`, slices for example. Testify does this using reflect.DeepEqual or as is proposed for v2; go-cmp (#535). This change would be a regression.

@jordaniversen
Copy link

jordaniversen commented Mar 31, 2022

Sorry for my ignorance as I'm new to Go and testify, but would support for generics allow the caveat described at https://pkg.go.dev/github.com/stretchr/testify/mock#hdr-Example_Usage to be voided? That is, would it prevent us from having to do a nil check in our mock method when the return value is a pointer in order to prevent the panic?

This may cause a panic if the object you are getting is nil (the type assertion will fail), in those cases you should check for nil first.

@brackendawson
Copy link
Collaborator

brackendawson commented Mar 31, 2022

Sorry for my ignorance as I'm new to Go and testify, but would support for generics allow the caveat described at https://pkg.go.dev/github.com/stretchr/testify/mock#hdr-Example_Usage to be voided? That is, would it prevent us from having to do a nil check in our mock method when the return value is a pointer in order to prevent the panic?

@jordaniversen I think you're asking if mock.Arguments can be a generic type to avoid needing to do type assertion of values returned from Get(). That's not going to be possible because the arguments are likely to be different types. Both of these would be invalid:

Arguments[any]{"seven", 7}
[]Argument[any]{{"seven"}, {7}}

So I don't think compile time type safety can be achieved here.

#967 solves your problem though.

@jordaniversen
Copy link

Ah thanks for that @brackendawson. Have you had any update on this?

@zhulik
Copy link

zhulik commented May 30, 2022

As a compromise, is it possible to write generic 'more type-safe' wrappers around assert for example and see how it's in real use? From the fisrt sight such an API should be fully backward compatible

@brackendawson
Copy link
Collaborator

As a compromise, is it possible to write generic 'more type-safe' wrappers around assert for example and see how it's in real use? From the fisrt sight such an API should be fully backward compatible

That would be possible and would give you a safer Assert for most purposes, it would have to have a different name (SaferAssert()?) and so would any others you wrap. It would not be backwards compatible at all because modules with a go directive <1.18 would not compile, even with a 1.18 compiler.

@zhulik
Copy link

zhulik commented May 31, 2022

@brackendawson by 'backward compatibility' I meant that safer assert would be a drop-in replacement for assert when built with go 1.18. One only needs to change the import string.

@anthony-chang
Copy link
Contributor

anthony-chang commented Nov 28, 2022

I had a stab at refactoring some of the assertion APIs using generics but there are a couple of roadblocks that make me feel like this is impossible without some major changes to testify and/or Golang itself.

  1. Golang does not support generics in methods with receivers (more detail on this issue here). This makes it impossible to actually genericise the public-facing APIs in the *_forward.go files without substantially changing the testify architecture (ie. stop using receiver methods).
  2. The type inferencing in Golang is not strong enough for arrays/slices/map types for some use cases. For example, consider the assert.Contains method. If we wanted to use generics to force the arguments to be the same slice/map/string type, we might do something like this:
func Contains[K comparable, V any, T []V | map[K]V | string](t TestingT, s T, contains V, msgAndArgs ...interface{})

But now when you call Contains, the compiler is not able to infer the argument types.

arr1 := []int{1, 2, 3}
arr2 := []int{1, 2, 3}
Contains(arr1, arr2) // error message: "cannot infer K"

So you would need to always call it like Contains[int, int](arr1, arr2), which is too intrusive in my opinion.

cc: @mchlp @TheAndrew2115 @KieranJQuan

@alokmenghrajani
Copy link

An example where having a generic Equal[T any] would make the test nicer to write:

	a := uint(1)
	assert.Equal(t, 1, a)

This is equivalent to being allowed to compare e.g. uint and int but only when one value is a literal -- something which can't be achieved using reflection.

@shoenig
Copy link

shoenig commented May 22, 2023

FWIW https://github.com/shoenig/test was lifted out of the HashiCorp Nomad project as a generics based re-imagination of testify. It leverages Google's go-cmp library and integrates with specifying custom cmp.Option values (particularly useful for protobuf stuff), as well as specifying custom output formats all via variadic ...Setting arguments. The library de-emphasises magic. As built-in types do not interoperate with generics naturally, you'll see variations of assertions specific to strings, maps, and slices. We've been using and iterating on the library for over a year now, and it's been growing in usage across more projects.

@peterldowns
Copy link

Is this still being considered as a potential improvement for the next version of testify?

(For anyone looking for a typesafe alternative using generics, with go-cmp for equality, zero other dependencies, and a much more reasonable set of assertion methods, check out my project https://github.com/peterldowns/testy.)

@brackendawson
Copy link
Collaborator

@peterldowns a v2 of testify is currently not being considered. I do recognise that there may be enhancements possible by making some of the assertions generic, but we won't be doing that in in v1.

@peterldowns
Copy link

@brackendawson got it, makes sense. Do you know if/when v2 will be considered? The line at the top of the README about it has been there for 4 years, I remember submitting the form myself ages ago, it seems a little misleading to keep it up there with the current wording if there are no plans to work on v2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests