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

Enhance errors for exception/warnings matching #8508

Merged
merged 4 commits into from Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog/8508.improvement.rst
@@ -0,0 +1,2 @@
Introduce multiline display for warning matching via :py:func:`pytest.warns` and
enhance match comparison for :py:func:`_pytest._code.ExceptionInfo.match` as returned by :py:func:`pytest.raises`.
RonnyPfannschmidt marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 5 additions & 4 deletions src/_pytest/_code/code.py
Expand Up @@ -672,10 +672,11 @@ def match(self, regexp: Union[str, Pattern[str]]) -> "Literal[True]":
If it matches `True` is returned, otherwise an `AssertionError` is raised.
"""
__tracebackhide__ = True
msg = "Regex pattern {!r} does not match {!r}."
if regexp == str(self.value):
msg += " Did you mean to `re.escape()` the regex?"
assert re.search(regexp, str(self.value)), msg.format(regexp, str(self.value))
value = str(self.value)
msg = f"Regex pattern did not match.\n Regex: {regexp!r}\n Input: {value!r}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it wouldn't be better to use a straight str here instead of repr. With the new line format the quoting doesn't seem needed, perhaps the opposite.

(Either way is fine with me)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe for the regex it helps to get the escapes printed more nicely readable, my personal opinion is that repr reads slightly better than just the str or the escape

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, this will show as

>>> regexp = r'py\.test'
>>> value = 'pytest'
>>> msg = f"Regex pattern did not match.\n Regex: {regexp!r}\n Input: {value!r}"
>>> print(msg)
Regex pattern did not match.
 Regex: 'py\\.test'
 Input: 'pytest'

I would have preferred py\.test over py\\.test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just experimented with this - if there is a simple way to print "raw" strings, i'll happily use that - however even in the pytest testsuite there is a number of cases that would turn confusing without

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is a simple way to print "raw" strings, i'll happily use that

That would be {regexp} instead of {regexp!r}, unless I'm misunderstanding.

however even in the pytest testsuite there is a number of cases that would turn confusing without

Can you give an example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm somewhat undecided here. I agree the double backslashes are a bit confusing, but at the same time I think it's good to print the repr for the input (considering that it can contain whitespace and such, and seeing that whitespace might be essential to find out why the pattern doesn't match).

But then if we use the repr for the input, it might be more consistent and thus less confusing if we use the repr for the pattern as well.

if regexp == value:
msg += "\n Did you mean to `re.escape()` the regex?"
assert re.search(regexp, value), msg
# Return True to allow for "assert excinfo.match()".
return True

Expand Down
36 changes: 18 additions & 18 deletions src/_pytest/recwarn.py
@@ -1,6 +1,7 @@
"""Record warnings during test function execution."""
import re
import warnings
from pprint import pformat
from types import TracebackType
from typing import Any
from typing import Callable
Expand Down Expand Up @@ -142,10 +143,11 @@ def warns(
__tracebackhide__ = True
if not args:
if kwargs:
msg = "Unexpected keyword arguments passed to pytest.warns: "
msg += ", ".join(sorted(kwargs))
msg += "\nUse context-manager form instead?"
raise TypeError(msg)
argnames = ", ".join(sorted(kwargs))
raise TypeError(
f"Unexpected keyword arguments passed to pytest.warns: {argnames}"
"\nUse context-manager form instead?"
)
return WarningsChecker(expected_warning, match_expr=match, _ispytest=True)
else:
func = args[0]
Expand Down Expand Up @@ -191,7 +193,7 @@ def pop(self, cls: Type[Warning] = Warning) -> "warnings.WarningMessage":
if issubclass(w.category, cls):
return self._list.pop(i)
__tracebackhide__ = True
raise AssertionError("%r not found in warning list" % cls)
raise AssertionError(f"{cls!r} not found in warning list")

def clear(self) -> None:
"""Clear the list of recorded warnings."""
Expand All @@ -202,7 +204,7 @@ def clear(self) -> None:
def __enter__(self) -> "WarningsRecorder": # type: ignore
if self._entered:
__tracebackhide__ = True
raise RuntimeError("Cannot enter %r twice" % self)
raise RuntimeError(f"Cannot enter {self!r} twice")
_list = super().__enter__()
# record=True means it's None.
assert _list is not None
Expand All @@ -218,7 +220,7 @@ def __exit__(
) -> None:
if not self._entered:
__tracebackhide__ = True
raise RuntimeError("Cannot exit %r without entering first" % self)
raise RuntimeError(f"Cannot exit {self!r} without entering first")

super().__exit__(exc_type, exc_val, exc_tb)

Expand Down Expand Up @@ -268,16 +270,17 @@ def __exit__(

__tracebackhide__ = True

def found_str():
return pformat([record.message for record in self], indent=2)
RonnyPfannschmidt marked this conversation as resolved.
Show resolved Hide resolved

# only check if we're not currently handling an exception
if exc_type is None and exc_val is None and exc_tb is None:
if self.expected_warning is not None:
if not any(issubclass(r.category, self.expected_warning) for r in self):
__tracebackhide__ = True
fail(
"DID NOT WARN. No warnings of type {} were emitted. "
"The list of emitted warnings is: {}.".format(
self.expected_warning, [each.message for each in self]
)
f"DID NOT WARN. No warnings of type {self.expected_warning} were emitted.\n"
f"The list of emitted warnings is: {found_str()}."
)
elif self.match_expr is not None:
for r in self:
Expand All @@ -286,11 +289,8 @@ def __exit__(
break
else:
fail(
"DID NOT WARN. No warnings of type {} matching"
" ('{}') were emitted. The list of emitted warnings"
" is: {}.".format(
self.expected_warning,
self.match_expr,
[each.message for each in self],
)
f"""\
RonnyPfannschmidt marked this conversation as resolved.
Show resolved Hide resolved
DID NOT WARN. No warnings of type {self.expected_warning} matching the regex were emitted.
Regex: {self.match_expr}
Emitted warnings: {found_str()}"""
)
14 changes: 8 additions & 6 deletions testing/code/test_excinfo.py
Expand Up @@ -420,18 +420,20 @@ def test_division_zero():
excinfo.match(r'[123]+')
"""
)
result = pytester.runpytest()
result = pytester.runpytest("--tb=short")
assert result.ret != 0

