Revert "presubmit_support: Fix tests on Python 3."

This reverts commit a834f39e21.

Reason for revert:
https://bugs.chromium.org/p/chromium/issues/detail?id=1017367

Original change's description:
> presubmit_support: Fix tests on Python 3.
>
> Bug: 1009814
> Change-Id: I2c2a835fc4fa0a7fc9e68dfff680fb0beacb491e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1869535
> Reviewed-by: Anthony Polito <apolito@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=ehmaldonado@chromium.org,apolito@google.com

Bug: 1009814
Change-Id: I48907caeca9b9b32e4e3a7cb5a636280d017a4c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1877350
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

.vpython3

@@ -1,23 +1 @@
 python_version: "3.8"
-
-# Used by:
-#   auth.py
-#   gerrit_util.py
-#   git_cl.py
-#   my_activity.py
-#   TODO(crbug.com/1002153): Add ninjalog_uploader.py
-wheel: <
-  name: "infra/python/wheels/httplib2-py3"
-  version: "version:0.13.1"
->
-
-# Used by:
-#   my_activity.py
-wheel: <
-  name: "infra/python/wheels/python-dateutil-py2_py3"
-  version: "version:2.7.3"
->
-wheel: <
-  name: "infra/python/wheels/six-py2_py3"
-  version: "version:1.10.0"
->

gerrit_util.py

@@ -12,6 +12,8 @@ from __future__ import print_function
 
 import base64
 import contextlib
+import cookielib
+import httplib  # Still used for its constants.
 import httplib2
 import json
 import logging
@@ -25,6 +27,8 @@ import sys
 import tempfile
 import time
 import urllib
+import urlparse
+from cStringIO import StringIO
 from multiprocessing.pool import ThreadPool
 
 import auth
@@ -33,17 +37,6 @@ import metrics
 import metrics_utils
 import subprocess2
 
-if sys.version_info.major == 2:
-  import cookielib
-  import httplib
-  import urlparse
-  from cStringIO import StringIO
-else:
-  import http.cookiejar as cookielib
-  import http.client as httplib
-  import urllib.parse as urlparse
-  from io import StringIO
-
 LOGGER = logging.getLogger()
 # With a starting sleep time of 1.5 seconds, 2^n exponential backoff, and seven
 # total tries, the sleep time between the first and last tries will be 94.5 sec.

git_cl.py

@@ -15,6 +15,7 @@ import base64
 import collections
 import datetime
 import glob
+import httplib
 import httplib2
 import itertools
 import json
@@ -29,6 +30,9 @@ import sys
 import tempfile
 import textwrap
 import time
+import urllib
+import urllib2
+import urlparse
 import uuid
 import webbrowser
 import zlib
@@ -54,17 +58,6 @@ import subcommand
 import subprocess2
 import watchlists
 
-if sys.version_info.major == 2:
-  import httplib
-  import urllib2 as urllib_request
-  import urllib2 as urllib_error
-  import urlparse
-else:
-  import http.client as httplib
-  import urllib.request as urllib_request
-  import urllib.error as urllib_error
-  import urllib.parse as urlparse
-
 __version__ = '2.0'
 
 # Traces for git push will be stored in a traces directory inside the
@@ -3189,7 +3182,7 @@ def urlretrieve(source, destination):
   This is necessary because urllib is broken for SSL connections via a proxy.
   """
   with open(destination, 'w') as f:
-    f.write(urllib_request.urlopen(source).read())
+    f.write(urllib2.urlopen(source).read())
 
 
 def hasSheBang(fname):
@@ -4662,7 +4655,7 @@ def GetTreeStatus(url=None):
   'unknown' or 'unset'."""
   url = url or settings.GetTreeStatusUrl(error_ok=True)
   if url:
-    status = urllib_request.urlopen(url).read().lower()
+    status = urllib2.urlopen(url).read().lower()
     if status.find('closed') != -1 or status == '0':
       return 'closed'
     elif status.find('open') != -1 or status == '1':
