From faea273c930985074783703e7d37d88441acf30e Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 19 Nov 2019 08:24:08 -0800 Subject: [PATCH] Revert "A warning is now issued when assertions are made for `None`" --- changelog/4639.feature.rst | 3 +++ src/_pytest/assertion/rewrite.py | 31 ---------------------- testing/test_warnings.py | 44 +------------------------------- 3 files changed, 4 insertions(+), 74 deletions(-) create mode 100644 changelog/4639.feature.rst diff --git a/changelog/4639.feature.rst b/changelog/4639.feature.rst new file mode 100644 index 00000000000..f296f164925 --- /dev/null +++ b/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. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 51ea1801b72..25e726f2c31 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -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) @@ -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. diff --git a/testing/test_warnings.py b/testing/test_warnings.py index c4af14dac0e..922c4c36782 100644 --- a/testing/test_warnings.py +++ b/testing/test_warnings.py @@ -543,7 +543,7 @@ def assert_result_warns(result, msg): def test_tuple_warning(self, testdir): testdir.makepyfile( - """ + """\ def test_foo(): assert (1,2) """ @@ -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"""