Escape both bytes and unicode strings for "ids" in Metafunc.parametrize#1463
Escape both bytes and unicode strings for "ids" in Metafunc.parametrize#1463ceridwen wants to merge 1 commit intopytest-dev:masterfrom ceridwen:master
Conversation
|
I don't think it's much of a problem to add a testing dependency, and I think it'd really make sense for pytest to use hypothesis for some kinds of tests. It's just some kind of a circular dependency then? 😆 /cc @DRMacIver |
|
I don't think circular test dependencies are a problem. Hypothesis only depends on py.test for its tests as well. |
|
For the record I don't see a problem either, we have several test-only dependencies. Is there any way to skip tests which use Hypothesis in case hypothesis is not installed? Just wondering. |
| return str(val) | ||
| elif isinstance(val, REGEX_TYPE): | ||
| return _escape_bytes(val.pattern) if isinstance(val.pattern, bytes) else val.pattern | ||
| return _escape_strings(val.pattern) if isinstance(val.pattern, bytes) else val.pattern |
There was a problem hiding this comment.
given the new idea for unicode, i feel we no longer need the conditional, please check if thats the case
There was a problem hiding this comment.
I ran the tests again with the conditional removed, and everything passed, so I assume it's okay.
|
wrt poin2, since we now escape, i feel we should test the new code - however i realized we also need to target the features branch with that |
|
What tests do you want me to change? How do you feel about using Hypothesis? How do I move this patch to the features branch? |
I'm not sure if GitHub allows you to edit the target of a pull request, but when you open a pull you can specify the target branch. For example features...ceridwen:master will show the changes between your master branch and the pytest features branch, and allows you to open a pull request. |
|
You can do something like |
I have two questions here: