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

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

Closed
4 tasks
lovetheguitar opened this issue Jun 21, 2024 · 1 comment · Fixed by #12526
Closed
4 tasks

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

lovetheguitar opened this issue Jun 21, 2024 · 1 comment · Fixed by #12526
Labels
good first issue easy issue that is friendly to new contributor

Comments

@lovetheguitar
Copy link
Contributor

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
@The-Compiler The-Compiler added the good first issue easy issue that is friendly to new contributor label Jun 21, 2024
@The-Compiler
Copy link
Member

The-Compiler commented Jun 21, 2024

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:

self.excinfo.match(self.match_expr)

assert re.search(regexp, value), msg

(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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue easy issue that is friendly to new contributor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants