Merge pull request #2292 from nicoddemus/defer-hook-checking

Verify hooks after collection completes

CHANGELOG.rst

@@ -53,11 +53,16 @@ Changes
 * Change exception raised by ``capture.DontReadFromInput.fileno()`` from ``ValueError``
   to ``io.UnsupportedOperation``. Thanks `@vlad-dragos`_ for the PR.
 
-* fix `#2013`_: turn RecordedWarning into namedtupe,
-  to give it a comprehensible repr while preventing unwarranted modification
+* fix `#2013`_: turn RecordedWarning into ``namedtuple``,
+  to give it a comprehensible repr while preventing unwarranted modification.
 
 * fix `#2208`_: ensure a iteration limit for _pytest.compat.get_real_func.
-  Thanks `@RonnyPfannschmidt`_ for the Report and PR
+  Thanks `@RonnyPfannschmidt`_ for the report and PR.
+
+* Hooks are now verified after collection is complete, rather than right after loading installed plugins. This
+  makes it easy to write hooks for plugins which will be loaded during collection, for example using the
+  ``pytest_plugins`` special variable (`#1821`_).
+  Thanks `@nicoddemus`_ for the PR.
 
 * Modify ``pytest_make_parametrize_id()`` hook to accept ``argname`` as an
   additional parameter.
@@ -96,6 +101,7 @@ Bug Fixes
 
 .. _#1407: https://github.com/pytest-dev/pytest/issues/1407
 .. _#1512: https://github.com/pytest-dev/pytest/issues/1512
+.. _#1821: https://github.com/pytest-dev/pytest/issues/1821
 .. _#1874: https://github.com/pytest-dev/pytest/pull/1874
 .. _#1952: https://github.com/pytest-dev/pytest/pull/1952
 .. _#2007: https://github.com/pytest-dev/pytest/issues/2007

_pytest/config.py

@@ -54,7 +54,6 @@ def main(args=None, plugins=None):
             return 4
         else:
             try:
-                config.pluginmanager.check_pending()
                 return config.hook.pytest_cmdline_main(config=config)
             finally:
                 config._ensure_unconfigure()

_pytest/main.py

@@ -309,9 +309,6 @@ class Node(object):
         fslocation = getattr(self, "location", None)
         if fslocation is None:
             fslocation = getattr(self, "fspath", None)
-        else:
-            fslocation = "%s:%s" % (fslocation[0], fslocation[1] + 1)
-
         self.ihook.pytest_logwarning.call_historic(kwargs=dict(
             code=code, message=message,
             nodeid=self.nodeid, fslocation=fslocation))
@@ -596,6 +593,7 @@ class Session(FSCollector):
         hook = self.config.hook
         try:
             items = self._perform_collect(args, genitems)
+            self.config.pluginmanager.check_pending()
             hook.pytest_collection_modifyitems(session=self,
                 config=self.config, items=items)
         finally:

_pytest/pytester.py

@@ -88,7 +88,7 @@ class LsofFdLeakChecker(object):
             return True
 
     @pytest.hookimpl(hookwrapper=True, tryfirst=True)
-    def pytest_runtest_item(self, item):
+    def pytest_runtest_protocol(self, item):
         lines1 = self.get_open_files()
         yield
         if hasattr(sys, "pypy_version_info"):
@@ -107,7 +107,8 @@ class LsofFdLeakChecker(object):
             error.extend([str(f) for f in lines2])
             error.append(error[0])
             error.append("*** function %s:%s: %s " % item.location)
-            pytest.fail("\n".join(error), pytrace=False)
+            error.append("See issue #2366")
+            item.warn('', "\n".join(error))
 
 
 # XXX copied from execnet's conftest.py - needs to be merged

_pytest/terminal.py

@@ -109,10 +109,10 @@ class WarningReport(object):
         if self.nodeid:
             return self.nodeid
         if self.fslocation:
-            if isinstance(self.fslocation, tuple) and len(self.fslocation) == 2:
-                filename, linenum = self.fslocation
+            if isinstance(self.fslocation, tuple) and len(self.fslocation) >= 2:
+                filename, linenum = self.fslocation[:2]
                 relpath = py.path.local(filename).relto(config.invocation_dir)
-                return '%s:%d' % (relpath, linenum)
+                return '%s:%s' % (relpath, linenum)
             else:
                 return str(self.fslocation)
         return None

testing/acceptance_test.py

@@ -807,3 +807,31 @@ def test_import_plugin_unicode_name(testdir):
     """)
     r = testdir.runpytest()
     assert r.ret == 0
+
+
+def test_deferred_hook_checking(testdir):
+    """
+    Check hooks as late as possible (#1821).
+    """
+    testdir.syspathinsert()
+    testdir.makepyfile(**{
+        'plugin.py': """
+        class Hooks:
+            def pytest_my_hook(self, config):
+                pass
+
+        def pytest_configure(config):
+            config.pluginmanager.add_hookspecs(Hooks)
+        """,
+        'conftest.py': """
+            pytest_plugins = ['plugin']
+            def pytest_my_hook(config):
+                return 40
+        """,
+        'test_foo.py': """
+            def test(request):
+                assert request.config.hook.pytest_my_hook(config=request.config) == [40]
+        """
+    })
+    result = testdir.runpytest()
+    result.stdout.fnmatch_lines(['* 1 passed *'])

testing/test_helpconfig.py

@@ -33,7 +33,7 @@ def test_hookvalidation_unknown(testdir):
     """)
     result = testdir.runpytest()
     assert result.ret != 0
-    result.stderr.fnmatch_lines([
+    result.stdout.fnmatch_lines([
         '*unknown hook*pytest_hello*'
     ])