@@ -4676,7 +4669,7 @@ def GetTreeStatusReason():
   with the reason for the tree to be opened or closed."""
   url = settings.GetTreeStatusUrl()
   json_url = urlparse.urljoin(url, '/current?format=json')
-  connection = urllib_request.urlopen(json_url)
+  connection = urllib2.urlopen(json_url)
   status = json.loads(connection.read())
   connection.close()
   return status['message']
@@ -5466,7 +5459,7 @@ def main(argv):
     return dispatcher.execute(OptionParser(), argv)
   except auth.LoginRequiredError as e:
     DieWithError(str(e))
-  except urllib_error.HTTPError as e:
+  except urllib2.HTTPError as e:
     if e.code != 500:
       raise
     DieWithError(

presubmit_canned_checks.py

@@ -5,7 +5,6 @@
 """Generic presubmit checks that can be reused by other presubmit checks."""
 
 from __future__ import print_function
-from __future__ import unicode_literals
 
 import os as _os
 _HERE = _os.path.dirname(_os.path.abspath(__file__))
@@ -246,7 +245,7 @@ def CheckGenderNeutral(input_api, output_api, source_file_filter=None):
   submitted.
   """
   gendered_re = input_api.re.compile(
-      r'(^|\s|\(|\[)([Hh]e|[Hh]is|[Hh]ers?|[Hh]im|[Ss]he|[Gg]uys?)\\b')
+      '(^|\s|\(|\[)([Hh]e|[Hh]is|[Hh]ers?|[Hh]im|[Ss]he|[Gg]uys?)\\b')
 
   errors = []
   for f in input_api.AffectedFiles(include_deletes=False,
@@ -571,7 +570,7 @@ def CheckTreeIsOpen(input_api, output_api,
     return []
   try:
     if json_url:
-      connection = input_api.urllib_request.urlopen(json_url)
+      connection = input_api.urllib2.urlopen(json_url)
       status = input_api.json.loads(connection.read())
       connection.close()
       if not status['can_commit_freely']:
@@ -580,7 +579,7 @@ def CheckTreeIsOpen(input_api, output_api,
         return [output_api.PresubmitError(short_text, long_text=long_text)]
     else:
       # TODO(bradnelson): drop this once all users are gone.
-      connection = input_api.urllib_request.urlopen(url)
+      connection = input_api.urllib2.urlopen(url)
       status = connection.read()
       connection.close()
       if input_api.re.match(closed, status):
@@ -825,7 +824,7 @@ def GetPylint(input_api, output_api, white_list=None, black_list=None,
 
   The default white_list enforces looking only at *.py files.
   """
-  white_list = tuple(white_list or (r'.*\.py$',))
+  white_list = tuple(white_list or ('.*\.py$',))
   black_list = tuple(black_list or input_api.DEFAULT_BLACK_LIST)
   extra_paths_list = extra_paths_list or []
 
@@ -871,11 +870,9 @@ def GetPylint(input_api, output_api, white_list=None, black_list=None,
   input_api.logging.info('Running pylint on %d files', len(files))
   input_api.logging.debug('Running pylint on: %s', files)
   env = input_api.environ.copy()
-  # We convert to str, since on Windows on Python 2 only strings are allowed
-  # as environment variables, but literals are unicode since we're importing
-  # unicode_literals from __future__.
-  env[str('PYTHONPATH')] = input_api.os_path.pathsep.join(extra_paths_list)
-  env.pop(str('VPYTHON_CLEAR_PYTHONPATH'), None)
+  env['PYTHONPATH'] = input_api.os_path.pathsep.join(
+    extra_paths_list).encode('utf8')
+  env.pop('VPYTHON_CLEAR_PYTHONPATH', None)
   input_api.logging.debug('  with extra PYTHONPATH: %r', extra_paths_list)
 
   def GetPylintCmd(flist, extra, parallel):
@@ -942,7 +939,7 @@ def RunPylint(input_api, *args, **kwargs):
 def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
     ignored):
   try:
-    connection = input_api.urllib_request.urlopen(url)
+    connection = input_api.urllib2.urlopen(url)
     raw_data = connection.read()
     connection.close()
   except IOError:
@@ -1007,7 +1004,7 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
       input_api.change.RepositoryRoot(),
       owner_email,
       reviewers,
-      fopen=open,
+      fopen=file,
       os_path=input_api.os_path,
       email_postfix='',
       disable_color=True,
@@ -1132,7 +1129,7 @@ def PanProjectChecks(input_api, output_api,
   # 2006-20xx string used on the oldest files. 2006-20xx is deprecated, but
   # tolerated on old files.
   current_year = int(input_api.time.strftime('%Y'))
-  allowed_years = (str(s) for s in reversed(range(2006, current_year + 1)))
+  allowed_years = (str(s) for s in reversed(xrange(2006, current_year + 1)))
   years_re = '(' + '|'.join(allowed_years) + '|2006-2008|2006-2009|2006-2010)'
 
   # The (c) is deprecated, but tolerate it until it's removed from all files.
@@ -1161,10 +1158,7 @@ def PanProjectChecks(input_api, output_api,
   snapshot_memory = []
   def snapshot(msg):
     """Measures & prints performance warning if a rule is running slow."""
-    if input_api.sys.version_info.major == 2:
-      dt2 = input_api.time.clock()
-    else:
-      dt2 = input_api.time.process_time()
+    dt2 = input_api.time.clock()
     if snapshot_memory:
       delta_ms = int(1000*(dt2 - snapshot_memory[0]))
       if delta_ms > 500:
@@ -1397,6 +1391,7 @@ def CheckChangedLUCIConfigs(input_api, output_api):
   import base64
   import json
   import logging
+  import urllib2
 
   import auth
   import git_cl
@@ -1433,16 +1428,16 @@ def CheckChangedLUCIConfigs(input_api, output_api):
   def request(endpoint, body=None):
     api_url = ('https://%s/_ah/api/config/v1/%s'
                % (LUCI_CONFIG_HOST_NAME, endpoint))
-    req = input_api.urllib_request.Request(api_url)
+    req = urllib2.Request(api_url)
     req.add_header('Authorization', 'Bearer %s' % acc_tkn.token)
     if body is not None:
       req.add_header('Content-Type', 'application/json')
       req.add_data(json.dumps(body))
-    return json.load(input_api.urllib_request.urlopen(req))
+    return json.load(urllib2.urlopen(req))
 
   try:
     config_sets = request('config-sets').get('config_sets')
-  except input_api.urllib_error.HTTPError as e:
+  except urllib2.HTTPError as e:
     return [output_api.PresubmitError(
         'Config set request to luci-config failed', long_text=str(e))]
   if not config_sets:
@@ -1480,7 +1475,7 @@ def CheckChangedLUCIConfigs(input_api, output_api):
         cs_to_files[cs].append({
           'path': file_path[len(dr):] if dr != '/' else file_path,
           'content': base64.b64encode(
-              '\n'.join(f.NewContents()).encode('utf-8')).decode('utf-8')
+              '\n'.join(f.NewContents()).encode('utf-8'))
         })
   outputs = []
   for cs, f in cs_to_files.items():
@@ -1488,7 +1483,7 @@ def CheckChangedLUCIConfigs(input_api, output_api):
       # TODO(myjang): parallelize
       res = request(
           'validate-config', body={'config_set': cs, 'files': f})
-    except input_api.urllib_error.HTTPError as e:
+    except urllib2.HTTPError as e:
       return [output_api.PresubmitError(
           'Validation request to luci-config failed', long_text=str(e))]
     for msg in res.get('messages', []):

presubmit_support.py

@@ -7,7 +7,6 @@
 """
 
 from __future__ import print_function
-from __future__ import unicode_literals
 
 __version__ = '1.8.0'
 
@@ -34,6 +33,7 @@ import sys  # Parts exposed through API.
 import tempfile  # Exposed through the API.
 import threading
 import time
+import types
 import unittest  # Exposed through the API.
 from warnings import warn
 
@@ -542,12 +542,10 @@ class InputApi(object):
     self.os_walk = os.walk
     self.re = re
     self.subprocess = subprocess
-    self.sys = sys
     self.tempfile = tempfile
     self.time = time
     self.unittest = unittest
-    if sys.version_info.major == 2:
-      self.urllib2 = urllib2
+    self.urllib2 = urllib2
     self.urllib_request = urllib_request
     self.urllib_error = urllib_error
 
@@ -577,7 +575,7 @@ class InputApi(object):
     # TODO(dpranke): figure out a list of all approved owners for a repo
     # in order to be able to handle wildcard OWNERS files?
     self.owners_db = owners.Database(change.RepositoryRoot(),
-                                     fopen=open, os_path=self.os_path)
+                                     fopen=file, os_path=self.os_path)
     self.owners_finder = owners_finder.OwnersFinder
     self.verbose = verbose
     self.Command = CommandData
@@ -611,9 +609,9 @@ class InputApi(object):
     if len(dir_with_slash) == 1:
       dir_with_slash = ''
 
-    return list(filter(
+    return filter(
         lambda x: normpath(x.AbsoluteLocalPath()).startswith(dir_with_slash),
-        self.change.AffectedFiles(include_deletes, file_filter)))
+        self.change.AffectedFiles(include_deletes, file_filter))
 
   def LocalPaths(self):
     """Returns local paths of input_api.AffectedFiles()."""
@@ -635,9 +633,8 @@ class InputApi(object):
                " is deprecated and ignored" % str(include_deletes),
            category=DeprecationWarning,
            stacklevel=2)
-    return list(filter(
-        lambda x: x.IsTestableFile(),
-        self.AffectedFiles(include_deletes=False, **kwargs)))
+    return filter(lambda x: x.IsTestableFile(),
+                  self.AffectedFiles(include_deletes=False, **kwargs))
 
   def AffectedTextFiles(self, include_deletes=None):
     """An alias to AffectedTestableFiles for backwards compatibility."""
@@ -670,7 +667,7 @@ class InputApi(object):
     """
     if not source_file:
       source_file = self.FilterSourceFile
-    return list(filter(source_file, self.AffectedTestableFiles()))
+    return filter(source_file, self.AffectedTestableFiles())
 
   def RightHandSideLines(self, source_file_filter=None):
     """An iterator over all text lines in "new" version of changed files.
@@ -1097,11 +1094,11 @@ class Change(object):
     Returns:
       [AffectedFile(path, action), AffectedFile(path, action)]
     """
-    affected = list(filter(file_filter, self._affected_files))
+    affected = filter(file_filter, self._affected_files)
 
     if include_deletes:
       return affected
-    return list(filter(lambda x: x.Action() != 'D', affected))
+    return filter(lambda x: x.Action() != 'D', affected)
 
   def AffectedTestableFiles(self, include_deletes=None, **kwargs):
     """Return a list of the existing text files in a change."""
@@ -1110,9 +1107,8 @@ class Change(object):
                " is deprecated and ignored" % str(include_deletes),
            category=DeprecationWarning,
            stacklevel=2)
-    return list(filter(
-        lambda x: x.IsTestableFile(),
-        self.AffectedFiles(include_deletes=False, **kwargs)))
+    return filter(lambda x: x.IsTestableFile(),
+                  self.AffectedFiles(include_deletes=False, **kwargs))
 
   def AffectedTextFiles(self, include_deletes=None):
     """An alias to AffectedTestableFiles for backwards compatibility."""
@@ -1449,9 +1445,9 @@ class PresubmitExecuter(object):
         logging.debug('Running %s done.', function_name)
         self.more_cc.extend(output_api.more_cc)
       finally:
-        for f in input_api._named_temporary_files:
-          os.remove(f)
-      if not isinstance(result, (tuple, list)):
+        map(os.remove, input_api._named_temporary_files)
+      if not (isinstance(result, types.TupleType) or
+              isinstance(result, types.ListType)):
         raise PresubmitFailure(
           'Presubmit functions must return a tuple or list')
       for item in result:
@@ -1573,8 +1569,7 @@ def DoPresubmitChecks(change,
         ]
       }
 
-      gclient_utils.FileWrite(
-          json_output, json.dumps(presubmit_results, sort_keys=True))
+      gclient_utils.FileWrite(json_output, json.dumps(presubmit_results))
 
     output.write('\n')
     for name, items in (('Messages', notifications),

tests/presubmit_unittest.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env vpython3
+#!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
@@ -7,8 +7,7 @@
 
 # pylint: disable=no-member,E1103
 
-from __future__ import unicode_literals
-
+import StringIO
 import functools
 import itertools
 import logging
@@ -20,6 +19,7 @@ import sys
 import tempfile
 import time
 import unittest
+import urllib2
 
 _ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, _ROOT)
@@ -38,15 +38,6 @@ import presubmit_support as presubmit
 import scm
 import subprocess2 as subprocess
 
-if sys.version_info.major == 2:
-  from cStringIO import StringIO
-  import urllib2 as urllib_request
-  BUILTIN_OPEN = '__builtin__.open'
-else:
-  from io import StringIO
-  import urllib.request as urllib_request
-  BUILTIN_OPEN = 'builtins.open'
-
 # Shortcut.
 presubmit_canned_checks = presubmit.presubmit_canned_checks
 
@@ -172,14 +163,11 @@ index fe3de7b..54ae6e1 100755
     mock.patch('scm.determine_scm').start()
     mock.patch('scm.GIT.GenerateDiff').start()
     mock.patch('subprocess2.Popen').start()
-    mock.patch('sys.stderr', StringIO()).start()
-    mock.patch('sys.stdout', StringIO()).start()
+    mock.patch('sys.stderr', StringIO.StringIO()).start()
+    mock.patch('sys.stdout', StringIO.StringIO()).start()
     mock.patch('tempfile.NamedTemporaryFile').start()
     mock.patch('multiprocessing.cpu_count', lambda: 2)
-    if sys.version_info.major == 2:
-      mock.patch('urllib2.urlopen').start()
-    else:
-      mock.patch('urllib.request.urlopen').start()
+    mock.patch('urllib2.urlopen').start()
     self.addCleanup(mock.patch.stopall)
 
   def checkstdout(self, value):
@@ -643,7 +631,7 @@ def CheckChangeOnCommit(input_api, output_api):
         ]
     }
 
-    fake_result_json = json.dumps(fake_result, sort_keys=True)
+    fake_result_json = json.dumps(fake_result)
 
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
@@ -669,7 +657,7 @@ def CheckChangeOnCommit(input_api, output_api):
     # Make a change with a single warning.
     change = self.ExampleChange(extra_lines=['PROMPT_WARNING=yes'])
 
-    input_buf = StringIO('n\n')  # say no to the warning
+    input_buf = StringIO.StringIO('n\n')  # say no to the warning
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=input_buf,
@@ -678,7 +666,7 @@ def CheckChangeOnCommit(input_api, output_api):
     self.assertFalse(output.should_continue())
     self.assertEqual(output.getvalue().count('??'), 2)
 
-    input_buf = StringIO('y\n')  # say yes to the warning
+    input_buf = StringIO.StringIO('y\n')  # say yes to the warning
     output = presubmit.DoPresubmitChecks(
         change=change, committing=False, verbose=True,
         output_stream=None, input_stream=input_buf,
@@ -752,7 +740,7 @@ def CheckChangeOnCommit(input_api, output_api):
         lambda d: [] if d == self.fake_root_dir else ['PRESUBMIT.py'])
     random.randint.return_value = 0
 
-    input_buf = StringIO('y\n')
+    input_buf = StringIO.StringIO('y\n')
 
     change = self.ExampleChange(extra_lines=['STORY=http://tracker/123'])
     output = presubmit.DoPresubmitChecks(
@@ -832,7 +820,7 @@ def CheckChangeOnCommit(input_api, output_api):
       'try2.cr': {'linux2': set(['defaulttests'])},
     }
     for permutation in itertools.permutations(parts):
-      self.assertEqual(expected, functools.reduce(merge, permutation, {}))
+      self.assertEqual(expected, reduce(merge, permutation, {}))
 
   def testDoGetTryMasters(self):
     root_text = (self.presubmit_trymaster
@@ -856,12 +844,12 @@ def CheckChangeOnCommit(input_api, output_api):
     change = presubmit.Change(
         'mychange', '', self.fake_root_dir, [], 0, 0, None)
 
-    output = StringIO()
+    output = StringIO.StringIO()
     self.assertEqual({'t1.cr': {'win': ['defaulttests']}},
                      presubmit.DoGetTryMasters(change, [filename],
                                                self.fake_root_dir,
                                                None, None, False, output))
-    output = StringIO()
+    output = StringIO.StringIO()
     expected = {
       't1.cr': {'win': ['defaulttests'], 'linux1': ['t1']},
       't2.cr': {'linux2': ['defaulttests']},
@@ -1096,7 +1084,7 @@ class InputApiUnittest(PresubmitTestsBase):
     self.assertEqual(len(input_api.DEFAULT_WHITE_LIST), 24)
     self.assertEqual(len(input_api.DEFAULT_BLACK_LIST), 12)
     for item in files:
-      results = list(filter(input_api.FilterSourceFile, item[0]))
+      results = filter(input_api.FilterSourceFile, item[0])
       for i in range(len(results)):
         self.assertEqual(results[i].LocalPath(),
                           presubmit.normpath(item[1][i]))
@@ -1301,33 +1289,33 @@ class OutputApiUnittest(PresubmitTestsBase):
     self.assertIsNotNone(output.should_continue())
     self.assertIsNotNone(output.getvalue().count('?see?'))
 
-    output = presubmit.PresubmitOutput(input_stream=StringIO('y'))
+    output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('y'))
     presubmit.OutputApi.PresubmitPromptWarning('???').handle(output)
     output.prompt_yes_no('prompt: ')
     self.assertIsNotNone(output.should_continue())
     self.assertIsNotNone(output.getvalue().count('???'))
 
-    output = presubmit.PresubmitOutput(input_stream=StringIO('\n'))
+    output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('\n'))
     presubmit.OutputApi.PresubmitPromptWarning('???').handle(output)
     output.prompt_yes_no('prompt: ')
     self.assertFalse(output.should_continue())
     self.assertIsNotNone(output.getvalue().count('???'))
 
     output_api = presubmit.OutputApi(True)
-    output = presubmit.PresubmitOutput(input_stream=StringIO('y'))
+    output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('y'))
     output_api.PresubmitPromptOrNotify('???').handle(output)
     output.prompt_yes_no('prompt: ')
     self.assertIsNotNone(output.should_continue())
     self.assertIsNotNone(output.getvalue().count('???'))
 
     output_api = presubmit.OutputApi(False)
-    output = presubmit.PresubmitOutput(input_stream=StringIO('y'))
+    output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('y'))
     output_api.PresubmitPromptOrNotify('???').handle(output)
     self.assertIsNotNone(output.should_continue())
     self.assertIsNotNone(output.getvalue().count('???'))
 
     output_api = presubmit.OutputApi(True)
-    output = presubmit.PresubmitOutput(input_stream=StringIO('\n'))
+    output = presubmit.PresubmitOutput(input_stream=StringIO.StringIO('\n'))
     output_api.PresubmitPromptOrNotify('???').handle(output)
     output.prompt_yes_no('prompt: ')
     self.assertFalse(output.should_continue())
@@ -1363,7 +1351,7 @@ class AffectedFileUnittest(PresubmitTestsBase):
     f_blob = os.path.join(self.fake_root_dir, blob)
     os.path.isfile.side_effect = lambda f: f in [f_blat, f_blob]
 
-    output = list(filter(lambda x: x.IsTestableFile(), files))
+    output = filter(lambda x: x.IsTestableFile(), files)
     self.assertEqual(2, len(output))
     self.assertEqual(files[:2], output[:2])
 
@@ -1444,13 +1432,11 @@ class CannedChecksUnittest(PresubmitTestsBase):
     input_api.os_path = os.path
     input_api.re = presubmit.re
     input_api.gerrit = mock.MagicMock(presubmit.GerritAccessor)
-    if sys.version_info.major == 2:
-      input_api.urllib2 = mock.MagicMock(presubmit.urllib2)
+    input_api.urllib2 = mock.MagicMock(presubmit.urllib2)
     input_api.urllib_request = mock.MagicMock(presubmit.urllib_request)
     input_api.urllib_error = mock.MagicMock(presubmit.urllib_error)
     input_api.unittest = unittest
     input_api.subprocess = subprocess
-    input_api.sys = sys
     class fake_CalledProcessError(Exception):
       def __str__(self):
         return 'foo'
@@ -1674,7 +1660,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
       'config_sets': [{'config_set': 'deadbeef',
                        'location': '%s/+/%s' % (host, branch)}]
     }
-    urllib_request.urlopen.return_value = http_resp
+    urllib2.urlopen.return_value = http_resp
     json.load.return_value = http_resp
 
     mockChangelist().GetRemoteBranch.return_value = ('remote', branch)
@@ -1982,14 +1968,14 @@ the current line as well!
 
   def testCannedCheckTreeIsOpenOpen(self):
     input_api = self.MockInputApi(None, True)
-    input_api.urllib_request.urlopen().read.return_value = 'The tree is open'
+    input_api.urllib2.urlopen().read.return_value = 'The tree is open'
     results = presubmit_canned_checks.CheckTreeIsOpen(
         input_api, presubmit.OutputApi, url='url_to_open', closed='.*closed.*')
     self.assertEqual(results, [])
 
   def testCannedCheckTreeIsOpenClosed(self):
     input_api = self.MockInputApi(None, True)
-    input_api.urllib_request.urlopen().read.return_value = (
+    input_api.urllib2.urlopen().read.return_value = (
         'Tree is closed for maintenance')
     results = presubmit_canned_checks.CheckTreeIsOpen(
         input_api, presubmit.OutputApi,
@@ -2005,7 +1991,7 @@ the current line as well!
         'general_state': 'open',
         'message': 'The tree is open'
     }
-    input_api.urllib_request.urlopen().read.return_value = json.dumps(status)
+    input_api.urllib2.urlopen().read.return_value = json.dumps(status)
     results = presubmit_canned_checks.CheckTreeIsOpen(
         input_api, presubmit.OutputApi, json_url='url_to_open')
     self.assertEqual(results, [])
@@ -2017,7 +2003,7 @@ the current line as well!
         'general_state': 'closed',
         'message': 'The tree is close',
     }
-    input_api.urllib_request.urlopen().read.return_value = json.dumps(status)
+    input_api.urllib2.urlopen().read.return_value = json.dumps(status)
     results = presubmit_canned_checks.CheckTreeIsOpen(
         input_api, presubmit.OutputApi, json_url='url_to_closed')
     self.assertEqual(len(results), 1)
@@ -2133,7 +2119,7 @@ the current line as well!
 
   def testCheckBuildbotPendingBuildsBad(self):
     input_api = self.MockInputApi(None, True)
-    input_api.urllib_request.urlopen().read.return_value = 'foo'
+    input_api.urllib2.urlopen().read.return_value = 'foo'
 
     results = presubmit_canned_checks.CheckBuildbotPendingBuilds(
         input_api, presubmit.OutputApi, 'uurl', 2, ('foo'))
@@ -2143,7 +2129,7 @@ the current line as well!
 
   def testCheckBuildbotPendingBuildsGood(self):
     input_api = self.MockInputApi(None, True)
-    input_api.urllib_request.urlopen().read.return_value = """
+    input_api.urllib2.urlopen().read.return_value = """
     {
       'b1': { 'pending_builds': [0, 1, 2, 3, 4, 5, 6, 7] },
       'foo': { 'pending_builds': [0, 1, 2, 3, 4, 5, 6, 7] },
@@ -2169,7 +2155,7 @@ the current line as well!
 
     os.path.exists = lambda _: True
 
-    owners_file = StringIO(owners_content)
+    owners_file = StringIO.StringIO(owners_content)
     fopen = lambda *args: owners_file
 
     input_api.owners_db = owners.Database('', fopen, os.path)
@@ -2279,7 +2265,7 @@ the current line as well!
         presubmit.OutputApi)
     for result in results:
       result.handle(output)
-    if expected_output:
+    if isinstance(expected_output, re._pattern_type):
       self.assertRegexpMatches(output.getvalue(), expected_output)
     else:
       self.assertEqual(output.getvalue(), expected_output)
@@ -2558,7 +2544,7 @@ the current line as well!
                            is_committing=False,
                            uncovered_files=set())
 
-  @mock.patch(BUILTIN_OPEN, mock.mock_open(read_data=''))
+  @mock.patch('__builtin__.open', mock.mock_open(read_data=''))
   def testCannedRunUnitTests(self):
     change = presubmit.Change(
         'foo1', 'description1', self.fake_root_dir, None, 0, 0, None)
@@ -2601,7 +2587,7 @@ the current line as well!
 
     self.checkstdout('')
 
-  @mock.patch(BUILTIN_OPEN, mock.mock_open())
+  @mock.patch('__builtin__.open', mock.mock_open())
   def testCannedRunUnitTestsPython3(self):
     open().readline.return_value = '#!/usr/bin/env python3'
     change = presubmit.Change(
@@ -2657,7 +2643,7 @@ the current line as well!
 
     self.checkstdout('')
 
-  @mock.patch(BUILTIN_OPEN, mock.mock_open())
+  @mock.patch('__builtin__.open', mock.mock_open())
   def testCannedRunUnitTestsDontRunOnPython2(self):
     open().readline.return_value = '#!/usr/bin/env python3'
     change = presubmit.Change(
@@ -2701,7 +2687,7 @@ the current line as well!
 
     self.checkstdout('')
 
-  @mock.patch(BUILTIN_OPEN, mock.mock_open())
+  @mock.patch('__builtin__.open', mock.mock_open())
   def testCannedRunUnitTestsDontRunOnPython3(self):
     open().readline.return_value = '#!/usr/bin/env python3'
     change = presubmit.Change(