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

ShouldEqual no longer works with functions #53

Closed
KevinAnthony opened this issue Jul 5, 2023 · 5 comments
Closed

ShouldEqual no longer works with functions #53

KevinAnthony opened this issue Jul 5, 2023 · 5 comments

Comments

@KevinAnthony
Copy link

KevinAnthony commented Jul 5, 2023

Starting in 1.15.0 ShouldEqual no longer functions properly.

func Test(t *testing.T) {
	f := func() {}
	str := assertions.ShouldEqual(f, f)

	if len(str) != 0 {
		panic(str)
	}
}

This panics with:

{"Message":"Expected: (func())(0x0000000104b26d40)
Actual:   (func())(0x0000000104b26d40)
(Should equal, but there is a type difference within the two)!","Expected":"(func())(0x0000000104b26d40)","Actual":"(func())(0x0000000104b26d40)"}

This work in 1.13.1

@mdwhatcott
Copy link
Contributor

Good catch. The spec is pretty clear in stating that:

Slice, map, and function types are not comparable (with ==). However, as a special case, a slice, map, or function value may be compared to the predeclared identifier nil.

Furthermore, reflect.DeepEqual won't even return true in the case that a nil func reference is compared with nil: https://go.dev/play/p/9DRZia53nFk

So, I'm trying to see the utility of supporting equality comparisons with ShouldEqual. Just curious, why are you comparing functions with ShouldEqual?

@KevinAnthony
Copy link
Author

KevinAnthony commented Jul 5, 2023

Interface implementation testing.

We have an interface that has a function, "GetHandler()" that returns the handler that was set on initialization.
This unit test ensures that the function passed into the struct isn't mutated, and is instead the same function that is passed back out"

@mdwhatcott
Copy link
Contributor

@KevinAnthony - Pull v1.15.1 and see if that does the trick.

@KevinAnthony
Copy link
Author

I know this is closed, but I wanted to update it and say it worked for us.

@KevinAnthony
Copy link
Author

can you look at this PR in goconvey
smartystreets/goconvey#676
To update this bug in goconvey?

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

No branches or pull requests

2 participants