introduce a new subset_hook_caller instead of remove make_hook_caller

and adapat and refine conftest/global plugin management accordingly

--HG--
branch : more_plugin

_pytest/config.py

@@ -99,7 +99,7 @@ class PytestPluginManager(PluginManager):
                                                   excludefunc=exclude_pytest_names)
         self._warnings = []
         self._plugin_distinfo = []
-        self._globalplugins = []
+        self._conftest_plugins = set()
 
         # state related to local conftest plugins
         self._path2confmods = {}
@@ -121,21 +121,12 @@ class PytestPluginManager(PluginManager):
     def register(self, plugin, name=None, conftest=False):
         ret = super(PytestPluginManager, self).register(plugin, name)
         if ret:
-            if not conftest:
-                self._globalplugins.append(plugin)
             self.hook.pytest_plugin_registered.call_historic(
                       kwargs=dict(plugin=plugin, manager=self))
         return ret
 
-    def unregister(self, plugin=None, name=None):
-        plugin = super(PytestPluginManager, self).unregister(plugin, name)
-        try:
-            self._globalplugins.remove(plugin)
-        except ValueError:
-            pass
-
     def getplugin(self, name):
-        # deprecated
+        # deprecated naming
         return self.get_plugin(name)
 
     def pytest_configure(self, config):
@@ -189,14 +180,20 @@ class PytestPluginManager(PluginManager):
         try:
             return self._path2confmods[path]
         except KeyError:
-            clist = []
+            if path.isfile():
-            for parent in path.parts():
+                clist = self._getconftestmodules(path.dirpath())
-                if self._confcutdir and self._confcutdir.relto(parent):
+            else:
-                    continue
+                # XXX these days we may rather want to use config.rootdir
-                conftestpath = parent.join("conftest.py")
+                # and allow users to opt into looking into the rootdir parent
-                if conftestpath.check(file=1):
+                # directories instead of requiring to specify confcutdir
-                    mod = self._importconftest(conftestpath)
+                clist = []
-                    clist.append(mod)
+                for parent in path.parts():
+                    if self._confcutdir and self._confcutdir.relto(parent):
+                        continue
+                    conftestpath = parent.join("conftest.py")
+                    if conftestpath.isfile():
+                        mod = self._importconftest(conftestpath)
+                        clist.append(mod)
 
             self._path2confmods[path] = clist
             return clist
@@ -222,6 +219,7 @@ class PytestPluginManager(PluginManager):
             except Exception:
                 raise ConftestImportFailure(conftestpath, sys.exc_info())
 
+            self._conftest_plugins.add(mod)
             self._conftestpath2mod[conftestpath] = mod
             dirpath = conftestpath.dirpath()
             if dirpath in self._path2confmods:
@@ -782,10 +780,6 @@ class Config(object):
             if not hasattr(self.option, opt.dest):
                 setattr(self.option, opt.dest, opt.default)
 
-    def _getmatchingplugins(self, fspath):
-        return self.pluginmanager._globalplugins + \
-               self.pluginmanager._getconftestmodules(fspath)
-
     def pytest_load_initial_conftests(self, early_config):
         self.pluginmanager._set_initial_conftests(early_config.known_args_namespace)
     pytest_load_initial_conftests.trylast = True

_pytest/core.py

@@ -238,7 +238,8 @@ class PluginManager(object):
 
     def subset_hook_caller(self, name, remove_plugins):
         """ Return a new HookCaller instance which manages calls to
-        the plugins but without hooks from remove_plugins taking part. """
+        the plugins but without hooks from the plugins in remove_plugins
+        taking part. """
         hc = getattr(self.hook, name)
         plugins_to_remove = [plugin for plugin in remove_plugins
                                     if hasattr(plugin, name)]
@@ -246,24 +247,6 @@ class PluginManager(object):
             hc = hc.clone()
             for plugin in plugins_to_remove:
                 hc._remove_plugin(plugin)
-            # we also keep track of this hook caller so it
-            # gets properly removed on plugin unregistration
-            self._plugin2hookcallers.setdefault(plugin, []).append(hc)
-        return hc
-
-    def make_hook_caller(self, name, plugins):
-        """ Return a new HookCaller instance which manages calls to
-        all methods named "name" in the plugins. The new hook caller
-        is registered internally such that when one of the plugins gets
-        unregistered, its method will be removed from the hook caller. """
-        caller = getattr(self.hook, name)
-        hc = HookCaller(caller.name, self._hookexec, caller._specmodule_or_class)
-        for plugin in plugins:
-            if hasattr(plugin, name):
-                hc._add_plugin(plugin)
-                # we also keep track of this hook caller so it
-                # gets properly removed on plugin unregistration
-                self._plugin2hookcallers.setdefault(plugin, []).append(hc)
         return hc
 
     def get_canonical_name(self, plugin):
@@ -271,8 +254,8 @@ class PluginManager(object):
         return getattr(plugin, "__name__", None) or str(id(plugin))
 
     def register(self, plugin, name=None):
-        """ Register a plugin and return its canonical name or None if it was
+        """ Register a plugin and return its canonical name or None if the name
-        blocked from registering.  Raise a ValueError if the plugin is already
+        is blocked from registering.  Raise a ValueError if the plugin is already
         registered. """
         plugin_name = name or self.get_canonical_name(plugin)
 
