From 428342a0a724e1d3b8a63d06c65e12f25bc5b4d8 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Thu, 10 Nov 2011 15:46:33 +0000 Subject: [PATCH] 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 | 8 ++++---- presubmit_support.py | 4 ++-- pylintrc | 3 ++- tests/breakpad_unittest.py | 6 ++++-- tests/checkout_test.py | 3 +-- tests/fix_encoding_test.py | 3 +-- tests/gcl_unittest.py | 18 ++++++++++++------ tests/gclient_scm_test.py | 18 +++++++++++------- tests/gclient_smoketest.py | 13 +++++++------ tests/gclient_test.py | 3 +-- tests/gclient_utils_test.py | 7 ++++--- tests/owners_unittest.py | 2 +- tests/patch_test.py | 5 +---- tests/presubmit_unittest.py | 19 ++++++++++++------- tests/rietveld_test.py | 10 +++++----- tests/scm_unittest.py | 7 ++++--- tests/subprocess2_test.py | 12 +++++------- tests/trychange_unittest.py | 7 +++++-- tests/watchlists_unittest.py | 8 ++++++-- 19 files changed, 88 insertions(+), 68 deletions(-) diff --git a/gcl.py b/gcl.py index 64804a543..368c5a3e9 100755 --- a/gcl.py +++ b/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 = { diff --git a/presubmit_support.py b/presubmit_support.py index b2a94bc24..8adbd65d7 100755 --- a/presubmit_support.py +++ b/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[A-Z][A-Z_0-9]*)\s*=\s*(?P.*?)\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: diff --git a/pylintrc b/pylintrc index 0412be85b..74d1ec2bc 100644 --- a/pylintrc +++ b/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] diff --git a/tests/breakpad_unittest.py b/tests/breakpad_unittest.py index a561b871a..44a3ba844 100755 --- a/tests/breakpad_unittest.py +++ b/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 diff --git a/tests/checkout_test.py b/tests/checkout_test.py index 1428bd730..5de734a00 100755 --- a/tests/checkout_test.py +++ b/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 diff --git a/tests/fix_encoding_test.py b/tests/fix_encoding_test.py index c8bfbc29d..28e368484 100755 --- a/tests/fix_encoding_test.py +++ b/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 diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index 41b40836d..a2fde152c 100755 --- a/tests/gcl_unittest.py +++ b/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) diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 908b322b2..4c57c3d01 100755 --- a/tests/gclient_scm_test.py +++ b/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() diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index ffaa7b0e8..41c49bcbc 100755 --- a/tests/gclient_smoketest.py +++ b/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) diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 767808dc3..7067785dc 100755 --- a/tests/gclient_test.py +++ b/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 diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index 3e24c01b1..caf04b64f 100755 --- a/tests/gclient_utils_test.py +++ b/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, diff --git a/tests/owners_unittest.py b/tests/owners_unittest.py index 93b72fe5a..aa623f544 100755 --- a/tests/owners_unittest.py +++ b/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 diff --git a/tests/patch_test.py b/tests/patch_test.py index 76778ea45..3286f6bb2 100755 --- a/tests/patch_test.py +++ b/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) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index c43bc4f8a..57660bb7d 100755 --- a/tests/presubmit_unittest.py +++ b/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('') diff --git a/tests/rietveld_test.py b/tests/rietveld_test.py index 798e84dd9..84d5d6365 100755 --- a/tests/rietveld_test.py +++ b/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) diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 7564678ec..2349beb2a 100755 --- a/tests/scm_unittest.py +++ b/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', diff --git a/tests/subprocess2_test.py b/tests/subprocess2_test.py index 9d30452c3..54e4e1512 100755 --- a/tests/subprocess2_test.py +++ b/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 diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 12157fdc8..c7330e13d 100755 --- a/tests/trychange_unittest.py +++ b/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') diff --git a/tests/watchlists_unittest.py b/tests/watchlists_unittest.py index 7721f8471..e72bb8bab 100755 --- a/tests/watchlists_unittest.py +++ b/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