pytest icon indicating copy to clipboard operation
pytest copied to clipboard

`pytest.raises` with invalid regex in match kwarg fails with internal `re.error`

Open lovetheguitar opened this issue 1 year ago • 1 comments

Using an invalid regex in pytest.raises fails in a messy way.

with pytest.raises(ValueError, match="invalid regex character ["):
    raise ValueError()
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ValueError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "C:\Users\user\Documents\Projects\external\pytest\src\_pytest\python_api.py", line 1011, in __exit__
    self.excinfo.match(self.match_expr)
  File "C:\Users\user\Documents\Projects\external\pytest\src\_pytest\_code\code.py", line 733, in match
    assert re.search(regexp, value), msg
  File "C:\Python38\lib\re.py", line 201, in search
    return _compile(pattern, flags).search(string)
  File "C:\Python38\lib\re.py", line 304, in _compile
    p = sre_compile.compile(pattern, flags)
  File "C:\Python38\lib\sre_compile.py", line 764, in compile
    p = sre_parse.parse(p, flags)
  File "C:\Python38\lib\sre_parse.py", line 948, in parse
    p = _parse_sub(source, state, flags & SRE_FLAG_VERBOSE, 0)
  File "C:\Python38\lib\sre_parse.py", line 443, in _parse_sub
    itemsappend(_parse(source, state, verbose, nested + 1,
  File "C:\Python38\lib\sre_parse.py", line 549, in _parse
    raise source.error("unterminated character set",
re.error: unterminated character set at position 24

Proposal would be to handle regex failures within pytest.raises and fail nicely for the user (UsageError).

  • [ ] a detailed description of the bug or problem you are having
  • [ ] output of pip list from the virtual environment you are using
  • [ ] pytest and operating system versions
  • [ ] minimal example if possible

lovetheguitar avatar Jun 21 '24 08:06 lovetheguitar

Here's how I would go about this:

  • Take the example from above and turn it into a test case
    • testing/python/raises.py is where the existing tests are for pytest.raises
    • Since we are mostly interested in the output of pytest, a test using pytester would be a good fit here.
    • You could use one of the cases in test_raises_as_contextmanager as inspiration, but instead of matching that the test case passed, you use result.stdout.fnmatch_lines to see what output pytest produces when the pattern is invalid.
    • I'd actually make the test pass initially, i.e. test the current output.
  • Improve pytest's output in that case.
    • If you look at the traceback from above, you can see that the relevant lines inside pytest where this is happening are:

https://github.com/pytest-dev/pytest/blob/329d3712146e69c471be3e30883d54bdde2f76cb/src/_pytest/python_api.py#L1005

https://github.com/pytest-dev/pytest/blob/329d3712146e69c471be3e30883d54bdde2f76cb/src/_pytest/_code/code.py#L728

(line numbers slightly different, not sure what version @lovetheguitar was on above)

  • Find the correct place do do a change like this
    • The latter (code.py) is the more internal machinery in pytest. It might be used across different places, some of which are not user-facing. So this would be the wrong place to do this kind of change.
    • The former (RaisesContext) seems like the better place.
    • In fact, a few lines above you can already spot a call to pytest.fail(...), which is how we explicitely fail a test case nicely.
    • ...so the only thing that's left would be to catch the exception (re.error) and turn it into a pytest.fail call with a nice message
    • Make sure you format the original exception text into the new message to retain the information about what's wrong exactly
  • Finally, adjust your test case to test the new output. Maybe add an assertion that makes sure that we don't print a full traceback, only a nice short message.
  • This will also need a changelog fragment - see the contribution docs for more details.

The-Compiler avatar Jun 21 '24 08:06 The-Compiler