From 9a89783fbb8e17984cf7c9ab8cb83afcb8a9f715 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 24 Jun 2019 16:09:39 +0200 Subject: [PATCH 01/24] Assertion passed hook --- changelog/3457.feature.rst | 2 + setup.py | 1 + src/_pytest/assertion/__init__.py | 8 ++- src/_pytest/assertion/rewrite.py | 89 +++++++++++++++++++++-------- src/_pytest/assertion/util.py | 4 ++ src/_pytest/hookspec.py | 15 +++++ testing/acceptance_test.py | 45 --------------- testing/fixture_values_leak_test.py | 53 +++++++++++++++++ testing/python/raises.py | 3 + testing/test_assertrewrite.py | 51 +++++++++++++++++ 10 files changed, 202 insertions(+), 69 deletions(-) create mode 100644 changelog/3457.feature.rst create mode 100644 testing/fixture_values_leak_test.py diff --git a/changelog/3457.feature.rst b/changelog/3457.feature.rst new file mode 100644 index 00000000000..3f676514431 --- /dev/null +++ b/changelog/3457.feature.rst @@ -0,0 +1,2 @@ +Adds ``pytest_assertion_pass`` hook, called with assertion context information +(original asssertion statement and pytest explanation) whenever an assertion passes. diff --git a/setup.py b/setup.py index 4c87c6429bb..7d953281611 100644 --- a/setup.py +++ b/setup.py @@ -13,6 +13,7 @@ "pluggy>=0.12,<1.0", "importlib-metadata>=0.12", "wcwidth", + "astor", ] diff --git a/src/_pytest/assertion/__init__.py b/src/_pytest/assertion/__init__.py index e52101c9f16..b59b1bfdffc 100644 --- a/src/_pytest/assertion/__init__.py +++ b/src/_pytest/assertion/__init__.py @@ -92,7 +92,7 @@ def pytest_collection(session): def pytest_runtest_setup(item): - """Setup the pytest_assertrepr_compare hook + """Setup the pytest_assertrepr_compare and pytest_assertion_pass hooks The newinterpret and rewrite modules will use util._reprcompare if it exists to use custom reporting via the @@ -129,9 +129,15 @@ def callbinrepr(op, left, right): util._reprcompare = callbinrepr + if item.ihook.pytest_assertion_pass.get_hookimpls(): + def call_assertion_pass_hook(lineno, expl, orig): + item.ihook.pytest_assertion_pass(item=item, lineno=lineno, orig=orig, expl=expl) + util._assertion_pass = call_assertion_pass_hook + def pytest_runtest_teardown(item): util._reprcompare = None + util._assertion_pass = None def pytest_sessionfinish(session): diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index ce698f368f2..77dab5242e9 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -1,5 +1,6 @@ """Rewrite assertion AST to produce nice error messages""" import ast +import astor import errno import imp import itertools @@ -357,6 +358,11 @@ def _rewrite_test(config, fn): state.trace("failed to parse: {!r}".format(fn)) return None, None rewrite_asserts(tree, fn, config) + + # TODO: REMOVE, THIS IS ONLY FOR DEBUG + with open(f'{str(fn)+"bak"}', "w", encoding="utf-8") as f: + f.write(astor.to_source(tree)) + try: co = compile(tree, fn.strpath, "exec", dont_inherit=True) except SyntaxError: @@ -434,7 +440,7 @@ def _format_assertmsg(obj): # contains a newline it gets escaped, however if an object has a # .__repr__() which contains newlines it does not get escaped. # However in either case we want to preserve the newline. - replaces = [("\n", "\n~"), ("%", "%%")] + replaces = [("\n", "\n~")] if not isinstance(obj, str): obj = saferepr(obj) replaces.append(("\\n", "\n~")) @@ -478,6 +484,17 @@ def _call_reprcompare(ops, results, expls, each_obj): return expl +def _call_assertion_pass(lineno, orig, expl): + if util._assertion_pass is not None: + util._assertion_pass(lineno=lineno, orig=orig, expl=expl) + + +def _check_if_assertionpass_impl(): + """Checks if any plugins implement the pytest_assertion_pass hook + in order not to generate explanation unecessarily (might be expensive)""" + return True if util._assertion_pass else False + + unary_map = {ast.Not: "not %s", ast.Invert: "~%s", ast.USub: "-%s", ast.UAdd: "+%s"} binop_map = { @@ -550,7 +567,8 @@ class AssertionRewriter(ast.NodeVisitor): original assert statement: it rewrites the test of an assertion to provide intermediate values and replace it with an if statement which raises an assertion error with a detailed explanation in - case the expression is false. + case the expression is false and calls pytest_assertion_pass hook + if expression is true. For this .visit_Assert() uses the visitor pattern to visit all the AST nodes of the ast.Assert.test field, each visit call returning @@ -568,9 +586,10 @@ class AssertionRewriter(ast.NodeVisitor): by statements. Variables are created using .variable() and have the form of "@py_assert0". - :on_failure: The AST statements which will be executed if the - assertion test fails. This is the code which will construct - the failure message and raises the AssertionError. + :expl_stmts: The AST statements which will be executed to get + data from the assertion. This is the code which will construct + the detailed assertion message that is used in the AssertionError + or for the pytest_assertion_pass hook. :explanation_specifiers: A dict filled by .explanation_param() with %-formatting placeholders and their corresponding @@ -720,7 +739,7 @@ def pop_format_context(self, expl_expr): The expl_expr should be an ast.Str instance constructed from the %-placeholders created by .explanation_param(). This will - add the required code to format said string to .on_failure and + add the required code to format said string to .expl_stmts and return the ast.Name instance of the formatted string. """ @@ -731,7 +750,8 @@ def pop_format_context(self, expl_expr): format_dict = ast.Dict(keys, list(current.values())) form = ast.BinOp(expl_expr, ast.Mod(), format_dict) name = "@py_format" + str(next(self.variable_counter)) - self.on_failure.append(ast.Assign([ast.Name(name, ast.Store())], form)) + self.format_variables.append(name) + self.expl_stmts.append(ast.Assign([ast.Name(name, ast.Store())], form)) return ast.Name(name, ast.Load()) def generic_visit(self, node): @@ -765,8 +785,9 @@ def visit_Assert(self, assert_): self.statements = [] self.variables = [] self.variable_counter = itertools.count() + self.format_variables = [] self.stack = [] - self.on_failure = [] + self.expl_stmts = [] self.push_format_context() # Rewrite assert into a bunch of statements. top_condition, explanation = self.visit(assert_.test) @@ -777,24 +798,46 @@ def visit_Assert(self, assert_): top_condition, module_path=self.module_path, lineno=assert_.lineno ) ) - # Create failure message. - body = self.on_failure negation = ast.UnaryOp(ast.Not(), top_condition) - self.statements.append(ast.If(negation, body, [])) + msg = self.pop_format_context(ast.Str(explanation)) if assert_.msg: assertmsg = self.helper("_format_assertmsg", assert_.msg) - explanation = "\n>assert " + explanation + gluestr = "\n>assert " else: assertmsg = ast.Str("") - explanation = "assert " + explanation - template = ast.BinOp(assertmsg, ast.Add(), ast.Str(explanation)) - msg = self.pop_format_context(template) - fmt = self.helper("_format_explanation", msg) + gluestr = "assert " + err_explanation = ast.BinOp(ast.Str(gluestr), ast.Add(), msg) + err_msg = ast.BinOp(assertmsg, ast.Add(), err_explanation) err_name = ast.Name("AssertionError", ast.Load()) + fmt = self.helper("_format_explanation", err_msg) + fmt_pass = self.helper("_format_explanation", msg) exc = ast.Call(err_name, [fmt], []) - raise_ = ast.Raise(exc, None) + if sys.version_info[0] >= 3: + raise_ = ast.Raise(exc, None) + else: + raise_ = ast.Raise(exc, None, None) + # Call to hook when passes + orig = astor.to_source(assert_.test).rstrip("\n").lstrip("(").rstrip(")") + hook_call_pass = ast.Expr( + self.helper( + "_call_assertion_pass", ast.Num(assert_.lineno), ast.Str(orig), fmt_pass + ) + ) - body.append(raise_) + # If any hooks implement assert_pass hook + hook_impl_test = ast.If( + self.helper("_check_if_assertionpass_impl"), + [hook_call_pass], + [], + ) + main_test = ast.If(negation, [raise_], [hook_impl_test]) + + self.statements.extend(self.expl_stmts) + self.statements.append(main_test) + if self.format_variables: + variables = [ast.Name(name, ast.Store()) for name in self.format_variables] + clear_format = ast.Assign(variables, _NameConstant(None)) + self.statements.append(clear_format) # Clear temporary variables by setting them to None. if self.variables: variables = [ast.Name(name, ast.Store()) for name in self.variables] @@ -848,7 +891,7 @@ def visit_BoolOp(self, boolop): app = ast.Attribute(expl_list, "append", ast.Load()) is_or = int(isinstance(boolop.op, ast.Or)) body = save = self.statements - fail_save = self.on_failure + fail_save = self.expl_stmts levels = len(boolop.values) - 1 self.push_format_context() # Process each operand, short-circuting if needed. @@ -856,14 +899,14 @@ def visit_BoolOp(self, boolop): if i: fail_inner = [] # cond is set in a prior loop iteration below - self.on_failure.append(ast.If(cond, fail_inner, [])) # noqa - self.on_failure = fail_inner + self.expl_stmts.append(ast.If(cond, fail_inner, [])) # noqa + self.expl_stmts = fail_inner self.push_format_context() res, expl = self.visit(v) body.append(ast.Assign([ast.Name(res_var, ast.Store())], res)) expl_format = self.pop_format_context(ast.Str(expl)) call = ast.Call(app, [expl_format], []) - self.on_failure.append(ast.Expr(call)) + self.expl_stmts.append(ast.Expr(call)) if i < levels: cond = res if is_or: @@ -872,7 +915,7 @@ def visit_BoolOp(self, boolop): self.statements.append(ast.If(cond, inner, [])) self.statements = body = inner self.statements = save - self.on_failure = fail_save + self.expl_stmts = fail_save expl_template = self.helper("_format_boolop", expl_list, ast.Num(is_or)) expl = self.pop_format_context(expl_template) return ast.Name(res_var, ast.Load()), self.explanation_param(expl) diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py index 762e5761d71..df4587985fe 100644 --- a/src/_pytest/assertion/util.py +++ b/src/_pytest/assertion/util.py @@ -12,6 +12,10 @@ # DebugInterpreter. _reprcompare = None +# Works similarly as _reprcompare attribute. Is populated with the hook call +# when pytest_runtest_setup is called. +_assertion_pass = None + def format_explanation(explanation): """This formats an explanation diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index d40a368116c..70301566caa 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -485,6 +485,21 @@ def pytest_assertrepr_compare(config, op, left, right): """ +def pytest_assertion_pass(item, lineno, orig, expl): + """Process explanation when assertions are valid. + + Use this hook to do some processing after a passing assertion. + The original assertion information is available in the `orig` string + and the pytest introspected assertion information is available in the + `expl` string. + + :param _pytest.nodes.Item item: pytest item object of current test + :param int lineno: line number of the assert statement + :param string orig: string with original assertion + :param string expl: string with assert explanation + """ + + # ------------------------------------------------------------------------- # hooks for influencing reporting (invoked from _pytest_terminal) # ------------------------------------------------------------------------- diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 60cc21c4a01..ccb69dd79d1 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1047,51 +1047,6 @@ def test(request): result.stdout.fnmatch_lines(["* 1 passed *"]) -def test_fixture_values_leak(testdir): - """Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected - life-times (#2981). - """ - testdir.makepyfile( - """ - import attr - import gc - import pytest - import weakref - - @attr.s - class SomeObj(object): - name = attr.ib() - - fix_of_test1_ref = None - session_ref = None - - @pytest.fixture(scope='session') - def session_fix(): - global session_ref - obj = SomeObj(name='session-fixture') - session_ref = weakref.ref(obj) - return obj - - @pytest.fixture - def fix(session_fix): - global fix_of_test1_ref - obj = SomeObj(name='local-fixture') - fix_of_test1_ref = weakref.ref(obj) - return obj - - def test1(fix): - assert fix_of_test1_ref() is fix - - def test2(): - gc.collect() - # fixture "fix" created during test1 must have been destroyed by now - assert fix_of_test1_ref() is None - """ - ) - result = testdir.runpytest() - result.stdout.fnmatch_lines(["* 2 passed *"]) - - def test_fixture_order_respects_scope(testdir): """Ensure that fixtures are created according to scope order, regression test for #2405 """ diff --git a/testing/fixture_values_leak_test.py b/testing/fixture_values_leak_test.py new file mode 100644 index 00000000000..6f4c90d3e07 --- /dev/null +++ b/testing/fixture_values_leak_test.py @@ -0,0 +1,53 @@ +"""Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected +life-times (#2981). + +This comes from the old acceptance_test.py::test_fixture_values_leak(testdir): +This used pytester before but was not working when using pytest_assert_reprcompare +because pytester tracks hook calls and it would hold a reference (ParsedCall object), +preventing garbage collection + +, + 'op': 'is', + 'left': SomeObj(name='local-fixture'), + 'right': SomeObj(name='local-fixture')})> +""" +import attr +import gc +import pytest +import weakref + + +@attr.s +class SomeObj(object): + name = attr.ib() + + +fix_of_test1_ref = None +session_ref = None + + +@pytest.fixture(scope="session") +def session_fix(): + global session_ref + obj = SomeObj(name="session-fixture") + session_ref = weakref.ref(obj) + return obj + + +@pytest.fixture +def fix(session_fix): + global fix_of_test1_ref + obj = SomeObj(name="local-fixture") + fix_of_test1_ref = weakref.ref(obj) + return obj + + +def test1(fix): + assert fix_of_test1_ref() is fix + + +def test2(): + gc.collect() + # fixture "fix" created during test1 must have been destroyed by now + assert fix_of_test1_ref() is None diff --git a/testing/python/raises.py b/testing/python/raises.py index 89cef38f1a2..c9ede412ae9 100644 --- a/testing/python/raises.py +++ b/testing/python/raises.py @@ -202,6 +202,9 @@ def __call__(self): assert sys.exc_info() == (None, None, None) del t + # Make sure this does get updated in locals dict + # otherwise it could keep a reference + locals() # ensure the t instance is not stuck in a cyclic reference for o in gc.get_objects(): diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 0e6f42239f2..8304cf05771 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1305,3 +1305,54 @@ def test(): ) result = testdir.runpytest() result.stdout.fnmatch_lines(["* 1 passed in *"]) + + +class TestAssertionPass: + def test_hook_call(self, testdir): + testdir.makeconftest( + """ + def pytest_assertion_pass(item, lineno, orig, expl): + raise Exception("Assertion Passed: {} {} at line {}".format(orig, expl, lineno)) + """ + ) + testdir.makepyfile( + """ + def test_simple(): + a=1 + b=2 + c=3 + d=0 + + assert a+b == c+d + """ + ) + result = testdir.runpytest() + print(testdir.tmpdir) + result.stdout.fnmatch_lines( + "*Assertion Passed: a + b == c + d (1 + 2) == (3 + 0) at line 7*" + ) + + def test_hook_not_called_without_hookimpl(self, testdir, monkeypatch): + """Assertion pass should not be called (and hence formatting should + not occur) if there is no hook declared for pytest_assertion_pass""" + + def raise_on_assertionpass(*_, **__): + raise Exception("Assertion passed called when it shouldn't!") + + monkeypatch.setattr(_pytest.assertion.rewrite, + "_call_assertion_pass", + raise_on_assertionpass) + + testdir.makepyfile( + """ + def test_simple(): + a=1 + b=2 + c=3 + d=0 + + assert a+b == c+d + """ + ) + result = testdir.runpytest() + result.assert_outcomes(passed=1) From 52e695b3295cbef9a243fbcb9d80b69e805dfbe1 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 24 Jun 2019 17:47:48 +0200 Subject: [PATCH 02/24] Removed debug code. --- src/_pytest/assertion/rewrite.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 77dab5242e9..528c3500bb4 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -358,11 +358,6 @@ def _rewrite_test(config, fn): state.trace("failed to parse: {!r}".format(fn)) return None, None rewrite_asserts(tree, fn, config) - - # TODO: REMOVE, THIS IS ONLY FOR DEBUG - with open(f'{str(fn)+"bak"}', "w", encoding="utf-8") as f: - f.write(astor.to_source(tree)) - try: co = compile(tree, fn.strpath, "exec", dont_inherit=True) except SyntaxError: From 98b212cbfb2b58abeaa58609e53667058ba23429 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 10:35:09 +0200 Subject: [PATCH 03/24] Added "experimental" note. --- src/_pytest/hookspec.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 70301566caa..c22b4c12a17 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -497,6 +497,13 @@ def pytest_assertion_pass(item, lineno, orig, expl): :param int lineno: line number of the assert statement :param string orig: string with original assertion :param string expl: string with assert explanation + + .. note:: + + This hook is still *experimental*, so its parameters or even the hook itself might + be changed/removed without warning in any future pytest release. + + If you find this hook useful, please share your feedback opening an issue. """ From f8c9a7b86daaa265c934e7a775841aa98c734fcd Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 10:35:42 +0200 Subject: [PATCH 04/24] Formatting and removed py2 support. --- src/_pytest/assertion/rewrite.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 528c3500bb4..ca3f18cf322 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -807,10 +807,7 @@ def visit_Assert(self, assert_): fmt = self.helper("_format_explanation", err_msg) fmt_pass = self.helper("_format_explanation", msg) exc = ast.Call(err_name, [fmt], []) - if sys.version_info[0] >= 3: - raise_ = ast.Raise(exc, None) - else: - raise_ = ast.Raise(exc, None, None) + raise_ = ast.Raise(exc, None) # Call to hook when passes orig = astor.to_source(assert_.test).rstrip("\n").lstrip("(").rstrip(")") hook_call_pass = ast.Expr( @@ -821,9 +818,7 @@ def visit_Assert(self, assert_): # If any hooks implement assert_pass hook hook_impl_test = ast.If( - self.helper("_check_if_assertionpass_impl"), - [hook_call_pass], - [], + self.helper("_check_if_assertionpass_impl"), [hook_call_pass], [] ) main_test = ast.If(negation, [raise_], [hook_impl_test]) From 2280f28596b8443d5d3522b683d59bb6705ff2d8 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 10:36:01 +0200 Subject: [PATCH 05/24] Black formatting. --- src/_pytest/assertion/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/_pytest/assertion/__init__.py b/src/_pytest/assertion/__init__.py index b59b1bfdffc..f670afe4f71 100644 --- a/src/_pytest/assertion/__init__.py +++ b/src/_pytest/assertion/__init__.py @@ -130,8 +130,12 @@ def callbinrepr(op, left, right): util._reprcompare = callbinrepr if item.ihook.pytest_assertion_pass.get_hookimpls(): + def call_assertion_pass_hook(lineno, expl, orig): - item.ihook.pytest_assertion_pass(item=item, lineno=lineno, orig=orig, expl=expl) + item.ihook.pytest_assertion_pass( + item=item, lineno=lineno, orig=orig, expl=expl + ) + util._assertion_pass = call_assertion_pass_hook From 81e3f3cf95c95465ba515ae9fba2546cd8b6233a Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 10:41:11 +0200 Subject: [PATCH 06/24] Black formatting --- testing/test_assertrewrite.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 8304cf05771..d3c50511ff4 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1339,9 +1339,9 @@ def test_hook_not_called_without_hookimpl(self, testdir, monkeypatch): def raise_on_assertionpass(*_, **__): raise Exception("Assertion passed called when it shouldn't!") - monkeypatch.setattr(_pytest.assertion.rewrite, - "_call_assertion_pass", - raise_on_assertionpass) + monkeypatch.setattr( + _pytest.assertion.rewrite, "_call_assertion_pass", raise_on_assertionpass + ) testdir.makepyfile( """ From db50a975fd2377e28bafd82edbd90bc2e9aeeb1b Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 17:23:14 +0200 Subject: [PATCH 07/24] Reverted leak fixture test. --- testing/acceptance_test.py | 45 ++++++++++++++++++++++++ testing/fixture_values_leak_test.py | 53 ----------------------------- 2 files changed, 45 insertions(+), 53 deletions(-) delete mode 100644 testing/fixture_values_leak_test.py diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index ccb69dd79d1..60cc21c4a01 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1047,6 +1047,51 @@ def test(request): result.stdout.fnmatch_lines(["* 1 passed *"]) +def test_fixture_values_leak(testdir): + """Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected + life-times (#2981). + """ + testdir.makepyfile( + """ + import attr + import gc + import pytest + import weakref + + @attr.s + class SomeObj(object): + name = attr.ib() + + fix_of_test1_ref = None + session_ref = None + + @pytest.fixture(scope='session') + def session_fix(): + global session_ref + obj = SomeObj(name='session-fixture') + session_ref = weakref.ref(obj) + return obj + + @pytest.fixture + def fix(session_fix): + global fix_of_test1_ref + obj = SomeObj(name='local-fixture') + fix_of_test1_ref = weakref.ref(obj) + return obj + + def test1(fix): + assert fix_of_test1_ref() is fix + + def test2(): + gc.collect() + # fixture "fix" created during test1 must have been destroyed by now + assert fix_of_test1_ref() is None + """ + ) + result = testdir.runpytest() + result.stdout.fnmatch_lines(["* 2 passed *"]) + + def test_fixture_order_respects_scope(testdir): """Ensure that fixtures are created according to scope order, regression test for #2405 """ diff --git a/testing/fixture_values_leak_test.py b/testing/fixture_values_leak_test.py deleted file mode 100644 index 6f4c90d3e07..00000000000 --- a/testing/fixture_values_leak_test.py +++ /dev/null @@ -1,53 +0,0 @@ -"""Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected -life-times (#2981). - -This comes from the old acceptance_test.py::test_fixture_values_leak(testdir): -This used pytester before but was not working when using pytest_assert_reprcompare -because pytester tracks hook calls and it would hold a reference (ParsedCall object), -preventing garbage collection - -, - 'op': 'is', - 'left': SomeObj(name='local-fixture'), - 'right': SomeObj(name='local-fixture')})> -""" -import attr -import gc -import pytest -import weakref - - -@attr.s -class SomeObj(object): - name = attr.ib() - - -fix_of_test1_ref = None -session_ref = None - - -@pytest.fixture(scope="session") -def session_fix(): - global session_ref - obj = SomeObj(name="session-fixture") - session_ref = weakref.ref(obj) - return obj - - -@pytest.fixture -def fix(session_fix): - global fix_of_test1_ref - obj = SomeObj(name="local-fixture") - fix_of_test1_ref = weakref.ref(obj) - return obj - - -def test1(fix): - assert fix_of_test1_ref() is fix - - -def test2(): - gc.collect() - # fixture "fix" created during test1 must have been destroyed by now - assert fix_of_test1_ref() is None From cfbfa53f2b0d38de84928fba8a8709003162ea6d Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 17:46:56 +0200 Subject: [PATCH 08/24] Using pytester subprocess to avoid keeping references in the HookRecorder. --- testing/acceptance_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 60cc21c4a01..3f339366e52 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1088,7 +1088,10 @@ def test2(): assert fix_of_test1_ref() is None """ ) - result = testdir.runpytest() + # Running on subprocess does not activate the HookRecorder + # which holds itself a reference to objects in case of the + # pytest_assert_reprcompare hook + result = testdir.runpytest_subprocess() result.stdout.fnmatch_lines(["* 2 passed *"]) From 4db5488ed888a4cd20cd458405ad6cf4dddca476 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 19:49:05 +0200 Subject: [PATCH 09/24] Now dependent on command line option. --- src/_pytest/assertion/__init__.py | 8 +++ src/_pytest/assertion/rewrite.py | 101 +++++++++++++++++++----------- src/_pytest/config/__init__.py | 6 +- src/_pytest/hookspec.py | 2 + testing/test_assertrewrite.py | 35 ++++++++++- 5 files changed, 113 insertions(+), 39 deletions(-) diff --git a/src/_pytest/assertion/__init__.py b/src/_pytest/assertion/__init__.py index f670afe4f71..9e53c79f4d2 100644 --- a/src/_pytest/assertion/__init__.py +++ b/src/_pytest/assertion/__init__.py @@ -24,6 +24,14 @@ def pytest_addoption(parser): expression information.""", ) + group = parser.getgroup("experimental") + group.addoption( + "--enable-assertion-pass-hook", + action="store_true", + help="Enables the pytest_assertion_pass hook." + "Make sure to delete any previously generated pyc cache files.", + ) + def register_assert_rewrite(*names): """Register one or more module names to be rewritten on import. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index ca3f18cf322..5477927b761 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -745,7 +745,8 @@ def pop_format_context(self, expl_expr): format_dict = ast.Dict(keys, list(current.values())) form = ast.BinOp(expl_expr, ast.Mod(), format_dict) name = "@py_format" + str(next(self.variable_counter)) - self.format_variables.append(name) + if getattr(self.config._ns, "enable_assertion_pass_hook", False): + self.format_variables.append(name) self.expl_stmts.append(ast.Assign([ast.Name(name, ast.Store())], form)) return ast.Name(name, ast.Load()) @@ -780,7 +781,10 @@ def visit_Assert(self, assert_): self.statements = [] self.variables = [] self.variable_counter = itertools.count() - self.format_variables = [] + + if getattr(self.config._ns, "enable_assertion_pass_hook", False): + self.format_variables = [] + self.stack = [] self.expl_stmts = [] self.push_format_context() @@ -793,41 +797,68 @@ def visit_Assert(self, assert_): top_condition, module_path=self.module_path, lineno=assert_.lineno ) ) - negation = ast.UnaryOp(ast.Not(), top_condition) - msg = self.pop_format_context(ast.Str(explanation)) - if assert_.msg: - assertmsg = self.helper("_format_assertmsg", assert_.msg) - gluestr = "\n>assert " - else: - assertmsg = ast.Str("") - gluestr = "assert " - err_explanation = ast.BinOp(ast.Str(gluestr), ast.Add(), msg) - err_msg = ast.BinOp(assertmsg, ast.Add(), err_explanation) - err_name = ast.Name("AssertionError", ast.Load()) - fmt = self.helper("_format_explanation", err_msg) - fmt_pass = self.helper("_format_explanation", msg) - exc = ast.Call(err_name, [fmt], []) - raise_ = ast.Raise(exc, None) - # Call to hook when passes - orig = astor.to_source(assert_.test).rstrip("\n").lstrip("(").rstrip(")") - hook_call_pass = ast.Expr( - self.helper( - "_call_assertion_pass", ast.Num(assert_.lineno), ast.Str(orig), fmt_pass + if getattr(self.config._ns, "enable_assertion_pass_hook", False): + ### Experimental pytest_assertion_pass hook + negation = ast.UnaryOp(ast.Not(), top_condition) + msg = self.pop_format_context(ast.Str(explanation)) + if assert_.msg: + assertmsg = self.helper("_format_assertmsg", assert_.msg) + gluestr = "\n>assert " + else: + assertmsg = ast.Str("") + gluestr = "assert " + err_explanation = ast.BinOp(ast.Str(gluestr), ast.Add(), msg) + err_msg = ast.BinOp(assertmsg, ast.Add(), err_explanation) + err_name = ast.Name("AssertionError", ast.Load()) + fmt = self.helper("_format_explanation", err_msg) + fmt_pass = self.helper("_format_explanation", msg) + exc = ast.Call(err_name, [fmt], []) + raise_ = ast.Raise(exc, None) + # Call to hook when passes + orig = astor.to_source(assert_.test).rstrip("\n").lstrip("(").rstrip(")") + hook_call_pass = ast.Expr( + self.helper( + "_call_assertion_pass", + ast.Num(assert_.lineno), + ast.Str(orig), + fmt_pass, + ) ) - ) - # If any hooks implement assert_pass hook - hook_impl_test = ast.If( - self.helper("_check_if_assertionpass_impl"), [hook_call_pass], [] - ) - main_test = ast.If(negation, [raise_], [hook_impl_test]) - - self.statements.extend(self.expl_stmts) - self.statements.append(main_test) - if self.format_variables: - variables = [ast.Name(name, ast.Store()) for name in self.format_variables] - clear_format = ast.Assign(variables, _NameConstant(None)) - self.statements.append(clear_format) + # If any hooks implement assert_pass hook + hook_impl_test = ast.If( + self.helper("_check_if_assertionpass_impl"), [hook_call_pass], [] + ) + main_test = ast.If(negation, [raise_], [hook_impl_test]) + + self.statements.extend(self.expl_stmts) + self.statements.append(main_test) + if self.format_variables: + variables = [ + ast.Name(name, ast.Store()) for name in self.format_variables + ] + clear_format = ast.Assign(variables, _NameConstant(None)) + self.statements.append(clear_format) + else: + ### Original assertion rewriting + # Create failure message. + body = self.expl_stmts + negation = ast.UnaryOp(ast.Not(), top_condition) + self.statements.append(ast.If(negation, body, [])) + if assert_.msg: + assertmsg = self.helper("_format_assertmsg", assert_.msg) + explanation = "\n>assert " + explanation + else: + assertmsg = ast.Str("") + explanation = "assert " + explanation + template = ast.BinOp(assertmsg, ast.Add(), ast.Str(explanation)) + msg = self.pop_format_context(template) + fmt = self.helper("_format_explanation", msg) + err_name = ast.Name("AssertionError", ast.Load()) + exc = ast.Call(err_name, [fmt], []) + raise_ = ast.Raise(exc, None) + + body.append(raise_) # Clear temporary variables by setting them to None. if self.variables: variables = [ast.Name(name, ast.Store()) for name in self.variables] diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index e6de86c3619..f947a4c3255 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -746,8 +746,10 @@ def _consider_importhook(self, args): and find all the installed plugins to mark them for rewriting by the importhook. """ - ns, unknown_args = self._parser.parse_known_and_unknown_args(args) - mode = getattr(ns, "assertmode", "plain") + # Saving _ns so it can be used for other assertion rewriting purposes + # e.g. experimental assertion pass hook + self._ns, self._unknown_args = self._parser.parse_known_and_unknown_args(args) + mode = getattr(self._ns, "assertmode", "plain") if mode == "rewrite": try: hook = _pytest.assertion.install_importhook(self) diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index c22b4c12a17..5cb1d9ce5a6 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -503,6 +503,8 @@ def pytest_assertion_pass(item, lineno, orig, expl): This hook is still *experimental*, so its parameters or even the hook itself might be changed/removed without warning in any future pytest release. + It should be enabled using the `--enable-assertion-pass-hook` command line option. + If you find this hook useful, please share your feedback opening an issue. """ diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index d3c50511ff4..e74e6df8395 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1326,8 +1326,7 @@ def test_simple(): assert a+b == c+d """ ) - result = testdir.runpytest() - print(testdir.tmpdir) + result = testdir.runpytest("--enable-assertion-pass-hook") result.stdout.fnmatch_lines( "*Assertion Passed: a + b == c + d (1 + 2) == (3 + 0) at line 7*" ) @@ -1343,6 +1342,38 @@ def raise_on_assertionpass(*_, **__): _pytest.assertion.rewrite, "_call_assertion_pass", raise_on_assertionpass ) + testdir.makepyfile( + """ + def test_simple(): + a=1 + b=2 + c=3 + d=0 + + assert a+b == c+d + """ + ) + result = testdir.runpytest("--enable-assertion-pass-hook") + result.assert_outcomes(passed=1) + + def test_hook_not_called_without_cmd_option(self, testdir, monkeypatch): + """Assertion pass should not be called (and hence formatting should + not occur) if there is no hook declared for pytest_assertion_pass""" + + def raise_on_assertionpass(*_, **__): + raise Exception("Assertion passed called when it shouldn't!") + + monkeypatch.setattr( + _pytest.assertion.rewrite, "_call_assertion_pass", raise_on_assertionpass + ) + + testdir.makeconftest( + """ + def pytest_assertion_pass(item, lineno, orig, expl): + raise Exception("Assertion Passed: {} {} at line {}".format(orig, expl, lineno)) + """ + ) + testdir.makepyfile( """ def test_simple(): From 80ac910a24ab8bfcf8b1821d459a7a4b98ece3f8 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 25 Jun 2019 19:49:57 +0200 Subject: [PATCH 10/24] Added msg to docstring for cleaning pyc. --- src/_pytest/hookspec.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 5cb1d9ce5a6..3b1aa277d20 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -504,6 +504,7 @@ def pytest_assertion_pass(item, lineno, orig, expl): be changed/removed without warning in any future pytest release. It should be enabled using the `--enable-assertion-pass-hook` command line option. + Remember to clean the .pyc files in your project directory and interpreter libraries. If you find this hook useful, please share your feedback opening an issue. """ From 7efdd5063bb0a64e09b0f3db3a109474d78c2132 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 10:50:27 +0200 Subject: [PATCH 11/24] Update src/_pytest/assertion/rewrite.py Co-Authored-By: Bruno Oliveira --- 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 5477927b761..4d99c1a7887 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -484,7 +484,7 @@ def _call_assertion_pass(lineno, orig, expl): util._assertion_pass(lineno=lineno, orig=orig, expl=expl) -def _check_if_assertionpass_impl(): +def _check_if_assertion_pass_impl(): """Checks if any plugins implement the pytest_assertion_pass hook in order not to generate explanation unecessarily (might be expensive)""" return True if util._assertion_pass else False From 0fb52416b112ced523757471d4599cfd7340d2f4 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 17:00:37 +0200 Subject: [PATCH 12/24] Reverted changes. --- src/_pytest/config/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index f947a4c3255..437b38853cd 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -748,8 +748,8 @@ def _consider_importhook(self, args): """ # Saving _ns so it can be used for other assertion rewriting purposes # e.g. experimental assertion pass hook - self._ns, self._unknown_args = self._parser.parse_known_and_unknown_args(args) - mode = getattr(self._ns, "assertmode", "plain") + _ns, _unknown_args = self._parser.parse_known_and_unknown_args(args) + mode = getattr(_ns, "assertmode", "plain") if mode == "rewrite": try: hook = _pytest.assertion.install_importhook(self) From d638da5821246caf1782a1a0519308987befad47 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 17:08:09 +0200 Subject: [PATCH 13/24] Using ini-file option instead of cmd option. --- src/_pytest/assertion/__init__.py | 9 ++++----- src/_pytest/assertion/rewrite.py | 10 +++++++--- src/_pytest/hookspec.py | 2 +- testing/test_assertrewrite.py | 21 +++++++++++++++++++-- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/_pytest/assertion/__init__.py b/src/_pytest/assertion/__init__.py index 9e53c79f4d2..53b33fe3836 100644 --- a/src/_pytest/assertion/__init__.py +++ b/src/_pytest/assertion/__init__.py @@ -23,11 +23,10 @@ def pytest_addoption(parser): test modules on import to provide assert expression information.""", ) - - group = parser.getgroup("experimental") - group.addoption( - "--enable-assertion-pass-hook", - action="store_true", + parser.addini( + "enable_assertion_pass_hook", + type="bool", + default="False", help="Enables the pytest_assertion_pass hook." "Make sure to delete any previously generated pyc cache files.", ) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 4d99c1a7887..1a41f662706 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -604,6 +604,10 @@ def __init__(self, module_path, config): super().__init__() self.module_path = module_path self.config = config + if config is not None: + self.enable_assertion_pass_hook = config.getini("enable_assertion_pass_hook") + else: + self.enable_assertion_pass_hook = False def run(self, mod): """Find all assert statements in *mod* and rewrite them.""" @@ -745,7 +749,7 @@ def pop_format_context(self, expl_expr): format_dict = ast.Dict(keys, list(current.values())) form = ast.BinOp(expl_expr, ast.Mod(), format_dict) name = "@py_format" + str(next(self.variable_counter)) - if getattr(self.config._ns, "enable_assertion_pass_hook", False): + if self.enable_assertion_pass_hook: self.format_variables.append(name) self.expl_stmts.append(ast.Assign([ast.Name(name, ast.Store())], form)) return ast.Name(name, ast.Load()) @@ -782,7 +786,7 @@ def visit_Assert(self, assert_): self.variables = [] self.variable_counter = itertools.count() - if getattr(self.config._ns, "enable_assertion_pass_hook", False): + if self.enable_assertion_pass_hook: self.format_variables = [] self.stack = [] @@ -797,7 +801,7 @@ def visit_Assert(self, assert_): top_condition, module_path=self.module_path, lineno=assert_.lineno ) ) - if getattr(self.config._ns, "enable_assertion_pass_hook", False): + if self.enable_assertion_pass_hook: ### Experimental pytest_assertion_pass hook negation = ast.UnaryOp(ast.Not(), top_condition) msg = self.pop_format_context(ast.Str(explanation)) diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 3b1aa277d20..268348eac05 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -503,7 +503,7 @@ def pytest_assertion_pass(item, lineno, orig, expl): This hook is still *experimental*, so its parameters or even the hook itself might be changed/removed without warning in any future pytest release. - It should be enabled using the `--enable-assertion-pass-hook` command line option. + It should be enabled using the `enable_assertion_pass_hook` ini-file option. Remember to clean the .pyc files in your project directory and interpreter libraries. If you find this hook useful, please share your feedback opening an issue. diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index e74e6df8395..040972791c7 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1308,6 +1308,7 @@ def test(): class TestAssertionPass: + def test_hook_call(self, testdir): testdir.makeconftest( """ @@ -1315,6 +1316,12 @@ def pytest_assertion_pass(item, lineno, orig, expl): raise Exception("Assertion Passed: {} {} at line {}".format(orig, expl, lineno)) """ ) + + testdir.makeini(""" + [pytest] + enable_assertion_pass_hook = True + """) + testdir.makepyfile( """ def test_simple(): @@ -1326,7 +1333,7 @@ def test_simple(): assert a+b == c+d """ ) - result = testdir.runpytest("--enable-assertion-pass-hook") + result = testdir.runpytest() result.stdout.fnmatch_lines( "*Assertion Passed: a + b == c + d (1 + 2) == (3 + 0) at line 7*" ) @@ -1342,6 +1349,11 @@ def raise_on_assertionpass(*_, **__): _pytest.assertion.rewrite, "_call_assertion_pass", raise_on_assertionpass ) + testdir.makeini(""" + [pytest] + enable_assertion_pass_hook = True + """) + testdir.makepyfile( """ def test_simple(): @@ -1353,7 +1365,7 @@ def test_simple(): assert a+b == c+d """ ) - result = testdir.runpytest("--enable-assertion-pass-hook") + result = testdir.runpytest() result.assert_outcomes(passed=1) def test_hook_not_called_without_cmd_option(self, testdir, monkeypatch): @@ -1374,6 +1386,11 @@ def pytest_assertion_pass(item, lineno, orig, expl): """ ) + testdir.makeini(""" + [pytest] + enable_assertion_pass_hook = False + """) + testdir.makepyfile( """ def test_simple(): From f755ff6af15384b8807d31ba2c3e3028d1966104 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 17:08:35 +0200 Subject: [PATCH 14/24] Black formatting. --- src/_pytest/assertion/rewrite.py | 4 +++- testing/test_assertrewrite.py | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 1a41f662706..25e1ce0750d 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -605,7 +605,9 @@ def __init__(self, module_path, config): self.module_path = module_path self.config = config if config is not None: - self.enable_assertion_pass_hook = config.getini("enable_assertion_pass_hook") + self.enable_assertion_pass_hook = config.getini( + "enable_assertion_pass_hook" + ) else: self.enable_assertion_pass_hook = False diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 040972791c7..129eca6806d 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1308,7 +1308,6 @@ def test(): class TestAssertionPass: - def test_hook_call(self, testdir): testdir.makeconftest( """ @@ -1317,10 +1316,12 @@ def pytest_assertion_pass(item, lineno, orig, expl): """ ) - testdir.makeini(""" + testdir.makeini( + """ [pytest] enable_assertion_pass_hook = True - """) + """ + ) testdir.makepyfile( """ @@ -1349,10 +1350,12 @@ def raise_on_assertionpass(*_, **__): _pytest.assertion.rewrite, "_call_assertion_pass", raise_on_assertionpass ) - testdir.makeini(""" + testdir.makeini( + """ [pytest] enable_assertion_pass_hook = True - """) + """ + ) testdir.makepyfile( """ @@ -1386,10 +1389,12 @@ def pytest_assertion_pass(item, lineno, orig, expl): """ ) - testdir.makeini(""" + testdir.makeini( + """ [pytest] enable_assertion_pass_hook = False - """) + """ + ) testdir.makepyfile( """ From d91a5d3cd73a390f7cc5b03c56678f66aeab5853 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 17:32:35 +0200 Subject: [PATCH 15/24] Further reverting changes. --- src/_pytest/config/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 437b38853cd..e6de86c3619 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -746,10 +746,8 @@ def _consider_importhook(self, args): and find all the installed plugins to mark them for rewriting by the importhook. """ - # Saving _ns so it can be used for other assertion rewriting purposes - # e.g. experimental assertion pass hook - _ns, _unknown_args = self._parser.parse_known_and_unknown_args(args) - mode = getattr(_ns, "assertmode", "plain") + ns, unknown_args = self._parser.parse_known_and_unknown_args(args) + mode = getattr(ns, "assertmode", "plain") if mode == "rewrite": try: hook = _pytest.assertion.install_importhook(self) From 9a34d88c8d369edae357094816b6aeef4d3a914d Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 18:09:51 +0200 Subject: [PATCH 16/24] Explanation variables only defined if failed or passed with plugins implementing the hook. --- src/_pytest/assertion/rewrite.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 25e1ce0750d..68fe8fd0940 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -803,10 +803,12 @@ def visit_Assert(self, assert_): top_condition, module_path=self.module_path, lineno=assert_.lineno ) ) - if self.enable_assertion_pass_hook: - ### Experimental pytest_assertion_pass hook + + if self.enable_assertion_pass_hook: # Experimental pytest_assertion_pass hook negation = ast.UnaryOp(ast.Not(), top_condition) msg = self.pop_format_context(ast.Str(explanation)) + + # Failed if assert_.msg: assertmsg = self.helper("_format_assertmsg", assert_.msg) gluestr = "\n>assert " @@ -817,10 +819,14 @@ def visit_Assert(self, assert_): err_msg = ast.BinOp(assertmsg, ast.Add(), err_explanation) err_name = ast.Name("AssertionError", ast.Load()) fmt = self.helper("_format_explanation", err_msg) - fmt_pass = self.helper("_format_explanation", msg) exc = ast.Call(err_name, [fmt], []) raise_ = ast.Raise(exc, None) - # Call to hook when passes + statements_fail = [] + statements_fail.extend(self.expl_stmts) + statements_fail.append(raise_) + + # Passed + fmt_pass = self.helper("_format_explanation", msg) orig = astor.to_source(assert_.test).rstrip("\n").lstrip("(").rstrip(")") hook_call_pass = ast.Expr( self.helper( @@ -830,14 +836,16 @@ def visit_Assert(self, assert_): fmt_pass, ) ) - # If any hooks implement assert_pass hook hook_impl_test = ast.If( self.helper("_check_if_assertionpass_impl"), [hook_call_pass], [] ) - main_test = ast.If(negation, [raise_], [hook_impl_test]) + statements_pass = [] + statements_pass.extend(self.expl_stmts) + statements_pass.append(hook_impl_test) - self.statements.extend(self.expl_stmts) + # Test for assertion condition + main_test = ast.If(negation, statements_fail, statements_pass) self.statements.append(main_test) if self.format_variables: variables = [ @@ -845,8 +853,8 @@ def visit_Assert(self, assert_): ] clear_format = ast.Assign(variables, _NameConstant(None)) self.statements.append(clear_format) - else: - ### Original assertion rewriting + + else: # Original assertion rewriting # Create failure message. body = self.expl_stmts negation = ast.UnaryOp(ast.Not(), top_condition) @@ -865,6 +873,7 @@ def visit_Assert(self, assert_): raise_ = ast.Raise(exc, None) body.append(raise_) + # Clear temporary variables by setting them to None. if self.variables: variables = [ast.Name(name, ast.Store()) for name in self.variables] From 53234bf6136c3821df917cf89507ed4cb5fde476 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 19:00:31 +0200 Subject: [PATCH 17/24] Added config back to AssertionWriter and fixed typo in check_if_assertion_pass_impl function call. --- src/_pytest/assertion/rewrite.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 2afe76b82a0..a79a5215728 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -135,7 +135,7 @@ def exec_module(self, module): co = _read_pyc(fn, pyc, state.trace) if co is None: state.trace("rewriting {!r}".format(fn)) - source_stat, co = _rewrite_test(fn) + source_stat, co = _rewrite_test(fn, self.config) if write: self._writing_pyc = True try: @@ -279,13 +279,13 @@ def _write_pyc(state, co, source_stat, pyc): return True -def _rewrite_test(fn): +def _rewrite_test(fn, config): """read and rewrite *fn* and return the code object.""" stat = os.stat(fn) with open(fn, "rb") as f: source = f.read() tree = ast.parse(source, filename=fn) - rewrite_asserts(tree, fn) + rewrite_asserts(tree, fn, config) co = compile(tree, fn, "exec", dont_inherit=True) return stat, co @@ -327,9 +327,9 @@ def _read_pyc(source, pyc, trace=lambda x: None): return co -def rewrite_asserts(mod, module_path=None): +def rewrite_asserts(mod, module_path=None, config=None): """Rewrite the assert statements in mod.""" - AssertionRewriter(module_path).run(mod) + AssertionRewriter(module_path, config).run(mod) def _saferepr(obj): @@ -523,7 +523,7 @@ class AssertionRewriter(ast.NodeVisitor): """ - def __init__(self, module_path): + def __init__(self, module_path, config): super().__init__() self.module_path = module_path self.config = config @@ -761,7 +761,7 @@ def visit_Assert(self, assert_): ) # If any hooks implement assert_pass hook hook_impl_test = ast.If( - self.helper("_check_if_assertionpass_impl"), [hook_call_pass], [] + self.helper("_check_if_assertion_pass_impl"), [hook_call_pass], [] ) statements_pass = [] statements_pass.extend(self.expl_stmts) From 6854ff2acc1f291d4781d97bf78d563bb2113c5e Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Wed, 26 Jun 2019 19:05:17 +0200 Subject: [PATCH 18/24] Fixed import order pep8. --- 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 a79a5215728..62d8dbb6ac5 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -1,6 +1,5 @@ """Rewrite assertion AST to produce nice error messages""" import ast -import astor import errno import importlib.machinery import importlib.util @@ -11,6 +10,7 @@ import sys import types +import astor import atomicwrites from _pytest._io.saferepr import saferepr From eb90f3d1c89a10a00b5cd34a5df3aaaaec500177 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 17:54:24 -0300 Subject: [PATCH 19/24] Fix default value of 'enable_assertion_pass_hook' --- src/_pytest/assertion/__init__.py | 2 +- testing/test_assertrewrite.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/_pytest/assertion/__init__.py b/src/_pytest/assertion/__init__.py index 53b33fe3836..126929b6ad9 100644 --- a/src/_pytest/assertion/__init__.py +++ b/src/_pytest/assertion/__init__.py @@ -26,7 +26,7 @@ def pytest_addoption(parser): parser.addini( "enable_assertion_pass_hook", type="bool", - default="False", + default=False, help="Enables the pytest_assertion_pass hook." "Make sure to delete any previously generated pyc cache files.", ) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 0c01be28c98..5c680927924 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1335,6 +1335,10 @@ def test(): class TestAssertionPass: + def test_option_default(self, testdir): + config = testdir.parseconfig() + assert config.getini("enable_assertion_pass_hook") is False + def test_hook_call(self, testdir): testdir.makeconftest( """ From fcbe66feba83b5991d093cc5e42a46001e46cab0 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 18:51:14 -0300 Subject: [PATCH 20/24] Restore proper handling of '%' in assertion messages --- 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 62d8dbb6ac5..7b098ad1b52 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -358,7 +358,7 @@ def _format_assertmsg(obj): # contains a newline it gets escaped, however if an object has a # .__repr__() which contains newlines it does not get escaped. # However in either case we want to preserve the newline. - replaces = [("\n", "\n~")] + replaces = [("\n", "\n~"), ("%", "%%")] if not isinstance(obj, str): obj = saferepr(obj) replaces.append(("\\n", "\n~")) From 3afee36ebb0720bb15f4ca67ea0f1305434e1cc7 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 19:10:54 -0300 Subject: [PATCH 21/24] Improve docs and reference --- changelog/3457.feature.rst | 6 ++++-- doc/en/reference.rst | 7 +++---- src/_pytest/hookspec.py | 25 ++++++++++++++++++------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/changelog/3457.feature.rst b/changelog/3457.feature.rst index 3f676514431..c309430706c 100644 --- a/changelog/3457.feature.rst +++ b/changelog/3457.feature.rst @@ -1,2 +1,4 @@ -Adds ``pytest_assertion_pass`` hook, called with assertion context information -(original asssertion statement and pytest explanation) whenever an assertion passes. +New `pytest_assertion_pass `__ +hook, called with context information when an assertion *passes*. + +This hook is still **experimental** so use it with caution. diff --git a/doc/en/reference.rst b/doc/en/reference.rst index 6750b17f082..5abb01f5023 100644 --- a/doc/en/reference.rst +++ b/doc/en/reference.rst @@ -665,15 +665,14 @@ Session related reporting hooks: .. autofunction:: pytest_fixture_post_finalizer .. autofunction:: pytest_warning_captured -And here is the central hook for reporting about -test execution: +Central hook for reporting about test execution: .. autofunction:: pytest_runtest_logreport -You can also use this hook to customize assertion representation for some -types: +Assertion related hooks: .. autofunction:: pytest_assertrepr_compare +.. autofunction:: pytest_assertion_pass Debugging/Interaction hooks diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 268348eac05..9e6d13fab44 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -486,13 +486,27 @@ def pytest_assertrepr_compare(config, op, left, right): def pytest_assertion_pass(item, lineno, orig, expl): - """Process explanation when assertions are valid. + """ + **(Experimental)** + + Hook called whenever an assertion *passes*. Use this hook to do some processing after a passing assertion. The original assertion information is available in the `orig` string and the pytest introspected assertion information is available in the `expl` string. + This hook must be explicitly enabled by the ``enable_assertion_pass_hook`` + ini-file option: + + .. code-block:: ini + + [pytest] + enable_assertion_pass_hook=true + + You need to **clean the .pyc** files in your project directory and interpreter libraries + when enabling this option, as assertions will require to be re-written. + :param _pytest.nodes.Item item: pytest item object of current test :param int lineno: line number of the assert statement :param string orig: string with original assertion @@ -500,13 +514,10 @@ def pytest_assertion_pass(item, lineno, orig, expl): .. note:: - This hook is still *experimental*, so its parameters or even the hook itself might - be changed/removed without warning in any future pytest release. - - It should be enabled using the `enable_assertion_pass_hook` ini-file option. - Remember to clean the .pyc files in your project directory and interpreter libraries. + This hook is **experimental**, so its parameters or even the hook itself might + be changed/removed without warning in any future pytest release. - If you find this hook useful, please share your feedback opening an issue. + If you find this hook useful, please share your feedback opening an issue. """ From 8edf68f3c0f3f2435fb8b0751dfe82bbd9ecd478 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 19:20:34 -0300 Subject: [PATCH 22/24] Add a trivial note about astor --- changelog/3457.trivial.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/3457.trivial.rst diff --git a/changelog/3457.trivial.rst b/changelog/3457.trivial.rst new file mode 100644 index 00000000000..f1888763440 --- /dev/null +++ b/changelog/3457.trivial.rst @@ -0,0 +1 @@ +pytest now also depends on the `astor `__ package. From 629eb3ec6a5428ff254646ca568a8cfeb96820c6 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 19:26:12 -0300 Subject: [PATCH 23/24] Move formatting variables under the "has impls" if Small optimization, move the generation of the intermediate formatting variables inside the 'if _check_if_assertion_pass_impl():' block. --- src/_pytest/assertion/rewrite.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 7b098ad1b52..8810c156cc1 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -761,11 +761,11 @@ def visit_Assert(self, assert_): ) # If any hooks implement assert_pass hook hook_impl_test = ast.If( - self.helper("_check_if_assertion_pass_impl"), [hook_call_pass], [] + self.helper("_check_if_assertion_pass_impl"), + self.expl_stmts + [hook_call_pass], + [], ) - statements_pass = [] - statements_pass.extend(self.expl_stmts) - statements_pass.append(hook_impl_test) + statements_pass = [hook_impl_test] # Test for assertion condition main_test = ast.If(negation, statements_fail, statements_pass) From 2ea22218ff1070a4afd4f7116b22275c58d090ea Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 20:46:31 -0300 Subject: [PATCH 24/24] Cover assertions with messages when enable_assertion_pass_hook is enabled --- testing/test_assertrewrite.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 5c680927924..8d1c7a5f000 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -1363,6 +1363,10 @@ def test_simple(): d=0 assert a+b == c+d + + # cover failing assertions with a message + def test_fails(): + assert False, "assert with message" """ ) result = testdir.runpytest()