testify icon indicating copy to clipboard operation
testify copied to clipboard

Using generics

Open borosr opened this issue 2 years ago • 15 comments

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

borosr avatar Jan 15 '22 12:01 borosr

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!

glesica avatar Jan 15 '22 16:01 glesica

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

brackendawson avatar Jan 15 '22 16:01 brackendawson

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.

jan-xyz avatar Mar 18 '22 10:03 jan-xyz

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

borosr avatar Mar 19 '22 10:03 borosr

@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.

jan-xyz avatar Mar 20 '22 16:03 jan-xyz

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 https://github.com/pellared/fluentassert/pull/26. I would appreciate any feedback.

pellared avatar Mar 21 '22 09:03 pellared

@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.

brackendawson avatar Mar 22 '22 12:03 brackendawson

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.

jordaniversen avatar Mar 31 '22 03:03 jordaniversen

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.

brackendawson avatar Mar 31 '22 12:03 brackendawson

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

jordaniversen avatar Mar 31 '22 13:03 jordaniversen

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

zhulik avatar May 30 '22 23:05 zhulik

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.

brackendawson avatar May 31 '22 08:05 brackendawson

@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.

zhulik avatar May 31 '22 18:05 zhulik

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

anthony-chang avatar Nov 28 '22 08:11 anthony-chang

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.

alokmenghrajani avatar Dec 04 '22 23:12 alokmenghrajani

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.

shoenig avatar May 22 '23 13:05 shoenig