Standardize the sys.path fix up and fix a few pylint warnings.

Disable temporarily W0403, will be reenabled on the next CL

R=dpranke@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/8508017

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@109435 0039d316-1c4b-4281-b951-d872f2087c98

gcl.py

@@ -278,7 +278,7 @@ class ChangeInfo(object):
     rietveld: rietveld server for this change
   """
   # Kept for unit test support. This is for the old format, it's deprecated.
-  _SEPARATOR = "\n-----\n"
+  SEPARATOR = "\n-----\n"
 
   def __init__(self, name, issue, patchset, description, files, local_root,
                rietveld_url, needs_upload):
@@ -576,15 +576,15 @@ class ChangeInfo(object):
   def _LoadOldFormat(content):
     # The info files have the following format:
     # issue_id, patchset\n   (, patchset is optional)
-    # _SEPARATOR\n
+    # SEPARATOR\n
     # filepath1\n
     # filepath2\n
     # .
     # .
     # filepathn\n
-    # _SEPARATOR\n
+    # SEPARATOR\n
     # description
-    split_data = content.split(ChangeInfo._SEPARATOR, 2)
+    split_data = content.split(ChangeInfo.SEPARATOR, 2)
     if len(split_data) != 3:
       raise ValueError('Bad change format')
     values = {

presubmit_support.py

@@ -662,7 +662,7 @@ class Change(object):
   _AFFECTED_FILES = AffectedFile
 
   # Matches key/value (or "tag") lines in changelist descriptions.
-  _TAG_LINE_RE = re.compile(
+  TAG_LINE_RE = re.compile(
       '^\s*(?P<key>[A-Z][A-Z_0-9]*)\s*=\s*(?P<value>.*?)\s*$')
   scm = ''
 
@@ -683,7 +683,7 @@ class Change(object):
     description_without_tags = []
     self.tags = {}
     for line in self._full_description.splitlines():
-      m = self._TAG_LINE_RE.match(line)
+      m = self.TAG_LINE_RE.match(line)
       if m:
         self.tags[m.group('key')] = m.group('value')
       else:

pylintrc

@@ -57,13 +57,14 @@ load-plugins=
 # W0141: Used builtin function ''
 # W0142: Used * or ** magic
 # W0402: Uses of a deprecated module 'string'
+# W0403: Relative import 'X', should be 'Y.X'
 # W0404: 41: Reimport 'XX' (imported line NN)
 # W0511: TODO
 # W0603: Using the global statement
 # W0613: Unused argument ''
 # W0703: Catch "Exception"
 # W1201: Specify string format arguments as logging function parameters
-disable=C0103,C0111,C0302,I0010,I0011,R0401,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0402,W0404,W0511,W0603,W0613,W0703,W1201
+disable=C0103,C0111,C0302,I0010,I0011,R0401,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0402,W0403,W0404,W0511,W0603,W0613,W0703,W1201
 
 
 [REPORTS]

tests/breakpad_unittest.py

@@ -5,9 +5,11 @@
 
 """Unit tests for breakpad.py."""
 
-# pylint: disable=W0403
+import os
+import sys
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-# Fixes include path.
 from super_mox import SuperMoxTestBase
 
 import breakpad

tests/checkout_test.py

@@ -14,8 +14,7 @@ import unittest
 from xml.etree import ElementTree
 
 ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
-BASE_DIR = os.path.join(ROOT_DIR, '..')
-sys.path.insert(0, BASE_DIR)
+sys.path.insert(0, os.path.dirname(ROOT_DIR))
 
 import checkout
 import patch

tests/fix_encoding_test.py

@@ -10,8 +10,7 @@ import os
 import sys
 import unittest
 
-ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
-sys.path.insert(0, ROOT_DIR)
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import fix_encoding
 

tests/gcl_unittest.py

@@ -5,10 +5,13 @@
 
 """Unit tests for gcl.py."""
 
-# pylint is too confused.
-# pylint: disable=E1101,E1103,E1120,W0212,W0403
+# pylint: disable=E1103,E1101,E1120
+
+import os
+import sys
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-# Fixes include path.
 from super_mox import mox, SuperMoxTestBase
 
 import gcl
@@ -180,6 +183,7 @@ class ChangeInfoUnittest(GclTestsBase):
       'CloseIssue', 'Delete', 'Exists', 'GetFiles', 'GetFileNames',
       'GetLocalRoot', 'GetIssueDescription', 'Load', 'MissingTests',
       'NeedsUpload', 'PrimeLint', 'RpcServer', 'Save', 'SendToRietveld',
+      'SEPARATOR',
       'UpdateRietveldDescription',
       'description', 'issue', 'name',
       'needs_upload', 'patch', 'patchset', 'reviewers', 'rietveld',
@@ -217,7 +221,7 @@ class ChangeInfoUnittest(GclTestsBase):
     gcl.GetChangelistInfoFile('bleh').AndReturn('bleeeh')
     gcl.os.path.exists('bleeeh').AndReturn(True)
     gcl.gclient_utils.FileRead('bleeeh').AndReturn(
-      gcl.ChangeInfo._SEPARATOR.join(["42, 53", "G      b.cc"] + description))
+      gcl.ChangeInfo.SEPARATOR.join(["42, 53", "G      b.cc"] + description))
     gcl.GetCodeReviewSetting('CODE_REVIEW_SERVER').AndReturn('foo')
     # Does an upgrade.
     gcl.GetChangelistInfoFile('bleh').AndReturn('bleeeh')
@@ -229,7 +233,7 @@ class ChangeInfoUnittest(GclTestsBase):
     self.assertEquals(change_info.issue, 42)
     self.assertEquals(change_info.patchset, 53)
     self.assertEquals(change_info.description,
-                      gcl.ChangeInfo._SEPARATOR.join(description))
+                      gcl.ChangeInfo.SEPARATOR.join(description))
     self.assertEquals(change_info.GetFiles(), [('G      ', 'b.cc')])
 
   def testLoadEmpty(self):
@@ -237,7 +241,7 @@ class ChangeInfoUnittest(GclTestsBase):
     gcl.GetChangelistInfoFile('bleh').AndReturn('bleeeh')
     gcl.os.path.exists('bleeeh').AndReturn(True)
     gcl.gclient_utils.FileRead('bleeeh').AndReturn(
-        gcl.ChangeInfo._SEPARATOR.join(["", "", ""]))
+        gcl.ChangeInfo.SEPARATOR.join(["", "", ""]))
     gcl.GetCodeReviewSetting('CODE_REVIEW_SERVER').AndReturn('foo')
     # Does an upgrade.
     gcl.GetChangelistInfoFile('bleh').AndReturn('bleeeh')
@@ -566,6 +570,7 @@ class CMDCommitUnittest(GclTestsBase):
 
     self.assertEquals(retval, 0)
     self.assertEquals(change_info.description, 'deescription')
+    # pylint: disable=W0212
     self.assertFalse(change_info._deleted)
     self.assertFalse(change_info._closed)
 
@@ -581,6 +586,7 @@ class CMDCommitUnittest(GclTestsBase):
     self.assertEquals(retval, 0)
     self.assertEquals(change_info.description,
         'deescription\n\nCommitted: http://view/12345')
+    # pylint: disable=W0212
     self.assertTrue(change_info._deleted)
     self.assertTrue(change_info._closed)
 

tests/gclient_scm_test.py

@@ -5,21 +5,25 @@
 
 """Unit tests for gclient_scm.py."""
 
-# pylint: disable=E1101,E1103,W0403
+# pylint: disable=E1103
 
 # Import before super_mox to keep valid references.
 from os import rename
 from shutil import rmtree
 from subprocess import Popen, PIPE, STDOUT
+
+import logging
+import os
+import sys
 import tempfile
 import unittest
 import __builtin__
 
-# Fixes include path.
-from super_mox import mox, StdoutCheck, TestCaseUtils, SuperMoxTestBase
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+from super_mox import mox, StdoutCheck, SuperMoxTestBase
+from super_mox import TestCaseUtils
 
-import logging
-import sys
 import gclient_scm
 import subprocess2
 
@@ -30,6 +34,7 @@ join = gclient_scm.os.path.join
 class GCBaseTestCase(object):
   def assertRaisesError(self, msg, fn, *args, **kwargs):
     """Like unittest's assertRaises() but checks for Gclient.Error."""
+    # pylint: disable=E1101
     try:
       fn(*args, **kwargs)
     except gclient_scm.gclient_utils.Error, e:
@@ -252,9 +257,8 @@ class SVNWrapperTestCase(BaseTestCase):
     gclient_scm.os.path.islink(file_path).AndReturn(False)
     gclient_scm.os.path.isdir(file_path).AndReturn(True)
     gclient_scm.gclient_utils.RemoveDirectory(file_path)
-    gclient_scm.os.path.isdir(self.base_path).AndReturn(False)
-    # The mock is unbound so self is not necessary.
     # pylint: disable=E1120
+    gclient_scm.os.path.isdir(self.base_path).AndReturn(False)
     gclient_scm.SVNWrapper.update(options, [], ['.'])
 
     self.mox.ReplayAll()

tests/gclient_smoketest.py

@@ -10,8 +10,6 @@ Shell out 'gclient' and run basic conformance tests.
 This test assumes GClientSmokeBase.URL_BASE is valid.
 """
 
-# pylint: disable=E1103,W0403
-
 import logging
 import os
 import re
@@ -19,12 +17,14 @@ import subprocess
 import sys
 import unittest
 
-ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.dirname(ROOT_DIR))
-from tests.fake_repos import join, write, FakeReposTestBase
+ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+sys.path.insert(0, ROOT_DIR)
+
+from fake_repos import join, write, FakeReposTestBase
+
 import subprocess2
 
-GCLIENT_PATH = os.path.join(os.path.dirname(ROOT_DIR), 'gclient')
+GCLIENT_PATH = os.path.join(ROOT_DIR, 'gclient')
 COVERAGE = False
 
 
@@ -50,6 +50,7 @@ class GClientSmokeBase(FakeReposTestBase):
     (stdout, stderr) = process.communicate()
     logging.debug("XXX: %s\n%s\nXXX" % (' '.join(cmd), stdout))
     logging.debug("YYY: %s\n%s\nYYY" % (' '.join(cmd), stderr))
+    # pylint: disable=E1103
     return (stdout.replace('\r\n', '\n'), stderr.replace('\r\n', '\n'),
             process.returncode)
 

tests/gclient_test.py

@@ -15,8 +15,7 @@ import os
 import sys
 import unittest
 
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.dirname(BASE_DIR))
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import gclient
 import gclient_utils

tests/gclient_utils_test.py

@@ -3,12 +3,12 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-# pylint: disable=E1101,W0403
-
 import os
 import StringIO
+import sys
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-# Fixes include path.
 from super_mox import SuperMoxTestBase
 import trial_dir
 
@@ -57,6 +57,7 @@ class CheckCallAndFilterTestCase(GclientUtilBase):
         '\n________ running \'boo foo bar\' in \'bleh\'\n')
     for i in test_string:
       gclient_utils.sys.stdout.write(i)
+    # pylint: disable=E1101
     subprocess2.Popen(
         args,
         cwd=cwd,

tests/owners_unittest.py

@@ -9,7 +9,7 @@ import os
 import sys
 import unittest
 
-sys.path.insert(0, os.path.dirname(os.path.dirname(__file__)))
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import owners
 from tests import filesystem_mock

tests/patch_test.py

@@ -11,8 +11,7 @@ import posixpath
 import sys
 import unittest
 
-ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.join(ROOT_DIR, '..'))
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import patch
 from tests.patches_data import GIT, RAW
@@ -92,8 +91,6 @@ class PatchTest(unittest.TestCase):
         p, 'foo', GIT.NEW, is_new=True, is_git_diff=True, patchlevel=1)
 
   def testValidSvn(self):
-    # pylint: disable=R0201
-    # Method could be a function
     # Should not throw.
     p = patch.FilePatchDiff('chrome/file.cc', RAW.PATCH, [])
     lines = RAW.PATCH.splitlines(True)

tests/presubmit_unittest.py

@@ -5,20 +5,22 @@
 
 """Unit tests for presubmit_support.py and presubmit_canned_checks.py."""
 
-# pylint is too confused.
-# pylint: disable=E1101,E1103,R0201,W0212,W0403
+# pylint: disable=E1101,E1103
 
 import logging
+import os
 import StringIO
 import sys
 import time
 
-# Fixes include path.
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
 from super_mox import mox, SuperMoxTestBase
 
 import owners
 import presubmit_support as presubmit
 import rietveld
+
 # Shortcut.
 presubmit_canned_checks = presubmit.presubmit_canned_checks
 
@@ -233,7 +235,7 @@ class PresubmitUnittest(PresubmitTestsBase):
 
   def testTagLineRe(self):
     self.mox.ReplayAll()
-    m = presubmit.Change._TAG_LINE_RE.match(' BUG =1223, 1445  \t')
+    m = presubmit.Change.TAG_LINE_RE.match(' BUG =1223, 1445  \t')
     self.failUnless(m)
     self.failUnlessEqual(m.group('key'), 'BUG')
     self.failUnlessEqual(m.group('value'), '1223, 1445')
@@ -1346,6 +1348,7 @@ class ChangeUnittest(PresubmitTestsBase):
         'AbsoluteLocalPaths', 'AffectedFiles', 'AffectedTextFiles',
         'DescriptionText', 'FullDescriptionText', 'LocalPaths', 'Name',
         'RepositoryRoot', 'RightHandSideLines', 'ServerPaths',
+        'TAG_LINE_RE',
         'author_email', 'issue', 'patchset', 'scm', 'tags',
     ]
     # If this test fails, you should add the relevant test.
@@ -1372,10 +1375,8 @@ class ChangeUnittest(PresubmitTestsBase):
 class CannedChecksUnittest(PresubmitTestsBase):
   """Tests presubmit_canned_checks.py."""
 
-  def setUp(self):
-    PresubmitTestsBase.setUp(self)
-
   def MockInputApi(self, change, committing):
+    # pylint: disable=R0201
     input_api = self.mox.CreateMock(presubmit.InputApi)
     input_api.cStringIO = presubmit.cStringIO
     input_api.json = presubmit.json
@@ -1684,6 +1685,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
     self.assertEquals(len(results1), 1)
     self.assertEquals(results1[0].__class__,
         presubmit.OutputApi.PresubmitPromptWarning)
+    # pylint: disable=W0212
     self.assertEquals(results1[0]._long_text,
         'makefile.foo, line 46')
 
@@ -1973,6 +1975,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
     self.assertEquals(len(results), 1)
     self.assertEquals(results[0].__class__,
                       presubmit.OutputApi.PresubmitNotifyResult)
+    # pylint: disable=W0212
     self.assertEquals('test_module failed!\nfoo', results[0]._message)
 
   def testRunPythonUnitTestsFailureCommitting(self):
@@ -1987,6 +1990,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
         input_api, presubmit.OutputApi, ['test_module'])
     self.assertEquals(len(results), 1)
     self.assertEquals(results[0].__class__, presubmit.OutputApi.PresubmitError)
+    # pylint: disable=W0212
     self.assertEquals('test_module failed!\nfoo', results[0]._message)
 
   def testRunPythonUnitTestsSuccess(self):
@@ -2227,6 +2231,7 @@ class CannedChecksUnittest(PresubmitTestsBase):
         project_name=None,
         owners_check=True)
     self.assertEqual(1, len(results))
+    # pylint: disable=W0212
     self.assertEqual(
         'Found line ending with white spaces in:', results[0]._message)
     self.checkstdout('')

tests/rietveld_test.py

@@ -10,16 +10,12 @@ import os
 import sys
 import unittest
 
-ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(0, os.path.join(ROOT_DIR, '..'))
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import patch
 import rietveld
 from tests.patches_data import GIT, RAW
 
-# Access to a protected member XX of a client class
-# pylint: disable=W0212
-
 
 def _api(files):
   """Mock a rietveld api request."""
@@ -40,12 +36,16 @@ def _file(
 
 class RietveldTest(unittest.TestCase):
   def setUp(self):
+    super(RietveldTest, self).setUp()
+    # Access to a protected member XX of a client class
+    # pylint: disable=W0212
     self.rietveld = rietveld.Rietveld('url', 'email', 'password')
     self.rietveld._send = self._rietveld_send
     self.requests = []
 
   def tearDown(self):
     self.assertEquals([], self.requests)
+    super(RietveldTest, self).tearDown()
 
   def _rietveld_send(self, url, *args, **kwargs):
     self.assertTrue(self.requests, url)

tests/scm_unittest.py

@@ -5,13 +5,14 @@
 
 """Unit tests for scm.py."""
 
-# pylint: disable=E1101,W0403
 from __future__ import with_statement
 import logging
+import os
 import sys
 import unittest
 
-# Fixes include path.
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
 from super_mox import SuperMoxTestBase
 
 import fake_repos
@@ -102,11 +103,11 @@ class SVNTestCase(BaseSCMTestCase):
     self.compareMembers(scm.SVN, members)
 
   def testGetCheckoutRoot(self):
+    # pylint: disable=E1103
     self.mox.StubOutWithMock(scm.SVN, 'CaptureInfo')
     self.mox.StubOutWithMock(scm, 'GetCasedPath')
     scm.os.path.abspath = lambda x: x
     scm.GetCasedPath = lambda x: x
-    # pylint: disable=E1103
     scm.SVN.CaptureInfo(self.root_dir + '/foo/bar').AndReturn({
         'Repository Root': 'svn://svn.chromium.org/chrome',
         'URL': 'svn://svn.chromium.org/chrome/trunk/src',

tests/subprocess2_test.py

@@ -11,14 +11,10 @@ import sys
 import time
 import unittest
 
-ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
-sys.path.insert(0, ROOT_DIR)
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import subprocess2
 
-# Method could be a function
-# pylint: disable=R0201
-
 class Subprocess2Test(unittest.TestCase):
   # Can be mocked in a test.
   TO_SAVE = {
@@ -62,7 +58,8 @@ class Subprocess2Test(unittest.TestCase):
         assert not results
         results.update(kwargs)
         results['args'] = args
-      def communicate(self):
+      @staticmethod
+      def communicate():
         return None, None
     subprocess2.Popen = fake_Popen
     return results
@@ -76,7 +73,8 @@ class Subprocess2Test(unittest.TestCase):
         assert not results
         results.update(kwargs)
         results['args'] = args
-      def communicate(self):
+      @staticmethod
+      def communicate():
         return None, None
     subprocess2.subprocess.Popen = fake_Popen
     return results

tests/trychange_unittest.py

@@ -5,9 +5,11 @@
 
 """Unit tests for trychange.py."""
 
-# pylint: disable=E1103,W0403
+import os
+import sys
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-# Fixes include path.
 from super_mox import SuperMoxTestBase
 
 import subprocess2
@@ -91,6 +93,7 @@ class GITUnittest(TryChangeTestsBase):
     self.compareMembers(trychange.GIT, members)
 
   def testBasic(self):
+    # pylint: disable=E1103
     trychange.os.path.abspath(self.fake_root).AndReturn(self.fake_root)
     trychange.scm.GIT.GetCheckoutRoot(self.fake_root).AndReturn(self.fake_root)
     trychange.scm.GIT.GetUpstreamBranch(self.fake_root).AndReturn('somewhere')

tests/watchlists_unittest.py

@@ -5,8 +5,12 @@
 
 """Unit tests for watchlists.py."""
 
-# pylint is too confused.
-# pylint: disable=E1103,E1120,W0212,W0403
+# pylint: disable=E1103,E1120,W0212
+
+import os
+import sys
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import super_mox
 import watchlists