Confusion between message and match parameters in pytest.raises

[rth]

After analyzing several code bases, it appears there is frequent confusion by users between the match and message parameters in pytest.raises (and consequentially pytest.warns).

In most cases, users probably would want match but use message by accident, because is it visually similar and the naming does not differentiate their functionality enough. As a result the written test produces false positive which is quite dangerous.

For instance, here are instances of such errors in popular open-source projects,

• TST Replace unittest.TestCase.fail calls, use pytest.raises instead of legacy constructs pandas-dev/pandas#22681 (comment)
• TST Use assert_raises "match" parameter instead of the "message" parameter. scipy/scipy#9264
• [MRG]Fix pytest message that silently passes no matter if it should scikit-learn/scikit-learn#12001

the fact that these cases ended up in the code despite fairly thorough review is telling.

There was also an earlier issue illustrating this confusion in #3350

Searching on github for "pytest.raises(ValueError, message=" for instance, also shows a number of examples that meant to use "match" instead of "message" (though sometimes it's hard to tell).

One possibility could be to deprecate the message parameter and rename it to something more explicit. I saw there were some deprecations related to the use of node.warn in 3.8, not sure if that's related. However, following the same logic even if pytest.raises(ValueError('expected message')) worked it would still not address all the existing code that currently uses the message parameter incorrectly.

---

EDIT by @nicoddemus

The message parameter has been deprecated and will be removed in pytest 5.0, please see the docs for an alternative which provides the same functionality:

https://docs.pytest.org/en/latest/deprecations.html#raises-message-deprecated

[RonnyPfannschmidt]

after checking we should deprecate the message name and rename it to something like failure_message

[rth]

Thanks for the feedback @RonnyPfannschmidt !

Naming is tricky: at some level even pytest.raises(ValueEror, failure_message='some message') could be interpreted as the expected failure message of that ValueError. Something like show_failure_mesage might be bit more explicit, but it's probably too long and not very practical..

[RonnyPfannschmidt]

@rth yes - the hardness of naming this the bane here ^^

[asottile]

big +1 to this, we have an internal linter that looks for pytest.raises(...message= and just bans it -- maybe we should consider removing the parameter altogether

[nicoddemus]

Hi @rth thanks for the excellent description and examples, they are extremely helpful in making the case about doing something about this matter.

So we have two proposals:

1. Deprecate message and use another name. I think using a longer name like show_failure_message is perfectly fine, my gut feeling is that this parameter is rarely used (only misused). This option to me is safest.

2. Removing the parameter altogether (after deprecating it of course). This would make it match (heh) pytest.warn signature as well. This is an excellent option in the long term if there are no good use-cases for it.

I'm writing an e-mail to the mailing list to ask opinions to this, in particular if there are good cases for message/show_failure_message that I'm not visualizing.

[nicoddemus]

Message posted to the mailing list: https://mail.python.org/pipermail/pytest-dev/2018-September/004599.html

[nicoddemus]

While reading our Backward Compatibility Policy:

... while giving you enough time to adjust your tests or raise concerns if there are valid reasons to keep deprecated functionality around.

I realized that option 2) can use a warning like this:

with pytest.raises(RuntimeError, message="some regex"):
    ...

   did you mean to use `match="some regex"` to check the exception message? the "message" parameter is scheduled for removal, please join https://github.com/pytest-dev/pytest/issues/3974 if you have concerns.

The feature keeps working, people can chime in with valid use cases, and we don't introduce a new parameter to keep around forever. Hmm.

[flub]

I'm tentatively agreeing with removing it fully to match pytest.warn(). But we'll probably upset some people by doing this.

From a consistency point of view we'd want the message parameter, if it is to stay, to also match pytest.fail() and pytest.skip() I think however.

[nicoddemus]

Hi @flub,

One of the options would be to deprecate the parameter first, without introducing an alternative, and see if there's feedback (#3974 (comment)). If people speak up, we can proceed with keeping it deprecated but we can introduce it as a new name to avoid the problems presented here. What do you think of that?

[flub]

seems reasonable

…

On Sat, 15 Sep 2018 at 22:38, Bruno Oliveira ***@***.***> wrote: Hi @flub <https://github.com/flub>, One of the options would be to deprecate the parameter first, without introducing an alternative, and see if there's feedback (#3974 (comment) <#3974 (comment)>). If people speak up, we can proceed with keeping it deprecated but we can introduce it as a new name to avoid the problems presented here. What do you think of that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3974 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI9IE28EwLa9PsYzdO_FeAfGycmIr98ks5ubWUkgaJpZM4Wm5hN> .