From 6b5152ae131ea2bd15f6c998de223b72b0c9e65d Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Sat, 13 Apr 2019 18:58:38 +0300 Subject: [PATCH 01/14] Sanity tests for loop unrolling --- testing/test_assertrewrite.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 72bfbcc55..c22ae7c6e 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -656,6 +656,12 @@ class TestAssertionRewrite(object): else: assert lines == ["assert 0 == 1\n + where 1 = \\n{ \\n~ \\n}.a"] + def test_all_unroll(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(object): From 765f75a8f1d38413fa85ae51f61e6be969aac9d5 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Sat, 13 Apr 2019 19:13:29 +0300 Subject: [PATCH 02/14] Replace asserts of `any` with an assert in a for --- src/_pytest/assertion/rewrite.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 18506d2e1..68ff2b3ef 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -964,6 +964,8 @@ warn_explicit( """ visit `ast.Call` nodes on Python3.5 and after """ + if call.func.id == "all": + return self.visit_all(call) new_func, func_expl = self.visit(call.func) arg_expls = [] new_args = [] @@ -987,6 +989,22 @@ warn_explicit( outer_expl = "%s\n{%s = %s\n}" % (res_expl, res_expl, expl) return res, outer_expl + def visit_all(self, call): + """Special rewrite for the builtin all function, see #5602""" + if not isinstance(call.args[0], ast.GeneratorExp): + return + gen_exp = call.args[0] + for_loop = ast.For( + iter=gen_exp.generators[0].iter, + target=gen_exp.generators[0].target, + body=[ + self.visit(ast.Assert(test=gen_exp.elt, lineno=1, msg="", col_offset=1)) + ], + ) + ast.fix_missing_locations(for_loop) + for_loop = ast.copy_location(for_loop, call) + return 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) From 470e686a70df788ddc76ed45607f7cea1e85ec71 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Sat, 13 Apr 2019 20:11:11 +0300 Subject: [PATCH 03/14] Rewrite unrolled assertion with a new rewriter,correctly append the unrolled for loop --- src/_pytest/assertion/rewrite.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 68ff2b3ef..adf252009 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -994,16 +994,21 @@ warn_explicit( if not isinstance(call.args[0], ast.GeneratorExp): return gen_exp = call.args[0] + assertion_module = ast.Module( + body=[ast.Assert(test=gen_exp.elt, lineno=1, msg="", col_offset=1)] + ) + AssertionRewriter(None, None).run(assertion_module) for_loop = ast.For( iter=gen_exp.generators[0].iter, target=gen_exp.generators[0].target, - body=[ - self.visit(ast.Assert(test=gen_exp.elt, lineno=1, msg="", col_offset=1)) - ], + body=assertion_module.body, + orelse=[], ) - ast.fix_missing_locations(for_loop) - for_loop = ast.copy_location(for_loop, call) - return for_loop, "" + 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 From ddbe733666d56b7c997bd901071f1608a0a90873 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Sat, 13 Apr 2019 20:57:05 +0300 Subject: [PATCH 04/14] Add changelog entry for 5062 --- changelog/5062.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/5062.feature.rst diff --git a/changelog/5062.feature.rst b/changelog/5062.feature.rst new file mode 100644 index 000000000..e311d161d --- /dev/null +++ b/changelog/5062.feature.rst @@ -0,0 +1 @@ +Unroll calls to ``all`` to full for-loops for better failure messages, especially when using Generator Expressions. From a0dbf2ab995780140ce335fb96e0f797c661764b Mon Sep 17 00:00:00 2001 From: danielx123 Date: Mon, 22 Apr 2019 17:58:07 -0400 Subject: [PATCH 05/14] Adding test cases for unrolling an iterable #5062 --- testing/test_assertrewrite.py | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index c22ae7c6e..8add65e0a 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -677,6 +677,53 @@ class TestAssertionRewrite(object): assert "UnicodeDecodeError" not in msg assert "UnicodeEncodeError" not in msg + def test_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_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(object): def test_pycache_is_a_file(self, testdir): From 0996f3dbc54d5b0d096e7341cb6718ffd16da100 Mon Sep 17 00:00:00 2001 From: danielx123 Date: Mon, 22 Apr 2019 18:23:51 -0400 Subject: [PATCH 06/14] Displaying pip list command's packages and versions #5062 --- src/_pytest/terminal.py | 6 ++++++ testing/test_terminal.py | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 2d7132259..98cbaa571 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -9,6 +9,7 @@ from __future__ import print_function import argparse import collections import platform +import subprocess import sys import time from functools import partial @@ -581,6 +582,11 @@ class TerminalReporter(object): ): msg += " -- " + str(sys.executable) self.write_line(msg) + pipe = subprocess.Popen("pip list", shell=True, stdout=subprocess.PIPE).stdout + package_msg = pipe.read() + package_msg = package_msg[:-2] + if package_msg: + self.write_line(package_msg) lines = self.config.hook.pytest_report_header( config=self.config, startdir=self.startdir ) diff --git a/testing/test_terminal.py b/testing/test_terminal.py index feacc242d..bfa928d74 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -279,6 +279,15 @@ class TestTerminal(object): tr.rewrite("hey", erase=True) assert f.getvalue() == "hello" + "\r" + "hey" + (6 * " ") + def test_packages_display(self, testdir): + testdir.makepyfile( + """ + def test_pass(num): + assert 1 == 1""" + ) + result = testdir.runpytest() + result.stdout.fnmatch_lines(["*Package*", "*Version*"]) + class TestCollectonly(object): def test_collectonly_basic(self, testdir): From c607697400affcc0e50e51baa329aec81ba3cf7b Mon Sep 17 00:00:00 2001 From: danielx123 Date: Mon, 22 Apr 2019 21:29:08 -0400 Subject: [PATCH 07/14] Fixed test case --- testing/test_terminal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/test_terminal.py b/testing/test_terminal.py index bfa928d74..0b57114e5 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -286,7 +286,7 @@ class TestTerminal(object): assert 1 == 1""" ) result = testdir.runpytest() - result.stdout.fnmatch_lines(["*Package*", "*Version*"]) + result.stdout.fnmatch_lines(["*Package Version *"]) class TestCollectonly(object): From ecd2de25a122cd79f2e9b37b267fe883272383c6 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Thu, 9 May 2019 17:39:24 +0300 Subject: [PATCH 08/14] Revert "Displaying pip list command's packages and versions #5062" This reverts commit 043fdb7c4065e5eb54f3fb5776077bb8fd651ce6. These tests were part of the PR #5155 but weren't relevant to #5602 --- src/_pytest/terminal.py | 6 ------ testing/test_terminal.py | 9 --------- 2 files changed, 15 deletions(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 98cbaa571..2d7132259 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -9,7 +9,6 @@ from __future__ import print_function import argparse import collections import platform -import subprocess import sys import time from functools import partial @@ -582,11 +581,6 @@ class TerminalReporter(object): ): msg += " -- " + str(sys.executable) self.write_line(msg) - pipe = subprocess.Popen("pip list", shell=True, stdout=subprocess.PIPE).stdout - package_msg = pipe.read() - package_msg = package_msg[:-2] - if package_msg: - self.write_line(package_msg) lines = self.config.hook.pytest_report_header( config=self.config, startdir=self.startdir ) diff --git a/testing/test_terminal.py b/testing/test_terminal.py index 0b57114e5..feacc242d 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -279,15 +279,6 @@ class TestTerminal(object): tr.rewrite("hey", erase=True) assert f.getvalue() == "hello" + "\r" + "hey" + (6 * " ") - def test_packages_display(self, testdir): - testdir.makepyfile( - """ - def test_pass(num): - assert 1 == 1""" - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines(["*Package Version *"]) - class TestCollectonly(object): def test_collectonly_basic(self, testdir): From e37ff3042e711dd930ed8cd315b3b93340546a25 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Thu, 9 May 2019 17:50:41 +0300 Subject: [PATCH 09/14] Check calls to all only if it's a name and not an attribute --- src/_pytest/assertion/rewrite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index adf252009..0a3713285 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -964,7 +964,7 @@ warn_explicit( """ visit `ast.Call` nodes on Python3.5 and after """ - if call.func.id == "all": + 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 = [] From 437d6452c1a37ef1412adb3fd79bc844f9d05c3a Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Thu, 9 May 2019 18:51:03 +0300 Subject: [PATCH 10/14] Expand list comprehensions as well --- src/_pytest/assertion/rewrite.py | 4 ++-- testing/test_assertrewrite.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 0a3713285..0e988a8f4 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -991,13 +991,13 @@ warn_explicit( def visit_all(self, call): """Special rewrite for the builtin all function, see #5602""" - if not isinstance(call.args[0], ast.GeneratorExp): + 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(None, None).run(assertion_module) + 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, diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 8add65e0a..c453fe57d 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -677,7 +677,7 @@ class TestAssertionRewrite(object): assert "UnicodeDecodeError" not in msg assert "UnicodeEncodeError" not in msg - def test_generator(self, testdir): + def test_unroll_generator(self, testdir): testdir.makepyfile( """ def check_even(num): @@ -692,7 +692,7 @@ class TestAssertionRewrite(object): result = testdir.runpytest() result.stdout.fnmatch_lines(["*assert False*", "*where False = check_even(1)*"]) - def test_list_comprehension(self, testdir): + def test_unroll_list_comprehension(self, testdir): testdir.makepyfile( """ def check_even(num): From 852fb6a4ae93c68bd03f36ca4daf7e9642d8e2d2 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Thu, 9 May 2019 19:02:50 +0300 Subject: [PATCH 11/14] Change basic test case to be consistent with existing assertion rewriting The code ``` x = 0 assert x == 1 ``` will give the failure message 0 == 1, so it shouldn't be different as part of an unroll --- testing/test_assertrewrite.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index c453fe57d..e819075b5 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -656,11 +656,11 @@ class TestAssertionRewrite(object): else: assert lines == ["assert 0 == 1\n + where 1 = \\n{ \\n~ \\n}.a"] - def test_all_unroll(self): + def test_unroll_expression(self): def f(): assert all(x == 1 for x in range(10)) - assert "0 != 1" in getmsg(f) + assert "0 == 1" in getmsg(f) def test_custom_repr_non_ascii(self): def f(): From 58149459a59c7b7d2f34e7291482d74e39d8ad69 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Thu, 9 May 2019 19:08:10 +0300 Subject: [PATCH 12/14] Mark visit_all as a private method --- src/_pytest/assertion/rewrite.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 0e988a8f4..bb3d36a9c 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -965,7 +965,7 @@ warn_explicit( visit `ast.Call` nodes on Python3.5 and after """ if isinstance(call.func, ast.Name) and call.func.id == "all": - return self.visit_all(call) + return self._visit_all(call) new_func, func_expl = self.visit(call.func) arg_expls = [] new_args = [] @@ -989,7 +989,7 @@ warn_explicit( outer_expl = "%s\n{%s = %s\n}" % (res_expl, res_expl, expl) return res, outer_expl - def visit_all(self, call): + def _visit_all(self, call): """Special rewrite for the builtin all function, see #5602""" if not isinstance(call.args[0], (ast.GeneratorExp, ast.ListComp)): return From 322a0f0a331494746c20a56f7776aec1f08d9240 Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Thu, 9 May 2019 19:12:58 +0300 Subject: [PATCH 13/14] Fix mention of issue #5062 in docstrings --- src/_pytest/assertion/rewrite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index bb3d36a9c..5690e0e33 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -990,7 +990,7 @@ warn_explicit( return res, outer_expl def _visit_all(self, call): - """Special rewrite for the builtin all function, see #5602""" + """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] From 22d91a3c3a248761506a38fdf26c859b341e82be Mon Sep 17 00:00:00 2001 From: Tomer Keren Date: Sat, 18 May 2019 14:27:47 +0300 Subject: [PATCH 14/14] Unroll calls to all on python 2 --- src/_pytest/assertion/rewrite.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 5690e0e33..e3870d435 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -1020,6 +1020,8 @@ warn_explicit( """ visit `ast.Call nodes on 3.4 and below` """ + 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 = []