simplify plugins bookkeeping further, refine API

--HG--
branch : more_plugin

_pytest/config.py

@@ -127,13 +127,17 @@ class PytestPluginManager(PluginManager):
                       kwargs=dict(plugin=plugin, manager=self))
         return ret
 
-    def unregister(self, plugin):
+    def unregister(self, plugin=None, name=None):
-        super(PytestPluginManager, self).unregister(plugin)
+        plugin = super(PytestPluginManager, self).unregister(plugin, name)
         try:
             self._globalplugins.remove(plugin)
         except ValueError:
             pass
 
+    def getplugin(self, name):
+        # deprecated
+        return self.get_plugin(name)
+
     def pytest_configure(self, config):
         config.addinivalue_line("markers",
             "tryfirst: mark a hook implementation function such that the "
@@ -238,17 +242,14 @@ class PytestPluginManager(PluginManager):
         except ImportError:
             return # XXX issue a warning
         for ep in iter_entry_points('pytest11'):
-            name = ep.name
+            if self.get_plugin(ep.name) or ep.name in self._name2plugin:
-            if name.startswith("pytest_"):
-                name = name[7:]
-            if ep.name in self._name2plugin or name in self._name2plugin:
                 continue
             try:
                 plugin = ep.load()
             except DistributionNotFound:
                 continue
+            self.register(plugin, name=ep.name)
             self._plugin_distinfo.append((ep.dist, plugin))
-            self.register(plugin, name=name)
 
     def consider_preparse(self, args):
         for opt1,opt2 in zip(args, args[1:]):
@@ -257,14 +258,9 @@ class PytestPluginManager(PluginManager):
 
     def consider_pluginarg(self, arg):
         if arg.startswith("no:"):
-            name = arg[3:]
+            self.set_blocked(arg[3:])
-            plugin = self.getplugin(name)
-            if plugin is not None:
-                self.unregister(plugin)
-            self._name2plugin[name] = -1
         else:
-            if self.getplugin(arg) is None:
+            self.import_plugin(arg)
-                self.import_plugin(arg)
 
     def consider_conftest(self, conftestmodule):
         if self.register(conftestmodule, name=conftestmodule.__file__,
@@ -290,7 +286,7 @@ class PytestPluginManager(PluginManager):
         # basename for historic purposes but must be imported with the
         # _pytest prefix.
         assert isinstance(modname, str)
-        if self.getplugin(modname) is not None:
+        if self.get_plugin(modname) is not None:
             return
         if modname in builtin_plugins:
             importspec = "_pytest." + modname
@@ -736,7 +732,7 @@ class Config(object):
                                     fslocation=None, nodeid=None)
 
     def get_terminal_writer(self):
-        return self.pluginmanager.getplugin("terminalreporter")._tw
+        return self.pluginmanager.get_plugin("terminalreporter")._tw
 
     def pytest_cmdline_parse(self, pluginmanager, args):
         # REF1 assert self == pluginmanager.config, (self, pluginmanager.config)

_pytest/core.py

@@ -112,10 +112,13 @@ class TagTracerSub:
     def __init__(self, root, tags):
         self.root = root
         self.tags = tags
+
     def __call__(self, *args):
         self.root.processmessage(self.tags, args)
+
     def setmyprocessor(self, processor):
         self.root.setprocessor(self.tags, processor)
+
     def get(self, name):
         return self.__class__(self.root, self.tags + (name,))
 
@@ -125,6 +128,7 @@ def raise_wrapfail(wrap_controller, msg):
     raise RuntimeError("wrap_controller at %r %s:%d %s" %
                    (co.co_name, co.co_filename, co.co_firstlineno, msg))
 
+
 def wrapped_call(wrap_controller, func):
     """ Wrap calling to a function with a generator which needs to yield
     exactly once.  The yield point will trigger calling the wrapped function
@@ -208,7 +212,6 @@ class PluginManager(object):
         self._prefix = prefix
         self._excludefunc = excludefunc
         self._name2plugin = {}
-        self._plugins = []
         self._plugin2hookcallers = {}
         self.trace = TagTracer().get("pluginmanage")
         self.hook = HookRelay(self.trace.root.get("hook"))
@@ -244,22 +247,25 @@ class PluginManager(object):
                 self._plugin2hookcallers.setdefault(plugin, []).append(hc)
         return hc
 
-    def register(self, plugin, name=None):
+    def get_canonical_name(self, plugin):
-        """ Register a plugin with the given name and ensure that all its
+        """ Return canonical name for the plugin object. """
-        hook implementations are integrated.  If the name is not specified
+        return getattr(plugin, "__name__", None) or str(id(plugin))
-        we use the ``__name__`` attribute of the plugin object or, if that
-        doesn't exist, the id of the plugin.  This method will raise a
-        ValueError if the eventual name is already registered. """
-        name = name or self._get_canonical_name(plugin)
-        if self._name2plugin.get(name, None) == -1:
-            return
-        if self.hasplugin(name):
-            raise ValueError("Plugin already registered: %s=%s\n%s" %(
-                              name, plugin, self._name2plugin))
-        self._name2plugin[name] = plugin
-        self._plugins.append(plugin)
 
-        # register prefix-matching hooks of the plugin
+    def register(self, plugin, name=None):
+        """ Register a plugin and return its canonical name or None if it was
+        blocked from registering.  Raise a ValueError if the plugin is already
+        registered. """
+        plugin_name = name or self.get_canonical_name(plugin)
+
+        if plugin_name in self._name2plugin or plugin in self._plugin2hookcallers:
+            if self._name2plugin.get(plugin_name, -1) is None:
+                return  # blocked plugin, return None to indicate no registration
+            raise ValueError("Plugin already registered: %s=%s\n%s" %(
+                              plugin_name, plugin, self._name2plugin))
+
+        self._name2plugin[plugin_name] = plugin
+
+        # register prefix-matching hook specs of the plugin
         self._plugin2hookcallers[plugin] = hookcallers = []
         for name in dir(plugin):
             if name.startswith(self._prefix):
@@ -274,18 +280,33 @@ class PluginManager(object):
                     hook._maybe_apply_history(getattr(plugin, name))
                 hookcallers.append(hook)
                 hook._add_plugin(plugin)
-        return True
+        return plugin_name
 
-    def unregister(self, plugin):
+    def unregister(self, plugin=None, name=None):
-        """ unregister the plugin object and all its contained hook implementations
+        """ unregister a plugin object and all its contained hook implementations
-        from internal data structures. """
+        from internal data structures.  One of ``plugin`` or ``name`` needs to
-        self._plugins.remove(plugin)
+        be specified. """
-        for name, value in list(self._name2plugin.items()):
+        if name is None:
-            if value == plugin:
+            assert plugin is not None
-                del self._name2plugin[name]
+            name = self.get_canonical_name(plugin)
-        for hookcaller in self._plugin2hookcallers.pop(plugin):
+
+        if plugin is None:
+            plugin = self.get_plugin(name)
+
+        # None signals blocked registrations, don't delete it
+        if self._name2plugin.get(name):
+            del self._name2plugin[name]
+
+        for hookcaller in self._plugin2hookcallers.pop(plugin, []):
             hookcaller._remove_plugin(plugin)
 
+        return plugin
+
+    def set_blocked(self, name):
+        """ block registrations of the given name, unregister if already registered. """
+        self.unregister(name=name)
+        self._name2plugin[name] = None
+
     def addhooks(self, module_or_class):
         """ add new hook definitions from the given module_or_class using
         the prefix/excludefunc with which the PluginManager was initialized. """
@@ -302,33 +323,27 @@ class PluginManager(object):
                     for plugin in hc._plugins:
                         self._verify_hook(hc, plugin)
                 names.append(name)
+
         if not names:
             raise ValueError("did not find new %r hooks in %r"
                              %(self._prefix, module_or_class))
 
-    def getplugins(self):
+    def get_plugins(self):
-        """ return the complete list of registered plugins. NOTE that
+        """ return the set of registered plugins. """
-        you will get the internal list and need to make a copy if you
+        return set(self._plugin2hookcallers)
-        modify the list."""
-        return self._plugins
 
-    def isregistered(self, plugin):
+    def is_registered(self, plugin):
-        """ Return True if the plugin is already registered under its
+        """ Return True if the plugin is already registered. """
-        canonical name. """
+        return plugin in self._plugin2hookcallers
-        return self.hasplugin(self._get_canonical_name(plugin)) or \
-               plugin in self._plugins
 
-    def hasplugin(self, name):
+    def get_plugin(self, name):
-        """ Return True if there is a registered with the given name. """
-        return name in self._name2plugin
-
-    def getplugin(self, name):
         """ Return a plugin or None for the given name. """
         return self._name2plugin.get(name)
 
     def _verify_hook(self, hook, plugin):
         method = getattr(plugin, hook.name)
-        pluginname = self._get_canonical_name(plugin)
+        pluginname = self.get_canonical_name(plugin)
+
         if hook.is_historic() and hasattr(method, "hookwrapper"):
             raise PluginValidationError(
                 "Plugin %r\nhook %r\nhistoric incompatible to hookwrapper" %(
@@ -344,6 +359,8 @@ class PluginManager(object):
                        ", ".join(hook.argnames)))
 
     def check_pending(self):
+        """ Verify that all hooks which have not been verified against
+        a hook specification are optional, otherwise raise PluginValidationError"""
         for name in self.hook.__dict__:
             if name.startswith(self._prefix):
                 hook = getattr(self.hook, name)
@@ -354,10 +371,6 @@ class PluginManager(object):
                             raise PluginValidationError(
                                 "unknown hook %r in plugin %r" %(name, plugin))
 
-    def _get_canonical_name(self, plugin):
-        return getattr(plugin, "__name__", None) or str(id(plugin))
-
-
 
 class MultiCall:
     """ execute a call into multiple python functions/methods. """

_pytest/pytester.py

@@ -11,7 +11,7 @@ import subprocess
 import py
 import pytest
 from py.builtin import print_
-from _pytest.core import HookCaller, TracedHookExecution
+from _pytest.core import TracedHookExecution
 
 from _pytest.main import Session, EXIT_OK
 

testing/test_config.py

@@ -313,7 +313,7 @@ def test_plugin_preparse_prevents_setuptools_loading(testdir, monkeypatch):
     monkeypatch.setattr(pkg_resources, 'iter_entry_points', my_iter)
     config = testdir.parseconfig("-p", "no:mytestplugin")
     plugin = config.pluginmanager.getplugin("mytestplugin")
-    assert plugin == -1
+    assert plugin is None
 
 def test_cmdline_processargs_simple(testdir):
     testdir.makeconftest("""

testing/test_core.py

@@ -17,20 +17,34 @@ class TestPluginManager:
         pm.register(42, name="abc")
         with pytest.raises(ValueError):
             pm.register(42, name="abc")
+        with pytest.raises(ValueError):
+            pm.register(42, name="def")
 
     def test_pm(self, pm):
         class A: pass
         a1, a2 = A(), A()
         pm.register(a1)
-        assert pm.isregistered(a1)
+        assert pm.is_registered(a1)
         pm.register(a2, "hello")
-        assert pm.isregistered(a2)
+        assert pm.is_registered(a2)
-        l = pm.getplugins()
+        l = pm.get_plugins()
         assert a1 in l
         assert a2 in l
-        assert pm.getplugin('hello') == a2
+        assert pm.get_plugin('hello') == a2
-        pm.unregister(a1)
+        assert pm.unregister(a1) == a1
-        assert not pm.isregistered(a1)
+        assert not pm.is_registered(a1)
+
+    def test_set_blocked(self, pm):
+        class A: pass
+        a1 = A()
+        name = pm.register(a1)
+        assert pm.is_registered(a1)
+        pm.set_blocked(name)
+        assert not pm.is_registered(a1)
+
+        pm.set_blocked("somename")
+        assert not pm.register(A(), "somename")
+        pm.unregister(name="somename")
 
     def test_register_mismatch_method(self, pytestpm):
         class hello:
@@ -53,29 +67,16 @@ class TestPluginManager:
             pass
         my = MyPlugin()
         pm.register(my)
-        assert pm.getplugins()
+        assert pm.get_plugins()
         my2 = MyPlugin()
         pm.register(my2)
-        assert pm.getplugins()[-2:] == [my, my2]
+        assert set([my,my2]).issubset(pm.get_plugins())
 
-        assert pm.isregistered(my)
+        assert pm.is_registered(my)
-        assert pm.isregistered(my2)
+        assert pm.is_registered(my2)
         pm.unregister(my)
-        assert not pm.isregistered(my)
+        assert not pm.is_registered(my)
-        assert pm.getplugins()[-1:] == [my2]
+        assert my not in pm.get_plugins()
-
-    def test_register_unknown_hooks(self, pm):
-        class Plugin1:
-            def he_method1(self, arg):
-                return arg + 1
-
-        pm.register(Plugin1())
-        class Hooks:
-            def he_method1(self, arg):
-                pass
-        pm.addhooks(Hooks)
-        #assert not pm._unverified_hooks
-        assert pm.hook.he_method1(arg=1) == [2]
 
     def test_register_unknown_hooks(self, pm):
         class Plugin1:
@@ -231,6 +232,26 @@ class TestAddMethodOrdering:
             pass
         assert hc._nonwrappers == [he_method1, he_method1_middle, he_method1_b]
 
+    def test_adding_nonwrappers_trylast3(self, hc, addmeth):
+        @addmeth()
+        def he_method1_a():
+            pass
+
+        @addmeth(trylast=True)
+        def he_method1_b():
+            pass
+
+        @addmeth()
+        def he_method1_c():
+            pass
+
+        @addmeth(trylast=True)
+        def he_method1_d():
+            pass
+        assert hc._nonwrappers == [he_method1_d, he_method1_b,
+                                   he_method1_a, he_method1_c]
+
+
     def test_adding_nonwrappers_trylast2(self, hc, addmeth):
         @addmeth()
         def he_method1_middle():
@@ -259,24 +280,6 @@ class TestAddMethodOrdering:
             pass
         assert hc._nonwrappers == [he_method1_middle, he_method1_b, he_method1]
 
-    def test_adding_nonwrappers_trylast(self, hc, addmeth):
-        @addmeth()
-        def he_method1_a():
-            pass
-
-        @addmeth(trylast=True)
-        def he_method1_b():
-            pass
-
-        @addmeth()
-        def he_method1_c():
-            pass
-
-        @addmeth(trylast=True)
-        def he_method1_d():
-            pass
-        assert hc._nonwrappers == [he_method1_d, he_method1_b, he_method1_a, he_method1_c]
-
     def test_adding_wrappers_ordering(self, hc, addmeth):
         @addmeth(hookwrapper=True)
         def he_method1():
@@ -361,7 +364,7 @@ class TestPytestPluginInteractions:
         pm.hook.pytest_addhooks.call_historic(
                                 kwargs=dict(pluginmanager=config.pluginmanager))
         config.pluginmanager._importconftest(conf)
-        #print(config.pluginmanager.getplugins())
+        #print(config.pluginmanager.get_plugins())
         res = config.hook.pytest_myhook(xyz=10)
         assert res == [11]
 
@@ -814,21 +817,21 @@ class TestPytestPluginManager:
         pm = PytestPluginManager()
         mod = py.std.types.ModuleType("x.y.pytest_hello")
         pm.register(mod)
-        assert pm.isregistered(mod)
+        assert pm.is_registered(mod)
-        l = pm.getplugins()
+        l = pm.get_plugins()
         assert mod in l
         pytest.raises(ValueError, "pm.register(mod)")
         pytest.raises(ValueError, lambda: pm.register(mod))
-        #assert not pm.isregistered(mod2)
+        #assert not pm.is_registered(mod2)
-        assert pm.getplugins() == l
+        assert pm.get_plugins() == l
 
     def test_canonical_import(self, monkeypatch):
         mod = py.std.types.ModuleType("pytest_xyz")
         monkeypatch.setitem(py.std.sys.modules, 'pytest_xyz', mod)
         pm = PytestPluginManager()
         pm.import_plugin('pytest_xyz')
-        assert pm.getplugin('pytest_xyz') == mod
+        assert pm.get_plugin('pytest_xyz') == mod
-        assert pm.isregistered(mod)
+        assert pm.is_registered(mod)
 
     def test_consider_module(self, testdir, pytestpm):
         testdir.syspathinsert()
@@ -837,8 +840,8 @@ class TestPytestPluginManager:
         mod = py.std.types.ModuleType("temp")
         mod.pytest_plugins = ["pytest_p1", "pytest_p2"]
         pytestpm.consider_module(mod)
-        assert pytestpm.getplugin("pytest_p1").__name__ == "pytest_p1"
+        assert pytestpm.get_plugin("pytest_p1").__name__ == "pytest_p1"
-        assert pytestpm.getplugin("pytest_p2").__name__ == "pytest_p2"
+        assert pytestpm.get_plugin("pytest_p2").__name__ == "pytest_p2"
 
     def test_consider_module_import_module(self, testdir):
         pytestpm = get_config().pluginmanager
@@ -880,13 +883,13 @@ class TestPytestPluginManager:
         testdir.syspathinsert()
         testdir.makepyfile(xy123="#")
         monkeypatch.setitem(os.environ, 'PYTEST_PLUGINS', 'xy123')
-        l1 = len(pytestpm.getplugins())
+        l1 = len(pytestpm.get_plugins())
         pytestpm.consider_env()
-        l2 = len(pytestpm.getplugins())
+        l2 = len(pytestpm.get_plugins())
         assert l2 == l1 + 1
-        assert pytestpm.getplugin('xy123')
+        assert pytestpm.get_plugin('xy123')
         pytestpm.consider_env()
-        l3 = len(pytestpm.getplugins())
+        l3 = len(pytestpm.get_plugins())
         assert l2 == l3
 
     def test_consider_setuptools_instantiation(self, monkeypatch, pytestpm):
@@ -904,7 +907,7 @@ class TestPytestPluginManager:
 
         monkeypatch.setattr(pkg_resources, 'iter_entry_points', my_iter)
         pytestpm.consider_setuptools_entrypoints()
-        plugin = pytestpm.getplugin("mytestplugin")
+        plugin = pytestpm.get_plugin("pytest_mytestplugin")
         assert plugin.x == 42
 
     def test_consider_setuptools_not_installed(self, monkeypatch, pytestpm):
@@ -918,7 +921,7 @@ class TestPytestPluginManager:
         p = testdir.makepyfile("""
             import pytest
             def test_hello(pytestconfig):
-                plugin = pytestconfig.pluginmanager.getplugin('pytest_x500')
+                plugin = pytestconfig.pluginmanager.get_plugin('pytest_x500')
                 assert plugin is not None
         """)
         monkeypatch.setenv('PYTEST_PLUGINS', 'pytest_x500', prepend=",")
@@ -934,13 +937,13 @@ class TestPytestPluginManager:
         pluginname = "pytest_hello"
         testdir.makepyfile(**{pluginname: ""})
         pytestpm.import_plugin("pytest_hello")
-        len1 = len(pytestpm.getplugins())
+        len1 = len(pytestpm.get_plugins())
         pytestpm.import_plugin("pytest_hello")
-        len2 = len(pytestpm.getplugins())
+        len2 = len(pytestpm.get_plugins())
         assert len1 == len2
-        plugin1 = pytestpm.getplugin("pytest_hello")
+        plugin1 = pytestpm.get_plugin("pytest_hello")
         assert plugin1.__name__.endswith('pytest_hello')
-        plugin2 = pytestpm.getplugin("pytest_hello")
+        plugin2 = pytestpm.get_plugin("pytest_hello")
         assert plugin2 is plugin1
 
     def test_import_plugin_dotted_name(self, testdir, pytestpm):
@@ -951,7 +954,7 @@ class TestPytestPluginManager:
         testdir.mkpydir("pkg").join("plug.py").write("x=3")
         pluginname = "pkg.plug"
         pytestpm.import_plugin(pluginname)
-        mod = pytestpm.getplugin("pkg.plug")
+        mod = pytestpm.get_plugin("pkg.plug")
         assert mod.x == 3
 
     def test_consider_conftest_deps(self, testdir, pytestpm):
@@ -967,15 +970,16 @@ class TestPytestPluginManagerBootstrapming:
 
     def test_plugin_prevent_register(self, pytestpm):
         pytestpm.consider_preparse(["xyz", "-p", "no:abc"])
-        l1 = pytestpm.getplugins()
+        l1 = pytestpm.get_plugins()
         pytestpm.register(42, name="abc")
-        l2 = pytestpm.getplugins()
+        l2 = pytestpm.get_plugins()
         assert len(l2) == len(l1)
+        assert 42 not in l2
 
     def test_plugin_prevent_register_unregistered_alredy_registered(self, pytestpm):
         pytestpm.register(42, name="abc")
-        l1 = pytestpm.getplugins()
+        l1 = pytestpm.get_plugins()
         assert 42 in l1
         pytestpm.consider_preparse(["xyz", "-p", "no:abc"])
-        l2 = pytestpm.getplugins()
+        l2 = pytestpm.get_plugins()
         assert 42 not in l2

testing/test_terminal.py

@@ -457,7 +457,7 @@ class TestTerminalFunctional:
         ])
         assert result.ret == 1
 
-        if not pytestconfig.pluginmanager.hasplugin("xdist"):
+        if not pytestconfig.pluginmanager.get_plugin("xdist"):
             pytest.skip("xdist plugin not installed")
 
         result = testdir.runpytest(p1, '-v', '-n 1')