add matching the error message to pytest.raises #2227

Kriechi · 2017-02-01T12:37:49Z

pytest.raises can now assert that the error message contains a certain text.
Example:

with raises(ValueError, match_info='must be 0'):
    raise ValueError("value must be 0 or None")

This is a more compact and IMHO cleaner way of doing it like this:

with raises(ValueError) as exc_info:
    raise ValueError("value must be <= 10")
assert "value must be <= 10" in str(exc_info.value)

The-Compiler · 2017-02-01T12:44:40Z

This PR should go to the features branch, as it's a new feature - you can change the branch of the existing PR, and then you might need to rebase and force-push your changes too (not sure about the latter).

Kriechi · 2017-02-01T12:47:28Z

Sorry about that - I changed it as requested.

coveralls · 2017-02-01T12:48:49Z

Coverage increased (+0.006%) to 92.838% when pulling dd4d61a on Kriechi:raises-info into 0931fe2 on pytest-dev:master.

coveralls · 2017-02-01T13:02:39Z

Coverage increased (+0.006%) to 92.73% when pulling 06fae84 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

The-Compiler · 2017-02-01T13:07:24Z

No worries! You'll also need to change the changelog to add this to a 3.1.0 section (I think?).

Looking at https://travis-ci.org/pytest-dev/pytest/jobs/197286855#L214 I also think you should use ... instead of .... in your docstring.

LGTM otherwise from a quick look, but I'm a bit in a hurry currently.

Kriechi · 2017-02-01T13:20:49Z

fixed & fixed 😄

coveralls · 2017-02-01T13:31:01Z

Coverage increased (+0.006%) to 92.73% when pulling cfb7bec on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

coveralls · 2017-02-01T13:40:10Z

Coverage increased (+0.006%) to 92.73% when pulling cfb7bec on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

coveralls · 2017-02-01T13:56:54Z

Coverage increased (+0.006%) to 92.73% when pulling 5d6a2e3 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

nicoddemus · 2017-02-01T14:12:23Z

Thanks for the PR! But I believe we already have similar functionality with excinfo.match (sorry I can't provide a link now because I'm on my phone)

RonnyPfannschmidt · 2017-02-01T14:32:37Z

@nicoddemus would it make sense to make "with raises(..., match=...)" a shortcut for the turnaround?

nicoddemus · 2017-02-01T17:51:37Z

IMHO it does not add much... that would be two ways to accomplish the same thing with little gain. For now I'm -0 on the idea.

nicoddemus · 2017-02-01T17:52:06Z

But by all means if other people think this is a good idea we should move this forward, of course,

mhils · 2017-02-02T14:53:21Z

We use these kind of assertions quite a lot - IMHO, a declarative "we expect this error with this message" style would be a bit nicer than the current

    with pytest.raises(ValueError) as excinfo:
        myfunc()
    excinfo.match(r'.* 123 .*')

construct where our expectations about the error object are split before and after the statement. It might just be me, but I'm always slightly irritated by using excinfo after the with context is closed.

RonnyPfannschmidt · 2017-02-02T15:07:40Z

im +1 on having match as kwarg and encouraging its usage over exceptioninfo

nicoddemus · 2017-02-02T15:11:46Z

@mhils thanks for the feedback, appreciate it!

@RonnyPfannschmidt OK then, let's go ahead with it. 👍

I would just suggest to call the keyword argument match so it matches (:grin:) the excinfo method.

with pytest.raises(ValueError, match=r'.* 123 .*'):
    myfunc()

Kriechi · 2017-02-02T15:12:05Z

At the moment this PR only does substring matching 'foo' in 'foobar'.
Do you think Regex-based matching makes sense here?
I'm happy to amend this PR to also handle regex's!

nicoddemus · 2017-02-02T15:13:02Z

Do you think Regex-based matching makes sense here?
I'm happy to amend this PR to also handle regex's!

I think so, mostly to be consistent with excinfo.match.

