Quite a bit of work on new public errors and their unit tests

ipalib/errors2.py

@@ -28,7 +28,9 @@ to the caller.
     =============  ========================================
     900            `PublicError`
     901            `InternalError`
-    902 - 999      *Reserved for future use*
+    902            `RemoteInternalError`
+    903            `VersionError`
+    904 - 999      *Reserved for future use*
     1000 - 1999    `AuthenticationError` and its subclasses
     2000 - 2999    `AuthorizationError` and its subclasses
     3000 - 3999    `InvocationError` and its subclasses
@@ -39,6 +41,7 @@ to the caller.
 
 from inspect import isclass
 from request import ugettext, ungettext
+from constants import TYPE_ERROR
 
 
 class PrivateError(StandardError):
@@ -157,23 +160,90 @@ class PublicError(StandardError):
     code = 900
 
     def __init__(self, message=None, **kw):
-        self.kw = kw
         if message is None:
-            message = self.get_format() % kw
+            message = self.get_format(ugettext) % kw
+            assert type(message) is unicode
+        elif type(message) is not unicode:
+            raise TypeError(
+                TYPE_ERROR % ('message', unicode, message, type(message))
+            )
+        self.message = message
+        for (key, value) in kw.iteritems():
+            assert not hasattr(self, key), 'conflicting kwarg %s.%s = %r' % (
+                self.__class__.__name__, key, value,
+            )
+            setattr(self, key, value)
         StandardError.__init__(self, message)
 
     def get_format(self, _):
         return _('')
 
 
-
 class InternalError(PublicError):
     """
     **901** Used to conceal a non-public exception.
+
+    For example:
+
+    >>> raise InternalError()
+    Traceback (most recent call last):
+      ...
+    InternalError: an internal error has occured
     """
 
     code = 901
 
+    def __init__(self, message=None):
+        """
+        Security issue: ignore any information given to constructor.
+        """
+        PublicError.__init__(self, self.get_format(ugettext))
+
+    def get_format(self, _):
+        return _('an internal error has occured')
+
+
+class RemoteInternalError(PublicError):
+    """
+    **902** Raised when client catches an `InternalError` from server.
+
+    For example:
+
+    >>> raise RemoteInternalError(uri='http://localhost:8888')
+    Traceback (most recent call last):
+      ...
+    RemoteInternalError: an internal error has occured on server 'http://localhost:8888'
+    """
+
+    code = 902
+
+    def get_format(self, _):
+        return _('an internal error has occured on server %(uri)r')
+
+
+class VersionError(PublicError):
+    """
+    **903** Raised when client and server versions are incompatible.
+
+    For example:
+
+    >>> raise VersionError(client='2.0', server='2.1', uri='http://localhost:8888')
+    Traceback (most recent call last):
+      ...
+    VersionError: 2.0 client incompatible with 2.1 server at 'http://localhost:8888'
+
+    """
+
+    code = 903
+
+    def get_format(self, _):
+        return _(
+            '%(client)s client incompatible with %(server)s server at %(uri)r'
+        )
+
+
+
+
 
 
 ##############################################################################
@@ -212,21 +282,38 @@ class InvocationError(PublicError):
 class CommandError(InvocationError):
     """
     **3001** Raised when an unknown command is called.
+
+    For example:
+
+    >>> raise CommandError(name='foobar')
+    Traceback (most recent call last):
+      ...
+    CommandError: unknown command 'foobar'
     """
 
     code = 3001
 
     def get_format(self, _):
-        return _('Unknown command %(name)r')
+        return _('unknown command %(name)r')
 
 
 class RemoteCommandError(InvocationError):
     """
-    **3002** Raised when client receives a `CommandError` from server.
+    **3002** Raised when client catches a `CommandError` from server.
+
+    For example:
+
+    >>> raise RemoteCommandError(name='foobar', uri='http://localhost:8888')
+    Traceback (most recent call last):
+      ...
+    RemoteCommandError: command 'foobar' unknown on server 'http://localhost:8888'
     """
 
     code = 3002
 
+    def get_format(self, _):
+        return _('command %(name)r unknown on server %(uri)r')
+
 
 class ArgumentError(InvocationError):
     """
@@ -254,7 +341,7 @@ class RequirementError(InvocationError):
 
 class ConversionError(InvocationError):
     """
-    **3006** Raised when a parameter value is the wrong type.
+    **3006** Raised when parameter value can't be converted to correct type.
     """
 
     code = 3006
@@ -286,7 +373,7 @@ class ExecutionError(PublicError):
 
 class GenericError(PublicError):
     """
