diff --git a/README.rst b/README.rst index 22f7cb7..c5acb72 100644 --- a/README.rst +++ b/README.rst @@ -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. @@ -312,6 +313,7 @@ Future ~~~~~~~~~ * B906: Ignore ``visit_`` functions with a ``_fields`` attribute that can't contain ast.AST subnodes. (#330) +* B017: Don't warn when ``pytest.raises()`` has a ``match`` argument. (#334) 23.1.17 ~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index aad3557..26e89cc 100644 --- a/bugbear.py +++ b/bugbear.py @@ -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 @@ -1428,11 +1426,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( diff --git a/tests/b017.py b/tests/b017.py index 1994a60..9d508a9 100644 --- a/tests/b017.py +++ b/tests/b017.py @@ -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 @@ -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: @@ -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() diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index 0fdd24f..950efc5 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -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):