@@ -303,16 +286,15 @@ class PluginManager(object):
 
     def unregister(self, plugin=None, name=None):
         """ unregister a plugin object and all its contained hook implementations
-        from internal data structures.  One of ``plugin`` or ``name`` needs to
+        from internal data structures. """
-        be specified. """
         if name is None:
-            assert plugin is not None
+            assert plugin is not None, "one of name or plugin needs to be specified"
             name = self.get_name(plugin)
 
         if plugin is None:
             plugin = self.get_plugin(name)
 
-        # None signals blocked registrations, don't delete it
+        # if self._name2plugin[name] == None registration was blocked: ignore
         if self._name2plugin.get(name):
             del self._name2plugin[name]
 
@@ -485,6 +467,7 @@ class HookCaller(object):
         self._wrappers = []
         self._nonwrappers = []
         self._hookexec = hook_execute
+        self._subcaller = []
         if specmodule_or_class is not None:
             self.set_specification(specmodule_or_class)
 
@@ -502,6 +485,21 @@ class HookCaller(object):
         if hasattr(specfunc, "historic"):
             self._call_history = []
 
+    def clone(self):
+        assert not self.is_historic()
+        hc = object.__new__(HookCaller)
+        hc.name = self.name
+        hc._plugins = list(self._plugins)
+        hc._wrappers = list(self._wrappers)
+        hc._nonwrappers = list(self._nonwrappers)
+        hc._hookexec = self._hookexec
+        hc.argnames = self.argnames
+        hc.firstresult = self.firstresult
+        # we keep track of this hook caller so it
+        # gets properly removed on plugin unregistration
+        self._subcaller.append(hc)
+        return hc
+
     def is_historic(self):
         return hasattr(self, "_call_history")
 
@@ -512,6 +510,10 @@ class HookCaller(object):
             self._nonwrappers.remove(meth)
         except ValueError:
             self._wrappers.remove(meth)
+        if hasattr(self, "_subcaller"):
+            for hc in self._subcaller:
+                if plugin in hc._plugins:
+                    hc._remove_plugin(plugin)
 
     def _add_plugin(self, plugin):
         self._plugins.append(plugin)

_pytest/main.py

@@ -151,18 +151,17 @@ def pytest_ignore_collect(path, config):
         ignore_paths.extend([py.path.local(x) for x in excludeopt])
     return path in ignore_paths
 
-class FSHookProxy(object):
+class FSHookProxy:
-    def __init__(self, fspath, config):
+    def __init__(self, fspath, pm, remove_mods):
         self.fspath = fspath
-        self.config = config
+        self.pm = pm
+        self.remove_mods = remove_mods
 
     def __getattr__(self, name):
-        plugins = self.config._getmatchingplugins(self.fspath)
+        x = self.pm.subset_hook_caller(name, remove_plugins=self.remove_mods)
-        x = self.config.pluginmanager.make_hook_caller(name, plugins)
         self.__dict__[name] = x
         return x
 
-
 def compatproperty(name):
     def fget(self):
         # deprecated - use pytest.name
@@ -538,8 +537,20 @@ class Session(FSCollector):
         try:
             return self._fs2hookproxy[fspath]
         except KeyError:
-            self._fs2hookproxy[fspath] = x = FSHookProxy(fspath, self.config)
+            # check if we have the common case of running
-            return x
+            # hooks with all conftest.py filesall conftest.py
+            pm = self.config.pluginmanager
+            my_conftestmodules = pm._getconftestmodules(fspath)
+            remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
+            if remove_mods:
+                # one or more conftests are not in use at this fspath
+                proxy = FSHookProxy(fspath, pm, remove_mods)
+            else:
+                # all plugis are active for this fspath
+                proxy = self.config.hook
+
+            self._fs2hookproxy[fspath] = proxy
+            return proxy
 
     def perform_collect(self, args=None, genitems=True):
         hook = self.config.hook

testing/test_core.py

@@ -176,25 +176,6 @@ class TestPluginManager:
         l = pm.hook.he_method1.call_extra([he_method1], dict(arg=1))
         assert l == [10]
 
-    def test_make_hook_caller_unregistered(self, pm):
-        class Hooks:
-            def he_method1(self, arg):
-                pass
-        pm.addhooks(Hooks)
-
-        l = []
-        class Plugin:
-            def he_method1(self, arg):
-                l.append(arg * 10)
-        plugin = Plugin()
-        pm.register(plugin)
-        hc = pm.make_hook_caller("he_method1", [plugin])
-        hc(arg=1)
-        assert l == [10]
-        pm.unregister(plugin)
-        hc(arg=2)
-        assert l == [10]
-
     def test_subset_hook_caller(self, pm):
         class Hooks:
             def he_method1(self, arg):
@@ -232,6 +213,11 @@ class TestPluginManager:
         pm.unregister(plugin1)
         hc(arg=2)
         assert l == []
+        l[:] = []
+
+        pm.hook.he_method1(arg=1)
+        assert l == [10]
+
 
 
 class TestAddMethodOrdering: