Add PRESUBMIT with pylint check for web-page-replay.

Review URL: https://codereview.appspot.com/234420043

PRESUBMIT.py

@@ -0,0 +1,28 @@
+# Copyright 2015 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.
+
+"""Presubmit script for changes affecting tools/perf/.
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details about the presubmit API built into depot_tools.
+"""
+
+def _CommonChecks(input_api, output_api):
+  """Performs common checks, which includes running pylint."""
+  results = []
+  results.extend(input_api.canned_checks.RunPylint(
+        input_api, output_api, black_list=[], pylintrc='pylintrc'))
+  return results
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  report = []
+  report.extend(_CommonChecks(input_api, output_api))
+  return report
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  report = []
+  report.extend(_CommonChecks(input_api, output_api))
+  return report

certutils.py

@@ -53,7 +53,7 @@
 def get_ssl_context(method=SSL_METHOD):
   # One of: One of SSLv2_METHOD, SSLv3_METHOD, SSLv23_METHOD, or TLSv1_METHOD
   if openssl_import_error:
-    raise openssl_import_error
+    raise openssl_import_error  # pylint: disable=raising-bad-type
   return SSL.Context(method)
 
 
@@ -112,7 +112,7 @@ def generate_dummy_ca_cert(subject='_WebPageReplayCert'):
     certificate
   """
   if openssl_import_error:
-    raise openssl_import_error
+    raise openssl_import_error  # pylint: disable=raising-bad-type
 
   key = crypto.PKey()
   key.generate_key(crypto.TYPE_RSA, 1024)
@@ -228,7 +228,7 @@ def generate_cert(root_ca_cert_str, server_cert_str, server_host):
     a PEM formatted certificate string
   """
   if openssl_import_error:
-    raise openssl_import_error
+    raise openssl_import_error  # pylint: disable=raising-bad-type
 
   common_name = server_host
   if server_cert_str:

customhandlers.py

@@ -28,7 +28,6 @@
 
 import base64
 import httparchive
-import httplib
 import json
 import logging
 import os
@@ -63,6 +62,7 @@ def __init__(self, options, http_archive):
       options: original options passed to the server.
       http_archive: reference to the HttpArchive object.
     """
+    self.server_manager = None
     self.options = options
     self.http_archive = http_archive
     self.handlers = [
@@ -104,6 +104,7 @@ def get_generator_url_response_code(self, request, url_suffix):
       On a match, an ArchivedHttpResponse.
       Otherwise, None.
     """
+    del request
     try:
       response_code = int(url_suffix)
       return SimpleResponse(response_code)

daemonserver.py

@@ -25,7 +25,8 @@ def __enter__(self):
     # the components do not need to communicate with each other. On Linux,
     # "taskset" could be used to assign each process to specific CPU/core.
     # Of course, only bother with this if the processing speed is an issue.
-    # Some related discussion: http://stackoverflow.com/questions/990102/python-global-interpreter-lock-gil-workaround-on-multi-core-systems-using-tasks
+    # Some related discussion: http://stackoverflow.com/questions/990102/python-
+    # global-interpreter-lock-gil-workaround-on-multi-core-systems-using-tasks
     thread = threading.Thread(target=self.serve_forever)
     thread.daemon = True  # Python exits when no non-daemon threads are left.
     thread.start()

dnsproxy.py

@@ -21,13 +21,10 @@
 import threading
 import time
 
-import third_party
-import dns.flags
-import dns.message
-import dns.rcode
-import dns.resolver
-import dns.rdatatype
-import ipaddr
+from third_party import dns
+from third_party.dns import rdatatype
+from third_party import ipaddr
+
 
 
 class DnsProxyException(Exception):
@@ -52,7 +49,7 @@ def _IsIPAddress(hostname):
     except socket.error:
       return False
 
-  def __call__(self, hostname, rdtype=dns.rdatatype.A):
+  def __call__(self, hostname, rdtype=rdatatype.A):
     """Return real IP for a host.
 
     Args:

httparchive.py

@@ -67,7 +67,7 @@ def wrapped(self, *args, **kwargs):
     try:
       return fn(self, *args, **kwargs)
     finally:
-      run_time = (time.time() - start_time) * 1000.0;
+      run_time = (time.time() - start_time) * 1000.0
       logging.debug('%s: %dms', fn.__name__, run_time)
   return wrapped
 
@@ -92,7 +92,7 @@ class HttpArchive(dict, persistentmixin.PersistentMixin):
         the archive to find potential matches.
   """
 
-  def __init__(self):
+  def __init__(self):  # pylint: disable=super-init-not-called
     self.responses_by_host = defaultdict(dict)
 
   def __setstate__(self, state):

httparchive_test.py

@@ -13,12 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import ast
 import calendar
 import email.utils
 import httparchive
-import os
-import time
 import unittest
 
 

httpclient.py

@@ -251,7 +251,7 @@ def _ToTuples(headers):
           logging.warning(
               'Response header in wrong format [%s]', line)
           continue
-        name, value = name_value
+        name, value = name_value  # pylint: disable=unpacking-non-sequence
       all_headers.append((name, value))
     return all_headers
 

httpproxy.py

@@ -72,9 +72,14 @@ def setup(self):
           self.server.traffic_shaping_down_bps)
 
   # Make request handler logging match our logging format.
-  def log_request(self, code='-', size='-'): pass
-  def log_error(self, format, *args): logging.error(format, *args)
-  def log_message(self, format, *args): logging.info(format, *args)
+  def log_request(self, code='-', size='-'):
+    pass
+
+  def log_error(self, format, *args):  # pylint:disable=redefined-builtin
+    logging.error(format, *args)
+
+  def log_message(self, format, *args):  # pylint:disable=redefined-builtin
+    logging.info(format, *args)
 
   def read_request_body(self):
     request_body = None

httpproxy_test.py

@@ -18,7 +18,6 @@
 import httplib
 import httpproxy
 import threading
-import time
 import unittest
 import util
 
@@ -33,6 +32,7 @@ def __init__(self, response):
     self._response = response
 
   def handle(self, request):
+    del request
     return self._response
 
 

platformsettings.py

@@ -37,7 +37,6 @@
 import stat
 import subprocess
 import sys
-import tempfile
 import time
 import urlparse
 
@@ -65,8 +64,9 @@ class NotAdministratorError(PlatformSettingsError):
 class CalledProcessError(PlatformSettingsError):
     """Raised when a _check_output() process returns a non-zero exit status."""
     def __init__(self, returncode, cmd):
-        self.returncode = returncode
-        self.cmd = cmd
+      super(CalledProcessError, self).__init__()
+      self.returncode = returncode
+      self.cmd = cmd
 
     def __str__(self):
         return 'Command "%s" returned non-zero exit status %d' % (
@@ -166,6 +166,7 @@ def get_httpproxy_ip_address(self, is_server_mode=False):
 
   def get_system_proxy(self, use_ssl):
     """Returns the system HTTP(S) proxy host, port."""
+    del use_ssl
     return SystemProxy(None, None)
 
   def _ipfw_cmd(self):
@@ -281,6 +282,9 @@ def set_temporary_primary_nameserver(self, nameserver):
 
 class _PosixPlatformSettings(_BasePlatformSettings):
 
+  # pylint: disable=abstract-method
+  # Suppress lint check for _get_primary_nameserver & _set_primary_nameserver
+
   def rerun_as_administrator(self):
     """If needed, rerun the program with administrative privileges.
 
@@ -573,6 +577,9 @@ def _set_primary_nameserver(self, dns):
 
 class _WindowsPlatformSettings(_BasePlatformSettings):
 
+  # pylint: disable=abstract-method
+  # Suppress lint check for _ipfw_cmd
+
   def get_system_logging_handler(self):
     """Return a handler for the logging module (optional).
 

platformsettings_test.py

@@ -180,6 +180,7 @@ def test_has_sni(self):
     platformsettings.HasSniSupport()
 
 
+# pylint: disable=abstract-method
 class Win7Settings(platformsettings._WindowsPlatformSettings):
   @classmethod
   def _ipconfig(cls, *args):

proxyshaper_test.py

@@ -44,7 +44,7 @@
 
 
 class TimedTestCase(unittest.TestCase):
-  def assertAlmostEqual(self, expected, actual, tolerance=0.05):
+  def assertValuesAlmostEqual(self, expected, actual, tolerance=0.05):
     """Like the following with nicer default message:
            assertTrue(expected <= actual + tolerance &&
                       expected >= actual - tolerance)
@@ -66,7 +66,7 @@ def testReadLimitedBasic(self):
     self.assertEqual(num_bytes, len(limited_f.read()))
     expected_ms = 8.0 * num_bytes / bps * 1000.0
     actual_ms = (proxyshaper.TIMER() - start) * 1000.0
-    self.assertAlmostEqual(expected_ms, actual_ms)
+    self.assertValuesAlmostEqual(expected_ms, actual_ms)
 
   def testReadlineLimitedBasic(self):
     num_bytes = 1024 * 8 + 512
@@ -78,7 +78,7 @@ def testReadlineLimitedBasic(self):
     self.assertEqual(num_bytes, len(limited_f.readline()))
     expected_ms = 8.0 * num_bytes / bps * 1000.0
     actual_ms = (proxyshaper.TIMER() - start) * 1000.0
-    self.assertAlmostEqual(expected_ms, actual_ms)
+    self.assertValuesAlmostEqual(expected_ms, actual_ms)
 
   def testReadLimitedSlowedByMultipleRequests(self):
     num_bytes = 1024
@@ -92,7 +92,7 @@ def testReadLimitedSlowedByMultipleRequests(self):
     self.assertEqual(num_bytes, len(num_read_bytes))
     expected_ms = 8.0 * num_bytes / (bps / float(request_count)) * 1000.0
     actual_ms = (proxyshaper.TIMER() - start) * 1000.0
-    self.assertAlmostEqual(expected_ms, actual_ms)
+    self.assertValuesAlmostEqual(expected_ms, actual_ms)
 
   def testWriteLimitedBasic(self):
     num_bytes = 1024 * 10 + 350
@@ -105,7 +105,7 @@ def testWriteLimitedBasic(self):
     self.assertEqual(num_bytes, len(limited_f.getvalue()))
     expected_ms = 8.0 * num_bytes / bps * 1000.0
     actual_ms = (proxyshaper.TIMER() - start) * 1000.0
-    self.assertAlmostEqual(expected_ms, actual_ms)
+    self.assertValuesAlmostEqual(expected_ms, actual_ms)
 
   def testWriteLimitedSlowedByMultipleRequests(self):
     num_bytes = 1024 * 10
@@ -119,7 +119,7 @@ def testWriteLimitedSlowedByMultipleRequests(self):
     self.assertEqual(num_bytes, len(limited_f.getvalue()))
     expected_ms = 8.0 * num_bytes / (bps / float(request_count)) * 1000.0
     actual_ms = (proxyshaper.TIMER() - start) * 1000.0
-    self.assertAlmostEqual(expected_ms, actual_ms)
+    self.assertValuesAlmostEqual(expected_ms, actual_ms)
 
 
 class GetBitsPerSecondTest(unittest.TestCase):

pylintrc

@@ -0,0 +1,17 @@
+[MESSAGES CONTROL]
+
+# Disable the message, report, category or checker with the given id(s).
+# TODO(wpr-owners): Reduce this list to as small as possible.
+disable=I0010,I0011,abstract-class-little-used,abstract-class-not-used,anomalous-backslash-in-string,bad-builtin,bad-context-manager,bad-continuation,bad-indentation,bad-str-strip-call,bad-whitespace,broad-except,cell-var-from-loop,deprecated-lambda,deprecated-module,duplicate-code,eval-used,exec-used,fixme,function-redefined,global-statement,interface-not-implemented,invalid-name,locally-enabled,logging-not-lazy,missing-docstring,missing-final-newline,no-init,no-member,no-name-in-module,no-self-use,no-self-use,not-callable,old-style-class,reimported,star-args,super-on-old-class,too-few-public-methods,too-many-ancestors,too-many-arguments,too-many-branches,too-many-function-args,too-many-instance-attributes,too-many-lines,too-many-locals,too-many-public-methods,too-many-return-statements,too-many-statements,trailing-whitespace,useless-else-on-loop,unused-variable,attribute-defined-outside-init,protected-access
+
+
+[REPORTS]
+
+# Don't write out full reports, just messages.
+reports=no
+
+
+[FORMAT]
+
+# We use two spaces for indents, instead of the usual four spaces or tab.
+indent-string='  '

replay.py

@@ -47,7 +47,6 @@
 import sys
 import traceback
 
-import certutils
 import customhandlers
 import dnsproxy
 import httparchive
@@ -202,7 +201,7 @@ def _CheckValidIp(self, name):
     if value:
       try:
         socket.inet_aton(value)
-      except:
+      except Exception:
         self._parser.error('Option --%s must be a valid IPv4 address.' % name)
 
   def _CheckFeatureSupport(self):
@@ -346,7 +345,7 @@ def replay(options, replay_filename):
           platformsettings.DnsUpdateError) as e:
     logging.critical('%s: %s', e.__class__.__name__, e)
     exit_status = 1
-  except:
+  except Exception:
     logging.critical(traceback.format_exc())
     exit_status = 2
 

servermanager.py

@@ -123,14 +123,15 @@ def Run(self):
         time.sleep(1)
         if self.should_exit:
           break
-    except:
+    except Exception:
       exception_info = sys.exc_info()
     finally:
       for server_exit in server_exits:
         try:
           if server_exit(*exception_info):
             exception_info = (None, None, None)
-        except:
+        except Exception:
           exception_info = sys.exc_info()
       if exception_info != (None, None, None):
+        # pylint: disable=raising-bad-type
         raise exception_info[0], exception_info[1], exception_info[2]

sslproxy.py

@@ -71,6 +71,7 @@ def finish(self):
 def wrap_handler(handler_class):
   """Wraps a BaseHTTPHandler with SSL MITM certificates."""
   if certutils.openssl_import_error:
+    # pylint: disable=raising-bad-type
     raise certutils.openssl_import_error
 
   class WrappedHandler(SslHandshakeHandler, handler_class):

test_runner.py

@@ -2,7 +2,6 @@
 # Copyright (c) 2014 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.
-import inspect
 import unittest
 import sys
 import os

third_party/__init__.py

@@ -22,4 +22,5 @@
 third_party_dir = os.path.dirname(os.path.abspath(__file__))
 ipaddr_dir = os.path.join(third_party_dir, "ipaddr")
 sys.path.append(ipaddr_dir)  # workaround for no __init__.py
+import ipaddr
 sys.path.append(third_party_dir)