Skip to content

Commit

Permalink
B017: don't warn when pytest.raises() has a match argument
Browse files Browse the repository at this point in the history
Fix PyCQA#334 .

Also add more tests for when we should and shouldn't emit B017.
In particular we should check ``assertRaises`` and ``pytest.raises``
calls outside of with statements.
  • Loading branch information
nsoranzo committed Jan 18, 2023
1 parent 4435548 commit 2cba141
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 21 deletions.
13 changes: 7 additions & 6 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,13 @@ waste CPU instructions. Either prepend ``assert`` or remove it.
**B016**: Cannot raise a literal. Did you intend to return it or raise
an Exception?

**B017**: ``self.assertRaises(Exception):`` should be considered evil. It can lead
to your test passing even if the code being tested is never executed due to a typo.
Either assert for a more specific exception (builtin or custom), use
``assertRaisesRegex``, or use the context manager form of assertRaises
(``with self.assertRaises(Exception) as ex:``) with an assertion against the
data available in ``ex``.
**B017**: ``assertRaises(Exception)`` and ``pytest.raises(Exception)`` should
be considered evil. They can lead to your test passing even if the
code being tested is never executed due to a typo. Assert for a more
specific exception (builtin or custom), or use ``assertRaisesRegex``
(if using ``assertRaises``), or add the ``match`` keyword argument (if
using ``pytest.raises``), or use the context manager form with a target
(e.g. ``with self.assertRaises(Exception) as ex:``).

**B018**: Found useless expression. Either assign it to a variable or remove it.

Expand Down
21 changes: 10 additions & 11 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -534,16 +534,14 @@ def check_for_b017(self, node):

if (
hasattr(item_context, "func")
and isinstance(item_context.func, ast.Attribute)
and (
(
hasattr(item_context.func, "attr")
and item_context.func.attr == "assertRaises"
)
item_context.func.attr == "assertRaises"
or (
isinstance(item_context.func, ast.Attribute)
and item_context.func.attr == "raises"
item_context.func.attr == "raises"
and isinstance(item_context.func.value, ast.Name)
and item_context.func.value.id == "pytest"
and "match" not in [kwd.arg for kwd in item_context.keywords]
)
)
and len(item_context.args) == 1
Expand Down Expand Up @@ -1411,11 +1409,12 @@ def visit_Lambda(self, node):
)
B017 = Error(
message=(
"B017 assertRaises(Exception): or pytest.raises(Exception) should "
"be considered evil. It can lead to your test passing even if the "
"code being tested is never executed due to a typo. Either assert "
"for a more specific exception (builtin or custom), use "
"assertRaisesRegex, or use the context manager form of assertRaises."
"B017 `assertRaises(Exception)` and `pytest.raises(Exception)` should "
"be considered evil. They can lead to your test passing even if the "
"code being tested is never executed due to a typo. Assert for a more "
"specific exception (builtin or custom), or use `assertRaisesRegex` "
"(if using `assertRaises`), or add the `match` keyword argument (if "
"using `pytest.raises`), or use the context manager form with a target."
)
)
B018 = Error(
Expand Down
15 changes: 12 additions & 3 deletions tests/b017.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""
Should emit:
B017 - on lines 24 and 26.
B017 - on lines 24, 26, 28, 31 and 32.
"""
import asyncio
import unittest
Expand All @@ -23,8 +23,13 @@ class Foobar(unittest.TestCase):
def evil_raises(self) -> None:
with self.assertRaises(Exception):
raise Exception("Evil I say!")
with self.assertRaises(Exception, msg="Generic exception"):
raise Exception("Evil I say!")
with pytest.raises(Exception):
raise Exception("Evil I say!")
# These are evil as well but we are only testing inside a with statement
self.assertRaises(Exception, lambda x, y: x / y, 1, y=0)
pytest.raises(Exception, lambda x, y: x / y, 1, y=0)

def context_manager_raises(self) -> None:
with self.assertRaises(Exception) as ex:
Expand All @@ -33,14 +38,18 @@ def context_manager_raises(self) -> None:
raise Exception("Context manager is good")

self.assertEqual("Context manager is good", str(ex.exception))
self.assertEqual("Context manager is good", str(pyt_ex.exception))
self.assertEqual("Context manager is good", str(pyt_ex.value))

def regex_raises(self) -> None:
with self.assertRaisesRegex(Exception, "Regex is good"):
raise Exception("Regex is good")
with pytest.raises(Exception, "Regex is good"):
with pytest.raises(Exception, match="Regex is good"):
raise Exception("Regex is good")

def non_context_manager_raises(self) -> None:
self.assertRaises(ZeroDivisionError, lambda x, y: x / y, 1, y=0)
pytest.raises(ZeroDivisionError, lambda x, y: x / y, 1, y=0)

def raises_with_absolute_reference(self):
with self.assertRaises(asyncio.CancelledError):
Foo()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ def test_b017(self):
filename = Path(__file__).absolute().parent / "b017.py"
bbc = BugBearChecker(filename=str(filename))
errors = list(bbc.run())
expected = self.errors(B017(24, 8), B017(26, 8))
expected = self.errors(B017(24, 8), B017(26, 8), B017(28, 8))
self.assertEqual(errors, expected)

def test_b018_functions(self):
Expand Down

0 comments on commit 2cba141

Please sign in to comment.