exc_msg = "Regex pattern '[[]123[]]+' does not match 'division by zero'."
result.stdout.fnmatch_lines([f"E * AssertionError: {exc_msg}"])
match = [
r"E .* AssertionError: Regex pattern did not match.",
r"E .* Regex: '\[123\]\+'",
r"E .* Input: 'division by zero'",
]
result.stdout.re_match_lines(match)
result.stdout.no_fnmatch_line("*__tracebackhide__ = True*")

result = pytester.runpytest("--fulltrace")
assert result.ret != 0
result.stdout.fnmatch_lines(
["*__tracebackhide__ = True*", f"E * AssertionError: {exc_msg}"]
)
result.stdout.re_match_lines([r".*__tracebackhide__ = True.*", *match])


class TestFormattedExcinfo:
Expand Down
17 changes: 10 additions & 7 deletions testing/python/raises.py
Expand Up @@ -191,10 +191,12 @@ def test_raises_match(self) -> None:
int("asdf")

msg = "with base 16"
expr = "Regex pattern {!r} does not match \"invalid literal for int() with base 10: 'asdf'\".".format(
msg
expr = (
"Regex pattern did not match.\n"
f" Regex: {msg!r}\n"
" Input: \"invalid literal for int() with base 10: 'asdf'\""
)
with pytest.raises(AssertionError, match=re.escape(expr)):
with pytest.raises(AssertionError, match="(?m)" + re.escape(expr)):
with pytest.raises(ValueError, match=msg):
int("asdf", base=10)

Expand All @@ -217,7 +219,7 @@ def test_match_failure_string_quoting(self):
with pytest.raises(AssertionError, match="'foo"):
raise AssertionError("'bar")
(msg,) = excinfo.value.args
assert msg == 'Regex pattern "\'foo" does not match "\'bar".'
assert msg == '''Regex pattern did not match.\n Regex: "'foo"\n Input: "'bar"'''

def test_match_failure_exact_string_message(self):
message = "Oh here is a message with (42) numbers in parameters"
Expand All @@ -226,9 +228,10 @@ def test_match_failure_exact_string_message(self):
raise AssertionError(message)
(msg,) = excinfo.value.args
assert msg == (
"Regex pattern 'Oh here is a message with (42) numbers in "
"parameters' does not match 'Oh here is a message with (42) "
"numbers in parameters'. Did you mean to `re.escape()` the regex?"
"Regex pattern did not match.\n"
" Regex: 'Oh here is a message with (42) numbers in parameters'\n"
" Input: 'Oh here is a message with (42) numbers in parameters'\n"
" Did you mean to `re.escape()` the regex?"
)

def test_raises_match_wrong_type(self):
Expand Down
25 changes: 10 additions & 15 deletions testing/test_recwarn.py
@@ -1,4 +1,3 @@
import re
import warnings
from typing import Optional

Expand Down Expand Up @@ -263,23 +262,23 @@ def test_as_contextmanager(self) -> None:
with pytest.warns(RuntimeWarning):
warnings.warn("user", UserWarning)
excinfo.match(
r"DID NOT WARN. No warnings of type \(.+RuntimeWarning.+,\) were emitted. "
r"DID NOT WARN. No warnings of type \(.+RuntimeWarning.+,\) were emitted.\n"
r"The list of emitted warnings is: \[UserWarning\('user',?\)\]."
RonnyPfannschmidt marked this conversation as resolved.
Show resolved Hide resolved
)

with pytest.raises(pytest.fail.Exception) as excinfo:
with pytest.warns(UserWarning):
warnings.warn("runtime", RuntimeWarning)
excinfo.match(
r"DID NOT WARN. No warnings of type \(.+UserWarning.+,\) were emitted. "
r"The list of emitted warnings is: \[RuntimeWarning\('runtime',?\)\]."
r"DID NOT WARN. No warnings of type \(.+UserWarning.+,\) were emitted.\n"
r"The list of emitted warnings is: \[RuntimeWarning\('runtime',?\)]."
)

with pytest.raises(pytest.fail.Exception) as excinfo:
with pytest.warns(UserWarning):
pass
excinfo.match(
r"DID NOT WARN. No warnings of type \(.+UserWarning.+,\) were emitted. "
r"DID NOT WARN. No warnings of type \(.+UserWarning.+,\) were emitted.\n"
r"The list of emitted warnings is: \[\]."
)

Expand All @@ -289,18 +288,14 @@ def test_as_contextmanager(self) -> None:
warnings.warn("runtime", RuntimeWarning)
warnings.warn("import", ImportWarning)

message_template = (
"DID NOT WARN. No warnings of type {0} were emitted. "
"The list of emitted warnings is: {1}."
)
excinfo.match(
re.escape(
message_template.format(
warning_classes, [each.message for each in warninfo]
)
)
messages = [each.message for each in warninfo]
expected_str = (
f"DID NOT WARN. No warnings of type {warning_classes} were emitted.\n"
f"The list of emitted warnings is: {messages}."
)

assert str(excinfo.value) == expected_str

def test_record(self) -> None:
with pytest.warns(UserWarning) as record:
warnings.warn("user", UserWarning)
Expand Down