Skip to content

Commit e3fe79b

Browse files
authored
B017: don't warn when pytest.raises() has a match argument (#337)
Fix #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.
1 parent e7137ec commit e3fe79b

File tree

4 files changed

+31
-21
lines changed

4 files changed

+31
-21
lines changed

Diff for: README.rst

+8-6
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,13 @@ waste CPU instructions. Either prepend ``assert`` or remove it.
127127
**B016**: Cannot raise a literal. Did you intend to return it or raise
128128
an Exception?
129129

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

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

@@ -312,6 +313,7 @@ Future
312313
~~~~~~~~~
313314

314315
* B906: Ignore ``visit_`` functions with a ``_fields`` attribute that can't contain ast.AST subnodes. (#330)
316+
* B017: Don't warn when ``pytest.raises()`` has a ``match`` argument. (#334)
315317

316318
23.1.17
317319
~~~~~~~~~

Diff for: bugbear.py

+10-11
Original file line numberDiff line numberDiff line change
@@ -534,16 +534,14 @@ def check_for_b017(self, node):
534534

535535
if (
536536
hasattr(item_context, "func")
537+
and isinstance(item_context.func, ast.Attribute)
537538
and (
538-
(
539-
hasattr(item_context.func, "attr")
540-
and item_context.func.attr == "assertRaises"
541-
)
539+
item_context.func.attr == "assertRaises"
542540
or (
543-
isinstance(item_context.func, ast.Attribute)
544-
and item_context.func.attr == "raises"
541+
item_context.func.attr == "raises"
545542
and isinstance(item_context.func.value, ast.Name)
546543
and item_context.func.value.id == "pytest"
544+
and "match" not in [kwd.arg for kwd in item_context.keywords]
547545
)
548546
)
549547
and len(item_context.args) == 1
@@ -1428,11 +1426,12 @@ def visit_Lambda(self, node):
14281426
)
14291427
B017 = Error(
14301428
message=(
1431-
"B017 assertRaises(Exception): or pytest.raises(Exception) should "
1432-
"be considered evil. It can lead to your test passing even if the "
1433-
"code being tested is never executed due to a typo. Either assert "
1434-
"for a more specific exception (builtin or custom), use "
1435-
"assertRaisesRegex, or use the context manager form of assertRaises."
1429+
"B017 `assertRaises(Exception)` and `pytest.raises(Exception)` should "
1430+
"be considered evil. They can lead to your test passing even if the "
1431+
"code being tested is never executed due to a typo. Assert for a more "
1432+
"specific exception (builtin or custom), or use `assertRaisesRegex` "
1433+
"(if using `assertRaises`), or add the `match` keyword argument (if "
1434+
"using `pytest.raises`), or use the context manager form with a target."
14361435
)
14371436
)
14381437
B018 = Error(

Diff for: tests/b017.py

+12-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""
22
Should emit:
3-
B017 - on lines 24 and 26.
3+
B017 - on lines 24, 26, 28, 31 and 32.
44
"""
55
import asyncio
66
import unittest
@@ -23,8 +23,13 @@ class Foobar(unittest.TestCase):
2323
def evil_raises(self) -> None:
2424
with self.assertRaises(Exception):
2525
raise Exception("Evil I say!")
26+
with self.assertRaises(Exception, msg="Generic exception"):
27+
raise Exception("Evil I say!")
2628
with pytest.raises(Exception):
2729
raise Exception("Evil I say!")
30+
# These are evil as well but we are only testing inside a with statement
31+
self.assertRaises(Exception, lambda x, y: x / y, 1, y=0)
32+
pytest.raises(Exception, lambda x, y: x / y, 1, y=0)
2833

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

3540
self.assertEqual("Context manager is good", str(ex.exception))
36-
self.assertEqual("Context manager is good", str(pyt_ex.exception))
41+
self.assertEqual("Context manager is good", str(pyt_ex.value))
3742

3843
def regex_raises(self) -> None:
3944
with self.assertRaisesRegex(Exception, "Regex is good"):
4045
raise Exception("Regex is good")
41-
with pytest.raises(Exception, "Regex is good"):
46+
with pytest.raises(Exception, match="Regex is good"):
4247
raise Exception("Regex is good")
4348

49+
def non_context_manager_raises(self) -> None:
50+
self.assertRaises(ZeroDivisionError, lambda x, y: x / y, 1, y=0)
51+
pytest.raises(ZeroDivisionError, lambda x, y: x / y, 1, y=0)
52+
4453
def raises_with_absolute_reference(self):
4554
with self.assertRaises(asyncio.CancelledError):
4655
Foo()

Diff for: tests/test_bugbear.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ def test_b017(self):
254254
filename = Path(__file__).absolute().parent / "b017.py"
255255
bbc = BugBearChecker(filename=str(filename))
256256
errors = list(bbc.run())
257-
expected = self.errors(B017(24, 8), B017(26, 8))
257+
expected = self.errors(B017(24, 8), B017(26, 8), B017(28, 8))
258258
self.assertEqual(errors, expected)
259259

260260
def test_b018_functions(self):

0 commit comments

Comments
 (0)