launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> === modified file 'lib/lp/services/librarianserver/tests/test_web.py'
> --- lib/lp/services/librarianserver/tests/test_web.py	2018-01-02 10:54:31 +0000
> +++ lib/lp/services/librarianserver/tests/test_web.py	2018-11-01 18:49:40 +0000
> @@ -108,29 +105,34 @@
>          # displaying Ubuntu build logs in the browser.  The mimetype should be
>          # "text/plain" for these files.
>          client = LibrarianClient()
> -        contents = 'Build log...'
> -        build_log = StringIO(contents)
> +        contents = b'Build log...'
> +        build_log = BytesIO()
> +        with GzipFile(mode='wb', fileobj=build_log) as f:

Sorry, I should have explained what was going on here.  I had to do this as part of the port to requests because, unlike urllib2, requests understands the Content-Encoding and tries to decode it, so without this change you get something like:

Error in test test_checkGzipEncoding (lp.services.librarianserver.tests.test_web.LibrarianWebTestCase)
Traceback (most recent call last):
_StringException: Traceback (most recent call last):
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/lib/lp/services/librarianserver/tests/test_web.py", line 118, in test_checkGzipEncoding
    response = requests.get(url)
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/env/local/lib/python2.7/site-packages/requests/api.py", line 69, in get
    return request('get', url, params=params, **kwargs)
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/env/local/lib/python2.7/site-packages/requests/api.py", line 50, in request
    response = session.request(method=method, url=url, **kwargs)
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/env/local/lib/python2.7/site-packages/requests/sessions.py", line 465, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/env/local/lib/python2.7/site-packages/requests/sessions.py", line 605, in send
    r.content
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/env/local/lib/python2.7/site-packages/requests/models.py", line 750, in content
    self._content = bytes().join(self.iter_content(CONTENT_CHUNK_SIZE)) or bytes()
  File "/home/cjwatson/src/canonical/launchpad/lp-branches/librarianserver-test-web-requests/env/local/lib/python2.7/site-packages/requests/models.py", line 678, in generate
    raise ContentDecodingError(e)
requests.exceptions.ContentDecodingError: ('Received response with content-encoding: gzip, but failed to decode it.', error('Error -3 while decompressing: incorrect header check',))

Making the allegedly-gzipped test content actually be gzipped was the path of least resistance here, and seems a lot less weird anyway.

> +            f.write(contents)
> +        build_log.seek(0)
>          alias_id = client.addFile(name="build_log.txt.gz",
> -                                  size=len(contents),
> +                                  size=len(build_log.getvalue()),
>                                    file=build_log,
>                                    contentType="text/plain")
>  
>          self.commit()
>  
>          url = client.getURLForAlias(alias_id)
> -        fileObj = urlopen(url)
> -        mimetype = fileObj.headers['content-type']
> -        encoding = fileObj.headers['content-encoding']
> +        response = requests.get(url)
> +        response.raise_for_status()
> +        mimetype = response.headers['content-type']
> +        encoding = response.headers['content-encoding']
>          self.assertTrue(mimetype == "text/plain; charset=utf-8",
>                          "Wrong mimetype. %s != 'text/plain'." % mimetype)
>          self.assertTrue(encoding == "gzip",
>                          "Wrong encoding. %s != 'gzip'." % encoding)
> +        self.assertEqual(contents, response.content)

I can't use StringIO here for the reasons explained above, but I suppose it's worth making sure that all the response headers are such that requests can decode the text.  Done.

>  
>      def test_checkNoEncoding(self):
>          # Other files should have no encoding.
>          client = LibrarianClient()
> -        contents = 'Build log...'
> -        build_log = StringIO(contents)
> +        contents = b'Build log...'
> +        build_log = BytesIO(contents)
>          alias_id = client.addFile(name="build_log.tgz",
>                                    size=len(contents),
>                                    file=build_log,


-- 
https://code.launchpad.net/~cjwatson/launchpad/librarianserver-test-web-requests/+merge/358189
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/librarianserver-test-web-requests into lp:launchpad.