-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Introduce pytest.not_raises #2339
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -154,6 +154,75 @@ x=1/y=3. | |
comma-separated-string syntax is now advertised first because | ||
it's easier to write and produces less line noise. | ||
|
||
|
||
``pytest.mark.parametrize`` and exception expectations | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
.. versionadded:: 3.1 | ||
|
||
Sometimes you might want to make sure a function raises an exception for | ||
a set of inputs but should not raise anything for another set of inputs. | ||
|
||
For example, consider this function: | ||
|
||
.. code-block:: python | ||
|
||
def check_python_identifier(ident): | ||
"""raise ValueError if ``ident`` is not a valid Python identifier.""" | ||
|
||
|
||
This is a natural job for a ``pytest.mark.parametrize``, so you might write | ||
a test like this: | ||
|
||
.. code-block:: python | ||
|
||
@pytest.mark.parametrize('ident, valid', [ | ||
('foobar', True), | ||
('Foobar1', True), | ||
('Foobar_', True), | ||
('Foo_bar_', True), | ||
('Foo bar_', False), | ||
('foo bar_', False), | ||
('1foo bar_', False), | ||
]) | ||
def test_check_python_identifier(ident, valid): | ||
if not valid: | ||
with pytest.raises(ValueError): | ||
check_python_identifier(ident) | ||
else: | ||
check_python_identifier(ident) # should not raise | ||
|
||
|
||
But this makes us repeat ourselves on the test function. | ||
|
||
For these situations, where you need to ``parametrize`` a test function | ||
so that sometimes it raises an exception and others do not, you can use | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "and others do not" -> "and sometimes doesn't"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, that is better! 👍 |
||
the convenient ``pytest.not_raises`` helper: | ||
|
||
.. code-block:: python | ||
|
||
@pytest.mark.parametrize('ident, valid', [ | ||
('foobar', pytest.not_raises()), | ||
('Foobar1', pytest.not_raises()), | ||
('Foobar_', pytest.not_raises()), | ||
('Foo_bar_', pytest.not_raises()), | ||
('Foo bar_', pytest.raises(ValueError)), | ||
('foo bar_', pytest.raises(ValueError)), | ||
('1foo bar_', pytest.raises(ValueError)), | ||
]) | ||
def test_check_python_identifier(ident, expectation): | ||
with pytest.raises(expectation): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't either the function signature be Also, shouldn't the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @wcooley but I'm declining this PR for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nicoddemus So I noticed after finding & reading over the mailing list threads... For the sake of posterity for anyone finding this PR and wondering what to do, (I believe but have not actually tried) this can be achieved with a simple no-op context manager:
Then:
|
||
check_python_identifier(ident) | ||
|
||
How does it work? ``pytest.not_raises`` is a context-manager which **does nothing**. It is merely | ||
a convenience helper to use in conjunction with ``parametrize`` for testing that functions | ||
raise errors sometimes and other times do not. | ||
|
||
.. note:: | ||
Do not use ``pytest.not_raises()`` in a non-parametrized test just to | ||
check if some code does not raise something; just let the original code propagate | ||
any exception to make the test fail. | ||
|
||
.. _`pytest_generate_tests`: | ||
|
||
Basic ``pytest_generate_tests`` example | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have a strong opinion on naming it
wont_raise
in order to give acroos the semantics betterUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like
not_raises
due to its similarity toraises
, but I don't have a strong opinion on it. @The-Compiler do you mind changing towont_raise
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both sound weird, so no strong opinion here 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, an additonal note, why do we make it a callable?
i believe making it a singleton context-manager named after exactly the context its going to encompass is the best we can do,
else we might see people wanting to creep in more specific behavior causing grief down the line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It felt more natural, because you can use
pytest.raises(...)
. No problem changing it to a singleton if you feel it is more appropriate.What do you mean? People asking to some weird parameter to it to give additional behavior? Can't we just say no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About
wont_raise
... I'm having some second thoughts that I would like you to consider @RonnyPfannschmidt:pytest.raises
maps well toassert raises
, and so doespytest.not_raises
that also maps well toassert not raises
.pytest.wont_raises
is a contraction ofwill not
, which is not so nice in general;Hmm on second thought I think we should ask others for their opinion on this. It is important to nail this down right now than regret it later. I'll send an email to the list! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicoddemus wrt feature creep, sometimes it just happens, see the major mess non-strict xfail got us into because we did a too cheap job of handling flaky tests better
we should either clearly limit it, or clearly support passing exception types from the get go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I don't have a strong objection here.
Also asked in the mailing list if anyone sees a problem in making this a singleton. 👍