Remaining Python 3 fixes for lib/amo (except lib/git.py) (mozilla#10598)

src/olympia/amo/cron.py

@@ -91,7 +91,7 @@ def days_ago(days):
 
         output = Popen(cmd, stdout=PIPE).communicate()[0]
 
-        for line in output.split("\n"):
+        for line in output.split(b'\n'):
             log.debug(line)
 
     else:
@@ -106,7 +106,7 @@ def days_ago(days):
                '-exec', 'rm', '{}', ';')
         output = Popen(cmd, stdout=PIPE).communicate()[0]
 
-        for line in output.split("\n"):
+        for line in output.split(b'\n'):
             log.debug(line)
 
     # Delete stale FileUploads.

src/olympia/amo/pagination.py

@@ -80,13 +80,13 @@ def page(self, number):
             page = Page(result.hits, number, self)
 
             # Overwrite the `count` with the total received from ES results.
-            self.count = page.object_list.total
+            self.count = int(page.object_list.total)
         else:
             page = Page(self.object_list[bottom:top], number, self)
 
             # Force the search to evaluate and then attach the count.
             list(page.object_list)
-            self.count = page.object_list.count()
+            self.count = int(page.object_list.count())
 
         # Now that we have the count validate that the page number isn't higher
         # than the possible number of pages and adjust accordingly.

src/olympia/amo/templatetags/jinja_helpers.py

@@ -164,7 +164,7 @@ def __init__(self, pager):
         self.pager = pager
 
         self.max = 10
-        self.span = (self.max - 1) / 2
+        self.span = (self.max - 1) // 2
 
         self.page = pager.number
         self.num_pages = pager.paginator.num_pages

src/olympia/amo/tests/test_decorators.py

@@ -4,6 +4,7 @@
 from django.contrib.auth.models import AnonymousUser
 from django.core.exceptions import PermissionDenied
 from django.test import RequestFactory
+from django.utils.encoding import force_text
 
 import mock
 import pytest
@@ -38,7 +39,7 @@ def func(request):
 
     response = decorators.json_view(func)(mock.Mock())
     assert isinstance(response, http.HttpResponse)
-    assert response.content == '{"x": 1}'
+    assert force_text(response.content) == '{"x": 1}'
     assert response['Content-Type'] == 'application/json'
     assert response.status_code == 200
 
@@ -59,7 +60,7 @@ def test_json_view_error():
     """json_view.error returns 400 responses."""
     response = decorators.json_view.error({'msg': 'error'})
     assert isinstance(response, http.HttpResponseBadRequest)
-    assert response.content == '{"msg": "error"}'
+    assert force_text(response.content) == '{"msg": "error"}'
     assert response['Content-Type'] == 'application/json'
 
 
@@ -73,7 +74,7 @@ def func(request):
 
 def test_json_view_response_status():
     response = decorators.json_response({'msg': 'error'}, status_code=202)
-    assert response.content == '{"msg": "error"}'
+    assert force_text(response.content) == '{"msg": "error"}'
     assert response['Content-Type'] == 'application/json'
     assert response.status_code == 202
 

src/olympia/amo/tests/test_middleware.py

@@ -116,8 +116,8 @@ class TestNoDjangoDebugToolbar(TestCase):
     def test_no_django_debug_toolbar(self):
         with self.settings(DEBUG=False):
             res = self.client.get(reverse('home'), follow=True)
-            assert 'djDebug' not in res.content
-            assert 'debug_toolbar' not in res.content
+            assert b'djDebug' not in res.content
+            assert b'debug_toolbar' not in res.content
 
 
 def test_hide_password_middleware():

src/olympia/amo/tests/test_pagination.py

@@ -2,6 +2,7 @@
     EmptyPage, InvalidPage, PageNotAnInteger, Paginator)
 
 import pytest
+import six
 
 from mock import MagicMock, Mock
 
@@ -26,7 +27,7 @@ def mock_pager(page_number, num_pages, count):
 
 def assert_range(page_number, num_pages, expected):
     p = PaginationRenderer(mock_pager(page_number, num_pages, 100))
-    assert p.range() == expected
+    assert list(p.range()) == expected
 
 
 def test_page_range():
@@ -105,7 +106,7 @@ def test_invalid_page(self):
         # unfortunately since `pytest.raises` won't check the exact
         # instance but also accepts parent exceptions inherited.
         assert (
-            exc.value.message ==
+            six.text_type(exc.value) ==
             'That page number is too high for the current page size')
         assert isinstance(exc.value, InvalidPage)
 