nicoddemus · 2017-02-02T15:14:46Z

@Kriechi also, it would be nice if you could review the relevant documentation and change the examples to use the new match keyword instead of excinfo.match as suggested by @RonnyPfannschmidt .

coveralls · 2017-02-02T16:02:49Z

Coverage decreased (-0.005%) to 92.719% when pulling bc904f0 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

coveralls · 2017-02-02T16:13:26Z

Coverage decreased (-0.002%) to 92.722% when pulling 9377784 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

RonnyPfannschmidt · 2017-02-02T16:14:39Z

testing/python/raises.py

+            int('asdf')
+
+        msg = "with base 16"
+        try:


for mind-bending fun we could nest pytest.rasies here 🤣

RonnyPfannschmidt

well done, good work :)

coveralls · 2017-02-02T16:23:39Z

Coverage increased (+0.008%) to 92.732% when pulling 6170607 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

coveralls · 2017-02-02T16:34:01Z

Coverage increased (+0.008%) to 92.732% when pulling 79ec87f on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

nicoddemus · 2017-02-02T18:19:18Z

_pytest/python.py

@@ -1246,6 +1260,11 @@ def __exit__(self, *tp):
        suppress_exception = issubclass(self.excinfo.type, self.expected_exception)
        if sys.version_info[0] == 2 and suppress_exception:
            sys.exc_clear()
+        if self.match_expr:
+            try:


Hmm this bare try/except makes me a little uneasy... why is it needed? Wouldn't be enough to just call self.excinfo.match(self.match_expr)?

if self.match_expr: self.excinfo.match(self.match_expr)

I wasn't sure how to handle this properly, because ExceptionInfo.match raises an AssertionError if no match is found, but we want to call pytest.fail in that case...
Is there a better way of handling this?

I think just calling self.excinfo.match should be enough, as this will produce the same behavior as:

with pytest.raises(...) as excinfo: func() excinfo.match(text)

Or am I missing something?

I guess one issue in that case is that the traceback will point to pytest's internals, but to prevent that we just need to declare a __tracebackhide__ variable:

__tracebackhide__ = True if self.match_expr: self.excinfo.match(self.match_expr)

Done. The __tracebackhide__ is already there.

Great, thanks! 👍

nicoddemus · 2017-02-02T18:21:35Z

CHANGELOG.rst

@@ -13,6 +13,9 @@ New Features
 * ``pytest.warns`` now checks for subclass relationship rather than
  class equality. Thanks `@lesteve`_ for the PR (`#2166`_)

+* ``pytest.raises`` now asserts that the error message matches a text or regex
+  with the `match` keyword argument. Thanks `@Kriechi`_ for the PR.


Small nitpick, but "match" should be surrounded by double back ticks to get a monospaced font, like you did with "pytest.raises"

Thanks - fixed now.

coveralls · 2017-02-02T18:37:17Z

Coverage increased (+0.008%) to 92.732% when pulling 3c4c6f0 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

nicoddemus

Thanks @Kriechi for the patience and following through with the PR! 👍

I will merge as soon as CI passes!

coveralls · 2017-02-02T19:05:10Z

Coverage increased (+0.006%) to 92.73% when pulling 43662ce on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

The-Compiler

I like how this turned out, definitely also prefer this more declaritive/simple API! Just a simple fix for Python 2.6 left, LGTM otherwise.

The-Compiler · 2017-02-02T19:53:37Z

testing/python/raises.py

+            int('asdf')
+
+        msg = "with base 16"
+        expr = r"Pattern '{}' not found in 'invalid literal for int\(\) with base 10: 'asdf''".format(msg)


This should be {0} for Python 2.6 compatibility.

Right, pushed a quick fix. 👍

The-Compiler · 2017-02-02T20:11:28Z

oh, right - I always forget about that "maintainers can push too" feature 😄

coveralls · 2017-02-02T20:17:37Z

