Merge pull request #221 from GuillaumeGomez/port-to-python3

Port highfive to python3

.travis.yml

@@ -1,6 +1,6 @@
 language: python
 python:
-  - "2.7"
+  - "3.6"
 
 env:
   RUSTINFRA_IMAGE_TAG: highfive

Dockerfile

@@ -2,10 +2,10 @@ FROM ubuntu:bionic
 
 RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
     ca-certificates \
-    python \
-    python-setuptools \
-    python-wheel \
-    python-pip
+    python3 \
+    python3-setuptools \
+    python3-wheel \
+    python3-pip
 
 RUN mkdir /highfive
 WORKDIR /highfive

highfive/newpr.py

@@ -1,13 +1,13 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
-import urllib2
+import urllib
 import cgi
 import cgitb
 from copy import deepcopy
 import json
 import random
-import ConfigParser
-from StringIO import StringIO
+from configparser import ConfigParser
+from io import StringIO
 import gzip
 import re
 import os
@@ -90,14 +90,14 @@ def modifies_submodule(self, diff):
     def api_req(self, method, url, data=None, media_type=None):
         data = None if not data else json.dumps(data)
         headers = {} if not data else {'Content-Type': 'application/json'}
-        req = urllib2.Request(url, data, headers)
+        req = urllib.request.Request(url, data, headers)
         req.get_method = lambda: method
         if self.integration_token:
             req.add_header("Authorization", "token %s" % self.integration_token)
 
         if media_type:
             req.add_header("Accept", media_type)
-        f = urllib2.urlopen(req)
+        f = urllib.request.urlopen(req)
         header = f.info()
         if header.get('Content-Encoding') == 'gzip':
             buf = StringIO(f.read())
@@ -111,7 +111,7 @@ def set_assignee(self, assignee, owner, repo, issue, user, author, to_mention):
                 "PATCH", issue_url % (owner, repo, issue),
                 {"assignee": assignee}
             )['body']
-        except urllib2.HTTPError, e:
+        except urllib.error.HTTPError as e:
             if e.code == 201:
                 pass
             else:
