Comment 34 for bug 846044

Revision history for this message
In , Simon McVittie (smcv) wrote :

Comment on attachment 68472
Patch that allows exceptions with unicode error messages

Review of attachment 68472:
-----------------------------------------------------------------

"git format-patch"-formatted patches would be appreciated in future; dbus-python, like all other dbus subprojects, is maintained in git.

Please actually run the tests during "make check". I realise dbus-python's test framework is pretty convoluted: the right place to plug in these tests is test/run-test.sh, near test-standalone.py.

If the tests will compile, but fail, under their non-target version of Python, you could do something like:

    import dbus._compat
    if not dbus._compat.is_py3:
        print "SKIP: not the appropriate Python version"
        raise SystemExit(0)

If they won't even compile under their non-target version of Python, you may have to do something like this:

case `$PYTHON -c "print(__import__('sys').version)"` in
    (2*)
        echo "running test-exception-py2.py"
        $PYTHON "$DBUS_TOP_SRCDIR"/test/test-exception-py2.py || die "... failed"
        ;;
    (3*)
        echo "running test-exception-py3.py"
        $PYTHON "$DBUS_TOP_SRCDIR"/test/test-exception-py3.py || die "... failed"
        ;;
esac

::: test/test-exception-py2.py
@@ +4,5 @@
> +import unittest
> +
> +import dbus
> +
> +class DbusExceptionTestCase(unittest.TestCase):

Pet annoyance: it's spelled "D-Bus", or DBus when you're restricted to C-style identifiers. (Not Dbus or D-BUS.)

@@ +5,5 @@
> +
> +import dbus
> +
> +class DbusExceptionTestCase(unittest.TestCase):
> + """Test the DBusException str/unicode behavior with py2"""

Please also test each case with a subclass of DBusException that sets the _dbus_error_name class member, like ServerError in test-service.py's RaiseDBusException:

        class ServerError(dbus.DBusException):
            """Exception representing a normal "environmental" error"""
            include_traceback = False
            _dbus_error_name = 'com.example.Networking.ServerError'

(Every DBusException raised across D-Bus by an interoperable service should set its _dbus_error_name.)

The most "realistic" version is where isinstance(_dbus_error_name, str), whatever that means in this Python version. I don't think anyone deliberately sets it to a unicode in Python 2 or a bytestring in Python 3.

The expected result is that get_dbus_message() will still return "baaa", but str() and unicode() will return something more like "com.example.Networking.ServerError: baaa".

::: test/test-exception-py3.py
@@ +8,5 @@
> +class DbusExceptionTestCase(unittest.TestCase):
> +
> + def test_dbus_exception(self):
> + e = dbus.exceptions.DBusException("bäää")
> + msg= e.get_dbus_message()

Coding style: msg = e...

As in the Python 2 case, please test that unicode(e) also works and gives the desired result.

As in the Python 2 case, please also test with a subclass that sets _dbus_error_name.