src/olympia/amo/tests/test_send_mail.py

@@ -220,7 +220,7 @@ def test_send_html_mail_jinja(self):
     def test_send_attachment(self):
         path = os.path.join(ATTACHMENTS_DIR, 'bacon.txt')
         attachments = [(
-            os.path.basename(path), storage.open(path).read(),
+            os.path.basename(path), storage.open(path, 'r').read(),
             mimetypes.guess_type(path)[0])]
         send_mail('test subject', 'test body', from_email='a@example.com',
                   recipient_list=['b@example.com'], attachments=attachments)

src/olympia/amo/tests/test_settings.py

@@ -43,7 +43,7 @@ def test_raven_release_config():
 
     # Cleanup for tests
     if os.path.exists(version_json):
-        with open(version_json, 'rb') as fobj:
+        with open(version_json) as fobj:
             original = fobj.read()
 
         os.remove(version_json)
@@ -52,7 +52,7 @@ def test_raven_release_config():
     assert len(get_raven_release()) == 40
 
     # It fetches `version` from the version.json
-    with open(version_json, 'wb') as fobj:
+    with open(version_json, 'w') as fobj:
         fobj.write(json.dumps({
             'version': '2018.07.19'
         }))
@@ -62,19 +62,19 @@ def test_raven_release_config():
     os.remove(version_json)
 
     # Or tries to get the commit from version.json alternatively
-    with open(version_json, 'wb') as fobj:
+    with open(version_json, 'w') as fobj:
         fobj.write(json.dumps({
             'commit': '1111111'
         }))
 
     assert get_raven_release() == '1111111'
 
     if original:
-        with open(version_json, 'wb') as fobj:
+        with open(version_json, 'w') as fobj:
             fobj.write(original)
 
     # Usual state of things, version is empty but commit is set
