Merge lp:~diegosarmentero/ubuntu-sso-client/reset-error into lp:ubuntu-sso-client

[Diego Sarmentero]

- Converting to unicode some data returned by webclient in QByteArray format (LP: 961315).

[Natalia Bidart (nataliabidart)]

The fix applied to account.py should be applied to the webclient module instead, since despite we're ensuring having unicode for NewPasswordError, we may leak QBytes from other calls, into other exceptions or results, and that's not good.

So, unless is not doable and I'm missing something (please advice), this branch needs to address that, providing a test for the webclient module ensuring that no qt types are leaked.

Thanks!

review: Needs Fixing

[lp:~diegosarmentero/ubuntu-sso-client/reset-error]

933. By Diego Sarmentero on 2012-03-23

Improving tests.

934. By Diego Sarmentero on 2012-03-23

Tests fixed.

935. By Diego Sarmentero on 2012-03-23

merge

[Natalia Bidart (nataliabidart)]

A couple of minor neat picks:

* the 'pass' sentence in the MyException class is not needed

* the docstring for test_webclienterror_not_string says "The returned content are bytes." but your code makes the content unicode, not bytes...

* the way to test this:

        try:
            yield self.wc.request(self.base_iri + THROWERROR)
        except Exception, e:
            for error in e.args:
                self.assertTrue(isinstance(error, basestring))

like I mentioned over IRC, is:

    deferred = self.wc.request(self.base_iri + THROWERROR)
    failure = yield self.assertFailure(deferred, webclient.WebClientError)
    # do the asserts over failure (failure.value has the real exception)

review: Needs Fixing

[lp:~diegosarmentero/ubuntu-sso-client/reset-error]

936. By Diego Sarmentero on 2012-03-26

Minor fixes.

[Diego Sarmentero (diegosarmentero)]

> A couple of minor neat picks:
>
> * the 'pass' sentence in the MyException class is not needed
>
> * the docstring for test_webclienterror_not_string says "The returned content
> are bytes." but your code makes the content unicode, not bytes...
>
> * the way to test this:
>
> try:
> yield self.wc.request(self.base_iri + THROWERROR)
> except Exception, e:
> for error in e.args:
> self.assertTrue(isinstance(error, basestring))
>
> like I mentioned over IRC, is:
>
> deferred = self.wc.request(self.base_iri + THROWERROR)
> failure = yield self.assertFailure(deferred, webclient.WebClientError)
> # do the asserts over failure (failure.value has the real exception)

Fixed!

[lp:~diegosarmentero/ubuntu-sso-client/reset-error]

937. By Diego Sarmentero on 2012-03-26

removing unnecessary variable.

[Natalia Bidart (nataliabidart)]

Looks great!

review: Approve

[Roberto Alsina (ralsina)]

+1

review: Approve

=== modified file 'ubuntu_sso/main/tests/test_common.py'
--- ubuntu_sso/main/tests/test_common.py	2012-02-17 18:43:17 +0000
+++ ubuntu_sso/main/tests/test_common.py	2012-03-26 14:31:18 +0000
@@ -118,6 +118,18 @@
         self.assertEqual(result["errtype"], e.__class__.__name__)
         self.assertEqual(result["message"], sample_string)
 
+    def test_first_arg_is_exception(self):
+        """If the first arg is an Exception, use it's message attribute'."""
+
+        class MyException(Exception):
+            """Custom Exception."""
+
+        message = u'My custom error for ♥ Ubuntu'
+        my_exc = MyException(message)
+        result = except_to_errdict(my_exc)
+        self.assertEqual(result["errtype"], my_exc.__class__.__name__)
+        self.assertEqual(result["message"], message)
+
     def test_first_arg_is_unicode(self):
         """If the first arg is a unicode, use it as the message."""
         sample_string = u"a sample string"

=== modified file 'ubuntu_sso/utils/webclient/qtnetwork.py'
--- ubuntu_sso/utils/webclient/qtnetwork.py	2012-03-19 23:53:06 +0000
+++ ubuntu_sso/utils/webclient/qtnetwork.py	2012-03-26 14:31:18 +0000
@@ -188,7 +188,8 @@
             response = Response(bytes(content), headers)
             d.callback(response)
         else:
-            error_string = reply.errorString()
+            content = unicode(content)
+            error_string = unicode(reply.errorString())
             logger.debug('_handle_finished error (%s,%s).', error,
                          error_string)
             if error == QNetworkReply.AuthenticationRequiredError:

=== modified file 'ubuntu_sso/utils/webclient/tests/test_webclient.py'
--- ubuntu_sso/utils/webclient/tests/test_webclient.py	2012-03-19 23:43:56 +0000
+++ ubuntu_sso/utils/webclient/tests/test_webclient.py	2012-03-26 14:31:18 +0000
@@ -373,6 +373,14 @@
         self.assertTrue(isinstance(result.content, bytes),
                         "The type of %r must be bytes" % result.content)
 
+    @defer.inlineCallbacks
+    def test_webclienterror_not_string(self):
+        """The returned exception contains unicode data."""
+        deferred = self.wc.request(self.base_iri + THROWERROR)
+        failure = yield self.assertFailure(deferred, webclient.WebClientError)
+        for error in failure.args:
+            self.assertTrue(isinstance(error, basestring))
+
 
 class FakeSavingReactor(object):
     """A fake reactor that saves connection attempts."""