From 066bee6615af607286c7426abddb1902887e0ff3 Mon Sep 17 00:00:00 2001 From: qyearsley Date: Thu, 14 Jan 2016 09:58:33 -0800 Subject: [PATCH] Enable whitespace pylint checks. This CL makes whitespace formatting more consistent within the catapult repo. This means: * 2-space indent (Chromium style) * 4-space indent for line continuations * 2 spaces before one-line comments * 1 space around binary operators (suggested by PEP8, not a strict rule) * 2 blank lines around top-level functions and classes * 1 blank line around methods in a class Some of the changes were done automatically using: Run autopep8 --select=E2,W2,E3 --indent-size=2 --max-line-length=80 Others were done manually. BUG=catapult:#1888 Review URL: https://codereview.chromium.org/1589553002 --- catapult_base/catapult_base/__init__.py | 1 + catapult_base/catapult_base/cloud_storage.py | 9 +- .../catapult_base/cloud_storage_unittest.py | 20 +-- .../catapult_base/perfbot_stats/__init__.py | 1 - .../perfbot_stats/chrome_perf_stats.py | 8 +- .../chrome_perf_stats_unittest.py | 51 +++---- .../perfbot_stats/chrome_perf_step_timings.py | 64 ++++----- .../catapult_base/refactor/module.py | 1 + .../catapult_base/refactor/offset_token.py | 6 +- .../catapult_base/refactor/snippet.py | 4 +- .../catapult_base/refactor_util/move.py | 1 + catapult_base/catapult_base/util_unittest.py | 1 + catapult_build/dev_server.py | 6 +- catapult_build/js_checks.py | 2 +- catapult_build/run_dev_server_tests.py | 4 +- devil/devil/android/apk_helper.py | 1 + devil/devil/android/app_ui.py | 1 + devil/devil/android/app_ui_test.py | 3 +- devil/devil/android/battery_utils.py | 3 +- devil/devil/android/battery_utils_test.py | 54 +++---- devil/devil/android/decorators.py | 2 + devil/devil/android/decorators_test.py | 8 +- devil/devil/android/device_signal.py | 62 ++++---- devil/devil/android/device_temp_file.py | 1 + devil/devil/android/device_utils.py | 28 ++-- .../devil/android/device_utils_devicetest.py | 1 + devil/devil/android/device_utils_test.py | 18 ++- devil/devil/android/fastboot_utils.py | 1 + devil/devil/android/fastboot_utils_test.py | 4 +- devil/devil/android/forwarder.py | 1 + devil/devil/android/logcat_monitor.py | 1 + devil/devil/android/logcat_monitor_test.py | 38 ++--- devil/devil/android/md5sum_test.py | 4 +- .../android/perf/perf_control_devicetest.py | 2 + devil/devil/android/ports.py | 4 +- devil/devil/android/sdk/adb_wrapper.py | 19 ++- .../android/sdk/adb_wrapper_devicetest.py | 2 +- devil/devil/android/sdk/build_tools.py | 2 +- devil/devil/android/sdk/fastboot.py | 1 + devil/devil/android/sdk/gce_adb_wrapper.py | 9 +- devil/devil/android/sdk/intent.py | 1 + devil/devil/android/sdk/shared_prefs.py | 3 +- devil/devil/android/sdk/shared_prefs_test.py | 32 ++--- devil/devil/android/sdk/split_select.py | 2 + .../devil/android/tools/adb_run_shell_cmd.py | 2 +- devil/devil/android/tools/flash_device.py | 2 +- .../devil/android/tools/script_common_test.py | 2 +- devil/devil/devil_env.py | 3 +- devil/devil/devil_env_test.py | 3 +- devil/devil/utils/cmd_helper.py | 3 + devil/devil/utils/cmd_helper_test.py | 1 + devil/devil/utils/geometry_test.py | 3 + devil/devil/utils/mock_calls.py | 5 +- devil/devil/utils/mock_calls_test.py | 6 +- devil/devil/utils/parallelizer.py | 8 +- devil/devil/utils/parallelizer_test.py | 2 +- devil/devil/utils/reraiser_thread.py | 4 +- devil/devil/utils/reraiser_thread_unittest.py | 6 + devil/devil/utils/run_tests_helper.py | 4 +- devil/devil/utils/timeout_retry.py | 2 +- devil/devil/utils/timeout_retry_unittest.py | 2 + perf_insights/perf_insights/__init__.py | 2 +- perf_insights/perf_insights/cloud_config.py | 5 +- perf_insights/perf_insights/cloud_storage.py | 3 + perf_insights/perf_insights/corpus_driver.py | 2 + perf_insights/perf_insights/corpus_query.py | 17 ++- .../perf_insights/corpus_query_unittest.py | 3 + .../endpoints/cloud_mapper/cloud_helper.py | 5 + .../endpoints/cloud_mapper/create.py | 1 + .../endpoints/cloud_mapper/task.py | 6 +- .../endpoints/cloud_mapper/test.py | 4 +- .../endpoints/cloud_mapper/worker.py | 4 +- .../perf_insights/endpoints/corpus_cleanup.py | 5 +- .../perf_insights/function_handle_unittest.py | 3 +- .../perf_insights/gcs_trace_handle.py | 1 + .../perf_insights/in_memory_trace_handle.py | 1 + .../local_directory_corpus_driver.py | 5 + .../local_directory_corpus_driver_unittest.py | 2 + .../perf_insights/local_file_trace_handle.py | 1 + perf_insights/perf_insights/map_runner.py | 4 + .../perf_insights/map_single_trace.py | 12 +- .../map_single_trace_unittest.py | 11 +- .../perf_insights/map_traces_handler.py | 5 +- perf_insights/perf_insights/mre/failure.py | 2 +- .../perf_insights/mre/job_results.py | 4 +- .../perf_insights/mre/threaded_work_queue.py | 10 +- .../mre/threaded_work_queue_unittest.py | 3 + .../perf_insights_corpus_driver.py | 2 + .../perf_insights/progress_reporter.py | 5 +- .../perf_insights/results/__init__.py | 2 + .../results/gtest_progress_reporter.py | 1 + .../results/json_output_formatter.py | 3 +- .../perf_insights/results/output_formatter.py | 5 +- perf_insights/perf_insights/trace_handle.py | 2 + perf_insights/perf_insights/value/__init__.py | 10 +- perf_insights/perf_insights/value/run_info.py | 1 + .../perf_insights/value/value_unittest.py | 5 +- .../perf_insights_dev_server_config.py | 4 +- .../perf_insights_build/run_vinn_tests.py | 6 +- pylintrc | 3 - systrace/systrace/agents/atrace_agent.py | 42 +++--- .../systrace/agents/atrace_agent_unittest.py | 6 +- systrace/systrace/agents/ftrace_agent.py | 132 ++++++++++-------- .../systrace/agents/ftrace_agent_unittest.py | 23 +-- systrace/systrace/systrace-legacy.py | 27 ++-- systrace/systrace/systrace_agent.py | 1 + .../systrace/update_systrace_trace_viewer.py | 4 +- systrace/systrace/util.py | 3 +- systrace/systrace/util_unittest.py | 1 + tracing/tracing_build/check_common.py | 14 +- tracing/tracing_build/run_vinn_tests.py | 3 +- .../tracing_dev_server_config.py | 3 + .../vulcanize_trace_viewer_unittest.py | 1 + 113 files changed, 586 insertions(+), 389 deletions(-) diff --git a/catapult_base/catapult_base/__init__.py b/catapult_base/catapult_base/__init__.py index 653dd32922a..0ccc25281e2 100644 --- a/catapult_base/catapult_base/__init__.py +++ b/catapult_base/catapult_base/__init__.py @@ -10,6 +10,7 @@ from catapult_base import util + def _AddDirToPythonPath(*path_parts): path = os.path.abspath(os.path.join(*path_parts)) if os.path.isdir(path) and path not in sys.path: diff --git a/catapult_base/catapult_base/cloud_storage.py b/catapult_base/catapult_base/cloud_storage.py index fe8a2cb64f4..91d8b6deeac 100644 --- a/catapult_base/catapult_base/cloud_storage.py +++ b/catapult_base/catapult_base/cloud_storage.py @@ -49,8 +49,8 @@ _CROS_GSUTIL_HOME_WAR = '/home/chromeos-test/' - class CloudStorageError(Exception): + @staticmethod def _GetConfigInstructions(): command = _GSUTIL_PATH @@ -63,6 +63,7 @@ def _GetConfigInstructions(): class PermissionError(CloudStorageError): + def __init__(self): super(PermissionError, self).__init__( 'Attempted to access a file from Cloud Storage but you don\'t ' @@ -70,6 +71,7 @@ def __init__(self): class CredentialsError(CloudStorageError): + def __init__(self): super(CredentialsError, self).__init__( 'Attempted to access a file from Cloud Storage but you have no ' @@ -93,12 +95,14 @@ def _FindExecutableInPath(relative_executable_path, *extra_search_paths): return executable_path return None + def _EnsureExecutable(gsutil): """chmod +x if gsutil is not executable.""" st = os.stat(gsutil) if not st.st_mode & stat.S_IEXEC: os.chmod(gsutil, st.st_mode | stat.S_IEXEC) + def _RunCommand(args): # On cros device, as telemetry is running as root, home will be set to /root/, # which is not writable. gsutil will attempt to create a download tracker dir @@ -330,13 +334,14 @@ def GetFilesInDirectoryIfChanged(directory, bucket): continue GetIfChanged(path_name, bucket) + def CalculateHash(file_path): """Calculates and returns the hash of the file at file_path.""" sha1 = hashlib.sha1() with open(file_path, 'rb') as f: while True: # Read in 1mb chunks, so it doesn't all have to be loaded into memory. - chunk = f.read(1024*1024) + chunk = f.read(1024 * 1024) if not chunk: break sha1.update(chunk) diff --git a/catapult_base/catapult_base/cloud_storage_unittest.py b/catapult_base/catapult_base/cloud_storage_unittest.py index de583914d8d..b476fab2ad3 100644 --- a/catapult_base/catapult_base/cloud_storage_unittest.py +++ b/catapult_base/catapult_base/cloud_storage_unittest.py @@ -18,9 +18,11 @@ def _FakeReadHash(_): return 'hashthis!' + def _FakeCalulateHashMatchesRead(_): return 'hashthis!' + def _FakeCalulateHashNewHash(_): return 'omgnewhash' @@ -56,7 +58,7 @@ def _assertRunCommandRaisesError(self, communicate_strs, error): def testRunCommandCredentialsError(self): strs = ['You are attempting to access protected data with no configured', - 'Failure: No handler was ready to authenticate.'] + 'Failure: No handler was ready to authenticate.'] self._assertRunCommandRaisesError(strs, cloud_storage.CredentialsError) def testRunCommandPermissionError(self): @@ -95,8 +97,8 @@ def testExistsReturnsFalse(self, subprocess_mock): p_mock = mock.Mock() subprocess_mock.Popen.return_value = p_mock p_mock.communicate.return_value = ( - '', - 'CommandException: One or more URLs matched no objects.\n') + '', + 'CommandException: One or more URLs matched no objects.\n') p_mock.returncode_result = 1 self.assertFalse(cloud_storage.Exists('fake bucket', 'fake remote path')) @@ -172,11 +174,12 @@ def testGetIfChanged(self, lock_mock): 'https://github.com/catapult-project/catapult/issues/1861') def testGetFilesInDirectoryIfChanged(self): self.CreateFiles([ - 'real_dir_path/dir1/1file1.sha1', - 'real_dir_path/dir1/1file2.txt', - 'real_dir_path/dir1/1file3.sha1', - 'real_dir_path/dir2/2file.txt', - 'real_dir_path/dir3/3file1.sha1']) + 'real_dir_path/dir1/1file1.sha1', + 'real_dir_path/dir1/1file2.txt', + 'real_dir_path/dir1/1file3.sha1', + 'real_dir_path/dir2/2file.txt', + 'real_dir_path/dir3/3file1.sha1']) + def IncrementFilesUpdated(*_): IncrementFilesUpdated.files_updated += 1 IncrementFilesUpdated.files_updated = 0 @@ -197,6 +200,7 @@ def IncrementFilesUpdated(*_): def testCopy(self): orig_run_command = cloud_storage._RunCommand + def AssertCorrectRunCommandArgs(args): self.assertEqual(expected_args, args) cloud_storage._RunCommand = AssertCorrectRunCommandArgs diff --git a/catapult_base/catapult_base/perfbot_stats/__init__.py b/catapult_base/catapult_base/perfbot_stats/__init__.py index 1aaf0e179ff..50b23dff631 100644 --- a/catapult_base/catapult_base/perfbot_stats/__init__.py +++ b/catapult_base/catapult_base/perfbot_stats/__init__.py @@ -1,4 +1,3 @@ # 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. - diff --git a/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats.py b/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats.py index aee0b1b6a86..e98fddfd37d 100755 --- a/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats.py +++ b/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats.py @@ -51,7 +51,7 @@ def main(): days = range(1, calendar.monthrange(year, month)[1] + 1) else: day = int(sys.argv[3]) - if day > 31 or day <=0: + if day > 31 or day <= 0: print USAGE sys.exit(0) days = [day] @@ -84,6 +84,7 @@ def _UpdateSuccessRatesWithResult( success_rates[date_dict_str][builder]['count'] += count success_rates[date_dict_str][builder]['success_count'] += success_count + def _SummarizeSuccessRates(success_rates): overall_success_rates = [] for day, results in success_rates.iteritems(): @@ -102,8 +103,9 @@ def _SummarizeSuccessRates(success_rates): def UploadToPerfDashboard(success_rates): for success_rate in success_rates: - date_str = ('%s-%s-%s' % - (success_rate[0][0:4], success_rate[0][4:6], success_rate[0][6:8])) + date_str = '%s-%s-%s' % (success_rate[0][0:4], + success_rate[0][4:6], + success_rate[0][6:8]) dashboard_data = { 'master': 'WaterfallStats', 'bot': 'ChromiumPerf', diff --git a/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats_unittest.py b/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats_unittest.py index 78e57544c85..eeb078a3cc2 100644 --- a/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats_unittest.py +++ b/catapult_base/catapult_base/perfbot_stats/chrome_perf_stats_unittest.py @@ -19,49 +19,50 @@ def testUpdateSuccessRatesWithResult(self): 'invalid_builder') self.assertDictEqual({}, success_rates) chrome_perf_stats._UpdateSuccessRatesWithResult( - success_rates, - {'count': 5, 'failure_count': 3}, - '20151010', - 'android_nexus_10') + success_rates, + {'count': 5, 'failure_count': 3}, + '20151010', + 'android_nexus_10') self.assertDictEqual( {'20151010': {'android_nexus_10': {'count': 5, 'success_count': 2} - } - }, + } + }, success_rates) chrome_perf_stats._UpdateSuccessRatesWithResult( - success_rates, - {'count': 5, 'failure_count': 4}, - '20151010', - 'android_nexus_4') + success_rates, + {'count': 5, 'failure_count': 4}, + '20151010', + 'android_nexus_4') self.assertDictEqual( {'20151010': {'android_nexus_10': {'count': 5, 'success_count': 2}, 'android_nexus_4': {'count': 5, 'success_count': 1} - } - }, + } + }, success_rates) chrome_perf_stats._UpdateSuccessRatesWithResult( - success_rates, - {'count': 5, 'failure_count': 0}, - '20151009', - 'win_xp') + success_rates, + {'count': 5, 'failure_count': 0}, + '20151009', + 'win_xp') self.assertDictEqual( {'20151010': {'android_nexus_10': {'count': 5, 'success_count': 2}, 'android_nexus_4': {'count': 5, 'success_count': 1} - }, - '20151009': + }, + '20151009': {'win_xp': {'count': 5, 'success_count': 5} - } - }, + } + }, success_rates) + def testSummarizeSuccessRates(self): rates = chrome_perf_stats._SummarizeSuccessRates( {'20151010': @@ -69,14 +70,14 @@ def testSummarizeSuccessRates(self): {'count': 5, 'success_count': 2}, 'android_nexus_4': {'count': 5, 'success_count': 3} - }, - '20151009': + }, + '20151009': {'win_xp': {'count': 5, 'success_count': 5} - } - }) + } + }) self.assertListEqual([['20151010', 0.5], ['20151009', 1.0]], rates) if __name__ == '__main__': - unittest.main() + unittest.main() diff --git a/catapult_base/catapult_base/perfbot_stats/chrome_perf_step_timings.py b/catapult_base/catapult_base/perfbot_stats/chrome_perf_step_timings.py index 2b017c4c679..ebdb94de1cc 100755 --- a/catapult_base/catapult_base/perfbot_stats/chrome_perf_step_timings.py +++ b/catapult_base/catapult_base/perfbot_stats/chrome_perf_step_timings.py @@ -32,36 +32,36 @@ 'stats/last/chromium.perf/%s/%s/20') IGNORED_STEPS = [ - 'List Perf Tests', - 'Sharded Perf Tests', - 'authorize_adb_devices', - 'bot_update', - 'build__schedule__time__', - 'clean local files', - 'cleanup_temp', - 'device_status_check', - 'extract build', - 'gclient runhooks', - 'get compile targets for scripts', - 'get perf test list', - 'gsutil download_build_product', - 'host_info', - 'install ChromeShell.apk', - 'json.output cache', - 'json.output cache', - 'overall__build__result__', - 'overall__queued__time__', - 'provision_devices', - 'read test spec', - 'rmtree build directory', - 'setup_build', - 'spawn_logcat_monitor', - 'stack_tool_for_tombstones', - 'stack_tool_with_logcat_dump', - 'steps', - 'test_report', - 'unzip_build_product', - 'update_scripts' + 'List Perf Tests', + 'Sharded Perf Tests', + 'authorize_adb_devices', + 'bot_update', + 'build__schedule__time__', + 'clean local files', + 'cleanup_temp', + 'device_status_check', + 'extract build', + 'gclient runhooks', + 'get compile targets for scripts', + 'get perf test list', + 'gsutil download_build_product', + 'host_info', + 'install ChromeShell.apk', + 'json.output cache', + 'json.output cache', + 'overall__build__result__', + 'overall__queued__time__', + 'provision_devices', + 'read test spec', + 'rmtree build directory', + 'setup_build', + 'spawn_logcat_monitor', + 'stack_tool_for_tombstones', + 'stack_tool_with_logcat_dump', + 'steps', + 'test_report', + 'unzip_build_product', + 'update_scripts' ] KNOWN_TESTERS_LIST = [ @@ -117,7 +117,7 @@ threshold_time = datetime.now() - timedelta(days=2) col_names = [('builder', 'step', 'run_count', 'stddev', 'mean', 'maximum', - 'median', 'seventyfive', 'ninety', 'ninetynine')] + 'median', 'seventyfive', 'ninety', 'ninetynine')] with open(outfilename, 'wb') as f: writer = csv.writer(f) writer.writerows(col_names) @@ -128,7 +128,7 @@ response = urllib2.urlopen(url) results = json.load(response) steps = results['steps'] - steps.sort() # to group tests and their references together. + steps.sort() # to group tests and their references together. for step in steps: if step in IGNORED_STEPS: continue diff --git a/catapult_base/catapult_base/refactor/module.py b/catapult_base/catapult_base/refactor/module.py index d8b6fb86ae5..00d74668dd6 100644 --- a/catapult_base/catapult_base/refactor/module.py +++ b/catapult_base/catapult_base/refactor/module.py @@ -6,6 +6,7 @@ class Module(object): + def __init__(self, file_path): self._file_path = file_path diff --git a/catapult_base/catapult_base/refactor/offset_token.py b/catapult_base/catapult_base/refactor/offset_token.py index 2578f854864..5fa953e93fb 100644 --- a/catapult_base/catapult_base/refactor/offset_token.py +++ b/catapult_base/catapult_base/refactor/offset_token.py @@ -22,6 +22,7 @@ class OffsetToken(object): representing the content, and an offset. Using relative positions makes it easy to insert and remove tokens. """ + def __init__(self, token_type, string, offset): self._type = token_type self._string = string @@ -72,9 +73,10 @@ def Tokenize(f): else: erow, ecol = prev_token[3] if erow == srow: - offset_tokens.append(OffsetToken(token_type, string, (0, scol-ecol))) + offset_tokens.append(OffsetToken(token_type, string, (0, scol - ecol))) else: - offset_tokens.append(OffsetToken(token_type, string, (srow-erow, scol))) + offset_tokens.append(OffsetToken( + token_type, string, (srow - erow, scol))) return offset_tokens diff --git a/catapult_base/catapult_base/refactor/snippet.py b/catapult_base/catapult_base/refactor/snippet.py index ecb688af900..2277261c76d 100644 --- a/catapult_base/catapult_base/refactor/snippet.py +++ b/catapult_base/catapult_base/refactor/snippet.py @@ -93,6 +93,7 @@ class TokenSnippet(Snippet): A list of tokens may start with any number of comments and non-terminating newlines, but must end with a syntactically meaningful token. """ + def __init__(self, token_type, tokens): # For operators and delimiters, the TokenSnippet's type may be more specific # than the type of the constituent token. E.g. the TokenSnippet type is @@ -152,6 +153,7 @@ class Symbol(Snippet): """A Snippet containing sub-Snippets. The possible types and type_names are defined in Python's symbol module.""" + def __init__(self, symbol_type, children): self._type = symbol_type self._children = children @@ -194,7 +196,7 @@ def PrintTree(self, indent=0, stream=sys.stdout): print >> stream, node.type_name for child in node.children: - child.PrintTree(indent+2, stream) + child.PrintTree(indent + 2, stream) def Snippetize(f): diff --git a/catapult_base/catapult_base/refactor_util/move.py b/catapult_base/catapult_base/refactor_util/move.py index 108413b2864..9493e3b1ea4 100644 --- a/catapult_base/catapult_base/refactor_util/move.py +++ b/catapult_base/catapult_base/refactor_util/move.py @@ -39,6 +39,7 @@ def _Update(moves, module): class _Move(object): + def __init__(self, source, target): self._source_path = os.path.realpath(source) self._target_path = os.path.realpath(target) diff --git a/catapult_base/catapult_base/util_unittest.py b/catapult_base/catapult_base/util_unittest.py index c66fb021f5c..230e546dd43 100644 --- a/catapult_base/catapult_base/util_unittest.py +++ b/catapult_base/catapult_base/util_unittest.py @@ -10,6 +10,7 @@ @unittest.skipIf(sys.platform.startswith('win'), 'crbug.com/570512') class PathTest(unittest.TestCase): + def GetFileInTestDir(self, file_name): return os.path.join(os.path.dirname(__file__), 'test_data', file_name) diff --git a/catapult_build/dev_server.py b/catapult_build/dev_server.py index a27382764c3..b596195ba85 100644 --- a/catapult_build/dev_server.py +++ b/catapult_build/dev_server.py @@ -75,7 +75,7 @@ class TestsCompletedHandler(webapp2.RequestHandler): def post(self, *args, **kwargs): # pylint: disable=unused-argument msg = self.request.body sys.stdout.write(msg + '\n') - exit_code=(0 if 'ALL_PASSED' in msg else 1) + exit_code = 0 if 'ALL_PASSED' in msg else 1 if hasattr(self.app.server, 'please_exit'): self.app.server.please_exit(exit_code) return self.response.write('') @@ -272,8 +272,8 @@ def serve_forever(): try: real_serve_forever() except KeyboardInterrupt: - # allow CTRL+C to shutdown - return 255 + # allow CTRL+C to shutdown + return 255 if len(exitCodeAttempt) == 1: return exitCodeAttempt[0] diff --git a/catapult_build/js_checks.py b/catapult_build/js_checks.py index 000dea3ccf6..2c8fe3005ee 100644 --- a/catapult_build/js_checks.py +++ b/catapult_build/js_checks.py @@ -191,7 +191,7 @@ def CheckStrictMode(contents, is_html_file=False): statements_to_check.append(_FirstStatement(contents)) error_lines = [] for s in statements_to_check: - if s != "'use strict'": + if s != "'use strict'": error_lines.append('Expected "\'use strict\'" as first statement, ' 'but found "%s" instead.' % s) return error_lines diff --git a/catapult_build/run_dev_server_tests.py b/catapult_build/run_dev_server_tests.py index 0b67bd17d62..ef3af064868 100644 --- a/catapult_build/run_dev_server_tests.py +++ b/catapult_build/run_dev_server_tests.py @@ -107,7 +107,7 @@ def FindDepotTools(): # Check if depot_tools is in the path for path in os.environ['PATH'].split(os.pathsep): if IsDepotToolsPath(path): - return path.rstrip(os.sep) + return path.rstrip(os.sep) return None @@ -227,7 +227,7 @@ def Main(argv): port = '0' server_command = [server_path, '--no-install-hooks', '--port', port] if sys.platform.startswith('win'): - server_command = ['python.exe'] + server_command + server_command = ['python.exe'] + server_command print "Starting dev_server..." server_process = subprocess.Popen( server_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, diff --git a/devil/devil/android/apk_helper.py b/devil/devil/android/apk_helper.py index 81c49cf7596..61eeda06c4c 100644 --- a/devil/devil/android/apk_helper.py +++ b/devil/devil/android/apk_helper.py @@ -70,6 +70,7 @@ def _ParseManifestFromApk(apk_path): class ApkHelper(object): + def __init__(self, path): self._apk_path = path self._manifest = None diff --git a/devil/devil/android/app_ui.py b/devil/devil/android/app_ui.py index 20a0bf34b45..d5025f40520 100644 --- a/devil/devil/android/app_ui.py +++ b/devil/devil/android/app_ui.py @@ -23,6 +23,7 @@ class _UiNode(object): + def __init__(self, device, xml_node, package=None): """Object to interact with a UI node from an xml snapshot. diff --git a/devil/devil/android/app_ui_test.py b/devil/devil/android/app_ui_test.py index dac61227609..34729851185 100644 --- a/devil/devil/android/app_ui_test.py +++ b/devil/devil/android/app_ui_test.py @@ -13,7 +13,7 @@ from devil.utils import geometry with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error MOCK_XML_LOADING = ''' @@ -56,6 +56,7 @@ class UiAppTest(unittest.TestCase): + def setUp(self): self.device = mock.Mock() self.device.pixel_density = 320 # Each dp pixel is 2 real pixels. diff --git a/devil/devil/android/battery_utils.py b/devil/devil/android/battery_utils.py index 10dc49a7b61..ff0bd708ab1 100644 --- a/devil/devil/android/battery_utils.py +++ b/devil/devil/android/battery_utils.py @@ -176,7 +176,7 @@ def GetFuelGaugeChargeCounter(self, timeout=None, retries=None): device_errors.CommandFailedError: If fuel gauge chip not found. """ if self.SupportsFuelGauge(): - return int(self._device.ReadFile( + return int(self._device.ReadFile( self._cache['profile']['charge_counter'])) raise device_errors.CommandFailedError( 'Unable to find fuel gauge.') @@ -423,6 +423,7 @@ def _DischargeDevice(self, percent, wait_period=120): return self._HardwareSetCharging(False) + def device_discharged(): self._HardwareSetCharging(True) current_level = int(self.GetBatteryInfo().get('level')) diff --git a/devil/devil/android/battery_utils_test.py b/devil/devil/android/battery_utils_test.py index c34a90e05be..0f9e5016e88 100755 --- a/devil/devil/android/battery_utils_test.py +++ b/devil/devil/android/battery_utils_test.py @@ -20,7 +20,7 @@ from devil.utils import mock_calls with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error _DUMPSYS_OUTPUT = [ '9,0,i,uid,1000,test_package1', @@ -101,7 +101,9 @@ def testInitWithMissing_fails(self): with self.assertRaises(TypeError): battery_utils.BatteryUtils('') + class BatteryUtilsSetChargingTest(BatteryUtilsTest): + @mock.patch('time.sleep', mock.Mock()) def testHardwareSetCharging_enabled(self): self.battery._cache['profile'] = self._NEXUS_5 @@ -197,31 +199,31 @@ def testGetPowerData(self): self.assertEqual(data, check) def testGetPowerData_packageCollisionSame(self): - self.battery._cache['uids'] = {'test_package1': '1000'} - with self.assertCall( - self.call.device.RunShellCommand( - ['dumpsys', 'batterystats', '-c'], - check_return=True, large_output=True), - _DUMPSYS_OUTPUT): - data = self.battery.GetPowerData() - check = { - 'system_total': 2000.0, - 'per_package': { - 'test_package1': {'uid': '1000', 'data': [1.0]}, - 'test_package2': {'uid': '1001', 'data': [2.0]} - } + self.battery._cache['uids'] = {'test_package1': '1000'} + with self.assertCall( + self.call.device.RunShellCommand( + ['dumpsys', 'batterystats', '-c'], + check_return=True, large_output=True), + _DUMPSYS_OUTPUT): + data = self.battery.GetPowerData() + check = { + 'system_total': 2000.0, + 'per_package': { + 'test_package1': {'uid': '1000', 'data': [1.0]}, + 'test_package2': {'uid': '1001', 'data': [2.0]} } - self.assertEqual(data, check) + } + self.assertEqual(data, check) def testGetPowerData_packageCollisionDifferent(self): - self.battery._cache['uids'] = {'test_package1': '1'} - with self.assertCall( - self.call.device.RunShellCommand( - ['dumpsys', 'batterystats', '-c'], - check_return=True, large_output=True), - _DUMPSYS_OUTPUT): - with self.assertRaises(device_errors.CommandFailedError): - self.battery.GetPowerData() + self.battery._cache['uids'] = {'test_package1': '1'} + with self.assertCall( + self.call.device.RunShellCommand( + ['dumpsys', 'batterystats', '-c'], + check_return=True, large_output=True), + _DUMPSYS_OUTPUT): + with self.assertRaises(device_errors.CommandFailedError): + self.battery.GetPowerData() def testGetPowerData_cacheCleared(self): with self.assertCalls( @@ -301,11 +303,11 @@ def testDischargeDevice_percentageOutOfBounds(self): with self.assertCalls( (self.call.battery.GetBatteryInfo(), {'level': '100'})): with self.assertRaises(ValueError): - self.battery._DischargeDevice(100) + self.battery._DischargeDevice(100) with self.assertCalls( (self.call.battery.GetBatteryInfo(), {'level': '100'})): with self.assertRaises(ValueError): - self.battery._DischargeDevice(0) + self.battery._DischargeDevice(0) class BatteryUtilsGetBatteryInfoTest(BatteryUtilsTest): @@ -478,7 +480,7 @@ class BatteryUtilsGetFuelGaugeChargeCounterTest(BatteryUtilsTest): def testGetFuelGaugeChargeCounter_noFuelGauge(self): self.battery._cache['profile'] = self._NEXUS_5 with self.assertRaises(device_errors.CommandFailedError): - self.battery.GetFuelGaugeChargeCounter() + self.battery.GetFuelGaugeChargeCounter() def testGetFuelGaugeChargeCounter_fuelGaugePresent(self): self.battery._cache['profile'] = self._NEXUS_6 diff --git a/devil/devil/android/decorators.py b/devil/devil/android/decorators.py index 004ac8bcb95..3844b49a1ef 100644 --- a/devil/devil/android/decorators.py +++ b/devil/devil/android/decorators.py @@ -41,6 +41,7 @@ def timeout_retry_wrapper(*args, **kwargs): if pass_values: kwargs['timeout'] = timeout kwargs['retries'] = retries + @functools.wraps(f) def impl(): return f(*args, **kwargs) @@ -167,6 +168,7 @@ def get_timeout(inst, *_args, **kwargs): if min_default_timeout is not None: ret = max(min_default_timeout, ret) return kwargs.get('timeout', ret) + def get_retries(inst, *_args, **kwargs): return kwargs.get('retries', getattr(inst, default_retries_name)) return _TimeoutRetryWrapper(f, get_timeout, get_retries, pass_values=True) diff --git a/devil/devil/android/decorators_test.py b/devil/devil/android/decorators_test.py index a00b128d966..f60953e1f2e 100644 --- a/devil/devil/android/decorators_test.py +++ b/devil/devil/android/decorators_test.py @@ -19,12 +19,14 @@ _DEFAULT_TIMEOUT = 30 _DEFAULT_RETRIES = 3 + class DecoratorsTest(unittest.TestCase): _decorated_function_called_count = 0 def testFunctionDecoratorDoesTimeouts(self): """Tests that the base decorator handles the timeout logic.""" DecoratorsTest._decorated_function_called_count = 0 + @decorators.WithTimeoutAndRetries def alwaysTimesOut(timeout=None, retries=None): DecoratorsTest._decorated_function_called_count += 1 @@ -40,6 +42,7 @@ def alwaysTimesOut(timeout=None, retries=None): def testFunctionDecoratorDoesRetries(self): """Tests that the base decorator handles the retries logic.""" DecoratorsTest._decorated_function_called_count = 0 + @decorators.WithTimeoutAndRetries def alwaysRaisesCommandFailedError(timeout=None, retries=None): DecoratorsTest._decorated_function_called_count += 1 @@ -115,6 +118,7 @@ def alwaysRaisesNoAdbError(timeout=None, retries=None): def testDefaultsFunctionDecoratorDoesTimeouts(self): """Tests that the defaults decorator handles timeout logic.""" DecoratorsTest._decorated_function_called_count = 0 + @decorators.WithTimeoutAndRetriesDefaults(1, 0) def alwaysTimesOut(timeout=None, retries=None): DecoratorsTest._decorated_function_called_count += 1 @@ -137,6 +141,7 @@ def alwaysTimesOut(timeout=None, retries=None): def testDefaultsFunctionDecoratorDoesRetries(self): """Tests that the defaults decorator handles retries logic.""" DecoratorsTest._decorated_function_called_count = 0 + @decorators.WithTimeoutAndRetriesDefaults(30, 10) def alwaysRaisesCommandFailedError(timeout=None, retries=None): DecoratorsTest._decorated_function_called_count += 1 @@ -182,6 +187,7 @@ def alwaysRaisesProvidedException(exception, timeout=None, retries=None): def testExplicitFunctionDecoratorDoesTimeouts(self): """Tests that the explicit decorator handles timeout logic.""" DecoratorsTest._decorated_function_called_count = 0 + @decorators.WithExplicitTimeoutAndRetries(1, 0) def alwaysTimesOut(): DecoratorsTest._decorated_function_called_count += 1 @@ -197,6 +203,7 @@ def alwaysTimesOut(): def testExplicitFunctionDecoratorDoesRetries(self): """Tests that the explicit decorator handles retries logic.""" DecoratorsTest._decorated_function_called_count = 0 + @decorators.WithExplicitTimeoutAndRetries(30, 10) def alwaysRaisesCommandFailedError(): DecoratorsTest._decorated_function_called_count += 1 @@ -270,7 +277,6 @@ def alwaysRaisesProvidedException(self, exception, timeout=None, # pylint: enable=no-self-use - def testMethodDecoratorDoesTimeout(self): """Tests that the method decorator handles timeout logic.""" test_obj = self._MethodDecoratorTestObject(self) diff --git a/devil/devil/android/device_signal.py b/devil/devil/android/device_signal.py index 6a5b70999f8..2cec46d7d28 100644 --- a/devil/devil/android/device_signal.py +++ b/devil/devil/android/device_signal.py @@ -8,34 +8,34 @@ """ -SIGHUP = 1 # Hangup -SIGINT = 2 # Interrupt -SIGQUIT = 3 # Quit -SIGILL = 4 # Illegal instruction -SIGTRAP = 5 # Trap -SIGABRT = 6 # Aborted -SIGBUS = 7 # Bus error -SIGFPE = 8 # Floating point exception -SIGKILL = 9 # Killed -SIGUSR1 = 10 # User signal 1 -SIGSEGV = 11 # Segmentation fault -SIGUSR2 = 12 # User signal 2 -SIGPIPE = 13 # Broken pipe -SIGALRM = 14 # Alarm clock -SIGTERM = 15 # Terminated -SIGSTKFLT = 16 # Stack fault -SIGCHLD = 17 # Child exited -SIGCONT = 18 # Continue -SIGSTOP = 19 # Stopped (signal) -SIGTSTP = 20 # Stopped -SIGTTIN = 21 # Stopped (tty input) -SIGTTOU = 22 # Stopped (tty output) -SIGURG = 23 # Urgent I/O condition -SIGXCPU = 24 # CPU time limit exceeded -SIGXFSZ = 25 # File size limit exceeded -SIGVTALRM = 26 # Virtual timer expired -SIGPROF = 27 # Profiling timer expired -SIGWINCH = 28 # Window size changed -SIGIO = 29 # I/O possible -SIGPWR = 30 # Power failure -SIGSYS = 31 # Bad system call +SIGHUP = 1 # Hangup +SIGINT = 2 # Interrupt +SIGQUIT = 3 # Quit +SIGILL = 4 # Illegal instruction +SIGTRAP = 5 # Trap +SIGABRT = 6 # Aborted +SIGBUS = 7 # Bus error +SIGFPE = 8 # Floating point exception +SIGKILL = 9 # Killed +SIGUSR1 = 10 # User signal 1 +SIGSEGV = 11 # Segmentation fault +SIGUSR2 = 12 # User signal 2 +SIGPIPE = 13 # Broken pipe +SIGALRM = 14 # Alarm clock +SIGTERM = 15 # Terminated +SIGSTKFLT = 16 # Stack fault +SIGCHLD = 17 # Child exited +SIGCONT = 18 # Continue +SIGSTOP = 19 # Stopped (signal) +SIGTSTP = 20 # Stopped +SIGTTIN = 21 # Stopped (tty input) +SIGTTOU = 22 # Stopped (tty output) +SIGURG = 23 # Urgent I/O condition +SIGXCPU = 24 # CPU time limit exceeded +SIGXFSZ = 25 # File size limit exceeded +SIGVTALRM = 26 # Virtual timer expired +SIGPROF = 27 # Profiling timer expired +SIGWINCH = 28 # Window size changed +SIGIO = 29 # I/O possible +SIGPWR = 30 # Power failure +SIGSYS = 31 # Bad system call diff --git a/devil/devil/android/device_temp_file.py b/devil/devil/android/device_temp_file.py index af2c4892faf..75488c5090f 100644 --- a/devil/devil/android/device_temp_file.py +++ b/devil/devil/android/device_temp_file.py @@ -15,6 +15,7 @@ class DeviceTempFile(object): + def __init__(self, adb, suffix='', prefix='temp_file', dir='/data/local/tmp'): """Find an unused temporary file path on the device. diff --git a/devil/devil/android/device_utils.py b/devil/devil/android/device_utils.py index 05f3181b494..86f1f1d274b 100644 --- a/devil/devil/android/device_utils.py +++ b/devil/devil/android/device_utils.py @@ -103,6 +103,7 @@ _GETPROP_RE = re.compile(r'\[(.*?)\]: \[(.*?)\]') _IPV4_ADDRESS_RE = re.compile(r'([0-9]{1,3}\.){3}[0-9]{1,3}\:[0-9]{4,5}') + @decorators.WithExplicitTimeoutAndRetries( _DEFAULT_TIMEOUT, _DEFAULT_RETRIES) def GetAVDs(): @@ -709,7 +710,6 @@ def _CheckSdkLevel(self, required_sdk_level): (required_sdk_level, self.build_version_sdk)), device_serial=self.adb.GetDeviceSerial()) - @decorators.WithTimeoutAndRetriesFromInstance() def RunShellCommand(self, cmd, check_return=False, cwd=None, env=None, as_root=False, single_line=False, large_output=False, @@ -903,7 +903,7 @@ def KillAll(self, process_name, exact=False, signum=device_signal.SIGKILL, logging.info( 'KillAll(%r, ...) attempting to kill the following:', process_name) - for name, ids in procs_pids.iteritems(): + for name, ids in procs_pids.iteritems(): for i in ids: logging.info(' %05s %s', str(i), name) @@ -1303,12 +1303,12 @@ def _ApproximateDuration(adb_calls, file_count, byte_count, is_zipping): # c6: compression ratio (unitless) # All of these are approximations. - ADB_CALL_PENALTY = 0.1 # seconds - ADB_PUSH_PENALTY = 0.01 # seconds - ZIP_PENALTY = 2.0 # seconds - ZIP_RATE = 10000000.0 # bytes / second - TRANSFER_RATE = 2000000.0 # bytes / second - COMPRESSION_RATIO = 2.0 # unitless + ADB_CALL_PENALTY = 0.1 # seconds + ADB_PUSH_PENALTY = 0.01 # seconds + ZIP_PENALTY = 2.0 # seconds + ZIP_RATE = 10000000.0 # bytes / second + TRANSFER_RATE = 2000000.0 # bytes / second + COMPRESSION_RATIO = 2.0 # unitless adb_call_time = ADB_CALL_PENALTY * adb_calls adb_push_setup_time = ADB_PUSH_PENALTY * file_count @@ -1621,7 +1621,7 @@ def find_property(lines, property_name): else: properties[index] = new_line else: - assert index is not None # since new_value == '' and new_value != value + assert index is not None # since new_value == '' and new_value != value properties.pop(index) self.WriteFile(self.LOCAL_PROPERTIES_PATH, _JoinLines(properties)) @@ -1649,7 +1649,6 @@ def GetCountry(self, cache=False): """ return self.GetProp('persist.sys.country', cache=cache) - @property def screen_density(self): """Returns the screen density of the device.""" @@ -2079,6 +2078,7 @@ def parallel(cls, devices, async=False): @classmethod def HealthyDevices(cls, blacklist=None, **kwargs): blacklisted_devices = blacklist.Read() if blacklist else [] + def blacklisted(adb): if adb.GetDeviceSerial() in blacklisted_devices: logging.warning('Device %s is blacklisted.', adb.GetDeviceSerial()) @@ -2143,11 +2143,11 @@ def IsScreenOn(self, timeout=None, retries=None): device_errors.CommandFailedError: If screen state cannot be found. """ if self.build_version_sdk < version_codes.LOLLIPOP: - input_check = 'mScreenOn' - check_value = 'mScreenOn=true' + input_check = 'mScreenOn' + check_value = 'mScreenOn=true' else: - input_check = 'mInteractive' - check_value = 'mInteractive=true' + input_check = 'mInteractive' + check_value = 'mInteractive=true' dumpsys_out = self._RunPipedShellCommand( 'dumpsys input_method | grep %s' % input_check) if not dumpsys_out: diff --git a/devil/devil/android/device_utils_devicetest.py b/devil/devil/android/device_utils_devicetest.py index d4b823df4b8..9a50373823e 100755 --- a/devil/devil/android/device_utils_devicetest.py +++ b/devil/devil/android/device_utils_devicetest.py @@ -23,6 +23,7 @@ _SUB_DIR1 = "sub1" _SUB_DIR2 = "sub2" + class DeviceUtilsPushDeleteFilesTest(unittest.TestCase): def setUp(self): diff --git a/devil/devil/android/device_utils_test.py b/devil/devil/android/device_utils_test.py index 3b440b7ef12..6c569a32388 100755 --- a/devil/devil/android/device_utils_test.py +++ b/devil/devil/android/device_utils_test.py @@ -24,10 +24,11 @@ from devil.utils import mock_calls with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error class _MockApkHelper(object): + def __init__(self, path, package_name, perms=None): self.path = path self.package_name = package_name @@ -123,6 +124,7 @@ def name(self): class _PatchedFunction(object): + def __init__(self, patched=None, mocked=None): self.patched = patched self.mocked = mocked @@ -722,6 +724,7 @@ def testInstallSplitApk_downgrade(self): class DeviceUtilsUninstallTest(DeviceUtilsTest): + def testUninstall_callsThrough(self): with self.assertCalls( (self.call.device._GetApplicationPathsInternal('test.package'), @@ -736,6 +739,7 @@ def testUninstall_noop(self): class DeviceUtilsSuTest(DeviceUtilsTest): + def testSu_preM(self): with self.patch_call( self.call.device.build_version_sdk, @@ -989,7 +993,7 @@ def testKillAll_blocking(self): (self.call.device.GetPids('some.process'), {'some.processing.thing': ['5678']}), (self.call.device.GetPids('some.process'), - {'some.process': ['1111']})): # Other instance with different pid. + {'some.process': ['1111']})): # Other instance with different pid. self.assertEquals( 2, self.device.KillAll('some.process', blocking=True)) @@ -1360,6 +1364,7 @@ def testGoHome_obtainsFocusAfterGoingHome(self): ['mCurrentFocus Launcher'])): self.device.GoHome() + class DeviceUtilsForceStopTest(DeviceUtilsTest): def testForceStop(self): @@ -1692,7 +1697,7 @@ def testWriteFileWithPush_rejected(self): self.device._WriteFileWithPush('/path/to/device/file', contents) def testWriteFile_withPush(self): - contents = 'some large contents ' * 26 # 20 * 26 = 520 chars + contents = 'some large contents ' * 26 # 20 * 26 = 520 chars with self.assertCalls( self.call.device._WriteFileWithPush('/path/to/device/file', contents)): self.device.WriteFile('/path/to/device/file', contents) @@ -1704,7 +1709,7 @@ def testWriteFile_withPushForced(self): self.device.WriteFile('/path/to/device/file', contents, force_push=True) def testWriteFile_withPushAndSU(self): - contents = 'some large contents ' * 26 # 20 * 26 = 520 chars + contents = 'some large contents ' * 26 # 20 * 26 = 520 chars with self.assertCalls( (self.call.device.NeedsSU(), True), (mock.call.devil.android.device_temp_file.DeviceTempFile(self.adb), @@ -2185,8 +2190,8 @@ def testGrantPermissions_WriteExtrnalStorage(self): def testGrantPermissions_BlackList(self): with self.patch_call(self.call.device.build_version_sdk, return_value=version_codes.MARSHMALLOW): - self.device.GrantPermissions( - 'package', ['android.permission.ACCESS_MOCK_LOCATION']) + self.device.GrantPermissions( + 'package', ['android.permission.ACCESS_MOCK_LOCATION']) class DeviecUtilsIsScreenOn(DeviceUtilsTest): @@ -2254,7 +2259,6 @@ def testSetScreen_on(self): (self.call.device.IsScreenOn(), True)): self.device.SetScreen(True) - @mock.patch('time.sleep', mock.Mock()) def testSetScreen_off(self): with self.assertCalls( diff --git a/devil/devil/android/fastboot_utils.py b/devil/devil/android/fastboot_utils.py index 587f42fd02c..f1287d1adb6 100644 --- a/devil/devil/android/fastboot_utils.py +++ b/devil/devil/android/fastboot_utils.py @@ -29,6 +29,7 @@ 'cache', ] + class FastbootUtils(object): _FASTBOOT_WAIT_TIME = 1 diff --git a/devil/devil/android/fastboot_utils_test.py b/devil/devil/android/fastboot_utils_test.py index d82e1551114..8e6fc88b3c7 100755 --- a/devil/devil/android/fastboot_utils_test.py +++ b/devil/devil/android/fastboot_utils_test.py @@ -21,7 +21,7 @@ from devil.utils import mock_calls with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error _BOARD = 'board_type' _SERIAL = '0123456789abcdef' @@ -60,6 +60,7 @@ def _FastbootWrapperMock(test_serial): fastbooter.Devices.return_value = [test_serial] return fastbooter + def _DeviceUtilsMock(test_serial): device = mock.Mock(spec=device_utils.DeviceUtils) device.__str__ = mock.Mock(return_value=test_serial) @@ -232,6 +233,7 @@ def testVerifyBoard_noBoardInFileInvalidZip(self): with mock.patch('os.listdir', return_value=_INVALID_FILES): self.assertFalse(self.fastboot._VerifyBoard('test')) + class FastbootUtilsFindAndVerifyPartitionsAndImages(FastbootUtilsTest): def testFindAndVerifyPartitionsAndImages_valid(self): diff --git a/devil/devil/android/forwarder.py b/devil/devil/android/forwarder.py index 8794797ffce..aca4fefa7ab 100644 --- a/devil/devil/android/forwarder.py +++ b/devil/devil/android/forwarder.py @@ -25,6 +25,7 @@ class _FileLock(object): File locks are needed for cross-process synchronization when the multiprocessing Python module is used. """ + def __init__(self, path): self._fd = -1 self._path = path diff --git a/devil/devil/android/logcat_monitor.py b/devil/devil/android/logcat_monitor.py index 179002a8b0f..497a6bf92c0 100644 --- a/devil/devil/android/logcat_monitor.py +++ b/devil/devil/android/logcat_monitor.py @@ -221,6 +221,7 @@ def __del__(self): logging.warning('Need to call |Close| on the logcat monitor when done!') self._record_file.close() + class LogcatMonitorCommandError(device_errors.CommandFailedError): """Exception for errors with logcat monitor commands.""" pass diff --git a/devil/devil/android/logcat_monitor_test.py b/devil/devil/android/logcat_monitor_test.py index b096ef8db9d..8fb4d74bbc8 100755 --- a/devil/devil/android/logcat_monitor_test.py +++ b/devil/devil/android/logcat_monitor_test.py @@ -14,7 +14,7 @@ from devil.android.sdk import adb_wrapper with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error def _CreateTestLog(raw_logcat=None): @@ -23,23 +23,25 @@ def _CreateTestLog(raw_logcat=None): test_log = logcat_monitor.LogcatMonitor(test_adb, clear=False) return test_log + class LogcatMonitorTest(unittest.TestCase): _TEST_THREADTIME_LOGCAT_DATA = [ - '01-01 01:02:03.456 7890 0987 V LogcatMonitorTest: ' - 'verbose logcat monitor test message 1', - '01-01 01:02:03.457 8901 1098 D LogcatMonitorTest: ' - 'debug logcat monitor test message 2', - '01-01 01:02:03.458 9012 2109 I LogcatMonitorTest: ' - 'info logcat monitor test message 3', - '01-01 01:02:03.459 0123 3210 W LogcatMonitorTest: ' - 'warning logcat monitor test message 4', - '01-01 01:02:03.460 1234 4321 E LogcatMonitorTest: ' - 'error logcat monitor test message 5', - '01-01 01:02:03.461 2345 5432 F LogcatMonitorTest: ' - 'fatal logcat monitor test message 6', - '01-01 01:02:03.462 3456 6543 D LogcatMonitorTest: ' - 'last line',] + '01-01 01:02:03.456 7890 0987 V LogcatMonitorTest: ' + 'verbose logcat monitor test message 1', + '01-01 01:02:03.457 8901 1098 D LogcatMonitorTest: ' + 'debug logcat monitor test message 2', + '01-01 01:02:03.458 9012 2109 I LogcatMonitorTest: ' + 'info logcat monitor test message 3', + '01-01 01:02:03.459 0123 3210 W LogcatMonitorTest: ' + 'warning logcat monitor test message 4', + '01-01 01:02:03.460 1234 4321 E LogcatMonitorTest: ' + 'error logcat monitor test message 5', + '01-01 01:02:03.461 2345 5432 F LogcatMonitorTest: ' + 'fatal logcat monitor test message 6', + '01-01 01:02:03.462 3456 6543 D LogcatMonitorTest: ' + 'last line' + ] def assertIterEqual(self, expected_iter, actual_iter): for expected, actual in itertools.izip_longest(expected_iter, actual_iter): @@ -190,7 +192,8 @@ def testFindAll_filterLogLevel(self): ('8901', '1098', 'D', 'LogcatMonitorTest', 'debug logcat monitor test message 2'), ('0123', '3210', 'W', 'LogcatMonitorTest', - 'warning logcat monitor test message 4'),] + 'warning logcat monitor test message 4') + ] self.assertIterEqual(iter(expected_results), actual_results) test_log.Close() @@ -216,7 +219,8 @@ def testFindAll_filterComponent(self): ('2345', '5432', 'F', 'LogcatMonitorTest', 'fatal logcat monitor test message 6'), ('3456', '6543', 'D', 'LogcatMonitorTest', - 'last line'),] + 'last line') + ] self.assertIterEqual(iter(expected_results), actual_results) test_log.Close() diff --git a/devil/devil/android/md5sum_test.py b/devil/devil/android/md5sum_test.py index c1e8bbb4699..c9b4954540f 100755 --- a/devil/devil/android/md5sum_test.py +++ b/devil/devil/android/md5sum_test.py @@ -11,12 +11,13 @@ from devil.android import md5sum with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error TEST_OUT_DIR = os.path.join('test', 'out', 'directory') HOST_MD5_EXECUTABLE = os.path.join(TEST_OUT_DIR, 'md5sum_bin_host') MD5_DIST = os.path.join(TEST_OUT_DIR, 'md5sum_dist') + class Md5SumTest(unittest.TestCase): def setUp(self): @@ -202,7 +203,6 @@ def testCalculateDeviceMd5Sums_list_fileMissing(self): out['/storage/emulated/legacy/test/file0.dat']) self.assertEquals(1, len(device.RunShellCommand.call_args_list)) - def testCalculateDeviceMd5Sums_requiresBinary(self): test_path = '/storage/emulated/legacy/test/file.dat' diff --git a/devil/devil/android/perf/perf_control_devicetest.py b/devil/devil/android/perf/perf_control_devicetest.py index 68de63b5641..71bf3fbca37 100644 --- a/devil/devil/android/perf/perf_control_devicetest.py +++ b/devil/devil/android/perf/perf_control_devicetest.py @@ -12,7 +12,9 @@ from devil.android import device_utils from devil.android.perf import perf_control + class TestPerfControl(unittest.TestCase): + def setUp(self): if not os.getenv('BUILDTYPE'): os.environ['BUILDTYPE'] = 'Debug' diff --git a/devil/devil/android/ports.py b/devil/devil/android/ports.py index eff30ced0f5..4783082c88b 100644 --- a/devil/devil/android/ports.py +++ b/devil/devil/android/ports.py @@ -37,7 +37,7 @@ def ResetTestServerPortAllocation(): if os.path.exists(_TEST_SERVER_PORT_LOCKFILE): os.unlink(_TEST_SERVER_PORT_LOCKFILE) return True - except Exception: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except logging.exception('Error while resetting port allocation') return False @@ -69,7 +69,7 @@ def AllocateTestServerPort(): else: fp.seek(0, os.SEEK_SET) fp.write('%d' % (port + 1)) - except Exception: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except logging.exception('ERror while allocating port') finally: if fp_lock: diff --git a/devil/devil/android/sdk/adb_wrapper.py b/devil/devil/android/sdk/adb_wrapper.py index 30a8feed5e5..a65ab7cbc94 100644 --- a/devil/devil/android/sdk/adb_wrapper.py +++ b/devil/devil/android/sdk/adb_wrapper.py @@ -22,7 +22,7 @@ from devil.utils import timeout_retry with devil_env.SysPath(devil_env.DEPENDENCY_MANAGER_PATH): - import dependency_manager # pylint: disable=import-error + import dependency_manager # pylint: disable=import-error _DEFAULT_TIMEOUT = 30 @@ -103,7 +103,7 @@ def _BuildAdbCmd(cls, args, device_serial, cpu_affinity=None): cmd.extend(args) return cmd - #pylint: disable=unused-argument + # pylint: disable=unused-argument @classmethod @decorators.WithTimeoutAndConditionalRetries(_ShouldRetryAdbCmd) def _RunAdbCmd(cls, args, timeout=None, retries=None, device_serial=None, @@ -254,7 +254,7 @@ def GetDeviceSerial(self): """ return self._device_serial - def Push(self, local, remote, timeout=60*5, retries=_DEFAULT_RETRIES): + def Push(self, local, remote, timeout=60 * 5, retries=_DEFAULT_RETRIES): """Pushes a file from the host to the device. Args: @@ -266,7 +266,7 @@ def Push(self, local, remote, timeout=60*5, retries=_DEFAULT_RETRIES): VerifyLocalFileExists(local) self._RunDeviceAdbCmd(['push', local, remote], timeout, retries) - def Pull(self, remote, local, timeout=60*5, retries=_DEFAULT_RETRIES): + def Pull(self, remote, local, timeout=60 * 5, retries=_DEFAULT_RETRIES): """Pulls a file from the device to the host. Args: @@ -313,7 +313,7 @@ def Shell(self, command, expect_status=0, timeout=_DEFAULT_TIMEOUT, output_end = len(output) try: - status = int(output[output_end+1:]) + status = int(output[output_end + 1:]) except ValueError: logging.warning('exit status of shell command %r missing.', command) raise device_errors.AdbShellCommandFailedError( @@ -476,7 +476,7 @@ def JDWP(self, timeout=_DEFAULT_TIMEOUT, retries=_DEFAULT_RETRIES): self._RunDeviceAdbCmd(['jdwp'], timeout, retries).split('\n')] def Install(self, apk_path, forward_lock=False, allow_downgrade=False, - reinstall=False, sd_card=False, timeout=60*2, + reinstall=False, sd_card=False, timeout=60 * 2, retries=_DEFAULT_RETRIES): """Install an apk on the device. @@ -507,7 +507,7 @@ def Install(self, apk_path, forward_lock=False, allow_downgrade=False, def InstallMultiple(self, apk_paths, forward_lock=False, reinstall=False, sd_card=False, allow_downgrade=False, partial=False, - timeout=60*2, retries=_DEFAULT_RETRIES): + timeout=60 * 2, retries=_DEFAULT_RETRIES): """Install an apk with splits on the device. Args: @@ -602,7 +602,7 @@ def Restore(self, path, timeout=_DEFAULT_TIMEOUT, retries=_DEFAULT_RETRIES): VerifyLocalFileExists(path) self._RunDeviceAdbCmd(['restore'] + [path], timeout, retries) - def WaitForDevice(self, timeout=60*5, retries=_DEFAULT_RETRIES): + def WaitForDevice(self, timeout=60 * 5, retries=_DEFAULT_RETRIES): """Block until the device is online. Args: @@ -631,7 +631,6 @@ def GetState(self, timeout=_DEFAULT_TIMEOUT, retries=_DEFAULT_RETRIES): return line[1] return 'offline' - def GetDevPath(self, timeout=_DEFAULT_TIMEOUT, retries=_DEFAULT_RETRIES): """Gets the device path. @@ -648,7 +647,7 @@ def Remount(self, timeout=_DEFAULT_TIMEOUT, retries=_DEFAULT_RETRIES): """Remounts the /system partition on the device read-write.""" self._RunDeviceAdbCmd(['remount'], timeout, retries) - def Reboot(self, to_bootloader=False, timeout=60*5, + def Reboot(self, to_bootloader=False, timeout=60 * 5, retries=_DEFAULT_RETRIES): """Reboots the device. diff --git a/devil/devil/android/sdk/adb_wrapper_devicetest.py b/devil/devil/android/sdk/adb_wrapper_devicetest.py index 7c098a9059a..59755c00cbb 100644 --- a/devil/devil/android/sdk/adb_wrapper_devicetest.py +++ b/devil/devil/android/sdk/adb_wrapper_devicetest.py @@ -42,7 +42,7 @@ def testShell(self): output = self._adb.Shell('echo test') self.assertEqual(output.strip(), 'test') with self.assertRaises(device_errors.AdbCommandFailedError): - self._adb.Shell('echo test', expect_status=1) + self._adb.Shell('echo test', expect_status=1) def testPushLsPull(self): path = self._MakeTempFile('foo') diff --git a/devil/devil/android/sdk/build_tools.py b/devil/devil/android/sdk/build_tools.py index 36aafce2dee..99083d9904e 100644 --- a/devil/devil/android/sdk/build_tools.py +++ b/devil/devil/android/sdk/build_tools.py @@ -8,7 +8,7 @@ from devil.utils import lazy with devil_env.SysPath(devil_env.DEPENDENCY_MANAGER_PATH): - import dependency_manager # pylint: disable=import-error + import dependency_manager # pylint: disable=import-error def GetPath(build_tool): diff --git a/devil/devil/android/sdk/fastboot.py b/devil/devil/android/sdk/fastboot.py index aebff203fda..d9fa653bd5d 100644 --- a/devil/devil/android/sdk/fastboot.py +++ b/devil/devil/android/sdk/fastboot.py @@ -21,6 +21,7 @@ _DEFAULT_RETRIES = 3 _FLASH_TIMEOUT = _DEFAULT_TIMEOUT * 10 + class Fastboot(object): _fastboot_path = lazy.WeakConstant(lambda: os.path.join( diff --git a/devil/devil/android/sdk/gce_adb_wrapper.py b/devil/devil/android/sdk/gce_adb_wrapper.py index 60ec3644051..5ee7959c72b 100644 --- a/devil/devil/android/sdk/gce_adb_wrapper.py +++ b/devil/devil/android/sdk/gce_adb_wrapper.py @@ -25,11 +25,12 @@ class GceAdbWrapper(adb_wrapper.AdbWrapper): + def __init__(self, device_serial): super(GceAdbWrapper, self).__init__(device_serial) self._instance_ip = self.Shell('getprop net.gce.ip_address').strip() - #override + # override def Push(self, local, remote, **kwargs): """Pushes an object from the host to the gce instance. @@ -77,7 +78,7 @@ def _PushObject(self, local, remote): cmd, 'File not reachable on host: %s' % local, device_serial=str(self)) - #override + # override def Pull(self, remote, local, **kwargs): """Pulls a file from the gce instance to the host. @@ -110,7 +111,7 @@ def Pull(self, remote, local, **kwargs): cmd, 'File not reachable on host: %s' % local, device_serial=str(self)) - #override + # override def Install(self, apk_path, forward_lock=False, reinstall=False, sd_card=False, **kwargs): """Installs an apk on the gce instance @@ -139,7 +140,7 @@ def Install(self, apk_path, forward_lock=False, reinstall=False, raise device_errors.AdbCommandFailedError( cmd, output, device_serial=self._device_serial) - #override + # override @property def is_emulator(self): return True diff --git a/devil/devil/android/sdk/intent.py b/devil/devil/android/sdk/intent.py index 333b9f1f108..e612f76b164 100644 --- a/devil/devil/android/sdk/intent.py +++ b/devil/devil/android/sdk/intent.py @@ -8,6 +8,7 @@ Am command. """ + class Intent(object): def __init__(self, action='android.intent.action.VIEW', activity=None, diff --git a/devil/devil/android/sdk/shared_prefs.py b/devil/devil/android/sdk/shared_prefs.py index 2b430d5c4dc..50ff5c62a50 100644 --- a/devil/devil/android/sdk/shared_prefs.py +++ b/devil/devil/android/sdk/shared_prefs.py @@ -162,6 +162,7 @@ def set(self, value): class SharedPrefs(object): + def __init__(self, device, package, filename): """Helper object to read and update "Shared Prefs" of Android apps. @@ -252,7 +253,7 @@ def Load(self): def Clear(self): """Clear all of the preferences contained in this object.""" - if self._xml is not None and len(self): # only clear if not already empty + if self._xml is not None and len(self): # only clear if not already empty self._xml = None self._changed = True diff --git a/devil/devil/android/sdk/shared_prefs_test.py b/devil/devil/android/sdk/shared_prefs_test.py index e20fbaa1ef1..ff3b9a139f1 100755 --- a/devil/devil/android/sdk/shared_prefs_test.py +++ b/devil/devil/android/sdk/shared_prefs_test.py @@ -15,7 +15,7 @@ from devil.android.sdk import shared_prefs with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error def MockDeviceWithFiles(files=None): @@ -56,7 +56,7 @@ def setUp(self): def testPropertyLifetime(self): prefs = shared_prefs.SharedPrefs( self.device, 'com.some.package', 'prefs.xml') - self.assertEquals(len(prefs), 0) # collection is empty before loading + self.assertEquals(len(prefs), 0) # collection is empty before loading prefs.SetInt('myValue', 444) self.assertEquals(len(prefs), 1) self.assertEquals(prefs.GetInt('myValue'), 444) @@ -80,7 +80,7 @@ def testPropertyType(self): def testLoad(self): prefs = shared_prefs.SharedPrefs( self.device, 'com.some.package', 'prefs.xml') - self.assertEquals(len(prefs), 0) # collection is empty before loading + self.assertEquals(len(prefs), 0) # collection is empty before loading prefs.Load() self.assertEquals(len(prefs), len(self.expected_data)) self.assertEquals(prefs.AsDict(), self.expected_data) @@ -93,42 +93,42 @@ def testClear(self): self.assertEquals(prefs.AsDict(), self.expected_data) self.assertFalse(prefs.changed) prefs.Clear() - self.assertEquals(len(prefs), 0) # collection is empty now + self.assertEquals(len(prefs), 0) # collection is empty now self.assertTrue(prefs.changed) def testCommit(self): prefs = shared_prefs.SharedPrefs( self.device, 'com.some.package', 'other_prefs.xml') - self.assertFalse(self.device.FileExists(prefs.path)) # file does not exist + self.assertFalse(self.device.FileExists(prefs.path)) # file does not exist prefs.Load() - self.assertEquals(len(prefs), 0) # file did not exist, collection is empty + self.assertEquals(len(prefs), 0) # file did not exist, collection is empty prefs.SetInt('magicNumber', 42) prefs.SetFloat('myMetric', 3.14) prefs.SetLong('bigNumner', 6000000000) prefs.SetStringSet('apps', ['gmail', 'chrome', 'music']) - self.assertFalse(self.device.FileExists(prefs.path)) # still does not exist + self.assertFalse(self.device.FileExists(prefs.path)) # still does not exist self.assertTrue(prefs.changed) prefs.Commit() - self.assertTrue(self.device.FileExists(prefs.path)) # should exist now + self.assertTrue(self.device.FileExists(prefs.path)) # should exist now self.device.KillAll.assert_called_once_with(prefs.package, exact=True, as_root=True, quiet=True) self.assertFalse(prefs.changed) prefs = shared_prefs.SharedPrefs( self.device, 'com.some.package', 'other_prefs.xml') - self.assertEquals(len(prefs), 0) # collection is empty before loading + self.assertEquals(len(prefs), 0) # collection is empty before loading prefs.Load() self.assertEquals(prefs.AsDict(), { 'magicNumber': 42, 'myMetric': 3.14, 'bigNumner': 6000000000, - 'apps': ['gmail', 'chrome', 'music']}) # data survived roundtrip + 'apps': ['gmail', 'chrome', 'music']}) # data survived roundtrip def testAsContextManager_onlyReads(self): with shared_prefs.SharedPrefs( self.device, 'com.some.package', 'prefs.xml') as prefs: - self.assertEquals(prefs.AsDict(), self.expected_data) # loaded and ready - self.assertEquals(self.device.WriteFile.call_args_list, []) # did not write + self.assertEquals(prefs.AsDict(), self.expected_data) # loaded and ready + self.assertEquals(self.device.WriteFile.call_args_list, []) # did not write def testAsContextManager_readAndWrite(self): with shared_prefs.SharedPrefs( @@ -137,14 +137,14 @@ def testAsContextManager_readAndWrite(self): prefs.Remove('someHashValue') prefs.SetString('newString', 'hello') - self.assertTrue(self.device.WriteFile.called) # did write + self.assertTrue(self.device.WriteFile.called) # did write with shared_prefs.SharedPrefs( self.device, 'com.some.package', 'prefs.xml') as prefs: # changes persisted self.assertTrue(prefs.GetBoolean('featureEnabled')) self.assertFalse(prefs.HasProperty('someHashValue')) self.assertEquals(prefs.GetString('newString'), 'hello') - self.assertTrue(prefs.HasProperty('databaseVersion')) # still there + self.assertTrue(prefs.HasProperty('databaseVersion')) # still there def testAsContextManager_commitAborted(self): with self.assertRaises(TypeError): @@ -153,9 +153,9 @@ def testAsContextManager_commitAborted(self): prefs.SetBoolean('featureEnabled', True) prefs.Remove('someHashValue') prefs.SetString('newString', 'hello') - prefs.SetInt('newString', 123) # oops! + prefs.SetInt('newString', 123) # oops! - self.assertEquals(self.device.WriteFile.call_args_list, []) # did not write + self.assertEquals(self.device.WriteFile.call_args_list, []) # did not write with shared_prefs.SharedPrefs( self.device, 'com.some.package', 'prefs.xml') as prefs: # contents were not modified diff --git a/devil/devil/android/sdk/split_select.py b/devil/devil/android/sdk/split_select.py index b47a21f9fe0..6c3d231a77b 100644 --- a/devil/devil/android/sdk/split_select.py +++ b/devil/devil/android/sdk/split_select.py @@ -29,6 +29,7 @@ def _RunSplitSelectCmd(args): (' '.join(cmd), output)) return output + def _SplitConfig(device, allow_cached_props=False): """Returns a config specifying which APK splits are required by the device. @@ -42,6 +43,7 @@ def _SplitConfig(device, allow_cached_props=False): device.screen_density, device.product_cpu_abi)) + def SelectSplits(device, base_apk, split_apks, allow_cached_props=False): """Determines which APK splits the device requires. diff --git a/devil/devil/android/tools/adb_run_shell_cmd.py b/devil/devil/android/tools/adb_run_shell_cmd.py index 2f00b070c13..f995d272f7e 100755 --- a/devil/devil/android/tools/adb_run_shell_cmd.py +++ b/devil/devil/android/tools/adb_run_shell_cmd.py @@ -56,7 +56,7 @@ def main(): data = {} for device, output in zip(selected_devices, p_out): for line in output: - print '%s: %s' %(device, line) + print '%s: %s' % (device, line) data[str(device)] = output if args.json_output: diff --git a/devil/devil/android/tools/flash_device.py b/devil/devil/android/tools/flash_device.py index 5c42c15d187..50ed6961c8e 100755 --- a/devil/devil/android/tools/flash_device.py +++ b/devil/devil/android/tools/flash_device.py @@ -48,7 +48,7 @@ def flash(device): try: fastboot.FlashDevice(args.build_path, wipe=args.wipe) flashed_devices.append(device) - except Exception: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except logging.exception('Device %s failed to flash.', str(device)) failed_devices.append(device) diff --git a/devil/devil/android/tools/script_common_test.py b/devil/devil/android/tools/script_common_test.py index bb5e29efcf0..a226764557a 100755 --- a/devil/devil/android/tools/script_common_test.py +++ b/devil/devil/android/tools/script_common_test.py @@ -13,7 +13,7 @@ from devil.android.tools import script_common with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error class ScriptCommonTest(unittest.TestCase): diff --git a/devil/devil/devil_env.py b/devil/devil/devil_env.py index b23c2f4b1e5..b54e6f5bdac 100644 --- a/devil/devil/devil_env.py +++ b/devil/devil/devil_env.py @@ -17,6 +17,7 @@ PYMOCK_PATH = os.path.join( CATAPULT_ROOT_PATH, 'third_party', 'mock') + @contextlib.contextmanager def SysPath(path): sys.path.append(path) @@ -27,7 +28,7 @@ def SysPath(path): sys.path.pop() with SysPath(DEPENDENCY_MANAGER_PATH): - import dependency_manager # pylint: disable=import-error + import dependency_manager # pylint: disable=import-error _ANDROID_BUILD_TOOLS = {'aapt', 'dexdump', 'split-select'} diff --git a/devil/devil/devil_env_test.py b/devil/devil/devil_env_test.py index fcb5b3d30d1..e78221a0700 100755 --- a/devil/devil/devil_env_test.py +++ b/devil/devil/devil_env_test.py @@ -14,9 +14,10 @@ _sys_path_before = list(sys.path) with devil_env.SysPath(devil_env.PYMOCK_PATH): _sys_path_with_pymock = list(sys.path) - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error _sys_path_after = list(sys.path) + class DevilEnvTest(unittest.TestCase): def testSysPath(self): diff --git a/devil/devil/utils/cmd_helper.py b/devil/devil/utils/cmd_helper.py index 57e8987558d..b6237757424 100644 --- a/devil/devil/utils/cmd_helper.py +++ b/devil/devil/utils/cmd_helper.py @@ -22,6 +22,7 @@ _SafeShellChars = frozenset(string.ascii_letters + string.digits + '@%_-+=:,./') + def SingleQuote(s): """Return an shell-escaped version of the string using single quotes. @@ -39,6 +40,7 @@ def SingleQuote(s): """ return pipes.quote(s) + def DoubleQuote(s): """Return an shell-escaped version of the string using double quotes. @@ -176,6 +178,7 @@ def GetCmdStatusAndOutput(args, cwd=None, shell=False): logging.debug(stdout[:4096]) return (status, stdout) + def GetCmdStatusOutputAndError(args, cwd=None, shell=False): """Executes a subprocess and returns its exit code, output, and errors. diff --git a/devil/devil/utils/cmd_helper_test.py b/devil/devil/utils/cmd_helper_test.py index c01c31947a3..a04f1adf135 100755 --- a/devil/devil/utils/cmd_helper_test.py +++ b/devil/devil/utils/cmd_helper_test.py @@ -83,6 +83,7 @@ def testShrinkToSnippet_singleArg(self): self.assertEquals('foo \' \'"$a""$a"\' \'', cmd_helper.ShrinkToSnippet(['foo', ' barbar '], 'a', 'bar')) + class CmdHelperIterCmdOutputLinesTest(unittest.TestCase): """Test IterCmdOutputLines with some calls to the unix 'seq' command.""" diff --git a/devil/devil/utils/geometry_test.py b/devil/devil/utils/geometry_test.py index f6ee76c4268..af694429306 100644 --- a/devil/devil/utils/geometry_test.py +++ b/devil/devil/utils/geometry_test.py @@ -10,6 +10,7 @@ class PointTest(unittest.TestCase): + def testStr(self): p = g.Point(1, 2) self.assertEquals(str(p), '(1, 2)') @@ -42,7 +43,9 @@ def testMult_TypeErrorWithInvalidOperands(self): with self.assertRaises(TypeError): p * 4 # Can't multiply by a scalar on the right. + class RectangleTest(unittest.TestCase): + def testStr(self): r = g.Rectangle(g.Point(0, 1), g.Point(2, 3)) self.assertEquals(str(r), '[(0, 1), (2, 3)]') diff --git a/devil/devil/utils/mock_calls.py b/devil/devil/utils/mock_calls.py index 61ebb186751..5ae951e37da 100644 --- a/devil/devil/utils/mock_calls.py +++ b/devil/devil/utils/mock_calls.py @@ -11,12 +11,13 @@ from devil import devil_env with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error class TestCase(unittest.TestCase): """Adds assertCalls to TestCase objects.""" class _AssertCalls(object): + def __init__(self, test_case, expected_calls, watched): def call_action(pair): if isinstance(pair, type(mock.call)): @@ -45,7 +46,7 @@ def side_effect(*args, **kwargs): self._test_case = test_case self._expected_calls = [call_action(pair) for pair in expected_calls] - watched = watched.copy() # do not pollute the caller's dict + watched = watched.copy() # do not pollute the caller's dict watched.update((call.parent.name, call.parent) for call, _ in self._expected_calls) self._patched = [test_case.patch_call(call, side_effect=do_check(call)) diff --git a/devil/devil/utils/mock_calls_test.py b/devil/devil/utils/mock_calls_test.py index 3a1eea866ab..8eb4fc9da44 100755 --- a/devil/devil/utils/mock_calls_test.py +++ b/devil/devil/utils/mock_calls_test.py @@ -16,10 +16,11 @@ from devil.utils import mock_calls with devil_env.SysPath(devil_env.PYMOCK_PATH): - import mock # pylint: disable=import-error + import mock # pylint: disable=import-error class _DummyAdb(object): + def __str__(self): return '0123456789abcdef' @@ -44,6 +45,7 @@ def build_version_sdk(self): class TestCaseWithAssertCallsTest(mock_calls.TestCase): + def setUp(self): self.adb = _DummyAdb() @@ -154,7 +156,7 @@ def testAssertCalls_fails_extraCalls(self): self.adb.Reboot() def testAssertCalls_succeeds_NoCalls(self): - self.watchMethodCalls(self.call.adb) # we are watching all adb methods + self.watchMethodCalls(self.call.adb) # we are watching all adb methods with self.assertCalls(): pass diff --git a/devil/devil/utils/parallelizer.py b/devil/devil/utils/parallelizer.py index bac5b245fc4..b8a2824ea18 100644 --- a/devil/devil/utils/parallelizer.py +++ b/devil/devil/utils/parallelizer.py @@ -122,7 +122,7 @@ def __call__(self, *args, **kwargs): o, args=args, kwargs=kwargs, name='%s.%s' % (str(d), o.__name__)) for d, o in zip(self._orig_objs, self._objs)]) - r._objs.StartAll() # pylint: disable=W0212 + r._objs.StartAll() # pylint: disable=W0212 return r def pFinish(self, timeout): @@ -178,7 +178,7 @@ def pMap(self, f, *args, **kwargs): f, args=tuple([o] + list(args)), kwargs=kwargs, name='%s(%s)' % (f.__name__, d)) for d, o in zip(self._orig_objs, self._objs)]) - r._objs.StartAll() # pylint: disable=W0212 + r._objs.StartAll() # pylint: disable=W0212 return r def _assertNoShadow(self, attr_name): @@ -204,7 +204,7 @@ def _assertNoShadow(self, attr_name): class SyncParallelizer(Parallelizer): """A Parallelizer that blocks on function calls.""" - #override + # override def __call__(self, *args, **kwargs): """Emulate calling |self| with |args| and |kwargs|. @@ -220,7 +220,7 @@ def __call__(self, *args, **kwargs): r.pFinish(None) return r - #override + # override def pMap(self, f, *args, **kwargs): """Map a function across the current wrapped objects in parallel. diff --git a/devil/devil/utils/parallelizer_test.py b/devil/devil/utils/parallelizer_test.py index 0589a0430f6..3162a4f5d15 100644 --- a/devil/devil/utils/parallelizer_test.py +++ b/devil/devil/utils/parallelizer_test.py @@ -156,7 +156,7 @@ def testContained(self): self.assertEquals(str(i), results[i]) def testGetItem(self): - devices = [ParallelizerTestObject(range(i, i+10)) for i in xrange(0, 10)] + devices = [ParallelizerTestObject(range(i, i + 10)) for i in xrange(0, 10)] results = ParallelizerTestObject.parallel(devices)[9].pGet(1) self.assertEquals(range(9, 19), results) diff --git a/devil/devil/utils/reraiser_thread.py b/devil/devil/utils/reraiser_thread.py index e108fc754fb..56d95f3937e 100644 --- a/devil/devil/utils/reraiser_thread.py +++ b/devil/devil/utils/reraiser_thread.py @@ -74,12 +74,12 @@ def GetReturnValue(self): self.ReraiseIfException() return self._ret - #override + # override def run(self): """Overrides Thread.run() to add support for reraising exceptions.""" try: self._ret = self._func(*self._args, **self._kwargs) - except: # pylint: disable=W0702 + except: # pylint: disable=W0702 self._exc_info = sys.exc_info() diff --git a/devil/devil/utils/reraiser_thread_unittest.py b/devil/devil/utils/reraiser_thread_unittest.py index dcd243bc49e..e3c4e6bee81 100644 --- a/devil/devil/utils/reraiser_thread_unittest.py +++ b/devil/devil/utils/reraiser_thread_unittest.py @@ -17,6 +17,7 @@ class TestException(Exception): class TestReraiserThread(unittest.TestCase): """Tests for reraiser_thread.ReraiserThread.""" + def testNominal(self): result = [None, None] @@ -43,8 +44,10 @@ def f(): class TestReraiserThreadGroup(unittest.TestCase): """Tests for reraiser_thread.ReraiserThreadGroup.""" + def testInit(self): ran = [False] * 5 + def f(i): ran[i] = True @@ -57,6 +60,7 @@ def f(i): def testAdd(self): ran = [False] * 5 + def f(i): ran[i] = True @@ -81,6 +85,7 @@ def testJoinTimeout(self): def f(): pass event = threading.Event() + def g(): event.wait() group = reraiser_thread.ReraiserThreadGroup( @@ -94,6 +99,7 @@ def g(): class TestRunAsync(unittest.TestCase): """Tests for reraiser_thread.RunAsync.""" + def testNoArgs(self): results = reraiser_thread.RunAsync([]) self.assertEqual([], results) diff --git a/devil/devil/utils/run_tests_helper.py b/devil/devil/utils/run_tests_helper.py index 43f654ddcff..7df2da65859 100644 --- a/devil/devil/utils/run_tests_helper.py +++ b/devil/devil/utils/run_tests_helper.py @@ -12,14 +12,14 @@ class CustomFormatter(logging.Formatter): """Custom log formatter.""" - #override + # override def __init__(self, fmt='%(threadName)-4s %(message)s'): # Can't use super() because in older Python versions logging.Formatter does # not inherit from object. logging.Formatter.__init__(self, fmt=fmt) self._creation_time = time.time() - #override + # override def format(self, record): # Can't use super() because in older Python versions logging.Formatter does # not inherit from object. diff --git a/devil/devil/utils/timeout_retry.py b/devil/devil/utils/timeout_retry.py index e950e1160a8..95e90ee5b55 100644 --- a/devil/devil/utils/timeout_retry.py +++ b/devil/devil/utils/timeout_retry.py @@ -14,8 +14,8 @@ from devil.utils import watchdog_timer - class TimeoutRetryThreadGroup(reraiser_thread.ReraiserThreadGroup): + def __init__(self, timeout, threads=None): super(TimeoutRetryThreadGroup, self).__init__(threads) self._watcher = watchdog_timer.WatchdogTimer(timeout) diff --git a/devil/devil/utils/timeout_retry_unittest.py b/devil/devil/utils/timeout_retry_unittest.py index f2ceef7d604..84982889325 100755 --- a/devil/devil/utils/timeout_retry_unittest.py +++ b/devil/devil/utils/timeout_retry_unittest.py @@ -34,6 +34,7 @@ def testRun(self): def testTimeout(self): tries = [0] + def _sleep(): tries[0] += 1 time.sleep(1) @@ -64,6 +65,7 @@ def testCurrentTimeoutThreadGroup(self): def InnerFunc(): current_thread_group = timeout_retry.CurrentTimeoutThreadGroup() self.assertIsNotNone(current_thread_group) + def InnerInnerFunc(): self.assertEqual(current_thread_group, timeout_retry.CurrentTimeoutThreadGroup()) diff --git a/perf_insights/perf_insights/__init__.py b/perf_insights/perf_insights/__init__.py index 5f5d25ee4bf..987a3e1aeca 100644 --- a/perf_insights/perf_insights/__init__.py +++ b/perf_insights/perf_insights/__init__.py @@ -3,4 +3,4 @@ # found in the LICENSE file. import perf_insights_project -perf_insights_project.UpdateSysPathIfNeeded() \ No newline at end of file +perf_insights_project.UpdateSysPathIfNeeded() diff --git a/perf_insights/perf_insights/cloud_config.py b/perf_insights/perf_insights/cloud_config.py index 88115640a18..ae1fa6acb48 100644 --- a/perf_insights/perf_insights/cloud_config.py +++ b/perf_insights/perf_insights/cloud_config.py @@ -7,8 +7,9 @@ from google.appengine.api import app_identity from google.appengine.ext import ndb + def _is_devserver(): - server_software = os.environ.get('SERVER_SOFTWARE','') + server_software = os.environ.get('SERVER_SOFTWARE', '') return server_software and server_software.startswith('Development') _DEFAULT_CATAPULT_PATH = '/catapult' @@ -27,6 +28,7 @@ def _is_devserver(): _GCE_DEFAULT_ZONE = 'us-central1-f' _GCE_DEFAULT_MACHINE_TYPE = 'n1-standard-1' + class CloudConfig(ndb.Model): control_bucket_path = ndb.StringProperty(default=_DEFAULT_CONTROL_BUCKET_PATH) setup_scheme = 'http' if _is_devserver() else 'https' @@ -43,6 +45,7 @@ class CloudConfig(ndb.Model): default='%s/traces' % app_identity.get_default_gcs_bucket_name()) catapult_path = ndb.StringProperty(default=_DEFAULT_CATAPULT_PATH) + def Get(): config = CloudConfig.get_by_id(_CONFIG_KEY_NAME) if not config: diff --git a/perf_insights/perf_insights/cloud_storage.py b/perf_insights/perf_insights/cloud_storage.py index 809b91420a1..0df163867c3 100644 --- a/perf_insights/perf_insights/cloud_storage.py +++ b/perf_insights/perf_insights/cloud_storage.py @@ -12,6 +12,7 @@ class CloudStorageError(Exception): + @staticmethod def _GetConfigInstructions(): command = _GSUTIL_PATH @@ -22,6 +23,7 @@ def _GetConfigInstructions(): class PermissionError(CloudStorageError): + def __init__(self): super(PermissionError, self).__init__( 'Attempted to access a file from Cloud Storage but you don\'t ' @@ -29,6 +31,7 @@ def __init__(self): class CredentialsError(CloudStorageError): + def __init__(self): super(CredentialsError, self).__init__( 'Attempted to access a file from Cloud Storage but you have no ' diff --git a/perf_insights/perf_insights/corpus_driver.py b/perf_insights/perf_insights/corpus_driver.py index f2b571afb03..403d83cbf61 100644 --- a/perf_insights/perf_insights/corpus_driver.py +++ b/perf_insights/perf_insights/corpus_driver.py @@ -4,6 +4,8 @@ import os import sys + class CorpusDriver(object): + def GetTraceHandlesMatchingQuery(self, query): raise NotImplementedError() diff --git a/perf_insights/perf_insights/corpus_query.py b/perf_insights/perf_insights/corpus_query.py index d55d69b1e58..327598421cd 100644 --- a/perf_insights/perf_insights/corpus_query.py +++ b/perf_insights/perf_insights/corpus_query.py @@ -9,10 +9,13 @@ import re import datetime + def _InOp(a, b): return a in b + class _ReadField(object): + def __init__(self, fieldName): self.fieldName = fieldName @@ -22,7 +25,9 @@ def AsQueryString(self): def Eval(self, metadata): return metadata[self.fieldName] + class _Constant(object): + def __init__(self, constant): self.constant = constant @@ -43,6 +48,7 @@ def Eval(self, metadata): # pylint: disable=unused-argument return self.constant + def _StringToValue(s): try: constant = eval(s, {}, {}) @@ -74,12 +80,13 @@ def _StringToValue(s): '=': operator.eq, '<': operator.lt, '<=': operator.le, - '>': operator.gt, + '>': operator.gt, '>=': operator.ge, '!=': operator.ne, - ' IN ': _InOp # Spaces matter for proper parsing. + ' IN ': _InOp # Spaces matter for proper parsing. } + def _OperatorToString(op): for k, v in _OPERATORS.iteritems(): if op == v: @@ -93,7 +100,9 @@ def _OperatorToString(op): _TOKEN_SEARCH_ORDER = list(_OPERATORS.keys()) _TOKEN_SEARCH_ORDER.sort(lambda x, y: len(y) - len(x)) + class Filter(object): + def __init__(self, a, op, b): self.a = a self.op = op @@ -142,7 +151,9 @@ def FromString(s): _OPERATORS[found_op_key], rvalue) + class CorpusQuery(object): + def __init__(self): self.max_trace_handles = None self.filters = [] @@ -169,7 +180,7 @@ def AsGQLWhereClause(self): else: b_string = f.b.fieldName - filter_strings.append( '%s %s %s' % (a_string, + filter_strings.append('%s %s %s' % (a_string, _OperatorToString(f.op).strip(), b_string)) gql = 'WHERE ' + ' AND '.join(filter_strings) diff --git a/perf_insights/perf_insights/corpus_query_unittest.py b/perf_insights/perf_insights/corpus_query_unittest.py index 76ad536fbcc..06359d0baca 100644 --- a/perf_insights/perf_insights/corpus_query_unittest.py +++ b/perf_insights/perf_insights/corpus_query_unittest.py @@ -7,7 +7,9 @@ from perf_insights import corpus_query + class FilterTests(unittest.TestCase): + def testEqNumber(self): f = corpus_query.Filter.FromString("a = 3") @@ -65,6 +67,7 @@ def testDateComparison(self): class CorpusQueryTests(unittest.TestCase): + def testSimple(self): q = corpus_query.CorpusQuery.FromString('') self.assertTrue(q.Eval({'a': 1})) diff --git a/perf_insights/perf_insights/endpoints/cloud_mapper/cloud_helper.py b/perf_insights/perf_insights/endpoints/cloud_mapper/cloud_helper.py index 7ff108a3ec2..cda06a57258 100644 --- a/perf_insights/perf_insights/endpoints/cloud_mapper/cloud_helper.py +++ b/perf_insights/perf_insights/endpoints/cloud_mapper/cloud_helper.py @@ -16,9 +16,11 @@ max_retry_period=15) gcs.set_default_retry_params(default_retry_params) + def _remove_gcs_prefix(full_url): return full_url.split('gs:/')[1] + def WriteGCS(fullurl, data): gcs_file = gcs.open(_remove_gcs_prefix(fullurl), 'w', @@ -28,6 +30,7 @@ def WriteGCS(fullurl, data): gcs_file.write(data) gcs_file.close() + def ReadGCS(fullurl): gcs_file = gcs.open(_remove_gcs_prefix(fullurl), 'r', @@ -38,9 +41,11 @@ def ReadGCS(fullurl): return contents + def ReadGCSToFile(fullurl, output_file): output_file.write(ReadGCS(fullurl)) + def StatGCS(fullurl): try: return gcs.stat(_remove_gcs_prefix(fullurl)) diff --git a/perf_insights/perf_insights/endpoints/cloud_mapper/create.py b/perf_insights/perf_insights/endpoints/cloud_mapper/create.py index 1fd42c09903..1620a8c6b3b 100644 --- a/perf_insights/perf_insights/endpoints/cloud_mapper/create.py +++ b/perf_insights/perf_insights/endpoints/cloud_mapper/create.py @@ -10,6 +10,7 @@ from google.appengine.api import taskqueue from perf_insights.endpoints.cloud_mapper import job_info + class CreatePage(webapp2.RequestHandler): def post(self): diff --git a/perf_insights/perf_insights/endpoints/cloud_mapper/task.py b/perf_insights/perf_insights/endpoints/cloud_mapper/task.py index c87dd8186a3..8807372a584 100644 --- a/perf_insights/perf_insights/endpoints/cloud_mapper/task.py +++ b/perf_insights/perf_insights/endpoints/cloud_mapper/task.py @@ -43,9 +43,9 @@ def _DispatchTracesAndWaitForResult(self, job, traces, num_instances): def _slice_it(li, cols=2): start = 0 for i in xrange(cols): - stop = start + len(li[i::cols]) - yield li[start:stop] - start = stop + stop = start + len(li[i::cols]) + yield li[start:stop] + start = stop # TODO(simonhatch): In the future it might be possibly to only specify a # reducer and no mapper. Revisit this. diff --git a/perf_insights/perf_insights/endpoints/cloud_mapper/test.py b/perf_insights/perf_insights/endpoints/cloud_mapper/test.py index b3b348a99e7..85a03686c04 100644 --- a/perf_insights/perf_insights/endpoints/cloud_mapper/test.py +++ b/perf_insights/perf_insights/endpoints/cloud_mapper/test.py @@ -14,8 +14,9 @@ from perf_insights.endpoints.cloud_mapper import job_info from perf_insights import cloud_config + def _is_devserver(): - return os.environ.get('SERVER_SOFTWARE','').startswith('Development') + return os.environ.get('SERVER_SOFTWARE', '').startswith('Development') _DEFAULT_MAPPER = """ @@ -71,6 +72,7 @@ def _is_devserver(): """ + class TestPage(webapp2.RequestHandler): def get(self): diff --git a/perf_insights/perf_insights/endpoints/cloud_mapper/worker.py b/perf_insights/perf_insights/endpoints/cloud_mapper/worker.py index b9b3138a552..63dfa386d3e 100644 --- a/perf_insights/perf_insights/endpoints/cloud_mapper/worker.py +++ b/perf_insights/perf_insights/endpoints/cloud_mapper/worker.py @@ -23,6 +23,7 @@ class EnvVarModifier(object): + def __init__(self, **kwargs): self._vars = {} self._kwargs = kwargs @@ -39,7 +40,7 @@ def __exit__(self, *_): def _is_devserver(): - server_software = os.environ.get('SERVER_SOFTWARE','') + server_software = os.environ.get('SERVER_SOFTWARE', '') return server_software and server_software.startswith('Development') @@ -72,6 +73,7 @@ def _WorkLoop(): class TaskPage(webapp2.RequestHandler): + def post(self): os.putenv('PI_CLOUD_WORKER', '1') try: diff --git a/perf_insights/perf_insights/endpoints/corpus_cleanup.py b/perf_insights/perf_insights/endpoints/corpus_cleanup.py index 39a93171d20..9c7df2a997d 100644 --- a/perf_insights/perf_insights/endpoints/corpus_cleanup.py +++ b/perf_insights/perf_insights/endpoints/corpus_cleanup.py @@ -13,8 +13,9 @@ from perf_insights.trace_info import TraceInfo import third_party.cloudstorage as gcs -BATCH_SIZE=100 -MAX_DAYS=30 +BATCH_SIZE = 100 +MAX_DAYS = 30 + class CorpusCleanupPage(webapp2.RequestHandler): diff --git a/perf_insights/perf_insights/function_handle_unittest.py b/perf_insights/perf_insights/function_handle_unittest.py index ab48dc687d9..a6a63829899 100644 --- a/perf_insights/perf_insights/function_handle_unittest.py +++ b/perf_insights/perf_insights/function_handle_unittest.py @@ -6,6 +6,7 @@ from perf_insights import function_handle + class ModuleToLoadTests(unittest.TestCase): def testExactlyOneHrefOrFilename(self): @@ -69,7 +70,7 @@ def testAsDict(self): self.assertEquals( handle.AsDict(), { - 'modules_to_load' : [{'href': '/foo'}], + 'modules_to_load': [{'href': '/foo'}], 'function_name': 'Bar' }) diff --git a/perf_insights/perf_insights/gcs_trace_handle.py b/perf_insights/perf_insights/gcs_trace_handle.py index 1c169c3c004..84cebf3230f 100644 --- a/perf_insights/perf_insights/gcs_trace_handle.py +++ b/perf_insights/perf_insights/gcs_trace_handle.py @@ -8,6 +8,7 @@ class GCSTraceHandle(trace_handle.TraceHandle): + def __init__(self, run_info, cache_directory): super(GCSTraceHandle, self).__init__(run_info) file_name = run_info.run_id.split('/')[-1] diff --git a/perf_insights/perf_insights/in_memory_trace_handle.py b/perf_insights/perf_insights/in_memory_trace_handle.py index 300148039b2..02445b6d745 100644 --- a/perf_insights/perf_insights/in_memory_trace_handle.py +++ b/perf_insights/perf_insights/in_memory_trace_handle.py @@ -8,6 +8,7 @@ class InMemoryTraceHandle(trace_handle.TraceHandle): + def __init__(self, run_info, data): super(InMemoryTraceHandle, self).__init__(run_info) self.data = data diff --git a/perf_insights/perf_insights/local_directory_corpus_driver.py b/perf_insights/perf_insights/local_directory_corpus_driver.py index fcbf5cfcba3..c1f9a1e73f5 100644 --- a/perf_insights/perf_insights/local_directory_corpus_driver.py +++ b/perf_insights/perf_insights/local_directory_corpus_driver.py @@ -26,6 +26,7 @@ def _GetFilesIn(basedir): data_files.sort() return data_files + def _GetTagsForRelPath(relpath): # Tags. sub_dir = os.path.dirname(relpath) @@ -34,6 +35,7 @@ def _GetTagsForRelPath(relpath): parts = sub_dir.split(os.sep) return [p for p in parts if len(p) > 0] + def _GetMetadataForFilename(base_directory, filename): relpath = os.path.relpath(filename, base_directory) tags = _GetTagsForRelPath(relpath) @@ -43,10 +45,13 @@ def _GetMetadataForFilename(base_directory, filename): # TODO(nduca): Add modification time to metadata. return metadata + def _DefaultUrlResover(abspath): return 'file:///%s' % abspath + class LocalDirectoryCorpusDriver(corpus_driver.CorpusDriver): + def __init__(self, trace_directory, url_resolver=_DefaultUrlResover): self.directory = trace_directory self.url_resolver = url_resolver diff --git a/perf_insights/perf_insights/local_directory_corpus_driver_unittest.py b/perf_insights/perf_insights/local_directory_corpus_driver_unittest.py index c9f9ccaf9d9..71350795365 100644 --- a/perf_insights/perf_insights/local_directory_corpus_driver_unittest.py +++ b/perf_insights/perf_insights/local_directory_corpus_driver_unittest.py @@ -5,7 +5,9 @@ import unittest from perf_insights import local_directory_corpus_driver + class LocalDirectoryCorpusDriverTests(unittest.TestCase): + def testTags(self): self.assertEquals( local_directory_corpus_driver._GetTagsForRelPath('a.json'), []) diff --git a/perf_insights/perf_insights/local_file_trace_handle.py b/perf_insights/perf_insights/local_file_trace_handle.py index 0b28efe8fbe..206cd10af46 100644 --- a/perf_insights/perf_insights/local_file_trace_handle.py +++ b/perf_insights/perf_insights/local_file_trace_handle.py @@ -5,6 +5,7 @@ class LocalFileTraceHandle(trace_handle.TraceHandle): + def __init__(self, run_info, filename): super(LocalFileTraceHandle, self).__init__(run_info) self.filename = filename diff --git a/perf_insights/perf_insights/map_runner.py b/perf_insights/perf_insights/map_runner.py index 158cd483bb4..20aa13b01bb 100644 --- a/perf_insights/perf_insights/map_runner.py +++ b/perf_insights/perf_insights/map_runner.py @@ -17,12 +17,16 @@ AUTO_JOB_COUNT = -1 + class MapError(Exception): + def __init__(self, *args): super(MapError, self).__init__(*args) self.run_info = None + class MapRunner(object): + def __init__(self, trace_handles, map_function_handle, stop_on_error=False, progress_reporter=None, jobs=AUTO_JOB_COUNT, diff --git a/perf_insights/perf_insights/map_single_trace.py b/perf_insights/perf_insights/map_single_trace.py index 88fc651a7fd..d217d34ac13 100644 --- a/perf_insights/perf_insights/map_single_trace.py +++ b/perf_insights/perf_insights/map_single_trace.py @@ -15,6 +15,7 @@ class TemporaryMapScript(object): + def __init__(self, js): self.file = tempfile.NamedTemporaryFile() self.file.write(""" @@ -35,24 +36,29 @@ def __exit__(self, *args, **kwargs): @property def filename(self): - return self.file.name + return self.file.name class FunctionLoadingErrorValue(value_module.FailureValue): pass + class FunctionNotDefinedErrorValue(value_module.FailureValue): pass + class MapFunctionErrorValue(value_module.FailureValue): pass + class TraceImportErrorValue(value_module.FailureValue): pass + class NoResultsAddedErrorValue(value_module.FailureValue): pass + class InternalMapError(Exception): pass @@ -64,6 +70,7 @@ class InternalMapError(Exception): 'NoResultsAddedError': NoResultsAddedErrorValue } + def MapSingleTrace(results, trace_handle, map_function_handle): project = perf_insights_project.PerfInsightsProject() @@ -108,8 +115,7 @@ def MapSingleTrace(results, trace_handle, map_function_handle): 'vinn runtime error while mapping trace.', 'Unknown stack')) return - - found_at_least_one_result=False + found_at_least_one_result = False for line in res.stdout.split('\n'): m = re.match('^MAP_RESULT_VALUE: (.+)', line, re.DOTALL) if m: diff --git a/perf_insights/perf_insights/map_single_trace_unittest.py b/perf_insights/perf_insights/map_single_trace_unittest.py index 840c3740532..8e71139b488 100644 --- a/perf_insights/perf_insights/map_single_trace_unittest.py +++ b/perf_insights/perf_insights/map_single_trace_unittest.py @@ -17,6 +17,8 @@ def _Handle(filename): module = function_handle.ModuleToLoad(filename=filename) return function_handle.FunctionHandle(modules_to_load=[module], function_name='MyMapFunction') + + class MapSingleTraceTests(unittest.TestCase): def testPassingMapScript(self): @@ -31,7 +33,6 @@ def testPassingMapScript(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, json.dumps(events)) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" pi.FunctionRegistry.register( @@ -57,7 +58,6 @@ def testTraceDidntImport(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, trace_string) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" pi.FunctionRegistry.register( @@ -83,7 +83,6 @@ def testMapFunctionThatThrows(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, json.dumps(events)) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" pi.FunctionRegistry.register( @@ -110,7 +109,6 @@ def testMapperWithLoadeError(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, json.dumps(events)) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" throw new Error('Expected load error'); @@ -122,7 +120,6 @@ def testMapperWithLoadeError(self): v = results.all_values[0] self.assertIsInstance(v, map_single_trace.FunctionLoadingErrorValue) - def testNoMapper(self): run_info = run_info_module.RunInfo('file:///a.json', '/a.json', metadata={'m': 1}) @@ -135,7 +132,6 @@ def testNoMapper(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, json.dumps(events)) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" """) as map_script: @@ -146,7 +142,6 @@ def testNoMapper(self): v = results.all_values[0] self.assertIsInstance(v, map_single_trace.FunctionNotDefinedErrorValue) - def testMapperDoesntAddValues(self): run_info = run_info_module.RunInfo('file:///a.json', '/a.json', metadata={'m': 1}) @@ -159,7 +154,6 @@ def testMapperDoesntAddValues(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, json.dumps(events)) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" pi.FunctionRegistry.register( @@ -185,7 +179,6 @@ def testMapperSkips(self): trace_handle = in_memory_trace_handle.InMemoryTraceHandle( run_info, json.dumps(events)) - results = results_module.Results() with map_single_trace.TemporaryMapScript(""" pi.FunctionRegistry.register( diff --git a/perf_insights/perf_insights/map_traces_handler.py b/perf_insights/perf_insights/map_traces_handler.py index 47466a71aa9..ea9b0b6d533 100644 --- a/perf_insights/perf_insights/map_traces_handler.py +++ b/perf_insights/perf_insights/map_traces_handler.py @@ -3,9 +3,12 @@ # found in the LICENSE file. import webapp2 + def MapTrace(trace_corpus_driver): # pylint: disable=unused-argument pass + class MapTracesHandler(webapp2.RequestHandler): + def post(self, *args, **kwargs): # pylint: disable=unused-argument - pass \ No newline at end of file + pass diff --git a/perf_insights/perf_insights/mre/failure.py b/perf_insights/perf_insights/mre/failure.py index f464d1fb9a8..fef100b8d46 100644 --- a/perf_insights/perf_insights/mre/failure.py +++ b/perf_insights/perf_insights/mre/failure.py @@ -15,7 +15,7 @@ def __init__(self, job_guid, function_handle_guid, trace_guid, self.stack = stack def AsDict(self): - return { + return { 'job_guid': self.job_guid, 'function_handle_guid': self.function_handle_guid, 'trace_guid': self.trace_guid, diff --git a/perf_insights/perf_insights/mre/job_results.py b/perf_insights/perf_insights/mre/job_results.py index f7bd5dac944..21a2165f6b8 100644 --- a/perf_insights/perf_insights/mre/job_results.py +++ b/perf_insights/perf_insights/mre/job_results.py @@ -16,11 +16,11 @@ def __init__(self, failures=None, reduce_results=None): @property def failures(self): - return self._failures + return self._failures @property def reduce_results(self): - return self._reduce_results + return self._reduce_results def AsDict(self): return { diff --git a/perf_insights/perf_insights/mre/threaded_work_queue.py b/perf_insights/perf_insights/mre/threaded_work_queue.py index fccd16eecdc..bbb083a2138 100644 --- a/perf_insights/perf_insights/mre/threaded_work_queue.py +++ b/perf_insights/perf_insights/mre/threaded_work_queue.py @@ -9,7 +9,9 @@ import traceback import Queue + class ThreadedWorkQueue: + def __init__(self, num_threads): self._num_threads = num_threads @@ -99,10 +101,10 @@ def _RunSingleThreaded(self): def _RunMultiThreaded(self): threads = [] for _ in range(self._num_threads): - t = threading.Thread(target=self._ThreadMain) - t.setDaemon(True) - t.start() - threads.append(t) + t = threading.Thread(target=self._ThreadMain) + t.setDaemon(True) + t.start() + threads.append(t) while True: if self._stop: diff --git a/perf_insights/perf_insights/mre/threaded_work_queue_unittest.py b/perf_insights/perf_insights/mre/threaded_work_queue_unittest.py index ce693e468a8..f987a3c6c83 100644 --- a/perf_insights/perf_insights/mre/threaded_work_queue_unittest.py +++ b/perf_insights/perf_insights/mre/threaded_work_queue_unittest.py @@ -5,7 +5,9 @@ from perf_insights.mre import threaded_work_queue + class ThreadedWorkQueueTests(unittest.TestCase): + def testSingleThreaded(self): wq = threaded_work_queue.ThreadedWorkQueue(num_threads=1) self._RunSimpleDecrementingTest(wq) @@ -17,6 +19,7 @@ def testMultiThreaded(self): def _RunSimpleDecrementingTest(self, wq): remaining = [10] + def Decrement(): remaining[0] -= 1 if remaining[0]: diff --git a/perf_insights/perf_insights/perf_insights_corpus_driver.py b/perf_insights/perf_insights/perf_insights_corpus_driver.py index ceabff6d01e..5d35adc3ca7 100644 --- a/perf_insights/perf_insights/perf_insights_corpus_driver.py +++ b/perf_insights/perf_insights/perf_insights_corpus_driver.py @@ -14,7 +14,9 @@ _DEFAULT_PERF_INSIGHTS_SERVER = 'http://performance-insights.appspot.com' + class PerfInsightsCorpusDriver(corpus_driver.CorpusDriver): + def __init__(self, cache_directory, server=_DEFAULT_PERF_INSIGHTS_SERVER): self.directory = cache_directory self.server = server diff --git a/perf_insights/perf_insights/progress_reporter.py b/perf_insights/perf_insights/progress_reporter.py index b13270e35c3..6dd15f606cf 100644 --- a/perf_insights/perf_insights/progress_reporter.py +++ b/perf_insights/perf_insights/progress_reporter.py @@ -2,7 +2,9 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. + class RunReporter(object): + def __init__(self, run_info): self.run_info = run_info @@ -16,8 +18,9 @@ def DidRun(self, run_failed): # Derived from telemetry ProgressReporter. Should stay close in architecture # to telemetry ProgressReporter. class ProgressReporter(object): + def WillRun(self, run_info): return RunReporter(run_info) def DidFinishAllRuns(self, results): - pass \ No newline at end of file + pass diff --git a/perf_insights/perf_insights/results/__init__.py b/perf_insights/perf_insights/results/__init__.py index 61cd219f8f8..5127479e43a 100644 --- a/perf_insights/perf_insights/results/__init__.py +++ b/perf_insights/perf_insights/results/__init__.py @@ -4,7 +4,9 @@ from perf_insights import value as value_module + class Results(object): + def __init__(self): self.all_values = [] self._run_infos_that_have_failures = set() diff --git a/perf_insights/perf_insights/results/gtest_progress_reporter.py b/perf_insights/perf_insights/results/gtest_progress_reporter.py index cf9d778367d..680763d5156 100644 --- a/perf_insights/perf_insights/results/gtest_progress_reporter.py +++ b/perf_insights/perf_insights/results/gtest_progress_reporter.py @@ -9,6 +9,7 @@ class GTestRunReporter(progress_reporter.RunReporter): + def __init__(self, run_info, output_stream, timestamp): super(GTestRunReporter, self).__init__(run_info) self._output_stream = output_stream diff --git a/perf_insights/perf_insights/results/json_output_formatter.py b/perf_insights/perf_insights/results/json_output_formatter.py index 2ee487739cf..cb3b949c416 100644 --- a/perf_insights/perf_insights/results/json_output_formatter.py +++ b/perf_insights/perf_insights/results/json_output_formatter.py @@ -7,6 +7,7 @@ class JSONOutputFormatter(output_formatter.OutputFormatter): + def __init__(self, output_file): # TODO(nduca): Resolve output_file here vs output_stream in base class. super(JSONOutputFormatter, self).__init__(output_file) @@ -16,4 +17,4 @@ def Format(self, results): d = results.AsDict() json.dump(d, self.output_file, indent=2) if hasattr(self.output_file, 'flush'): - self.output_file.flush() \ No newline at end of file + self.output_file.flush() diff --git a/perf_insights/perf_insights/results/output_formatter.py b/perf_insights/perf_insights/results/output_formatter.py index 221c7a20b41..5a4be13acce 100644 --- a/perf_insights/perf_insights/results/output_formatter.py +++ b/perf_insights/perf_insights/results/output_formatter.py @@ -4,7 +4,10 @@ # Derived from telemetry OutputFormatter. Should stay close in architecture # to telemetry OutputFormatter. + + class OutputFormatter(object): + def __init__(self, output_stream): self._output_stream = output_stream @@ -13,4 +16,4 @@ def Format(self, results): @property def output_stream(self): - return self._output_stream \ No newline at end of file + return self._output_stream diff --git a/perf_insights/perf_insights/trace_handle.py b/perf_insights/perf_insights/trace_handle.py index a4abf979e96..23ef2a883bb 100644 --- a/perf_insights/perf_insights/trace_handle.py +++ b/perf_insights/perf_insights/trace_handle.py @@ -3,7 +3,9 @@ # found in the LICENSE file. import uuid + class TraceHandle(object): + def __init__(self, run_info): self.run_info = run_info diff --git a/perf_insights/perf_insights/value/__init__.py b/perf_insights/perf_insights/value/__init__.py index 7cf91eb4356..6848c8e5779 100644 --- a/perf_insights/perf_insights/value/__init__.py +++ b/perf_insights/perf_insights/value/__init__.py @@ -4,7 +4,10 @@ # Simplified version of telemetry Value system, just enough for us to get # perf_insights up and running. + + class Value(object): + def __init__(self, run_info, name, units, description=None, important=False, ir_stable_id=None): self.run_info = run_info @@ -53,7 +56,8 @@ def FromDict(cls, run_info, d): class DictValue(Value): - def __init__(self, run_info, name, value, description=None, important=False, + + def __init__(self, run_info, name, value, description=None, important=False, ir_stable_id=None): assert isinstance(value, dict) super(DictValue, self).__init__(run_info, name, units=None, @@ -81,13 +85,14 @@ def FromDict(cls, run_info, d): @property def value(self): - return self._value + return self._value def __getitem__(self, key): return self._value[key] class FailureValue(Value): + def __init__(self, run_info, failure_type_name, description, stack, important=False, ir_stable_id=None): super(FailureValue, self).__init__(run_info, @@ -122,6 +127,7 @@ def GetGTestPrintString(self): class SkipValue(Value): + def __init__(self, run_info, skipped_result_name, description=None, important=False, ir_stable_id=None): super(SkipValue, self).__init__(run_info, diff --git a/perf_insights/perf_insights/value/run_info.py b/perf_insights/perf_insights/value/run_info.py index fd74ef0995b..fc19dde8d4f 100644 --- a/perf_insights/perf_insights/value/run_info.py +++ b/perf_insights/perf_insights/value/run_info.py @@ -10,6 +10,7 @@ class RunInfo(object): + def __init__(self, url, display_name=None, run_id=None, metadata=None): if run_id is not None: self.run_id = run_id diff --git a/perf_insights/perf_insights/value/value_unittest.py b/perf_insights/perf_insights/value/value_unittest.py index 28e060e7aff..96ec4d79113 100644 --- a/perf_insights/perf_insights/value/value_unittest.py +++ b/perf_insights/perf_insights/value/value_unittest.py @@ -6,7 +6,9 @@ from perf_insights import value as value_module from perf_insights.value import run_info as run_info_module + class ValueTests(unittest.TestCase): + def testDict(self): run_info = run_info_module.RunInfo('file:///a.json', '/a.json', metadata={'m': 1}) @@ -23,7 +25,6 @@ def testDict(self): self.assertEquals(d, d2) - def testFailure(self): run_info = run_info_module.RunInfo('file:///a.json', '/a.json', metadata={'m': 1}) @@ -40,4 +41,4 @@ def testFailure(self): self.assertTrue(isinstance(v, value_module.FailureValue)) d2 = v.AsDict() - self.assertEquals(d, d2) \ No newline at end of file + self.assertEquals(d, d2) diff --git a/perf_insights/perf_insights_build/perf_insights_dev_server_config.py b/perf_insights/perf_insights_build/perf_insights_dev_server_config.py index 346b30e2900..a0737190e9f 100644 --- a/perf_insights/perf_insights_build/perf_insights_dev_server_config.py +++ b/perf_insights/perf_insights_build/perf_insights_dev_server_config.py @@ -44,8 +44,8 @@ def post(self, *args, **kwargs): # pylint: disable=unused-argument self.app) corpus_driver = local_directory_corpus_driver.LocalDirectoryCorpusDriver( - trace_directory = kwargs.pop('_pi_data_dir'), - url_resolver = self.app.GetURLForAbsFilename) + trace_directory=kwargs.pop('_pi_data_dir'), + url_resolver=self.app.GetURLForAbsFilename) # TODO(nduca): pass self.request.params to the map function [maybe]. query_string = self.request.get('corpus_query', 'True') diff --git a/perf_insights/perf_insights_build/run_vinn_tests.py b/perf_insights/perf_insights_build/run_vinn_tests.py index 21418d5e31c..e2263718340 100644 --- a/perf_insights/perf_insights_build/run_vinn_tests.py +++ b/perf_insights/perf_insights_build/run_vinn_tests.py @@ -30,14 +30,16 @@ def RunTests(): js_args=d8_test_module_filenames, stdout=sys.stdout, stdin=sys.stdin) return res.returncode + def Main(argv): parser = argparse.ArgumentParser( description='Run d8 tests.') parser.add_argument( - '--no-install-hooks', dest='install_hooks', action='store_false') + '--no-install-hooks', dest='install_hooks', action='store_false') parser.set_defaults(install_hooks=True) args = parser.parse_args(argv[1:]) if args.install_hooks: install.InstallHooks() - sys.exit(RunTests()) \ No newline at end of file + sys.exit(RunTests()) + diff --git a/pylintrc b/pylintrc index 3856f1b76c6..04e0feec2dd 100644 --- a/pylintrc +++ b/pylintrc @@ -31,12 +31,9 @@ disable = too-many-return-statements, too-many-statements, anomalous-backslash-in-string, - bad-indentation, - bad-whitespace, eval-used, function-redefined, import-error, - missing-final-newline, no-init, no-name-in-module, old-style-class, diff --git a/systrace/systrace/agents/atrace_agent.py b/systrace/systrace/agents/atrace_agent.py index 89544e95c89..c45f306b99c 100644 --- a/systrace/systrace/agents/atrace_agent.py +++ b/systrace/systrace/agents/atrace_agent.py @@ -38,16 +38,16 @@ # This list is based on the tags in frameworks/native/include/utils/Trace.h for # legacy platform. LEGACY_TRACE_TAG_BITS = ( - ('gfx', 1<<1), - ('input', 1<<2), - ('view', 1<<3), - ('webview', 1<<4), - ('wm', 1<<5), - ('am', 1<<6), - ('sm', 1<<7), - ('audio', 1<<8), - ('video', 1<<9), - ('camera', 1<<10), + ('gfx', 1 << 1), + ('input', 1 << 2), + ('view', 1 << 3), + ('webview', 1 << 4), + ('wm', 1 << 5), + ('am', 1 << 6), + ('sm', 1 << 7), + ('audio', 1 << 8), + ('video', 1 << 9), + ('camera', 1 << 10), ) @@ -75,6 +75,7 @@ def try_create_agent(options, categories): class AtraceAgent(systrace_agent.SystraceAgent): + def __init__(self, options, categories): super(AtraceAgent, self).__init__(options, categories) self._expect_trace = False @@ -312,7 +313,9 @@ def _preprocess_trace_data(self, trace_data): return trace_data + class AtraceLegacyAgent(AtraceAgent): + def _construct_list_categories_command(self): LEGACY_CATEGORIES = """ sched - CPU Scheduling freq - CPU Frequency @@ -376,6 +379,7 @@ def _construct_extra_trace_command(self): return extra_args + class BootAgent(AtraceAgent): """AtraceAgent that specializes in tracing the boot sequence.""" @@ -419,6 +423,7 @@ def _construct_trace_command(self): atrace_args + ['&&'] + setprop_args + ['&&'] + rm_args, self._options.device_serial) + class FileReaderThread(threading.Thread): """Reads data from a file/pipe on a worker thread. @@ -479,6 +484,7 @@ def set_chunk_size(self, chunk_size): assert chunk_size > 0 self._chunk_size = chunk_size + def get_default_categories(device_serial): categories_output, return_code = util.run_adb_shell(LIST_CATEGORIES_ARGS, device_serial) @@ -512,7 +518,7 @@ def extract_thread_list(trace_text): """ threads = {} - #start at line 1 to skip the top of the ps dump: + # start at line 1 to skip the top of the ps dump: text = trace_text.splitlines() for line in text[1:]: cols = line.split(None, 8) @@ -523,6 +529,7 @@ def extract_thread_list(trace_text): return threads + def extract_tgids(trace_text): """Removes the procfs dump from the given trace text Args: @@ -535,11 +542,12 @@ def extract_tgids(trace_text): for line in text: result = re.match('^/proc/([0-9]+)/task/([0-9]+)', line) if result: - parent_pid, tgid = result.group(1,2) + parent_pid, tgid = result.group(1, 2) tgid_2pid[tgid] = parent_pid return tgid_2pid + def strip_and_decompress_trace(trace_data): """Fixes new-lines and decompresses trace data. @@ -572,7 +580,6 @@ def strip_and_decompress_trace(trace_data): return trace_data - def fix_thread_names(trace_data, thread_names): """Replaces thread ids with their names. @@ -615,9 +622,9 @@ def repl(m): tid = m.group(2) if (int(tid) > 0 and m.group(1) != '' and m.group(3) == '(-----)' and tid in pid2_tgid): - # returns Proc_name-PID (TGID) - # Binder_2-381 (-----) becomes Binder_2-381 (128) - return m.group(1) + '-' + m.group(2) + ' ( '+ pid2_tgid[tid]+ ')' + # returns Proc_name-PID (TGID) + # Binder_2-381 (-----) becomes Binder_2-381 (128) + return m.group(1) + '-' + m.group(2) + ' ( ' + pid2_tgid[tid] + ')' return m.group(0) @@ -628,7 +635,6 @@ def repl(m): return trace_data - def fix_circular_traces(out): """Fix inconsistentcies in traces due to circular buffering. @@ -665,6 +671,7 @@ def fix_circular_traces(out): out = out[:end_of_header] + out[start_of_full_trace:] return out + def do_popen(args): try: adb = subprocess.Popen(args, stdout=subprocess.PIPE, @@ -678,6 +685,7 @@ def do_popen(args): return adb + def do_preprocess_adb_cmd(command, serial): args = [command] dump, ret_code = util.run_adb_shell(args, serial) diff --git a/systrace/systrace/agents/atrace_agent_unittest.py b/systrace/systrace/agents/atrace_agent_unittest.py index 7751169b6d1..63da46a41a5 100644 --- a/systrace/systrace/agents/atrace_agent_unittest.py +++ b/systrace/systrace/agents/atrace_agent_unittest.py @@ -55,6 +55,7 @@ class AtraceAgentTest(unittest.TestCase): + def test_construct_trace_command(self): options, categories = systrace.parse_options(SYSTRACE_CMD) agent = atrace_agent.AtraceAgent(options, categories) @@ -62,10 +63,9 @@ def test_construct_trace_command(self): self.assertEqual(' '.join(TRACE_CMD), ' '.join(tracer_args)) self.assertEqual(True, agent.expect_trace()) - def test_extract_thread_list(self): with contextlib.nested(open(ATRACE_EXTRACTED_THREADS, 'r'), - open(ATRACE_THREAD_LIST)) as (f1,f2): + open(ATRACE_THREAD_LIST)) as (f1, f2): atrace_result = f1.read() ps_dump = f2.read() @@ -150,6 +150,7 @@ def test_fix_missing_tgids(self): class AtraceLegacyAgentTest(unittest.TestCase): + def test_construct_trace_command(self): options, categories = systrace.parse_options(SYSTRACE_CMD) agent = atrace_agent.AtraceLegacyAgent(options, categories) @@ -159,6 +160,7 @@ def test_construct_trace_command(self): class BootAgentTest(unittest.TestCase): + def test_boot(self): options, categories = systrace.parse_options(SYSTRACE_BOOT_CMD) agent = atrace_agent.BootAgent(options, categories) diff --git a/systrace/systrace/agents/ftrace_agent.py b/systrace/systrace/agents/ftrace_agent.py index 47d8cfe9f4f..3c6ae51f1f0 100644 --- a/systrace/systrace/agents/ftrace_agent.py +++ b/systrace/systrace/agents/ftrace_agent.py @@ -8,7 +8,9 @@ import systrace_agent + class FtraceAgentIo(object): + @staticmethod def writeFile(path, data): with open(path, 'w') as f: @@ -23,53 +25,71 @@ def readFile(path): def haveWritePermissions(path): return os.access(path, os.W_OK) -FT_DIR = "/sys/kernel/debug/tracing/" -FT_CLOCK = FT_DIR + "trace_clock" -FT_BUFFER_SIZE = FT_DIR + "buffer_size_kb" -FT_TRACER = FT_DIR + "current_tracer" -FT_PRINT_TGID = FT_DIR + "options/print-tgid" -FT_TRACE_ON = FT_DIR + "tracing_on" -FT_TRACE = FT_DIR + "trace" +FT_DIR = "/sys/kernel/debug/tracing/" +FT_CLOCK = FT_DIR + "trace_clock" +FT_BUFFER_SIZE = FT_DIR + "buffer_size_kb" +FT_TRACER = FT_DIR + "current_tracer" +FT_PRINT_TGID = FT_DIR + "options/print-tgid" +FT_TRACE_ON = FT_DIR + "tracing_on" +FT_TRACE = FT_DIR + "trace" FT_TRACE_MARKER = FT_DIR + "trace_marker" -FT_OVERWRITE = FT_DIR + "options/overwrite" +FT_OVERWRITE = FT_DIR + "options/overwrite" all_categories = { - "sched" : {"desc" : "CPU Scheduling", - "req" : ["sched/sched_switch/", "sched/sched_wakeup/"]}, - "freq" : {"desc" : "CPU Frequency", - "req" : ["power/cpu_frequency/", "power/clock_set_rate/"]}, - "irq" : {"desc" : "CPU IRQS and IPIS", - "req" : ["irq/"], - "opt" : ["ipi/"]}, - "workq" : {"desc" : "Kernel workqueues", - "req" : ["workqueue/"]}, - "memreclaim" : {"desc" : "Kernel Memory Reclaim", - "req" : ["vmscan/mm_vmscan_direct_reclaim_begin/", - "vmscan/mm_vmscan_direct_reclaim_end/", - "vmscan/mm_vmscan_kswapd_wake/", - "vmscan/mm_vmscan_kswapd_sleep/"]}, - "idle" : {"desc" : "CPU Idle", - "req" : ["power/cpu_idle/"]}, - "regulators" : {"desc" : "Voltage and Current Regulators", - "req" : ["regulator/"]}, - "disk" : {"desc" : "Disk I/O", - "req" : ["block/block_rq_issue/", - "block/block_rq_complete/"], - "opt" : ["f2fs/f2fs_sync_file_enter/", - "f2fs/f2fs_sync_file_exit/", - "f2fs/f2fs_write_begin/", - "f2fs/f2fs_write_end/", - "ext4/ext4_da_write_begin/", - "ext4/ext4_da_write_end/", - "ext4/ext4_sync_file_enter/", - "ext4/ext4_sync_file_exit/"]} + "sched": { + "desc": "CPU Scheduling", + "req": ["sched/sched_switch/", "sched/sched_wakeup/"] + }, + "freq": { + "desc": "CPU Frequency", + "req": ["power/cpu_frequency/", "power/clock_set_rate/"] + }, + "irq": { + "desc": "CPU IRQS and IPIS", + "req": ["irq/"], + "opt": ["ipi/"] + }, + "workq": { + "desc": "Kernel workqueues", + "req": ["workqueue/"] + }, + "memreclaim": { + "desc": "Kernel Memory Reclaim", + "req": ["vmscan/mm_vmscan_direct_reclaim_begin/", + "vmscan/mm_vmscan_direct_reclaim_end/", + "vmscan/mm_vmscan_kswapd_wake/", + "vmscan/mm_vmscan_kswapd_sleep/"] + }, + "idle": { + "desc": "CPU Idle", + "req": ["power/cpu_idle/"] + }, + "regulators": { + "desc": "Voltage and Current Regulators", + "req": ["regulator/"] + }, + "disk": { + "desc": "Disk I/O", + "req": ["block/block_rq_issue/", + "block/block_rq_complete/"], + "opt": ["f2fs/f2fs_sync_file_enter/", + "f2fs/f2fs_sync_file_exit/", + "f2fs/f2fs_write_begin/", + "f2fs/f2fs_write_end/", + "ext4/ext4_da_write_begin/", + "ext4/ext4_da_write_end/", + "ext4/ext4_sync_file_enter/", + "ext4/ext4_sync_file_exit/"] + } } + def try_create_agent(options, categories): if options.target != 'linux': return False return FtraceAgent(options, categories, FtraceAgentIo) + class FtraceAgent(systrace_agent.SystraceAgent): def __init__(self, options, categories, fio=FtraceAgentIo): @@ -88,18 +108,18 @@ def __init__(self, options, categories, fio=FtraceAgentIo): self._expect_trace = False def _get_trace_buffer_size(self): - buffer_size = 4096 - if ((self._options.trace_buf_size is not None) - and (self._options.trace_buf_size > 0)): - buffer_size = self._options.trace_buf_size - return buffer_size + buffer_size = 4096 + if ((self._options.trace_buf_size is not None) + and (self._options.trace_buf_size > 0)): + buffer_size = self._options.trace_buf_size + return buffer_size def _get_trace_time(self): - wait_time = 5 - if ((self._options.trace_time is not None) - and (self._options.trace_time > 0)): - wait_time = self._options.trace_time - return wait_time + wait_time = 5 + if ((self._options.trace_time is not None) + and (self._options.trace_time > 0)): + wait_time = self._options.trace_time + return wait_time def start(self): """Start tracing. @@ -190,18 +210,18 @@ def _avail_categories(self): ret = [] for event in all_categories: if self._is_category_available(event): - ret.append(event) + ret.append(event) return ret def _print_avail_categories(self): - avail = self._avail_categories() - if len(avail): - print "tracing options:" - for category in self._avail_categories(): - desc = all_categories[category]["desc"] - print "{0: <16}".format(category), ": ", desc - else: - print "No tracing categories available - perhaps you need root?" + avail = self._avail_categories() + if len(avail): + print "tracing options:" + for category in self._avail_categories(): + desc = all_categories[category]["desc"] + print "{0: <16}".format(category), ": ", desc + else: + print "No tracing categories available - perhaps you need root?" def _category_enable_paths(self, category): events_dir = FT_DIR + "events/" diff --git a/systrace/systrace/agents/ftrace_agent_unittest.py b/systrace/systrace/agents/ftrace_agent_unittest.py index 035ac094a9d..5e4e1bac5c7 100644 --- a/systrace/systrace/agents/ftrace_agent_unittest.py +++ b/systrace/systrace/agents/ftrace_agent_unittest.py @@ -17,10 +17,12 @@ FT_EVENT_DIR = FT_DIR + "events/" FT_TRACE_ON = FT_DIR + "tracing_on" FT_TRACE = FT_DIR + "trace" -FT_BUFFER_SIZE = FT_DIR + "buffer_size_kb" +FT_BUFFER_SIZE = FT_DIR + "buffer_size_kb" + def make_test_io_interface(permitted_files): class TestIoImpl(object): + @staticmethod def writeFile(path, data): permitted_files[path] = data @@ -37,13 +39,14 @@ def haveWritePermissions(path): return path in permitted_files return TestIoImpl + class FtraceAgentTest(unittest.TestCase): def test_avail_categories(self): # sched only has required events permitted_files = { - FT_EVENT_DIR + "sched/sched_switch/enable" : "0", - FT_EVENT_DIR + "sched/sched_wakeup/enable" : "0" + FT_EVENT_DIR + "sched/sched_switch/enable": "0", + FT_EVENT_DIR + "sched/sched_wakeup/enable": "0" } io_interface = make_test_io_interface(permitted_files) options, categories = systrace.parse_options(SYSTRACE_HOST_CMD_DEFAULT) @@ -59,8 +62,8 @@ def test_avail_categories(self): # block has some required, some optional events permitted_files = { - FT_EVENT_DIR + "block/block_rq_complete/enable" : "0", - FT_EVENT_DIR + "block/block_rq_issue/enable" : "0" + FT_EVENT_DIR + "block/block_rq_complete/enable": "0", + FT_EVENT_DIR + "block/block_rq_issue/enable": "0" } io_interface = make_test_io_interface(permitted_files) options, categories = systrace.parse_options(SYSTRACE_HOST_CMD_DEFAULT) @@ -70,11 +73,11 @@ def test_avail_categories(self): def test_tracing_bootstrap(self): workq_event_path = FT_EVENT_DIR + "workqueue/enable" permitted_files = { - workq_event_path : "0", + workq_event_path: "0", FT_TRACE: "x" } io_interface = make_test_io_interface(permitted_files) - systrace_cmd = SYSTRACE_HOST_CMD_DEFAULT + ["workq"] + systrace_cmd = SYSTRACE_HOST_CMD_DEFAULT + ["workq"] options, categories = systrace.parse_options(systrace_cmd) agent = ftrace_agent.FtraceAgent(options, categories, io_interface) self.assertEqual(['workq'], agent._avail_categories()) @@ -104,11 +107,11 @@ def test_tracing_event_enable_disable(self): ipi_event_path = FT_EVENT_DIR + "ipi/enable" irq_event_path = FT_EVENT_DIR + "irq/enable" permitted_files = { - ipi_event_path : "0", - irq_event_path : "0" + ipi_event_path: "0", + irq_event_path: "0" } io_interface = make_test_io_interface(permitted_files) - systrace_cmd = SYSTRACE_HOST_CMD_DEFAULT + ["irq"] + systrace_cmd = SYSTRACE_HOST_CMD_DEFAULT + ["irq"] options, categories = systrace.parse_options(systrace_cmd) agent = ftrace_agent.FtraceAgent(options, categories, io_interface) self.assertEqual(['irq'], agent._avail_categories()) diff --git a/systrace/systrace/systrace-legacy.py b/systrace/systrace/systrace-legacy.py index 859a41643c0..4010fd7aa00 100755 --- a/systrace/systrace/systrace-legacy.py +++ b/systrace/systrace/systrace-legacy.py @@ -14,25 +14,27 @@ # This list is based on the tags in frameworks/native/include/utils/Trace.h. trace_tag_bits = { - 'gfx': 1<<1, - 'input': 1<<2, - 'view': 1<<3, - 'webview': 1<<4, - 'wm': 1<<5, - 'am': 1<<6, - 'sync': 1<<7, - 'audio': 1<<8, - 'video': 1<<9, - 'camera': 1<<10, + 'gfx': 1 << 1, + 'input': 1 << 2, + 'view': 1 << 3, + 'webview': 1 << 4, + 'wm': 1 << 5, + 'am': 1 << 6, + 'sync': 1 << 7, + 'audio': 1 << 8, + 'video': 1 << 9, + 'camera': 1 << 10, } flattened_html_file = 'systrace_trace_viewer.html' + def add_adb_serial(command, serial): if serial != None: command.insert(1, serial) command.insert(1, '-s') + def main(): parser = optparse.OptionParser() parser.add_option('-o', dest='output_file', help='write HTML to FILE', @@ -73,7 +75,7 @@ def main(): type='string', help='') parser.add_option('-e', '--serial', dest='device_serial', type='string', help='adb device serial number') - options, unused_args = parser.parse_args() # pylint: disable=unused-variable + options, unused_args = parser.parse_args() # pylint: disable=unused-variable if options.link_assets or options.asset_dir != 'trace-viewer': parser.error('--link-assets and --asset-dir is deprecated.') @@ -165,7 +167,7 @@ def main(): if line == 'TRACE:\n': sys.stdout.write("downloading trace...") sys.stdout.flush() - out = ''.join(lines[i+1:]) + out = ''.join(lines[i + 1:]) html_prefix = read_asset(script_dir, 'prefix.html') html_file = open(html_filename, 'w') html_file.write( @@ -204,6 +206,7 @@ def main(): print >> sys.stderr, ('An error occured while capturing the trace. Output ' 'file was not written.') + def read_asset(src_dir, filename): return open(os.path.join(src_dir, filename)).read() diff --git a/systrace/systrace/systrace_agent.py b/systrace/systrace/systrace_agent.py index 376d4f2507b..bca5e5e0bb3 100644 --- a/systrace/systrace/systrace_agent.py +++ b/systrace/systrace/systrace_agent.py @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. + class SystraceAgent(object): """The base class for systrace agents. diff --git a/systrace/systrace/update_systrace_trace_viewer.py b/systrace/systrace/update_systrace_trace_viewer.py index 71eece03cef..31f1689166c 100755 --- a/systrace/systrace/update_systrace_trace_viewer.py +++ b/systrace/systrace/update_systrace_trace_viewer.py @@ -22,7 +22,8 @@ def create_catapult_rev_str_(revision): - return '' + return '' + def get_catapult_rev_in_file_(): assert os.path.exists(SYSTRACE_TRACE_VIEWER_HTML_FILE_) @@ -36,6 +37,7 @@ def get_catapult_rev_in_file_(): break return rev + def get_catapult_rev_in_git_(): try: return subprocess.check_output( diff --git a/systrace/systrace/util.py b/systrace/systrace/util.py index 6566d084c60..283a6ab9a2e 100644 --- a/systrace/systrace/util.py +++ b/systrace/systrace/util.py @@ -71,6 +71,7 @@ def run_adb_shell(shell_args, device_serial): return (adb_output, adb_return_code) + def get_device_sdk_version(): """Uses adb to attempt to determine the SDK version of a running device.""" @@ -81,7 +82,7 @@ def get_device_sdk_version(): # command-line so we can send the adb command to the correct device. parser = OptionParserIgnoreErrors() parser.add_option('-e', '--serial', dest='device_serial', type='string') - options, unused_args = parser.parse_args() # pylint: disable=unused-variable + options, unused_args = parser.parse_args() # pylint: disable=unused-variable success = False diff --git a/systrace/systrace/util_unittest.py b/systrace/systrace/util_unittest.py index 585f6821e42..1b3fe47384f 100644 --- a/systrace/systrace/util_unittest.py +++ b/systrace/systrace/util_unittest.py @@ -13,6 +13,7 @@ class UtilTest(unittest.TestCase): + def test_construct_adb_shell_command(self): command = util.construct_adb_shell_command(LIST_TMP_ARGS, None) self.assertEqual(' '.join(command), 'adb shell ls /data/local/tmp') diff --git a/tracing/tracing_build/check_common.py b/tracing/tracing_build/check_common.py index 7c06e3c466b..7ac3cf432cb 100644 --- a/tracing/tracing_build/check_common.py +++ b/tracing/tracing_build/check_common.py @@ -14,13 +14,13 @@ def GetFileGroupFromFileName(filename): - extension = os.path.splitext(filename)[1] - return { - '.css': 'tracing_css_files', - '.html': 'tracing_js_html_files', - '.js': 'tracing_js_html_files', - '.png': 'tracing_img_files' - }[extension] + extension = os.path.splitext(filename)[1] + return { + '.css': 'tracing_css_files', + '.html': 'tracing_js_html_files', + '.js': 'tracing_js_html_files', + '.png': 'tracing_img_files' + }[extension] def CheckListedFilesSorted(src_file, group_name, listed_files): diff --git a/tracing/tracing_build/run_vinn_tests.py b/tracing/tracing_build/run_vinn_tests.py index 752d7202296..84aee7f1fd8 100644 --- a/tracing/tracing_build/run_vinn_tests.py +++ b/tracing/tracing_build/run_vinn_tests.py @@ -30,6 +30,7 @@ def RunTests(): js_args=d8_test_module_filenames, stdout=sys.stdout, stdin=sys.stdin) return res.returncode + def Main(argv): parser = argparse.ArgumentParser( description='Run d8 tests.') @@ -40,4 +41,4 @@ def Main(argv): if args.install_hooks: install.InstallHooks() - sys.exit(RunTests()) \ No newline at end of file + sys.exit(RunTests()) diff --git a/tracing/tracing_build/tracing_dev_server_config.py b/tracing/tracing_build/tracing_dev_server_config.py index 6d71792b38a..90cafeb66fc 100644 --- a/tracing/tracing_build/tracing_dev_server_config.py +++ b/tracing/tracing_build/tracing_dev_server_config.py @@ -20,7 +20,9 @@ def _RelPathToUnixPath(p): return p.replace(os.sep, '/') + class TestListHandler(webapp2.RequestHandler): + def get(self, *args, **kwargs): # pylint: disable=unused-argument project = tracing_project.TracingProject() test_relpaths = ['/' + _RelPathToUnixPath(x) @@ -33,6 +35,7 @@ def get(self, *args, **kwargs): # pylint: disable=unused-argument class TracingDevServerConfig(object): + def __init__(self): self.project = tracing_project.TracingProject() diff --git a/tracing/tracing_build/vulcanize_trace_viewer_unittest.py b/tracing/tracing_build/vulcanize_trace_viewer_unittest.py index 843f0f2c81f..b0c99818553 100644 --- a/tracing/tracing_build/vulcanize_trace_viewer_unittest.py +++ b/tracing/tracing_build/vulcanize_trace_viewer_unittest.py @@ -11,6 +11,7 @@ class Trace2HTMLTests(unittest.TestCase): + def test_writeHTMLForTracesToFile(self): try: # Note: We can't use "with" when working with tempfile.NamedTemporaryFile