deprecate and warn about __multicall__ usage in hooks, refine docs about hook ordering,

make hookwrappers respect tryfirst/trylast

--HG--
branch : more_plugin

CHANGELOG

@@ -39,6 +39,10 @@
 - fix issue732: properly unregister plugins from any hook calling
   sites allowing to have temporary plugins during test execution.
 
+- deprecate and warn about ``__multicall__`` argument in hook 
+  implementations.  Use the ``hookwrapper`` mechanism instead already 
+  introduced with pytest-2.7.
+
  
 2.7.1.dev (compared to 2.7.0)
 -----------------------------

_pytest/config.py

@@ -9,7 +9,7 @@ import py
 # DON't import pytest here because it causes import cycle troubles
 import sys, os
 from _pytest import hookspec # the extension point definitions
-from _pytest.core import PluginManager, hookimpl_opts
+from _pytest.core import PluginManager, hookimpl_opts, varnames
 
 # pytest startup
 #
@@ -117,6 +117,18 @@ class PytestPluginManager(PluginManager):
             self.trace.root.setwriter(err.write)
             self.enable_tracing()
 
+
+    def _verify_hook(self, hook, plugin):
+        super(PytestPluginManager, self)._verify_hook(hook, plugin)
+        method = getattr(plugin, hook.name)
+        if "__multicall__" in varnames(method):
+            fslineno = py.code.getfslineno(method)
+            warning = dict(code="I1",
+                           fslocation=fslineno,
+                           message="%r hook uses deprecated __multicall__ "
+                                   "argument" % (hook.name))
+            self._warnings.append(warning)
+
     def register(self, plugin, name=None):
         ret = super(PytestPluginManager, self).register(plugin, name)
         if ret:
@@ -138,7 +150,10 @@ class PytestPluginManager(PluginManager):
             "trylast: mark a hook implementation function such that the "
             "plugin machinery will try to call it last/as late as possible.")
         for warning in self._warnings:
-            config.warn(code="I1", message=warning)
+            if isinstance(warning, dict):
+                config.warn(**warning)
+            else:
+                config.warn(code="I1", message=warning)
 
     #
     # internal API for local conftest plugin handling
@@ -712,10 +727,10 @@ class Config(object):
             fin = self._cleanup.pop()
             fin()
 
-    def warn(self, code, message):
+    def warn(self, code, message, fslocation=None):
         """ generate a warning for this test session. """
         self.hook.pytest_logwarning(code=code, message=message,
-                                    fslocation=None, nodeid=None)
+                                    fslocation=fslocation, nodeid=None)
 
     def get_terminal_writer(self):
         return self.pluginmanager.get_plugin("terminalreporter")._tw

_pytest/core.py

@@ -408,6 +408,12 @@ class PluginManager(object):
 class MultiCall:
     """ execute a call into multiple python functions/methods. """
 
+    # XXX note that the __multicall__ argument is supported only
+    # for pytest compatibility reasons.  It was never officially
+    # supported there and is explicitely deprecated since 2.8
+    # so we can remove it soon, allowing to avoid the below recursion
+    # in execute() and simplify/speed up the execute loop.
+
     def __init__(self, methods, kwargs, firstresult=False):
         self.methods = methods
         self.kwargs = kwargs
@@ -527,20 +533,20 @@ class HookCaller(object):
 
     def _add_method(self, meth):
         if hasattr(meth, 'hookwrapper'):
-            self._wrappers.append(meth)
+            methods = self._wrappers
-        elif hasattr(meth, 'trylast'):
-            self._nonwrappers.insert(0, meth)
-        elif hasattr(meth, 'tryfirst'):
-            self._nonwrappers.append(meth)
         else:
-            # find the last nonwrapper which is not tryfirst marked
+            methods = self._nonwrappers
-            nonwrappers = self._nonwrappers
-            i = len(nonwrappers) - 1
-            while i >= 0 and hasattr(nonwrappers[i], "tryfirst"):
-                i -= 1
 
-            # and insert right in front of the tryfirst ones
+        if hasattr(meth, 'trylast'):
-            nonwrappers.insert(i+1, meth)
+            methods.insert(0, meth)
+        elif hasattr(meth, 'tryfirst'):
+            methods.append(meth)
+        else:
+            # find last non-tryfirst method
+            i = len(methods) - 1
+            while i >= 0 and hasattr(methods[i], "tryfirst"):
+                i -= 1
+            methods.insert(i + 1, meth)
 
     def __repr__(self):
         return "<HookCaller %r>" %(self.name,)

_pytest/terminal.py

@@ -164,6 +164,8 @@ class TerminalReporter:
 
     def pytest_logwarning(self, code, fslocation, message, nodeid):
         warnings = self.stats.setdefault("warnings", [])
+        if isinstance(fslocation, tuple):
+            fslocation = "%s:%d" % fslocation
         warning = WarningReport(code=code, fslocation=fslocation,
                                 message=message, nodeid=nodeid)
         warnings.append(warning)

doc/en/writing_plugins.txt

@@ -221,36 +221,21 @@ be "future-compatible": we can introduce new hook named parameters without
 breaking the signatures of existing hook implementations.  It is one of
 the reasons for the general long-lived compatibility of pytest plugins.
 
-Hook function results
+Note that hook functions other than ``pytest_runtest_*`` are not
----------------------
+allowed to raise exceptions.  Doing so will break the pytest run.
+
+
+
+firstresult: stop at first non-None result
+-------------------------------------------
 
 Most calls to ``pytest`` hooks result in a **list of results** which contains
 all non-None results of the called hook functions.
 
