Commit 66f9dbec authored by David 'Digit' Turner's avatar David 'Digit' Turner Committed by Commit Bot

build/android: Fixes to make pylint happy.

This CL contains fixes required to ensure that the presubmit
upload script does not fail after the Pylint update to 1.5.

More specifically:

- The pylintrc is modified to add wrong-order-position to
  the list of disabled warnings, since many of the scripts
  under build/android require sophisticated import logic
  due to the use of third_party/catapult/devil/

- Otherwise, scripts are locally updated by either changing
  the source code, or adding local pylint comments to disable
  a specific warning.

Without this CL, no Android-specific CL can pass the presubmit
test on Gerrit :-/

BUG=NONE
R=jbudorick@chromium.org, agrieve@chromium.org, vapier@chromium.org

Change-Id: I4812c76bf1b86918d56c230574c741c3c1328c5d
Reviewed-on: https://chromium-review.googlesource.com/1148339Reviewed-by: default avatarEgor Pasko <pasko@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577510}
parent 80daef89
...@@ -14,6 +14,7 @@ import sys ...@@ -14,6 +14,7 @@ import sys
from pylib import constants from pylib import constants
from pylib.constants import host_paths from pylib.constants import host_paths
# pylint: disable=wrong-import-order
# Uses symbol.py from third_party/android_platform, not python's. # Uses symbol.py from third_party/android_platform, not python's.
with host_paths.SysPath( with host_paths.SysPath(
host_paths.ANDROID_PLATFORM_DEVELOPMENT_SCRIPTS_PATH, host_paths.ANDROID_PLATFORM_DEVELOPMENT_SCRIPTS_PATH,
......
...@@ -104,7 +104,7 @@ class LighttpdServer(object): ...@@ -104,7 +104,7 @@ class LighttpdServer(object):
break break
self.process.close() self.process.close()
if self.fixed_port or not 'in use' in server_error: if self.fixed_port or 'in use' not in server_error:
print 'Client error:', client_error print 'Client error:', client_error
print 'Server error:', server_error print 'Server error:', server_error
return False return False
......
...@@ -152,6 +152,7 @@ class _ApkDelegate(object): ...@@ -152,6 +152,7 @@ class _ApkDelegate(object):
extras[gtest_test_instance.EXTRA_SHARD_NANO_TIMEOUT] = int( extras[gtest_test_instance.EXTRA_SHARD_NANO_TIMEOUT] = int(
kwargs['timeout'] * _SECONDS_TO_NANOS) kwargs['timeout'] * _SECONDS_TO_NANOS)
# pylint: disable=redefined-variable-type
command_line_file = _NullContextManager() command_line_file = _NullContextManager()
if flags: if flags:
if len(flags) > _MAX_INLINE_FLAGS_LENGTH: if len(flags) > _MAX_INLINE_FLAGS_LENGTH:
...@@ -169,6 +170,7 @@ class _ApkDelegate(object): ...@@ -169,6 +170,7 @@ class _ApkDelegate(object):
extras[_EXTRA_TEST_LIST] = test_list_file.name extras[_EXTRA_TEST_LIST] = test_list_file.name
else: else:
extras[_EXTRA_TEST] = test[0] extras[_EXTRA_TEST] = test[0]
# pylint: enable=redefined-variable-type
stdout_file = device_temp_file.DeviceTempFile( stdout_file = device_temp_file.DeviceTempFile(
device.adb, dir=device.GetExternalStoragePath(), suffix='.gtest_out') device.adb, dir=device.GetExternalStoragePath(), suffix='.gtest_out')
...@@ -283,11 +285,13 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun): ...@@ -283,11 +285,13 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun):
assert isinstance(test_instance, gtest_test_instance.GtestTestInstance) assert isinstance(test_instance, gtest_test_instance.GtestTestInstance)
super(LocalDeviceGtestRun, self).__init__(env, test_instance) super(LocalDeviceGtestRun, self).__init__(env, test_instance)
# pylint: disable=redefined-variable-type
if self._test_instance.apk: if self._test_instance.apk:
self._delegate = _ApkDelegate(self._test_instance, env.tool) self._delegate = _ApkDelegate(self._test_instance, env.tool)
elif self._test_instance.exe_dist_dir: elif self._test_instance.exe_dist_dir:
self._delegate = _ExeDelegate(self, self._test_instance.exe_dist_dir, self._delegate = _ExeDelegate(self, self._test_instance.exe_dist_dir,
self._env.tool) self._env.tool)
# pylint: enable=redefined-variable-type
self._crashes = set() self._crashes = set()
self._servers = collections.defaultdict(list) self._servers = collections.defaultdict(list)
......
...@@ -153,7 +153,11 @@ class LocalDeviceInstrumentationTestRun( ...@@ -153,7 +153,11 @@ class LocalDeviceInstrumentationTestRun(
self._replace_package_contextmanager = system_app.ReplaceSystemApp( self._replace_package_contextmanager = system_app.ReplaceSystemApp(
dev, self._test_instance.replace_system_package.package, dev, self._test_instance.replace_system_package.package,
self._test_instance.replace_system_package.replacement_apk) self._test_instance.replace_system_package.replacement_apk)
# Pylint is not smart enough to realize that this field has
# an __enter__ method, and will complain loudly.
# pylint: disable=no-member
self._replace_package_contextmanager.__enter__() self._replace_package_contextmanager.__enter__()
# pylint: enable=no-member
steps.append(replace_package) steps.append(replace_package)
...@@ -308,12 +312,15 @@ class LocalDeviceInstrumentationTestRun( ...@@ -308,12 +312,15 @@ class LocalDeviceInstrumentationTestRun(
valgrind_tools.SetChromeTimeoutScale(dev, None) valgrind_tools.SetChromeTimeoutScale(dev, None)
if self._replace_package_contextmanager: if self._replace_package_contextmanager:
# See pylint-related commend above with __enter__()
# pylint: disable=no-member
self._replace_package_contextmanager.__exit__(*sys.exc_info()) self._replace_package_contextmanager.__exit__(*sys.exc_info())
# pylint: enable=no-member
self._env.parallel_devices.pMap(individual_device_tear_down) self._env.parallel_devices.pMap(individual_device_tear_down)
def _CreateFlagChangerIfNeeded(self, device): def _CreateFlagChangerIfNeeded(self, device):
if not str(device) in self._flag_changers: if str(device) not in self._flag_changers:
self._flag_changers[str(device)] = flag_changer.FlagChanger( self._flag_changers[str(device)] = flag_changer.FlagChanger(
device, "test-cmdline-file") device, "test-cmdline-file")
......
...@@ -220,7 +220,7 @@ class LocalDeviceTestRun(test_run.TestRun): ...@@ -220,7 +220,7 @@ class LocalDeviceTestRun(test_run.TestRun):
if hash(self._GetUniqueTestName(t)) % total_shards == shard_index] if hash(self._GetUniqueTestName(t)) % total_shards == shard_index]
def GetTool(self, device): def GetTool(self, device):
if not str(device) in self._tools: if str(device) not in self._tools:
self._tools[str(device)] = valgrind_tools.CreateTool( self._tools[str(device)] = valgrind_tools.CreateTool(
self._env.tool, device) self._env.tool, device)
return self._tools[str(device)] return self._tools[str(device)]
......
...@@ -380,6 +380,7 @@ class JSONResultsGeneratorBase(object): ...@@ -380,6 +380,7 @@ class JSONResultsGeneratorBase(object):
urllib2.quote(self._test_type), urllib2.quote(self._test_type),
urllib2.quote(self._master_name))) urllib2.quote(self._master_name)))
# pylint: disable=redefined-variable-type
try: try:
# FIXME: We should talk to the network via a Host object. # FIXME: We should talk to the network via a Host object.
results_file = urllib2.urlopen(results_file_url) results_file = urllib2.urlopen(results_file_url)
...@@ -391,6 +392,7 @@ class JSONResultsGeneratorBase(object): ...@@ -391,6 +392,7 @@ class JSONResultsGeneratorBase(object):
error = http_error error = http_error
except urllib2.URLError, url_error: except urllib2.URLError, url_error:
error = url_error error = url_error
# pylint: enable=redefined-variable-type
if old_results: if old_results:
# Strip the prefix and suffix so we can get the actual JSON object. # Strip the prefix and suffix so we can get the actual JSON object.
......
...@@ -268,6 +268,7 @@ def create_suite_table(results_dict): ...@@ -268,6 +268,7 @@ def create_suite_table(results_dict):
def feedback_url(result_details_link): def feedback_url(result_details_link):
# pylint: disable=redefined-variable-type
url_args = [ url_args = [
('labels', 'Pri-2,Type-Bug,Restrict-View-Google'), ('labels', 'Pri-2,Type-Bug,Restrict-View-Google'),
('summary', 'Result Details Feedback:'), ('summary', 'Result Details Feedback:'),
...@@ -276,6 +277,7 @@ def feedback_url(result_details_link): ...@@ -276,6 +277,7 @@ def feedback_url(result_details_link):
if result_details_link: if result_details_link:
url_args.append(('comment', 'Please check out: %s' % result_details_link)) url_args.append(('comment', 'Please check out: %s' % result_details_link))
url_args = urllib.urlencode(url_args) url_args = urllib.urlencode(url_args)
# pylint: enable=redefined-variable-type
return 'https://bugs.chromium.org/p/chromium/issues/entry?%s' % url_args return 'https://bugs.chromium.org/p/chromium/issues/entry?%s' % url_args
...@@ -370,6 +372,7 @@ def ui_screenshot_set(json_path): ...@@ -370,6 +372,7 @@ def ui_screenshot_set(json_path):
# This will be reported as an error by result_details, no need to duplicate. # This will be reported as an error by result_details, no need to duplicate.
return None return None
ui_screenshots = [] ui_screenshots = []
# pylint: disable=too-many-nested-blocks
for testsuite_run in json_object['per_iteration_data']: for testsuite_run in json_object['per_iteration_data']:
for _, test_runs in testsuite_run.iteritems(): for _, test_runs in testsuite_run.iteritems():
for test_run in test_runs: for test_run in test_runs:
...@@ -388,6 +391,7 @@ def ui_screenshot_set(json_path): ...@@ -388,6 +391,7 @@ def ui_screenshot_set(json_path):
test_screenshots = json.loads( test_screenshots = json.loads(
screenshot_string) screenshot_string)
ui_screenshots.extend(test_screenshots) ui_screenshots.extend(test_screenshots)
# pylint: enable=too-many-nested-blocks
if ui_screenshots: if ui_screenshots:
return json.dumps(ui_screenshots) return json.dumps(ui_screenshots)
......
...@@ -79,10 +79,7 @@ def exists(name, bucket): ...@@ -79,10 +79,7 @@ def exists(name, bucket):
cmd = [_GSUTIL_PATH, '-q', 'stat', gs_path] cmd = [_GSUTIL_PATH, '-q', 'stat', gs_path]
return_code = cmd_helper.RunCmd(cmd) return_code = cmd_helper.RunCmd(cmd)
if return_code == 0: return return_code == 0
return True
else:
return False
# TODO(jbudorick): Delete this function. Only one user of it. # TODO(jbudorick): Delete this function. Only one user of it.
......
...@@ -4,7 +4,7 @@ max-line-length=80 ...@@ -4,7 +4,7 @@ max-line-length=80
[MESSAGES CONTROL] [MESSAGES CONTROL]
disable=abstract-class-not-used,bad-continuation,bad-indentation,duplicate-code,fixme,invalid-name,locally-disabled,locally-enabled,missing-docstring,star-args,too-few-public-methods,too-many-arguments,too-many-branches,too-many-instance-attributes,too-many-lines,too-many-locals,too-many-public-methods,too-many-statements, disable=abstract-class-not-used,bad-continuation,bad-indentation,duplicate-code,fixme,invalid-name,locally-disabled,locally-enabled,missing-docstring,star-args,too-few-public-methods,too-many-arguments,too-many-branches,too-many-instance-attributes,too-many-lines,too-many-locals,too-many-public-methods,too-many-statements,wrong-import-position
[REPORTS] [REPORTS]
......
...@@ -25,6 +25,7 @@ import unittest ...@@ -25,6 +25,7 @@ import unittest
# See http://crbug.com/724524 and https://bugs.python.org/issue7980. # See http://crbug.com/724524 and https://bugs.python.org/issue7980.
import _strptime # pylint: disable=unused-import import _strptime # pylint: disable=unused-import
# pylint: disable=ungrouped-imports
from pylib.constants import host_paths from pylib.constants import host_paths
if host_paths.DEVIL_PATH not in sys.path: if host_paths.DEVIL_PATH not in sys.path:
...@@ -49,7 +50,6 @@ from pylib.utils import logging_utils ...@@ -49,7 +50,6 @@ from pylib.utils import logging_utils
from py_utils import contextlib_ext from py_utils import contextlib_ext
_DEVIL_STATIC_CONFIG_FILE = os.path.abspath(os.path.join( _DEVIL_STATIC_CONFIG_FILE = os.path.abspath(os.path.join(
host_paths.DIR_SOURCE_ROOT, 'build', 'android', 'devil_config.json')) host_paths.DIR_SOURCE_ROOT, 'build', 'android', 'devil_config.json'))
...@@ -212,10 +212,12 @@ def AddCommonOptions(parser): ...@@ -212,10 +212,12 @@ def AddCommonOptions(parser):
def ProcessCommonOptions(args): def ProcessCommonOptions(args):
"""Processes and handles all common options.""" """Processes and handles all common options."""
run_tests_helper.SetLogLevel(args.verbose_count, add_handler=False) run_tests_helper.SetLogLevel(args.verbose_count, add_handler=False)
# pylint: disable=redefined-variable-type
if args.verbose_count > 0: if args.verbose_count > 0:
handler = logging_utils.ColorStreamHandler() handler = logging_utils.ColorStreamHandler()
else: else:
handler = logging.StreamHandler(sys.stdout) handler = logging.StreamHandler(sys.stdout)
# pylint: enable=redefined-variable-type
handler.setFormatter(run_tests_helper.CustomFormatter()) handler.setFormatter(run_tests_helper.CustomFormatter())
logging.getLogger().addHandler(handler) logging.getLogger().addHandler(handler)
...@@ -429,6 +431,7 @@ def AddInstrumentationTestOptions(parser): ...@@ -429,6 +431,7 @@ def AddInstrumentationTestOptions(parser):
split_arg = arg.split(',') split_arg = arg.split(',')
if len(split_arg) != 2: if len(split_arg) != 2:
raise argparse.ArgumentError( raise argparse.ArgumentError(
arg,
'Expected two comma-separated strings for --replace-system-package, ' 'Expected two comma-separated strings for --replace-system-package, '
'received %d' % len(split_arg)) 'received %d' % len(split_arg))
PackageReplacement = collections.namedtuple('PackageReplacement', PackageReplacement = collections.namedtuple('PackageReplacement',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment