Revert "A warning is now issued when assertions are made for `None`"
This commit is contained in:
parent
d1e2d12b3f
commit
faea273c93
|
@ -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.
|
|
@ -799,13 +799,6 @@ class AssertionRewriter(ast.NodeVisitor):
|
||||||
self.push_format_context()
|
self.push_format_context()
|
||||||
# Rewrite assert into a bunch of statements.
|
# Rewrite assert into a bunch of statements.
|
||||||
top_condition, explanation = self.visit(assert_.test)
|
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)
|
negation = ast.UnaryOp(ast.Not(), top_condition)
|
||||||
|
|
||||||
|
@ -887,30 +880,6 @@ class AssertionRewriter(ast.NodeVisitor):
|
||||||
set_location(stmt, assert_.lineno, assert_.col_offset)
|
set_location(stmt, assert_.lineno, assert_.col_offset)
|
||||||
return self.statements
|
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):
|
def visit_Name(self, name):
|
||||||
# Display the repr of the name if it's a local variable or
|
# Display the repr of the name if it's a local variable or
|
||||||
# _should_repr_global_name() thinks it's acceptable.
|
# _should_repr_global_name() thinks it's acceptable.
|
||||||
|
|
|
@ -543,7 +543,7 @@ class TestAssertionWarnings:
|
||||||
|
|
||||||
def test_tuple_warning(self, testdir):
|
def test_tuple_warning(self, testdir):
|
||||||
testdir.makepyfile(
|
testdir.makepyfile(
|
||||||
"""
|
"""\
|
||||||
def test_foo():
|
def test_foo():
|
||||||
assert (1,2)
|
assert (1,2)
|
||||||
"""
|
"""
|
||||||
|
@ -553,48 +553,6 @@ class TestAssertionWarnings:
|
||||||
result, "assertion is always true, perhaps remove parentheses?"
|
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():
|
def test_warnings_checker_twice():
|
||||||
"""Issue #4617"""
|
"""Issue #4617"""
|
||||||
|
|
Loading…
Reference in New Issue