-    **5000** Errors inappropriate for other categories (*5000 - 5999*).
+    **5000** Base class for errors that don't fit elsewhere (*5000 - 5999*).
     """
 
     code = 5000

tests/test_ipalib/test_error2.py

@@ -23,7 +23,9 @@ Test the `ipalib.error2` module.
 
 import re
 import inspect
+from tests.util import assert_equal, raises, DummyUGettext
 from ipalib import errors2
+from ipalib.constants import TYPE_ERROR
 
 
 class PrivateExceptionTester(object):
@@ -180,6 +182,80 @@ class test_PluginMissingOverrideError(PrivateExceptionTester):
         assert inst.message == str(inst)
 
 
+
+##############################################################################
+# Unit tests for public errors:
+
+class PublicExceptionTester(object):
+    _klass = None
+    __klass = None
+
+    def __get_klass(self):
+        if self.__klass is None:
+            self.__klass = self._klass
+        assert issubclass(self.__klass, StandardError)
+        assert issubclass(self.__klass, errors2.PublicError)
+        assert not issubclass(self.__klass, errors2.PrivateError)
+        assert type(self.__klass.code) is int
+        assert 900 <= self.__klass.code <= 5999
+        return self.__klass
+    klass = property(__get_klass)
+
+    def new(self, message=None, **kw):
+        # Test that TypeError is raised if message isn't unicode:
+        e = raises(TypeError, self.klass, 'The message')
+        assert str(e) == TYPE_ERROR % ('message', unicode, 'The message', str)
+
+        # Test the instance:
+        for (key, value) in kw.iteritems():
+            assert not hasattr(self.klass, key), key
+        inst = self.klass(message=message, **kw)
+        assert isinstance(inst, StandardError)
+        assert isinstance(inst, errors2.PublicError)
+        assert isinstance(inst, self.klass)
+        assert not isinstance(inst, errors2.PrivateError)
+        for (key, value) in kw.iteritems():
+            assert getattr(inst, key) is value
+        assert str(inst) == inst.get_format(lambda m: m) % kw
+        assert inst.message == str(inst)
+        return inst
+
+
+class test_PublicError(PublicExceptionTester):
+    """
+    Test the `ipalib.errors2.PublicError` exception.
+    """
+    _klass = errors2.PublicError
+
+    def test_init(self):
+        """
+        Test the `ipalib.errors2.PublicError.__init__` method.
+        """
+        inst = self.klass(key1='Value 1', key2='Value 2')
+        assert inst.key1 == 'Value 1'
+        assert inst.key2 == 'Value 2'
+        assert str(inst) == ''
+
+        # Test subclass and use of message, get_format():
+        class subclass(self.klass):
+            def get_format(self, _):
+                return _('%(true)r %(text)r %(number)r')
+
+        kw = dict(true=True, text='Hello!', number=18)
+        inst = subclass(**kw)
+        assert inst.true is True
+        assert inst.text is kw['text']
+        assert inst.number is kw['number']
+        assert_equal(inst.message, u'%(true)r %(text)r %(number)r' % kw)
+
+        # Test via PublicExceptionTester.new()
+        inst = self.new(**kw)
+        assert isinstance(inst, self.klass)
+        assert inst.true is True
+        assert inst.text is kw['text']
+        assert inst.number is kw['number']
+
+
 def test_public_errors():
     """
     Test the `ipalib.errors2.public_errors` module variable.
@@ -191,6 +267,10 @@ def test_public_errors():
         assert type(klass.code) is int
         assert 900 <= klass.code <= 5999
         doc = inspect.getdoc(klass)
+        assert doc is not None, 'need class docstring for %s' % klass.__name__
         m = re.match(r'^\*{2}(\d+)\*{2} ', doc)
-        assert m is not None, doc
-        assert int(m.group(1)) == klass.code, klass.__name__
+        assert m is not None, "need '**CODE**' in %s docstring" % klass.__name__
+        code = int(m.group(1))
+        assert code == klass.code, (
+            'docstring=%r but code=%r in %s' % (code, klass.code, klass.__name__)
+        )

tests/test_ipalib/test_request.py

@@ -23,16 +23,12 @@ Test the `ipalib.request` module.
 
 import threading
 import locale
-from tests.util import raises, TempDir, DummyUGettext, DummyUNGettext
+from tests.util import raises, assert_equal
+from tests.util import TempDir, DummyUGettext, DummyUNGettext
 from ipalib.constants import OVERRIDE_ERROR
 from ipalib import request
 
 
-def assert_equal(val1, val2):
-    assert type(val1) is type(val2), '%r != %r' % (val1, val2)
-    assert val1 == val2, '%r != %r' % (val1, val2)
-
-
 def test_ugettext():
     """
     Test the `ipalib.request.ugettext` function.

tests/util.py

@@ -99,6 +99,14 @@ class ExceptionNotRaised(Exception):
         return self.msg % self.expected.__name__
 
 
+def assert_equal(val1, val2):
+    """
+    Assert ``val1`` and ``val2`` are the same type and of equal value.
+    """
+    assert type(val1) is type(val2), '%r != %r' % (val1, val2)
+    assert val1 == val2, '%r != %r' % (val1, val2)
+
+
 def raises(exception, callback, *args, **kw):
     """
     Tests that the expected exception is raised; raises ExceptionNotRaised