-    with open(version_json, 'wb') as fobj:
+    with open(version_json, 'w') as fobj:
         fobj.write(json.dumps({
             'version': '',
             'commit': '1111111'
@@ -83,5 +83,5 @@ def test_raven_release_config():
     assert get_raven_release() == '1111111'
 
     if original:
-        with open(version_json, 'wb') as fobj:
+        with open(version_json, 'w') as fobj:
             fobj.write(original)

src/olympia/amo/tests/test_views.py

@@ -316,10 +316,10 @@ def test_jsi18n(self):
         self.assertCloseToNow(response['Expires'],
                               now=datetime.now() + timedelta(days=365))
 
-        en = self.client.get(reverse('jsi18n')).content
+        en = self.client.get(reverse('jsi18n')).content.decode('utf-8')
 
         with self.activate('fr'):
-            fr = self.client.get(reverse('jsi18n')).content
+            fr = self.client.get(reverse('jsi18n')).content.decode('utf-8')
 
         assert en != fr
 
@@ -408,7 +408,7 @@ def test_disable_collections(self):
         url = reverse('collections.list')
         response = self.client.get('/robots.txt')
         assert response.status_code == 200
-        assert 'Disallow: %s' % url in response.content
+        assert 'Disallow: %s' % url in response.content.decode('utf-8')
 
     @override_settings(ENGAGE_ROBOTS=True)
     def test_allow_mozilla_collections(self):
@@ -417,7 +417,7 @@ def test_allow_mozilla_collections(self):
                              settings.TASK_USER_ID)
         response = self.client.get('/robots.txt')
         assert response.status_code == 200
-        assert 'Allow: {}'.format(url) in response.content
+        assert 'Allow: {}'.format(url) in response.content.decode('utf-8')
 
 
 class TestAtomicRequests(WithDynamicEndpointsAndTransactions):

src/olympia/lib/crypto/signing.py

@@ -64,7 +64,7 @@ def autograph_sign_file(file_obj):
     conf = settings.AUTOGRAPH_CONFIG
 
     with storage.open(file_obj.current_file_path) as fobj:
-        input_data = b64encode(fobj.read())
+        input_data = force_text(b64encode(fobj.read()))
 
     signing_request = [{
         'input': input_data,
@@ -130,7 +130,7 @@ def autograph_sign_data(file_obj):
     signed_manifest = six.text_type(jar.signatures)
 
     signing_request = [{
-        'input': b64encode(force_bytes(signed_manifest)),
+        'input': force_text(b64encode(force_bytes(signed_manifest))),
         'keyid': conf['signer'],
         'options': {
             'id': get_id(file_obj.version.addon),

src/olympia/lib/crypto/tests/test_signing.py

@@ -175,8 +175,17 @@ def test_sign_file_non_ascii_filename(self):
         signing.sign_file(self.file_)
         self.assert_signed()
 
-    # FIXME: need to add a test with a unicode filename inside the package
-    # itself, and then check the signature.
+    def test_sign_file_with_utf8_filename_inside_package(self):
+        fpath = 'src/olympia/files/fixtures/files/unicode-filenames.xpi'
+        with amo.tests.copy_file(fpath, self.file_.file_path, overwrite=True):
+            self.assert_not_signed()
+            signing.sign_file(self.file_)
+            self.assert_signed()
+
+            with zipfile.ZipFile(self.file_.file_path, mode='r') as zf:
+                with zf.open('META-INF/manifest.mf', 'r') as manifest_mf:
+                    manifest_contents = manifest_mf.read().decode('utf-8')
+                    assert u'\u1109\u1161\u11a9' in manifest_contents
 
     def test_no_sign_missing_file(self):
         os.unlink(self.file_.file_path)

src/olympia/lib/tests/test_jingo_minify_helpers.py

@@ -4,6 +4,7 @@
 from django.test.utils import override_settings
 
 import mock
+import six
 
 from olympia.amo.utils import from_string
 
@@ -226,7 +227,7 @@ def test_css(getmtime, time):
     'css': {'compiled': ['css/impala/buttons.less']}})
 @mock.patch('olympia.lib.jingo_minify_helpers.os.path.getmtime')
 @mock.patch('olympia.lib.jingo_minify_helpers.subprocess')
-@mock.patch('__builtin__.open', spec=True)
+@mock.patch('%s.open' % ('__builtin__' if six.PY2 else 'builtins'), spec=True)
 def test_compiled_css(open_mock, subprocess_mock, getmtime_mock):
     getmtime_mock.side_effect = [
         # The first call is for the source

src/olympia/lib/tests/test_unicodehelper.py

@@ -8,11 +8,9 @@
 
 def _do_test(path):
     """Performs a test on a JS file"""
-    path = os.path.join(
-        os.path.abspath(os.path.dirname(__file__)),
-        path)
+    path = os.path.join(os.path.abspath(os.path.dirname(__file__)), path)
 
-    with open(path) as fobj:
+    with open(path, 'rb') as fobj:
         text = fobj.read()
 
     utext = unicodehelper.decode(text)

src/olympia/signing/tests/test_views.py

@@ -82,7 +82,7 @@ def request(self, method='PUT', url=None, version='3.0',
         if url is None:
             url = self.url(addon, version)
 
-        with open(filename) as upload:
+        with open(filename, 'rb') as upload:
             data = {'upload': upload}
             if method == 'POST' and version:
                 data['version'] = version
@@ -490,7 +490,7 @@ def test_akismet_reports_created_ham_outcome(self, comment_check_mock):
 
         validation_response = self.get(self.url(self.guid, '3.0'))
         assert validation_response.status_code == 200
-        assert 'spam' not in validation_response.content
+        assert b'spam' not in validation_response.content
 
     @override_switch('akismet-spam-check', active=True)
     @override_switch('akismet-addon-action', active=False)
@@ -513,7 +513,7 @@ def test_akismet_reports_created_spam_outcome_logging_only(self):
 
         validation_response = self.get(self.url(self.guid, '3.0'))
         assert validation_response.status_code == 200
-        assert 'spam' not in validation_response.content
+        assert b'spam' not in validation_response.content
 
     @override_switch('akismet-spam-check', active=True)
     @override_switch('akismet-addon-action', active=True)
@@ -536,8 +536,8 @@ def test_akismet_reports_created_spam_outcome_action_taken(self):
 
         validation_response = self.get(self.url(self.guid, '3.0'))
         assert validation_response.status_code == 200
-        assert 'spam' in validation_response.content
-        data = json.loads(validation_response.content)
+        assert b'spam' in validation_response.content
+        data = json.loads(validation_response.content.decode('utf-8'))
         assert data['validation_results']['messages'][0]['id'] == [
             u'validation', u'messages', u'akismet_is_spam_name'
         ]