From 2125d04501e00991f39460579f6415a239a43c4f Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 3 Jun 2019 08:34:25 -0700 Subject: [PATCH 1/3] Revert "Fix all() unroll for non-generators/non-list comprehensions (#5360)" This reverts commit 733f43b02eafe2934c2e86b7d0370e25dfe95a48, reversing changes made to e4fe41ebb754b3779a1535d43119ef3492ed6dcf. --- changelog/5358.bugfix.rst | 1 - src/_pytest/assertion/rewrite.py | 14 +++----------- testing/test_assertrewrite.py | 29 ++--------------------------- 3 files changed, 5 insertions(+), 39 deletions(-) delete mode 100644 changelog/5358.bugfix.rst diff --git a/changelog/5358.bugfix.rst b/changelog/5358.bugfix.rst deleted file mode 100644 index 181da1e0e..000000000 --- a/changelog/5358.bugfix.rst +++ /dev/null @@ -1 +0,0 @@ -Fix assertion rewriting of ``all()`` calls to deal with non-generators. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 9b431b984..27dcb58ee 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -903,21 +903,11 @@ warn_explicit( res = self.assign(ast.BinOp(left_expr, binop.op, right_expr)) return res, explanation - @staticmethod - def _is_any_call_with_generator_or_list_comprehension(call): - """Return True if the Call node is an 'any' call with a generator or list comprehension""" - return ( - isinstance(call.func, ast.Name) - and call.func.id == "all" - and len(call.args) == 1 - and isinstance(call.args[0], (ast.GeneratorExp, ast.ListComp)) - ) - def visit_Call(self, call): """ visit `ast.Call` nodes """ - if self._is_any_call_with_generator_or_list_comprehension(call): + if isinstance(call.func, ast.Name) and call.func.id == "all": return self._visit_all(call) new_func, func_expl = self.visit(call.func) arg_expls = [] @@ -944,6 +934,8 @@ warn_explicit( def _visit_all(self, call): """Special rewrite for the builtin all function, see #5062""" + if not isinstance(call.args[0], (ast.GeneratorExp, ast.ListComp)): + return gen_exp = call.args[0] assertion_module = ast.Module( body=[ast.Assert(test=gen_exp.elt, lineno=1, msg="", col_offset=1)] diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 6720a790f..1c309f50c 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -656,7 +656,7 @@ class TestAssertionRewrite: assert "UnicodeDecodeError" not in msg assert "UnicodeEncodeError" not in msg - def test_unroll_all_generator(self, testdir): + def test_unroll_generator(self, testdir): testdir.makepyfile( """ def check_even(num): @@ -671,7 +671,7 @@ class TestAssertionRewrite: result = testdir.runpytest() result.stdout.fnmatch_lines(["*assert False*", "*where False = check_even(1)*"]) - def test_unroll_all_list_comprehension(self, testdir): + def test_unroll_list_comprehension(self, testdir): testdir.makepyfile( """ def check_even(num): @@ -686,31 +686,6 @@ class TestAssertionRewrite: result = testdir.runpytest() result.stdout.fnmatch_lines(["*assert False*", "*where False = check_even(1)*"]) - def test_unroll_all_object(self, testdir): - """all() for non generators/non list-comprehensions (#5358)""" - testdir.makepyfile( - """ - def test(): - assert all((1, 0)) - """ - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines(["*assert False*", "*where False = all((1, 0))*"]) - - def test_unroll_all_starred(self, testdir): - """all() for non generators/non list-comprehensions (#5358)""" - testdir.makepyfile( - """ - def test(): - x = ((1, 0),) - assert all(*x) - """ - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines( - ["*assert False*", "*where False = all(*((1, 0),))*"] - ) - def test_for_loop(self, testdir): testdir.makepyfile( """ From 1b381d5277353082f626763210f418293bfe5479 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 3 Jun 2019 08:35:56 -0700 Subject: [PATCH 2/3] Revert "Unroll calls to any #5062 (#5103)" This reverts commit 2b9ca342809a4e05b4451a074d644c14acda844c, reversing changes made to 0a57124063a4ed63f85fb8a39dc57f6d6fc5b132. --- src/_pytest/assertion/rewrite.py | 23 -------------- testing/test_assertrewrite.py | 53 -------------------------------- 2 files changed, 76 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 27dcb58ee..ac9b85e67 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -907,8 +907,6 @@ warn_explicit( """ visit `ast.Call` nodes """ - if isinstance(call.func, ast.Name) and call.func.id == "all": - return self._visit_all(call) new_func, func_expl = self.visit(call.func) arg_expls = [] new_args = [] @@ -932,27 +930,6 @@ warn_explicit( outer_expl = "{}\n{{{} = {}\n}}".format(res_expl, res_expl, expl) return res, outer_expl - def _visit_all(self, call): - """Special rewrite for the builtin all function, see #5062""" - if not isinstance(call.args[0], (ast.GeneratorExp, ast.ListComp)): - return - gen_exp = call.args[0] - assertion_module = ast.Module( - body=[ast.Assert(test=gen_exp.elt, lineno=1, msg="", col_offset=1)] - ) - AssertionRewriter(module_path=None, config=None).run(assertion_module) - for_loop = ast.For( - iter=gen_exp.generators[0].iter, - target=gen_exp.generators[0].target, - body=assertion_module.body, - orelse=[], - ) - self.statements.append(for_loop) - return ( - ast.Num(n=1), - "", - ) # Return an empty expression, all the asserts are in the for_loop - def visit_Starred(self, starred): # From Python 3.5, a Starred node can appear in a function call res, expl = self.visit(starred.value) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 1c309f50c..61c5b9383 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -635,12 +635,6 @@ class TestAssertionRewrite: else: assert lines == ["assert 0 == 1\n + where 1 = \\n{ \\n~ \\n}.a"] - def test_unroll_expression(self): - def f(): - assert all(x == 1 for x in range(10)) - - assert "0 == 1" in getmsg(f) - def test_custom_repr_non_ascii(self): def f(): class A: @@ -656,53 +650,6 @@ class TestAssertionRewrite: assert "UnicodeDecodeError" not in msg assert "UnicodeEncodeError" not in msg - def test_unroll_generator(self, testdir): - testdir.makepyfile( - """ - def check_even(num): - if num % 2 == 0: - return True - return False - - def test_generator(): - odd_list = list(range(1,9,2)) - assert all(check_even(num) for num in odd_list)""" - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines(["*assert False*", "*where False = check_even(1)*"]) - - def test_unroll_list_comprehension(self, testdir): - testdir.makepyfile( - """ - def check_even(num): - if num % 2 == 0: - return True - return False - - def test_list_comprehension(): - odd_list = list(range(1,9,2)) - assert all([check_even(num) for num in odd_list])""" - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines(["*assert False*", "*where False = check_even(1)*"]) - - def test_for_loop(self, testdir): - testdir.makepyfile( - """ - def check_even(num): - if num % 2 == 0: - return True - return False - - def test_for_loop(): - odd_list = list(range(1,9,2)) - for num in odd_list: - assert check_even(num) - """ - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines(["*assert False*", "*where False = check_even(1)*"]) - class TestRewriteOnImport: def test_pycache_is_a_file(self, testdir): From 230f736fcdeca96107e58407214092d4ad9dc3b9 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 3 Jun 2019 08:38:42 -0700 Subject: [PATCH 3/3] Add changelog entries for reverting all() handling --- changelog/5370.bugfix.rst | 1 + changelog/5371.bugfix.rst | 1 + changelog/5372.bugfix.rst | 1 + 3 files changed, 3 insertions(+) create mode 100644 changelog/5370.bugfix.rst create mode 100644 changelog/5371.bugfix.rst create mode 100644 changelog/5372.bugfix.rst diff --git a/changelog/5370.bugfix.rst b/changelog/5370.bugfix.rst new file mode 100644 index 000000000..70def0d27 --- /dev/null +++ b/changelog/5370.bugfix.rst @@ -0,0 +1 @@ +Revert unrolling of ``all()`` to fix ``NameError`` on nested comprehensions. diff --git a/changelog/5371.bugfix.rst b/changelog/5371.bugfix.rst new file mode 100644 index 000000000..46ff5c890 --- /dev/null +++ b/changelog/5371.bugfix.rst @@ -0,0 +1 @@ +Revert unrolling of ``all()`` to fix incorrect handling of generators with ``if``. diff --git a/changelog/5372.bugfix.rst b/changelog/5372.bugfix.rst new file mode 100644 index 000000000..e9b644db2 --- /dev/null +++ b/changelog/5372.bugfix.rst @@ -0,0 +1 @@ +Revert unrolling of ``all()`` to fix incorrect assertion when using ``all()`` in an expression.