summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJason Madden <jamadden@gmail.com>2020-02-08 07:57:35 -0600
committerJason Madden <jamadden@gmail.com>2020-02-08 07:57:35 -0600
commitf6d2e9445d47dd104c146157065169be8df4d409 (patch)
tree0e8f22680524d135f8af30c720c3057f6397c07f
parentd6343eeaa7f67838db9de82974898b80938a7aef (diff)
downloadzope-interface-f6d2e9445d47dd104c146157065169be8df4d409.tar.gz
Make verifyObject/Class collect and raise all errors instead of only the first.issue171
Fixes #171.
-rw-r--r--CHANGES.rst8
-rw-r--r--docs/verify.rst64
-rw-r--r--src/zope/interface/exceptions.py92
-rw-r--r--src/zope/interface/tests/test_exceptions.py54
-rw-r--r--src/zope/interface/tests/test_verify.py33
-rw-r--r--src/zope/interface/verify.py153
6 files changed, 296 insertions, 108 deletions
diff --git a/CHANGES.rst b/CHANGES.rst
index b484a99..b21f54d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -108,6 +108,14 @@
.. caution:: This will break consumers (such as doctests) that
depended on the exact error messages.
+- Make ``verifyObject`` and ``verifyClass`` report all errors, if the
+ candidate object has multiple detectable violations. Previously they
+ reported only the first error. See `issue
+ <https://github.com/zopefoundation/zope.interface/issues/171>`_.
+
+ Like the above, this will break consumers depending on the exact
+ output of error messages if more than one error is present.
+
4.7.1 (2019-11-11)
==================
diff --git a/docs/verify.rst b/docs/verify.rst
index 6f48218..a921b8b 100644
--- a/docs/verify.rst
+++ b/docs/verify.rst
@@ -23,7 +23,7 @@ Attributes of the object, be they defined by its class or added by its
.. doctest::
>>> from zope.interface import Interface, Attribute, implementer
- >>> from zope.interface.exceptions import BrokenImplementation
+ >>> from zope.interface import Invalid
>>> class IFoo(Interface):
... x = Attribute("The X attribute")
... y = Attribute("The Y attribute")
@@ -47,7 +47,7 @@ exception. (We'll define a helper to make this easier to show.)
... foo = Foo()
... try:
... return verifyObject(IFoo, foo)
- ... except BrokenImplementation as e:
+ ... except Invalid as e:
... print(e)
>>> @implementer(IFoo)
@@ -62,6 +62,18 @@ exception. (We'll define a helper to make this easier to show.)
>>> verify_foo()
The object <Foo...> has failed to implement interface <InterfaceClass ...IFoo>: The IFoo.x attribute was not provided.
+If both attributes are missing, an exception is raised reporting
+both errors.
+
+.. doctest::
+
+ >>> @implementer(IFoo)
+ ... class Foo(object):
+ ... pass
+ >>> verify_foo()
+ The object <Foo ...> has failed to implement interface <InterfaceClass ...IFoo>:
+ The IFoo.x attribute was not provided
+ The IFoo.y attribute was not provided
If an attribute is implemented as a property that raises an ``AttributeError``
when trying to get its value, the attribute is considered missing:
@@ -124,18 +136,7 @@ that takes one argument. If we don't provide it, we get an error.
>>> verify_foo()
The object <Foo...> has failed to implement interface <InterfaceClass builtins.IFoo>: The IFoo.simple(arg1) attribute was not provided.
-Once they exist, they are checked for compatible signatures. This is a
-different type of exception, so we need an updated helper.
-
-.. doctest::
-
- >>> from zope.interface.exceptions import BrokenMethodImplementation
- >>> def verify_foo():
- ... foo = Foo()
- ... try:
- ... return verifyObject(IFoo, foo)
- ... except BrokenMethodImplementation as e:
- ... print(e)
+Once they exist, they are checked to be callable, and for compatible signatures.
Not being callable is an error.
@@ -143,7 +144,7 @@ Not being callable is an error.
>>> Foo.simple = 42
>>> verify_foo()
- The object <Foo...> violates its contract in IFoo.simple(arg1): implementation is not a method.
+ The object <Foo...> violates the contract of IFoo.simple(arg1) because implementation is not a method.
Taking too few arguments is an error.
@@ -151,7 +152,7 @@ Taking too few arguments is an error.
>>> Foo.simple = lambda: "I take no arguments"
>>> verify_foo()
- The object <Foo...> violates its contract in IFoo.simple(arg1): implementation doesn't allow enough arguments.
+ The object <Foo...> violates the contract of IFoo.simple(arg1) because implementation doesn't allow enough arguments.
Requiring too many arguments is an error. (Recall that the ``self``
argument is implicit.)
@@ -160,7 +161,7 @@ argument is implicit.)
>>> Foo.simple = lambda self, a, b: "I require two arguments"
>>> verify_foo()
- The object <Foo...> violates its contract in IFoo.simple(arg1): implementation requires too many arguments.
+ The object <Foo...> violates the contract of IFoo.simple(arg1) because implementation requires too many arguments.
Variable arguments can be used to implement the required number, as
can arguments with defaults.
@@ -185,7 +186,7 @@ variable keyword arguments, the implementation must also accept them.
... class Foo(object):
... def needs_kwargs(self, a=1, b=2): pass
>>> verify_foo()
- The object <Foo...> violates its contract in IFoo.needs_kwargs(**kwargs): implementation doesn't support keyword arguments.
+ The object <Foo...> violates the contract of IFoo.needs_kwargs(**kwargs) because implementation doesn't support keyword arguments.
>>> class IFoo(Interface):
... def needs_varargs(*args): pass
@@ -193,15 +194,32 @@ variable keyword arguments, the implementation must also accept them.
... class Foo(object):
... def needs_varargs(self, **kwargs): pass
>>> verify_foo()
- The object <Foo...> violates its contract in IFoo.needs_varargs(*args): implementation doesn't support variable arguments.
+ The object <Foo...> violates the contract of IFoo.needs_varargs(*args) because implementation doesn't support variable arguments.
+
+
+Of course, missing attributes are also found and reported.
+
+.. doctest::
+
+ >>> class IFoo(Interface):
+ ... x = Attribute('The X attribute')
+ ... def method(arg1): "Takes one positional argument"
+ >>> @implementer(IFoo)
+ ... class Foo(object):
+ ... def method(self): "I don't have enough arguments"
+ >>> verify_foo()
+ The object <Foo...> has failed to implement interface <InterfaceClass ...IFoo>:
+ The IFoo.x attribute was not provided
+ violates the contract of IFoo.method(arg1) because implementation doesn't allow enough arguments
Verifying Classes
=================
The function `verifyClass` is used to check that a class implements
an interface properly, meaning that its instances properly provide the
-interface. Most of the same things that `verifyObject` checks can be
-checked for classes.
+interface. Many of the same things that `verifyObject` checks can be
+checked for classes, but certain conditions, such as the presence of
+attributes, cannot be verified.
.. autofunction:: verifyClass
@@ -211,8 +229,8 @@ checked for classes.
>>> def verify_foo_class():
... try:
... return verifyClass(IFoo, Foo)
- ... except BrokenMethodImplementation as e:
+ ... except Invalid as e:
... print(e)
>>> verify_foo_class()
- The object <class 'Foo'> violates its contract in IFoo.needs_varargs(*args): implementation doesn't support variable arguments.
+ The object <class 'Foo'> violates the contract of IFoo.method(arg1) because implementation doesn't allow enough arguments.
diff --git a/src/zope/interface/exceptions.py b/src/zope/interface/exceptions.py
index 8d21f33..86caf3f 100644
--- a/src/zope/interface/exceptions.py
+++ b/src/zope/interface/exceptions.py
@@ -20,6 +20,7 @@ __all__ = [
'DoesNotImplement',
'BrokenImplementation',
'BrokenMethodImplementation',
+ 'MultipleInvalid',
# Other
'BadImplements',
'InvalidInterface',
@@ -29,18 +30,37 @@ class Invalid(Exception):
"""A specification is violated
"""
-_NotGiven = object()
+_NotGiven = '<Not Given>'
class _TargetMixin(object):
target = _NotGiven
+ interface = None
@property
- def _prefix(self):
+ def _target_prefix(self):
if self.target is _NotGiven:
return "An object"
return "The object %r" % (self.target,)
-class DoesNotImplement(Invalid, _TargetMixin):
+ _trailer = '.'
+
+ @property
+ def _general_description(self):
+ return "has failed to implement interface %s:" % (
+ self.interface
+ ) if self.interface is not None else ''
+
+
+ def __str__(self):
+ return "%s %s%s%s" % (
+ self._target_prefix,
+ self._general_description,
+ self._specifics,
+ self._trailer
+ )
+
+
+class DoesNotImplement(_TargetMixin, Invalid):
"""
The *target* (optional) does not implement the *interface*.
@@ -50,17 +70,17 @@ class DoesNotImplement(Invalid, _TargetMixin):
"""
def __init__(self, interface, target=_NotGiven):
- Invalid.__init__(self)
+ Invalid.__init__(self, interface, target)
self.interface = interface
self.target = target
- def __str__(self):
- return "%s does not implement the interface %s." % (
- self._prefix,
- self.interface
- )
+ _general_description = "does not implement the interface"
-class BrokenImplementation(Invalid, _TargetMixin):
+ @property
+ def _specifics(self):
+ return ' ' + str(self.interface)
+
+class BrokenImplementation(_TargetMixin, Invalid):
"""
The *target* (optional) is missing the attribute *name*.
@@ -72,19 +92,19 @@ class BrokenImplementation(Invalid, _TargetMixin):
"""
def __init__(self, interface, name, target=_NotGiven):
- Invalid.__init__(self)
+ Invalid.__init__(self, interface, name, target)
self.interface = interface
self.name = name
self.target = target
- def __str__(self):
- return "%s has failed to implement interface %s: The %s attribute was not provided." % (
- self._prefix,
- self.interface,
+
+ @property
+ def _specifics(self):
+ return " The %s attribute was not provided" % (
repr(self.name) if isinstance(self.name, str) else self.name
)
-class BrokenMethodImplementation(Invalid, _TargetMixin):
+class BrokenMethodImplementation(_TargetMixin, Invalid):
"""
The *target* (optional) has a *method* that violates
its contract in a way described by *mess*.
@@ -97,19 +117,49 @@ class BrokenMethodImplementation(Invalid, _TargetMixin):
"""
def __init__(self, method, mess, target=_NotGiven):
- Invalid.__init__(self)
+ Invalid.__init__(self, method, mess, target)
self.method = method
self.mess = mess
self.target = target
- def __str__(self):
- return "%s violates its contract in %s: %s." % (
- self._prefix,
+ @property
+ def _specifics(self):
+ return 'violates the contract of %s because %s' % (
repr(self.method) if isinstance(self.method, str) else self.method,
- self.mess
+ self.mess,
)
+class MultipleInvalid(_TargetMixin, Invalid):
+ """
+ The *target* has failed to implement the *iface* in
+ multiple ways.
+
+ The failures are described by *exceptions*, a collection of
+ other `Invalid` instances.
+
+ .. versionadded:: 5.0
+ """
+
+ def __init__(self, iface, target, exceptions):
+ exceptions = list(exceptions)
+ Invalid.__init__(self, iface, target, exceptions)
+ self.target = target
+ self.interface = iface
+ self.exceptions = exceptions
+
+ @property
+ def _specifics(self):
+ # It would be nice to use tabs here, but that
+ # is hard to represent in doctests.
+ return '\n ' + '\n '.join(
+ x._specifics.strip() if isinstance(x, _TargetMixin) else(str(x))
+ for x in self.exceptions
+ )
+
+ _trailer = ''
+
+
class InvalidInterface(Exception):
"""The interface has invalid contents
"""
diff --git a/src/zope/interface/tests/test_exceptions.py b/src/zope/interface/tests/test_exceptions.py
index 34ee071..42ea7eb 100644
--- a/src/zope/interface/tests/test_exceptions.py
+++ b/src/zope/interface/tests/test_exceptions.py
@@ -85,10 +85,60 @@ class BrokenMethodImplementationTests(unittest.TestCase):
dni = self._makeOne()
self.assertEqual(
str(dni),
- "An object violates its contract in 'aMethod': I said so.")
+ "An object violates the contract of 'aMethod' because I said so.")
def test___str__w_candidate(self):
dni = self._makeOne('candidate')
self.assertEqual(
str(dni),
- "The object 'candidate' violates its contract in 'aMethod': I said so.")
+ "The object 'candidate' violates the contract of 'aMethod' because I said so.")
+
+ def test___repr__w_candidate(self):
+ dni = self._makeOne('candidate')
+ self.assertEqual(
+ repr(dni),
+ "BrokenMethodImplementation('aMethod', 'I said so', 'candidate')"
+ )
+
+
+class MultipleInvalidTests(unittest.TestCase):
+
+ def _getTargetClass(self):
+ from zope.interface.exceptions import MultipleInvalid
+ return MultipleInvalid
+
+ def _makeOne(self, excs):
+ iface = _makeIface()
+ return self._getTargetClass()(iface, 'target', excs)
+
+ def test__str__(self):
+ from zope.interface.exceptions import BrokenMethodImplementation
+ excs = [
+ BrokenMethodImplementation('aMethod', 'I said so'),
+ Exception("Regular exception")
+ ]
+ dni = self._makeOne(excs)
+ self.assertEqual(
+ str(dni),
+ "The object 'target' has failed to implement interface "
+ "<InterfaceClass zope.interface.tests.test_exceptions.IDummy>:\n"
+ " violates the contract of 'aMethod' because I said so\n"
+ " Regular exception"
+ )
+
+ def test__repr__(self):
+ from zope.interface.exceptions import BrokenMethodImplementation
+ excs = [
+ BrokenMethodImplementation('aMethod', 'I said so'),
+ # Use multiple arguments to normalize repr; versions of Python
+ # prior to 3.7 add a trailing comma if there's just one.
+ Exception("Regular", "exception")
+ ]
+ dni = self._makeOne(excs)
+ self.assertEqual(
+ repr(dni),
+ "MultipleInvalid(<InterfaceClass zope.interface.tests.test_exceptions.IDummy>,"
+ " 'target',"
+ " [BrokenMethodImplementation('aMethod', 'I said so', '<Not Given>'),"
+ " Exception('Regular', 'exception')])"
+ )
diff --git a/src/zope/interface/tests/test_verify.py b/src/zope/interface/tests/test_verify.py
index 65e390a..3a68d20 100644
--- a/src/zope/interface/tests/test_verify.py
+++ b/src/zope/interface/tests/test_verify.py
@@ -554,6 +554,39 @@ class Test_verifyClass(unittest.TestCase):
self._callFUT(IReadSequence, tuple, tentative=True)
+ def test_multiple_invalid(self):
+ from zope.interface.exceptions import MultipleInvalid
+ from zope.interface.exceptions import DoesNotImplement
+ from zope.interface.exceptions import BrokenImplementation
+ from zope.interface import Interface
+ from zope.interface import classImplements
+
+ class ISeveralMethods(Interface):
+ def meth1(arg1):
+ "Method 1"
+ def meth2(arg1):
+ "Method 2"
+
+ class SeveralMethods(object):
+ pass
+
+ with self.assertRaises(MultipleInvalid) as exc:
+ self._callFUT(ISeveralMethods, SeveralMethods)
+
+ ex = exc.exception
+ self.assertEqual(3, len(ex.exceptions))
+ self.assertIsInstance(ex.exceptions[0], DoesNotImplement)
+ self.assertIsInstance(ex.exceptions[1], BrokenImplementation)
+ self.assertIsInstance(ex.exceptions[2], BrokenImplementation)
+
+ # If everything else is correct, only the single error is raised without
+ # the wrapper.
+ classImplements(SeveralMethods, ISeveralMethods)
+ SeveralMethods.meth1 = lambda self, arg1: "Hi"
+
+ with self.assertRaises(BrokenImplementation):
+ self._callFUT(ISeveralMethods, SeveralMethods)
+
class Test_verifyObject(Test_verifyClass):
@classmethod
diff --git a/src/zope/interface/verify.py b/src/zope/interface/verify.py
index a8dd952..a9b80a7 100644
--- a/src/zope/interface/verify.py
+++ b/src/zope/interface/verify.py
@@ -21,8 +21,12 @@ from types import MethodType
from zope.interface._compat import PYPY2
-from zope.interface.exceptions import BrokenImplementation, DoesNotImplement
+from zope.interface.exceptions import BrokenImplementation
from zope.interface.exceptions import BrokenMethodImplementation
+from zope.interface.exceptions import DoesNotImplement
+from zope.interface.exceptions import Invalid
+from zope.interface.exceptions import MultipleInvalid
+
from zope.interface.interface import fromMethod, fromFunction, Method
__all__ = [
@@ -55,8 +59,16 @@ def _verify(iface, candidate, tentative=False, vtype=None):
- Making sure the candidate defines all the necessary attributes
+ :return bool: Returns a true value if everything that could be
+ checked passed.
:raises zope.interface.Invalid: If any of the previous
conditions does not hold.
+
+ .. versionchanged:: 5.0
+ If multiple methods or attributes are invalid, all such errors
+ are collected and reported. Previously, only the first error was reported.
+ As a special case, if only one such error is present, it is raised
+ alone, like before.
"""
if vtype == 'c':
@@ -64,74 +76,91 @@ def _verify(iface, candidate, tentative=False, vtype=None):
else:
tester = iface.providedBy
+ excs = []
if not tentative and not tester(candidate):
- raise DoesNotImplement(iface)
+ excs.append(DoesNotImplement(iface))
- # Here the `desc` is either an `Attribute` or `Method` instance
for name, desc in iface.namesAndDescriptions(all=True):
try:
- attr = getattr(candidate, name)
- except AttributeError:
- if (not isinstance(desc, Method)) and vtype == 'c':
- # We can't verify non-methods on classes, since the
- # class may provide attrs in it's __init__.
- continue
-
- raise BrokenImplementation(iface, desc, candidate)
-
- if not isinstance(desc, Method):
- # If it's not a method, there's nothing else we can test
- continue
-
- if inspect.ismethoddescriptor(attr) or inspect.isbuiltin(attr):
- # The first case is what you get for things like ``dict.pop``
- # on CPython (e.g., ``verifyClass(IFullMapping, dict))``). The
- # second case is what you get for things like ``dict().pop`` on
- # CPython (e.g., ``verifyObject(IFullMapping, dict()))``.
- # In neither case can we get a signature, so there's nothing
- # to verify. Even the inspect module gives up and raises
- # ValueError: no signature found. The ``__text_signature__`` attribute
- # isn't typically populated either.
- #
- # Note that on PyPy 2 or 3 (up through 7.3 at least), these are
- # not true for things like ``dict.pop`` (but might be true for C extensions?)
- continue
-
- if isinstance(attr, FunctionType):
- if sys.version_info[0] >= 3 and isinstance(candidate, type) and vtype == 'c':
- # This is an "unbound method" in Python 3.
- # Only unwrap this if we're verifying implementedBy;
- # otherwise we can unwrap @staticmethod on classes that directly
- # provide an interface.
- meth = fromFunction(attr, iface, name=name,
- imlevel=1)
- else:
- # Nope, just a normal function
- meth = fromFunction(attr, iface, name=name)
- elif (isinstance(attr, MethodTypes)
- and type(attr.__func__) is FunctionType):
- meth = fromMethod(attr, iface, name)
- elif isinstance(attr, property) and vtype == 'c':
- # We without an instance we cannot be sure it's not a
- # callable.
- continue
- else:
- if not callable(attr):
- raise BrokenMethodImplementation(desc, "implementation is not a method", candidate)
- # sigh, it's callable, but we don't know how to introspect it, so
- # we have to give it a pass.
- continue
-
- # Make sure that the required and implemented method signatures are
- # the same.
- mess = _incompat(desc.getSignatureInfo(), meth.getSignatureInfo())
- if mess:
- if PYPY2 and _pypy2_false_positive(mess, candidate, vtype):
- continue
- raise BrokenMethodImplementation(desc, mess, candidate)
+ _verify_element(iface, name, desc, candidate, vtype)
+ except Invalid as e:
+ excs.append(e)
+
+ if excs:
+ if len(excs) == 1:
+ raise excs[0]
+ raise MultipleInvalid(iface, candidate, excs)
return True
+def _verify_element(iface, name, desc, candidate, vtype):
+ # Here the `desc` is either an `Attribute` or `Method` instance
+ try:
+ attr = getattr(candidate, name)
+ except AttributeError:
+ if (not isinstance(desc, Method)) and vtype == 'c':
+ # We can't verify non-methods on classes, since the
+ # class may provide attrs in it's __init__.
+ return
+
+ raise BrokenImplementation(iface, desc, candidate)
+
+ if not isinstance(desc, Method):
+ # If it's not a method, there's nothing else we can test
+ return
+
+ if inspect.ismethoddescriptor(attr) or inspect.isbuiltin(attr):
+ # The first case is what you get for things like ``dict.pop``
+ # on CPython (e.g., ``verifyClass(IFullMapping, dict))``). The
+ # second case is what you get for things like ``dict().pop`` on
+ # CPython (e.g., ``verifyObject(IFullMapping, dict()))``.
+ # In neither case can we get a signature, so there's nothing
+ # to verify. Even the inspect module gives up and raises
+ # ValueError: no signature found. The ``__text_signature__`` attribute
+ # isn't typically populated either.
+ #
+ # Note that on PyPy 2 or 3 (up through 7.3 at least), these are
+ # not true for things like ``dict.pop`` (but might be true for C extensions?)
+ return
+
+ if isinstance(attr, FunctionType):
+ if sys.version_info[0] >= 3 and isinstance(candidate, type) and vtype == 'c':
+ # This is an "unbound method" in Python 3.
+ # Only unwrap this if we're verifying implementedBy;
+ # otherwise we can unwrap @staticmethod on classes that directly
+ # provide an interface.
+ meth = fromFunction(attr, iface, name=name,
+ imlevel=1)
+ else:
+ # Nope, just a normal function
+ meth = fromFunction(attr, iface, name=name)
+ elif (isinstance(attr, MethodTypes)
+ and type(attr.__func__) is FunctionType):
+ meth = fromMethod(attr, iface, name)
+ elif isinstance(attr, property) and vtype == 'c':
+ # Without an instance we cannot be sure it's not a
+ # callable.
+ # TODO: This should probably check inspect.isdatadescriptor(),
+ # a more general form than ``property``
+ return
+
+ else:
+ if not callable(attr):
+ raise BrokenMethodImplementation(desc, "implementation is not a method", candidate)
+ # sigh, it's callable, but we don't know how to introspect it, so
+ # we have to give it a pass.
+ return
+
+ # Make sure that the required and implemented method signatures are
+ # the same.
+ mess = _incompat(desc.getSignatureInfo(), meth.getSignatureInfo())
+ if mess:
+ if PYPY2 and _pypy2_false_positive(mess, candidate, vtype):
+ return
+ raise BrokenMethodImplementation(desc, mess, candidate)
+
+
+
def verifyClass(iface, candidate, tentative=False):
"""
Verify that the *candidate* might correctly provide *iface*.