Coverage increased (+0.006%) to 92.73% when pulling abbff68 on Kriechi:raises-info into 3b47cb4 on pytest-dev:features.

RonnyPfannschmidt · 2017-02-03T11:31:59Z

the py26 env broke due to a zero-length format spec

RonnyPfannschmidt · 2017-02-03T11:32:20Z

whops, reload lag, sorry

Kriechi changed the base branch from master to features February 1, 2017 12:45

Kriechi force-pushed the raises-info branch from dd4d61a to 06fae84 Compare February 1, 2017 12:51

Kriechi force-pushed the raises-info branch 2 times, most recently from 3bca688 to cfb7bec Compare February 1, 2017 13:19

Kriechi force-pushed the raises-info branch from cfb7bec to 5d6a2e3 Compare February 1, 2017 13:41

Kriechi force-pushed the raises-info branch 2 times, most recently from bc904f0 to 9377784 Compare February 2, 2017 15:55

Kriechi force-pushed the raises-info branch from 9377784 to 6170607 Compare February 2, 2017 16:05

RonnyPfannschmidt reviewed Feb 2, 2017

View reviewed changes

RonnyPfannschmidt approved these changes Feb 2, 2017

View reviewed changes

Kriechi force-pushed the raises-info branch from 6170607 to 79ec87f Compare February 2, 2017 16:17

nicoddemus reviewed Feb 2, 2017

View reviewed changes

Kriechi force-pushed the raises-info branch from 79ec87f to 3c4c6f0 Compare February 2, 2017 18:26

allow error message matching in pytest.raises

43662ce

Kriechi force-pushed the raises-info branch from 3c4c6f0 to 43662ce Compare February 2, 2017 18:52

nicoddemus approved these changes Feb 2, 2017

View reviewed changes

The-Compiler requested changes Feb 2, 2017

View reviewed changes

Fix '{0}' format for py26

abbff68

The-Compiler approved these changes Feb 2, 2017

View reviewed changes

nicoddemus merged commit 208fae5 into pytest-dev:features Feb 3, 2017

Kriechi deleted the raises-info branch February 3, 2017 09:02

jorisvandenbossche mentioned this pull request Aug 16, 2017

TST: Converting to Pytest Idiom pandas-dev/pandas#15990

Closed

9 tasks

massich mentioned this pull request Aug 22, 2017

[MRG][feature] Change warns signature to mimic the raises call #2711

Merged

RonnyPfannschmidt mentioned this pull request Mar 14, 2018

Add syntactic sugar for attr.ib(default=attr.Factory) python-attrs/attrs#356

Merged

add matching the error message to pytest.raises #2227

add matching the error message to pytest.raises #2227

Conversation

Kriechi commented Feb 1, 2017 • edited Loading

The-Compiler commented Feb 1, 2017

Kriechi commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

The-Compiler commented Feb 1, 2017

Kriechi commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

nicoddemus commented Feb 1, 2017

RonnyPfannschmidt commented Feb 1, 2017

nicoddemus commented Feb 1, 2017

nicoddemus commented Feb 1, 2017

mhils commented Feb 2, 2017

RonnyPfannschmidt commented Feb 2, 2017

nicoddemus commented Feb 2, 2017

Kriechi commented Feb 2, 2017

nicoddemus commented Feb 2, 2017

nicoddemus commented Feb 2, 2017

coveralls commented Feb 2, 2017

coveralls commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

coveralls commented Feb 2, 2017

coveralls commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Feb 2, 2017

nicoddemus left a comment

Choose a reason for hiding this comment

coveralls commented Feb 2, 2017

The-Compiler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The-Compiler commented Feb 2, 2017

coveralls commented Feb 2, 2017

RonnyPfannschmidt commented Feb 3, 2017

RonnyPfannschmidt commented Feb 3, 2017 • edited Loading

Kriechi commented Feb 1, 2017 •

edited

Loading

RonnyPfannschmidt commented Feb 3, 2017 •

edited

Loading