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

The "ids" argument to "parametrize" again accepts unicode strings in Python 2 #1908

Merged
merged 1 commit into from
Sep 5, 2016

Conversation

nicoddemus
Copy link
Member

Fixes #1905

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.078% when pulling 1e10de5 on nicoddemus:parametrize-unicode-id into 722f9ea on pytest-dev:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.078% when pulling 1e10de5 on nicoddemus:parametrize-unicode-id into 722f9ea on pytest-dev:master.

@RonnyPfannschmidt
Copy link
Member

unicode support on python 2 got removed for some specific reason/regression - i think we want test id#s tobe native strings ins all cases

@RonnyPfannschmidt
Copy link
Member

i think i recalled the reason, environmental reasons may make python fail to print/output the unicode id's

i think we need to absolutely ensure the ids are native strings
please lets verify that and add a test

@The-Compiler
Copy link
Member

unicode support on python 2 got removed for some specific reason/regression

No, the check was added (by @nicoddemus) in response to #1857 to improve the error message when someone passes e.g. integers to parametrize.

@nicoddemus
Copy link
Member Author

Yep

@flub
Copy link
Member

flub commented Sep 5, 2016

So we can merge this anyway?

@nicoddemus
Copy link
Member Author

From my POV yes 😁

@The-Compiler The-Compiler merged commit 8a41b26 into pytest-dev:master Sep 5, 2016
@The-Compiler
Copy link
Member

Well then, I dared to click the button 😉

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

Successfully merging this pull request may close these issues.

5 participants