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

Allow assert.Equal on string type alias without panicking on failure #661

Merged
merged 1 commit into from Oct 2, 2018

Conversation

HaraldNordgren
Copy link
Contributor

@HaraldNordgren HaraldNordgren commented Sep 11, 2018

@ernesto-jimenez The old comparison against reflect.String is true for type aliases of string, but the type coercion panics. So let's compare the type instead of the kind, to avoid the panics.

Fixes #644
Fixes #646

if !Equal(mockT, &struct{}{}, &struct{}{}) {
t.Error("Equal should return true (pointer equality is based on equality of underlying value)")
}
var m map[string]interface{}
if Equal(mockT, m["bar"], "something") {
t.Error("Equal should return false")
}
if Equal(mockT, myType("1"), myType("2")) {
t.Error("Equal should return false")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test will panic if the fix in assert/assertions.go is reverted.

Copy link

@georgelesica-wf georgelesica-wf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you for your patience!

@HaraldNordgren
Copy link
Contributor Author

@ernesto-jimenez @georgelesica-wf Merge this? 😊

@georgelesica-wf
Copy link

@HaraldNordgren I'm waiting on merge permissions from @matryer - hang tight, sorry!

@georgelesica-wf georgelesica-wf merged commit f2347ac into stretchr:master Oct 2, 2018
@HaraldNordgren HaraldNordgren deleted the Equal_type_alias branch October 3, 2018 00:04
autarch added a commit to autarch/intro-to-go-class-exercises that referenced this pull request Jan 4, 2019
The last tagged version of testify, 1.2.2, has a bug that causes a lot of
issues for the class, fixed with stretchr/testify#661
markmandel added a commit to markmandel/agones that referenced this pull request Apr 3, 2019
Keep running into [googleforgames#661](stretchr/testify#661) -
Allow assert.Equal on string type alias without panicking on failure

So upgrading Testify to solve this.
markmandel added a commit to markmandel/agones that referenced this pull request Apr 3, 2019
Keep running into [googleforgames#661](stretchr/testify#661) -
Allow assert.Equal on string type alias without panicking on failure

So upgrading Testify to solve this.
markmandel added a commit to googleforgames/agones that referenced this pull request Apr 4, 2019
Keep running into [#661](stretchr/testify#661) -
Allow assert.Equal on string type alias without panicking on failure

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

Successfully merging this pull request may close these issues.

None yet

2 participants