@@ -154,12 +154,12 @@ def get_irc_nick(self, gh_name):
          or if the user has no `irc` field associated with their username
         """
         try:
-            data = urllib2.urlopen(rustaceans_api_url.format(username=gh_name))
+            data = urllib.urlopen(rustaceans_api_url.format(username=gh_name))
             if data.getcode() == 200:
                 rustacean_data = json.loads(data.read())
                 if rustacean_data:
                     return rustacean_data[0].get("irc")
-        except urllib2.HTTPError:
+        except urllib.error.HTTPError:
             pass
 
         return None
@@ -171,7 +171,7 @@ def is_collaborator(self, commenter, owner, repo):
                 "GET", user_collabo_url % (owner, repo, commenter), None
             )
             return True
-        except urllib2.HTTPError, e:
+        except urllib.error.HTTPError as e:
             if e.code == 404:
                 return False
             else:
@@ -195,7 +195,7 @@ def post_comment(self, body, owner, repo, issue):
             self.api_req(
                 "POST", post_comment_url % (owner, repo, issue), {"body": body}
             )['body']
-        except urllib2.HTTPError, e:
+        except urllib.error.HTTPError as e:
             if e.code == 201:
                 pass
             else:
@@ -242,7 +242,7 @@ def is_new_contributor(self, username, owner, repo):
                 'application/vnd.github.cloak-preview'
             )
             return json.loads(result['body'])['total_count'] == 0
-        except urllib2.HTTPError, e:
+        except urllib.error.HTTPError as e:
             if e.code == 422:
                 return True
             else:
@@ -266,7 +266,7 @@ def choose_reviewer(self, repo, owner, diff, exclude):
         # fill in the default groups, ensuring that overwriting is an
         # error.
         global_ = self._load_json_file('_global.json')
-        for name, people in global_['groups'].iteritems():
+        for name, people in global_['groups'].items():
             assert name not in groups, "group %s overlaps with _global.json" % name
             groups[name] = people
 
@@ -299,10 +299,10 @@ def choose_reviewer(self, repo, owner, diff, exclude):
 
             # Find the largest count.
             most_changes = 0
-            for dir, changes in counts.iteritems():
+            for directory, changes in counts.items():
                 if changes > most_changes:
                     most_changes = changes
-                    most_changed = dir
+                    most_changed = directory
 
         # lookup that directory in the json file to find the potential reviewers
         potential = groups['all']

highfive/tests/patcherize.py

@@ -9,9 +9,9 @@ def patcherize():
     def _patcherize(patcher_names=None):
         for n,p in (patcher_names or ()):
             patchers[n] = mock.patch(p)
-        return {n: p.start() for n,p in patchers.iteritems()}
+        return {n: p.start() for n,p in patchers.items()}
 
     yield _patcherize
 
-    for patcher in patchers.itervalues():
+    for patcher in patchers.values():
         patcher.stop()

highfive/tests/test_configs.py

@@ -102,10 +102,11 @@ def config_valid(fname):
 
 @pytest.mark.config
 @pytest.mark.hermetic
-def test_configs():
+@pytest.fixture
+def test_configs(request):
     """Check that the repo config files are valid JSON and contain the expected
     sorts of values."""
-    config_path = os.path.join(str(pytest.config.rootdir), 'highfive/configs')
+    config_path = os.path.join(str(request.config.rootdir), 'highfive/configs')
     for fname in os.listdir(config_path):
         if fname.endswith('.json'):
             assert config_valid(os.path.join(config_path, fname))

highfive/tests/test_newpr.py

@@ -10,7 +10,7 @@
 import os
 import pytest
 import responses
-from urllib2 import HTTPError
+from urllib.error import HTTPError
 
 @pytest.mark.unit
 @pytest.mark.hermetic
@@ -159,7 +159,7 @@ def test_load_json_file(self, mock_dirname):
         mock_dirname.return_value = '/the/path'
         contents = ['some json']
         with mock.patch(
-            '__builtin__.open', mock.mock_open(read_data=json.dumps(contents))
+            'builtins.open', mock.mock_open(read_data=json.dumps(contents))
         ) as mock_file:
             assert handler._load_json_file('a-config.json') == contents
             mock_file.assert_called_with('/the/path/configs/a-config.json')
@@ -340,51 +340,51 @@ def test_find_reviewer(self):
             assert handler.find_reviewer(msg) is None, \
                 "expected '%s' to have no reviewer extracted" % msg
 
-    def setup_get_irc_nick_mocks(self, mock_urllib2, status_code, data=None):
+    def setup_get_irc_nick_mocks(self, mock_urllib, status_code, data=None):
         if status_code != 200:
-            mock_urllib2.side_effect = HTTPError(
+            mock_urllib.side_effect = HTTPError(
                 None, status_code, None, None, None
             )
             return
 
         mock_data = mock.Mock()
         mock_data.getcode.return_value = status_code
         mock_data.read.return_value = data
-        mock_urllib2.urlopen.return_value = mock_data
+        mock_urllib.urlopen.return_value = mock_data
         return mock_data
 
-    @mock.patch('highfive.newpr.urllib2')
-    def test_get_irc_nick_non_200(self, mock_urllib2):
+    @mock.patch('highfive.newpr.urllib')
+    def test_get_irc_nick_non_200(self, mock_urllib):
         handler = HighfiveHandlerMock(Payload({})).handler
-        self.setup_get_irc_nick_mocks(mock_urllib2, 503)
+        self.setup_get_irc_nick_mocks(mock_urllib, 503)
         assert handler.get_irc_nick('foo') is None
 
-        mock_urllib2.urlopen.assert_called_with(
+        mock_urllib.urlopen.assert_called_with(
             'http://www.ncameron.org/rustaceans/user?username=foo'
         )
 
-    @mock.patch('highfive.newpr.urllib2')
-    def test_get_irc_nick_no_data(self, mock_urllib2):
+    @mock.patch('highfive.newpr.urllib')
+    def test_get_irc_nick_no_data(self, mock_urllib):
         handler = HighfiveHandlerMock(Payload({})).handler
-        mock_data = self.setup_get_irc_nick_mocks(mock_urllib2, 200, '[]')
+        mock_data = self.setup_get_irc_nick_mocks(mock_urllib, 200, '[]')
         assert handler.get_irc_nick('foo') is None
 
-        mock_urllib2.urlopen.assert_called_with(
+        mock_urllib.urlopen.assert_called_with(
             'http://www.ncameron.org/rustaceans/user?username=foo'
         )
         mock_data.getcode.assert_called()
         mock_data.read.assert_called()
 
-    @mock.patch('highfive.newpr.urllib2')
-    def test_get_irc_nick_has_data(self, mock_urllib2):
+    @mock.patch('highfive.newpr.urllib')
+    def test_get_irc_nick_has_data(self, mock_urllib):
         handler = HighfiveHandlerMock(Payload({})).handler
         mock_data = self.setup_get_irc_nick_mocks(
-            mock_urllib2, 200,
+            mock_urllib, 200,
             '[{"username":"nrc","name":"Nick Cameron","irc":"nrc","email":"nrc@ncameron.org","discourse":"nrc","reddit":"nick29581","twitter":"@nick_r_cameron","blog":"https://www.ncameron.org/blog","website":"https://www.ncameron.org","notes":"<p>I work on the Rust compiler, language design, and tooling. I lead the dev tools team and am part of the core team. I&#39;m part of the research team at Mozilla.</p>\\n","avatar":"https://avatars.githubusercontent.com/nrc","irc_channels":["rust-dev-tools","rust","rust-internals","rust-lang","rustc","servo"]}]'
         )
         assert handler.get_irc_nick('nrc') == 'nrc'
 
-        mock_urllib2.urlopen.assert_called_with(
+        mock_urllib.urlopen.assert_called_with(
             'http://www.ncameron.org/rustaceans/user?username=nrc'
         )
         mock_data.getcode.assert_called()
@@ -394,8 +394,8 @@ class TestApiReq(TestNewPR):
     @pytest.fixture(autouse=True)
     def make_defaults(cls, patcherize):
         cls.mocks = patcherize((
-            ('urlopen', 'urllib2.urlopen'),
-            ('Request', 'urllib2.Request'),
+            ('urlopen', 'urllib.request.urlopen'),
+            ('Request', 'urllib.request.Request'),
             ('StringIO', 'highfive.newpr.StringIO'),
             ('GzipFile', 'gzip.GzipFile'),
         ))
@@ -1138,7 +1138,7 @@ def choose_reviewers(self, diff, author, global_ = None):
         """
         chosen_reviewers = set()
         mention_list = set()
-        for _ in xrange(40):
+        for _ in range(40):
             reviewer = self.choose_reviewer(
                 'rust', 'rust-lang', diff, author, global_
             )