Skip to content
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

Add B017 support for pytest.raises #317

Merged
merged 2 commits into from
Dec 11, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ def check_for_b016(self, node):

def check_for_b017(self, node):
"""Checks for use of the evil syntax 'with assertRaises(Exception):'
or 'with pytest.raises(Exception)'.

This form of assertRaises will catch everything that subclasses
Exception, which happens to be the vast majority of Python internal
Expand All @@ -518,10 +519,21 @@ def check_for_b017(self, node):
"""
item = node.items[0]
item_context = item.context_expr

if (
hasattr(item_context, "func")
and hasattr(item_context.func, "attr")
and item_context.func.attr == "assertRaises"
and (
(
hasattr(item_context.func, "attr")
and item_context.func.attr == "assertRaises"
)
or (
isinstance(item_context.func, ast.Attribute)
and item_context.func.attr == "raises"
and isinstance(item_context.func.value, ast.Name)
and item_context.func.value.id == "pytest"
)
)
and len(item_context.args) == 1
and isinstance(item_context.args[0], ast.Name)
and item_context.args[0].id == "Exception"
Expand Down Expand Up @@ -1257,11 +1269,11 @@ def visit_Lambda(self, node):
)
B017 = Error(
message=(
"B017 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."
"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."
)
)
B018 = Error(
Expand Down
14 changes: 13 additions & 1 deletion tests/b017.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
"""
Should emit:
B017 - on lines 20
B017 - on lines 24 and 26.
"""
import asyncio
import unittest

import pytest

Comment on lines +8 to +9
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This confuses me how this works as we don't have pytest named as a dependency anywhere ...

Should we add it to dependencies or the dev optional dependencies now too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Not sure how this is being found. Agreed on adding to dev optional dependencies, though it would be good to know where it's currently being taken from.

CONSTANT = True


Expand All @@ -21,16 +23,26 @@ class Foobar(unittest.TestCase):
def evil_raises(self) -> None:
with self.assertRaises(Exception):
raise Exception("Evil I say!")
with pytest.raises(Exception):
raise Exception("Evil I say!")

def context_manager_raises(self) -> None:
with self.assertRaises(Exception) as ex:
raise Exception("Context manager is good")
with pytest.raises(Exception) as pyt_ex:
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))

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

def raises_with_absolute_reference(self):
with self.assertRaises(asyncio.CancelledError):
Foo()
with pytest.raises(asyncio.CancelledError):
Foo()
2 changes: 1 addition & 1 deletion tests/test_bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,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(22, 8))
expected = self.errors(B017(24, 8), B017(26, 8))
self.assertEqual(errors, expected)

def test_b018_functions(self):
Expand Down