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

Fix a panic when comparing types where the underlying type is string #622

Closed

Conversation

autarch
Copy link

@autarch autarch commented Jun 25, 2018

The code was checking if the type's kind was string and then attempting to
do a type assertion of var.(string). However, this assertion only works if
the type actually is a string. If it's some other type (like type foo string) then this ends up giving us a runtime panic.

The code was checking if the type's _kind_ was string and then attempting to
do a type assertion of `var.(string)`. However, this assertion only works if
the type actually _is_ a string. If it's some other type (like `type foo
string`) then this ends up giving us a runtime panic.
@autarch
Copy link
Author

autarch commented Jun 25, 2018

Travis failure is because of #619

@autarch
Copy link
Author

autarch commented Jul 7, 2018

Ping

@jwilder
Copy link

jwilder commented Jul 20, 2018

I'm hitting this issue w/ 1.2.2. Any chance we can get this merged? I verified the change fixes it for me as well.

jwilder added a commit to jwilder/testify that referenced this pull request Sep 14, 2018
@georgelesica-wf
Copy link

@autarch if you don't mind, could you confirm that #661 also fixes this issue?

@autarch
Copy link
Author

autarch commented Sep 18, 2018

@georgelesica-wf - the test in that other PR looks to be functionally the same as the one I added, so I'm sure it fixes the issue as best as I could test it. I don't have any other code that causes this issue to run the PR branch against.

@georgelesica-wf
Copy link

Cool, thanks! Would you have any objection if we merged the one I linked? I won't close this one until that's done, however.

@autarch
Copy link
Author

autarch commented Sep 18, 2018

Yeah, I'm fine with anything that fixes this bug.

@vitalyisaev2
Copy link
Contributor

Perhaps I'm little bit late to the party, tried to fix it too in #674

@autarch autarch closed this Nov 23, 2019
@autarch
Copy link
Author

autarch commented Nov 23, 2019

This was fixed a while back in a different commit.

@autarch autarch deleted the autarch/fix-type-assert-failure branch November 23, 2019 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants