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

Revert "A warning is now issued when assertions are made for None" #6234

Merged
merged 1 commit into from Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/4639.feature.rst
@@ -0,0 +1,3 @@
Revert "A warning is now issued when assertions are made for ``None``".
The warning proved to be less useful than initially expected and had quite a
few false positive cases.
31 changes: 0 additions & 31 deletions src/_pytest/assertion/rewrite.py
Expand Up @@ -799,13 +799,6 @@ def visit_Assert(self, assert_):
self.push_format_context()
# Rewrite assert into a bunch of statements.
top_condition, explanation = self.visit(assert_.test)
# If in a test module, check if directly asserting None, in order to warn [Issue #3191]
if self.module_path is not None:
self.statements.append(
self.warn_about_none_ast(
top_condition, module_path=self.module_path, lineno=assert_.lineno
)
)

negation = ast.UnaryOp(ast.Not(), top_condition)

Expand Down Expand Up @@ -887,30 +880,6 @@ def visit_Assert(self, assert_):
set_location(stmt, assert_.lineno, assert_.col_offset)
return self.statements

def warn_about_none_ast(self, node, module_path, lineno):
"""
Returns an AST issuing a warning if the value of node is `None`.
This is used to warn the user when asserting a function that asserts
internally already.
See issue #3191 for more details.
"""
val_is_none = ast.Compare(node, [ast.Is()], [ast.NameConstant(None)])
send_warning = ast.parse(
"""\
from _pytest.warning_types import PytestAssertRewriteWarning
from warnings import warn_explicit
warn_explicit(
PytestAssertRewriteWarning('asserting the value None, please use "assert is None"'),
category=None,
filename={filename!r},
lineno={lineno},
)
""".format(
filename=fspath(module_path), lineno=lineno
)
).body
return ast.If(val_is_none, send_warning, [])

def visit_Name(self, name):
# Display the repr of the name if it's a local variable or
# _should_repr_global_name() thinks it's acceptable.
Expand Down
44 changes: 1 addition & 43 deletions testing/test_warnings.py
Expand Up @@ -543,7 +543,7 @@ def assert_result_warns(result, msg):

def test_tuple_warning(self, testdir):
testdir.makepyfile(
"""
"""\
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not change code unnecessary, especially when it is only to please your editor in some way (IIRC).
For all others this adds visual disbalance, and often gets highlighted in a special way (line continuation).

Copy link
Contributor

Choose a reason for hiding this comment

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

?

def test_foo():
assert (1,2)
"""
Expand All @@ -553,48 +553,6 @@ def test_foo():
result, "assertion is always true, perhaps remove parentheses?"
)

@staticmethod
def create_file(testdir, return_none):
testdir.makepyfile(
"""
def foo(return_none):
if return_none:
return None
else:
return False

def test_foo():
assert foo({return_none})
""".format(
return_none=return_none
)
)

def test_none_function_warns(self, testdir):
self.create_file(testdir, True)
result = testdir.runpytest()
self.assert_result_warns(
result, 'asserting the value None, please use "assert is None"'
)

def test_assert_is_none_no_warn(self, testdir):
testdir.makepyfile(
"""
def foo():
return None

def test_foo():
assert foo() is None
"""
)
result = testdir.runpytest()
result.stdout.fnmatch_lines(["*1 passed in*"])

def test_false_function_no_warn(self, testdir):
self.create_file(testdir, False)
result = testdir.runpytest()
result.stdout.fnmatch_lines(["*1 failed in*"])


def test_warnings_checker_twice():
"""Issue #4617"""
Expand Down