pytest icon indicating copy to clipboard operation
pytest copied to clipboard

Fixed handling of invalid regex pattern

Open virendrapatil24 opened this issue 1 year ago • 5 comments

closes #12505

  • Updated the RaisesContext class in src/_pytest/python_api.py to catch re.error and call pytest.fail with a clear error message.
  • Added a test case in testing/python/raises.py to validate the new behavior.
  • Created a changelog fragment to document this improvement.

virendrapatil24 avatar Jun 23 '24 20:06 virendrapatil24

This definitely makes things a bit nicer:

test_raises.py:3: in <module>
    raise ValueError()
E   ValueError

During handling of the above exception, another exception occurred:
/usr/lib/python3.12/re/__init__.py:177: in search
    return _compile(pattern, flags).search(string)
/usr/lib/python3.12/re/__init__.py:307: in _compile
    p = _compiler.compile(pattern, flags)
/usr/lib/python3.12/re/_compiler.py:745: in compile
    p = _parser.parse(p, flags)
/usr/lib/python3.12/re/_parser.py:979: in parse
    p = _parse_sub(source, state, flags & SRE_FLAG_VERBOSE, 0)
/usr/lib/python3.12/re/_parser.py:460: in _parse_sub
    itemsappend(_parse(source, state, verbose, nested + 1,
/usr/lib/python3.12/re/_parser.py:568: in _parse
    raise source.error("unterminated character set",
E   re.error: unterminated character set at position 24

During handling of the above exception, another exception occurred:
test_raises.py:2: in <module>
    with pytest.raises(ValueError, match="invalid regex character ["):
E   Failed: Invalid regex pattern provided to 'match': unterminated character set at position 24

But it's still pretty verbose due to the chaining... This could be improved by using raise fail.Exception(...) from None:

test_raises.py:2: in <module>
    with pytest.raises(ValueError, match="invalid regex character ["):
E   Failed: Invalid regex pattern provided to 'match': unterminated character set at position 24

though we don't really do this kind of thing elsewhere so far. I'd wait to see what others think before proceeding with that.

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

I have added the changes you asked for, please let me know if anything else needs to be done. Thanks for the input!

virendrapatil24 avatar Jun 26 '24 19:06 virendrapatil24

@virendrapatil24 The proposed solution of catching the exception in __exit__ has the disadvantage that the fail is raised from the __exit__ (and is thus chained to the exception under test), and is not checked at all in the "did not raise" case. I therefore think it'd be better to check it eagerly in the __init__.

bluetech avatar Jun 27 '24 11:06 bluetech

@bluetech So I need to compile the regex pattern in the __init__ itself to check it for the valid pattern. Let me know if I have gotten this right.

virendrapatil24 avatar Jun 27 '24 12:06 virendrapatil24

Right, that would be a good way to do it.

bluetech avatar Jun 27 '24 13:06 bluetech