add ini option to disable string escape for parametrization #2537
Conversation
_pytest/python.py
Outdated
| @@ -909,7 +912,11 @@ def _idval(val, argname, idx, idfn, config=None): | |||
| return hook_id | |||
|
|
|||
| if isinstance(val, STRING_TYPES): | |||
| return _escape_strings(val) | |||
| if config is None: | |||
There was a problem hiding this comment.
for sanity and adding a unittest please extract this to a own function
_pytest/python.py
Outdated
| escape_option = False | ||
| else: | ||
| escape_option = config.getini("disable_test_id_escaping_and_forfeit_all_rights_to_community_support") | ||
| return val if escape_option else _escape_strings(val) | ||
| elif isinstance(val, (float, int, bool, NoneType)): | ||
| return str(val) | ||
| elif isinstance(val, REGEX_TYPE): |
There was a problem hiding this comment.
wrt regex its not clear if we should or should not escape
|
your re-factoring tool made a mess of the white space bu im inclined to still accept the changes as they make stuff better 👍 @nicoddemus can we merge master timely to sort out merge issues? |
|
@ApaDoctor sorry I hate to ask this, but could you remake the patch without all the whitespace/formatting changes? I'm sure you can disable that in your editor. Unfortunately as it stands the patch makes a mess of the history because we lose the authors of the changed lines. @RonnyPfannschmidt definitely about merging this with |
|
Sorry for the long pause. I'll fix whitespace problem in closest time |
|
No problem, thanks for the follow up |
|
So i completed all changes. PR can be approved |
_pytest/python.py
Outdated
| @@ -910,6 +914,14 @@ def _find_parametrized_scope(argnames, arg2fixturedefs, indirect): | |||
| return 'function' | |||
|
|
|||
|
|
|||
| def _escape_strings_checker(val, config=None): | |||
| if config is None: | |||
| escape_option = False | |||
There was a problem hiding this comment.
Maybe call this disable_escaping to make it more clear.
doc/en/parametrize.rst
Outdated
|
|
||
| pytest by default escapes any non-ascii characters used in unicode strings | ||
| for the parametrization because it has several downsides. | ||
| If however you would like to use them, use the option disable_test_id_escaping_and_forfeit_all_rights_to_community_support = False |
There was a problem hiding this comment.
Thanks @ApaDoctor for the follow up, we appreciate it!
Some final requests before we merge:
- Please add a changelog entry for this, I suggest
2482.featurewith this contents:
Include new ``disable_test_id_escaping_and_forfeit_all_rights_to_community_support`` option to disable ascii-escaping in parametrized values. This may cause a series of problems and as the name makes clear, use at your own risk.
- Can you please rebase all your commits in a single commit?
doc/en/parametrize.rst
Outdated
|
|
||
| pytest by default escapes any non-ascii characters used in unicode strings | ||
| for the parametrization because it has several downsides. | ||
| If however you would like to use them, use the option disable_test_id_escaping_and_forfeit_all_rights_to_community_support = True |
There was a problem hiding this comment.
Small nit:
If however you would like to use unicode strings in parametrization, use this option in your ``pytest.ini``:
.. code-block:: ini
[pytest]
disable_test_id_escaping_and_forfeit_all_rights_to_community_support = True
_pytest/python.py
Outdated
|
|
||
| parser.addini("disable_test_id_escaping_and_forfeit_all_rights_to_community_support", type="bool", | ||
| default=False, help="disable string escape non-ascii" | ||
| " characters, might cause unwanted side effects") |
There was a problem hiding this comment.
I think it worth adding a "(use at your own risk)" here.
ApaDoctor
force-pushed
the
disable-escape-test-id
branch
from
53fcdd2
to
a2da5a6
Compare
|
@ApaDoctor @nicoddemus What ever happened to this? It sounded ready to go, then it just got closed? |
|
Not sure, @ApaDoctor simply closed this, I assumed he wanted to withdraw his contribution... 😕 |
solving #2482