-Some hooks are specified so that the hook call only executes until the
+Some hook specifications use the ``firstresult=True`` option so that the hook
-first function returned a non-None value which is then also the
+call only executes until the first of N registered functions returns a
-result of the overall hook call.  The remaining hook functions will
+non-None result which is then taken as result of the overall hook call.
-not be called in this case.
+The remaining hook functions will not be called in this case.
-
-Note that hook functions other than ``pytest_runtest_*`` are not
-allowed to raise exceptions.  Doing so will break the pytest run.
-
-Hook function ordering
-----------------------
-
-For any given hook there may be more than one implementation and we thus
-generally view ``hook`` execution as a ``1:N`` function call where ``N``
-is the number of registered functions.  There are ways to
-influence if a hook implementation comes before or after others, i.e.
-the position in the ``N``-sized list of functions::
-
-    @pytest.hookimpl_spec(tryfirst=True)
-    def pytest_collection_modifyitems(items):
-        # will execute as early as possible
-
-    @pytest.hookimpl_spec(trylast=True)
-    def pytest_collection_modifyitems(items):
-        # will execute as late as possible
 
 
 hookwrapper: executing around other hooks
@@ -290,6 +275,47 @@ perform tracing or other side effects around the actual hook implementations.
 If the result of the underlying hook is a mutable object, they may modify
 that result, however.
 
+
+
+Hook function ordering / call example
+-------------------------------------
+
+For any given hook specification there may be more than one
+implementation and we thus generally view ``hook`` execution as a
+``1:N`` function call where ``N`` is the number of registered functions.
+There are ways to influence if a hook implementation comes before or
+after others, i.e.  the position in the ``N``-sized list of functions::
+
+    # Plugin 1
+    @pytest.hookimpl_spec(tryfirst=True)
+    def pytest_collection_modifyitems(items):
+        # will execute as early as possible
+
+    # Plugin 2
+    @pytest.hookimpl_spec(trylast=True)
+    def pytest_collection_modifyitems(items):
+        # will execute as late as possible
+
+    # Plugin 3
+    @pytest.hookimpl_spec(hookwrapper=True)
+    def pytest_collection_modifyitems(items):
+        # will execute even before the tryfirst one above!
+        outcome = yield
+        # will execute after all non-hookwrappers executed
+
+Here is the order of execution:
+
+1. Plugin3's pytest_collection_modifyitems called until the yield point
+2. Plugin1's pytest_collection_modifyitems is called
+3. Plugin2's pytest_collection_modifyitems is called
+4. Plugin3's pytest_collection_modifyitems called for executing after the yield
+   The yield receives a :py:class:`CallOutcome` instance which encapsulates
+   the result from calling the non-wrappers.  Wrappers cannot modify the result.
+
+It's possible to use ``tryfirst`` and ``trylast`` also in conjunction with
+``hookwrapper=True`` in which case it will influence the ordering of hookwrappers
+among each other.
+
 Declaring new hooks
 ------------------------
 

testing/conftest.py

@@ -66,13 +66,12 @@ def check_open_files(config):
         error.append(error[0])
         raise AssertionError("\n".join(error))
 
-@pytest.hookimpl_opts(trylast=True)
+@pytest.hookimpl_opts(hookwrapper=True, trylast=True)
-def pytest_runtest_teardown(item, __multicall__):
+def pytest_runtest_teardown(item):
+    yield
     item.config._basedir.chdir()
     if hasattr(item.config, '_openfiles'):
-        x = __multicall__.execute()
         check_open_files(item.config)
-        return x
 
 # XXX copied from execnet's conftest.py - needs to be merged
 winpymap = {

testing/test_core.py

@@ -336,6 +336,19 @@ class TestAddMethodOrdering:
         assert hc._nonwrappers == [he_method1_middle]
         assert hc._wrappers == [he_method1, he_method3]
 
+    def test_adding_wrappers_ordering_tryfirst(self, hc, addmeth):
+        @addmeth(hookwrapper=True, tryfirst=True)
+        def he_method1():
+            pass
+
+        @addmeth(hookwrapper=True)
+        def he_method2():
+            pass
+
+        assert hc._nonwrappers == []
+        assert hc._wrappers == [he_method2, he_method1]
+
+
     def test_hookspec_opts(self, pm):
         class HookSpec:
             @hookspec_opts()
@@ -530,6 +543,16 @@ class TestPytestPluginInteractions:
         finally:
             undo()
 
+    def test_warn_on_deprecated_multicall(self, pytestpm):
+        class Plugin:
+            def pytest_configure(self, __multicall__):
+                pass
+
+        before = list(pytestpm._warnings)
+        pytestpm.register(Plugin())
+        assert len(pytestpm._warnings) == len(before) + 1
+        assert "deprecated" in pytestpm._warnings[-1]["message"]
+
 
 def test_namespace_has_default_and_env_plugins(testdir):
     p = testdir.makepyfile("""
@@ -969,7 +992,7 @@ class TestPytestPluginManager:
         monkeypatch.setenv('PYTEST_PLUGINS', 'pytest_x500', prepend=",")
         result = testdir.runpytest(p)
         assert result.ret == 0
-        result.stdout.fnmatch_lines(["*1 passed in*"])
+        result.stdout.fnmatch_lines(["*1 passed*"])
 
     def test_import_plugin_importname(self, testdir, pytestpm):
         pytest.raises(ImportError, 'pytestpm.import_plugin("qweqwex.y")')