From 457b727213bf9ed288b87679289f0fa4aa42cd03 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Thu, 2 Aug 2018 09:53:01 -0700 Subject: Update to latest catapult (eae13a4) $ git log --oneline --no-merges 3fe65c60..eae13a4b tracing systrace 7110f0868 rendering: Add pipeline:draw in the report. 3a995feda [systrace] Fix update_systrace_trace_viewer 2acd8e035 Removed an old restriction on timestamp ordering of trace events 134364159 [mali importer] Add Exynos Mali job event parser 5c2d9e757 renderer: Add breakdown metrics for compositor pipeline. 94294baed Reland "rendering: Add pipeline metrics." 600cb5aa8 tabs: Report tab-switching request delay. e05f902a6 Revert "rendering: Add pipeline metrics." b9813d2fd Attempt to fix flakiness in webview_startup perf tests d8072e683 Fix lint errors for systrace 6a67dd77d Tracing: Fix native library stats for pre-N devices. 787a37892 rendering: Add pipeline metrics. f07f60ef0 Drop Scheduler and FrameScheduler prefixes to save space. 0ddcb32d1 [TBMv2] Add POINT_ID reserved diagnostic 557d8dca5 [Systrace] Raise error when specified device is not found be45355b4 Dashboard - Fix fast path in ReplaceSharedDiagnostic 66447ba64 trace-viewer: Show a link to codesearch. 68c3b1d86 Dashboard - Optimize histogram_set.ReplaceSharedDiagnostic 10041796a [ftrace importer] Use tid instead of pid for kernel threads 4c6f5a25f [ftrace importer] Fix recursive binder reply arguments 9e83c9e3c [TBMv2] Implement deduplication fast path for GenericSet 685c633d5 [TBMv2] Make timeInMsAutoFormat available in Python 0c92958e3 Refactor systrace/systrace/util.py to use device_utils 21acb2a82 Throw exception instead of dying in get_device_sdk_version 647cee134 Telemetry: frame times in TBMv2 0e9eb2f5c Android systrace: Fix printf/scanf format compiler warnings bea867a76 Android systrace: Fix undeclared use of strerr 3766fc728 Android systrace: Fix undeclared references to POSIX APIs d8e6a8203 [Telemetry] Remove GetRendererThreadFromTabId 7439a1c6b trace-recorder: Include 'viz' category for rendering. 3eb704bab Remove BattOr support from systrace 9f46241c6 Add documentation_link diagnostic 3836c556c androidStartupMetric: Allow multiple FCP events f76f0b440 [Systrace] remove duplicate --target option. 7ddf0ed7f Add 90th percentile to results.html. f989b6282 tracing: Add binder transaction size to events fdc825f43 androidStartupMetric: support multiple browser starts 11513e359 Add convenient method for running trace processing code in python 87eefd4f1 [fuchsia] Add thread priority to fuchsia importer 32a31d2e9 Do not compute TTI without ResourceLoad edf6c000f Add default bin boundaries for timeInMsAutoFormat. c87e4baed Support multiple story tags in ChartJsonConverter. 09b82f4a5 Plumb story tags through Telemetry Values. 63f44ee35 tabs: Ignore the duration for the new metric. 6160b40d1 HistogramSet - Merge TagMap diagnostics in add_reserved_diagnostics 9861a71fa tabs: Switch back to async trace events. 4c1bea249 [ftrace importer] Add fence parser a9d8c2947 tabs: Update the measurements. a7d578a29 tabs: Fix tabs metric. bf32270b0 Clean up telemetry/timeline/trace_data* caee0de0e [TBMv2] Add 'tasks' legacy_unit_info 385d3dd26 Add comments to document that variance/stddev use Bessel's correction c36ea2490 Remove deprecated trace event name for BlinkSchedulerAsyncSlice. 8fbdf9f10 Fix blink-gc metrics 0043a4a25 [Tracing UI] Fix raster task view 0111e13d0 [Tracing UI] Don't show "View in Picture Debugger" when not available 393b0fd9c [Tracing UI] Use ResizeObserver for picture chart resizing 0ceb1ecce [Tracing UI] Fix drag_handle when there are size constraints 0e9738361 [Tracing] Remove unused resize_sensor.html and css-element-queries 1986f5a95 Change "minutes" legacy unit to timeInMsAutoFormat. f1e34db09 Fix frame viewer ui issues 8dd790d49 Allow passing timestamps to Timing.mark. 49edbd3a2 Measure TBM2 runMetrics. 9aa395965 [trace-viewer] Update according to chrome side Frame Viewer trace format 5b4ef865e Roll eslint to 4.0. 183509ba6 Shorten normalizedPercentage units to a single decimal digit. 669dcb14a [trace-viewer] Fix "no tree" error in layer quad stack view a6ccbcf12 [trace-viewer] Fix exception when rasterizing zero-sized picture bc894f56c [vr] Add post_submit_draw_on_gpu metric a07c6a933 Histogram - Cache info types 3d2afdf63 [CSMMerging] Refactor breakdown_tree_helper with findEmptyRangesBetweenRanges 075ca3000 Support fuchsia project revisions in tooltips. b6e96e16f Introduce sequence_manager tracing category. 2048c078d [Dashboard] Fix stdio URLs for histogram uploads 504f52bbe Fix symbolization of heap profiles when so is mapped from apk 86d3e79d8 [Tracing] Android native library resident size statistics. 846cec5c4 HtmlEscape histogram json when rendering results.html 055c03395 Use top-level scheduler tasks in EQT metric. 59aabe8d4 Generate brighter color palette using SinebowColorGenerator. 222de1a4f [TBMv2] Add HAD_FAILURES diagnostic and use it to avoid merging a22719853 [vr] Increase VR Response UE to 1s 516a69786 HistogramSet - Merge by test paths in add_reserved_diagnostics. 1bae362e7 HistogramSet - Merge by only a subset of tags. 8ddb78550 [Dashboard] Merge histograms by tags in add_reserved_diagnostics 10232e9c9 Histogram - Missing json import in bin/add_reserved_diagnostics 71fd6b902 Revert "Do not add zero values to breakdown diagnostics" 7c43e1be4 Do not add zero values to breakdown diagnostics 1c69b2f42 androidStartupMetric: add first_contentful_paint_time b5e636db8 HistogramSet - Only merge histograms with stories. 0ad14f9aa [TBMv2] Test add_reserved_diagnostics 72c05e713 Fix frame grouping for [Web]FrameScheduler since it was renamed. 6b8f67cc1 VR: Remove UpdateTexturesAndSizes metric 5d35a2c71 Get navigation info without using FrameLoader snapshots 8b153cc7d Revert "HistogramSet - Only merge histograms with stories." bffbf166f HistogramSet - Only merge histograms with stories. 85462f12f [TBMv2] Add GenericSet.GetOnlyElement in Python fe230b673 [Build] Fix tracing Python tests 7e3fcbe35 Reland "More precise self time calculation" 1dfd083b6 Fix sorting results.html 718296623 First set of thread times metrics in TBMv2 17fc85e9c Replace --enable-heap-profiling with --memlog equivalent. f9a1ae1b3 Fix visual rect support for cc::DisplayItemList 580f1ca28 [Dashboard] Add IS_SUMMARY diagnostic b3d01529c [fuchsia] Add thread state to fuchsia importer. 1446cf3fe [vr] Fix draw metrics in frameCycleDurationMetric f73167a68 Ignore i-frame navigationStart events when computing LoadExpectation 4d4ed66a7 Revert "More precise self time calculation" 548a5bc4b androidStartupMetric: add request_start_time df668c312 Disable failing Snap-It test 27e44012b [Tracing] Fix merge_traces.py script a0688890f [results.html] Show All by default. 900b947e1 [vr] Add metrics for render step to frameCycleDurationMetric 2210f05b4 More precise self time calculation 875a659c7 Remove the 'deleted' suffix on path name 5c590a9bf Tracing: CPU time computation clean up b18c0b5df [vr] Fix submit_frame of frameCycleDuration metric 1c706a34f [TBMv2] Allow nulls to be stably stringified 0ac0bb6d1 [TBMv2] Factor mergeHistograms out in merge_histograms_cmdline Test: ./systrace.py -b 65536 gfx sync sched -t 5 Merged-In: I92804a98fc4fc438f1c5cac79e869a37bebd9416 (cherry picked from commit 367b65a310a72ba207744182de04a89d520b31ee) Change-Id: I803021272634cf4109b96fdd7f2f5ec93bf5ba51 --- UPSTREAM_REVISION | 2 +- catapult/common/battor/battor/__init__.py | 27 -- .../battor/battor/battor_binary_dependencies.json | 100 ----- catapult/common/battor/battor/battor_error.py | 8 - catapult/common/battor/battor/battor_wrapper.py | 436 ------------------- .../battor/battor/battor_wrapper_devicetest.py | 103 ----- .../battor/battor/battor_wrapper_unittest.py | 387 ----------------- catapult/common/battor/bin/run_py_tests | 23 - .../common/battor/bin/upload_battor_binaries.py | 54 --- .../common/bin/update_chrome_reference_binaries | 67 +-- catapult/common/node_runner/node_runner/minify | 58 +++ .../common/node_runner/node_runner/package.json | 8 +- catapult/common/py_utils/py_utils/__init__.py | 4 +- .../common/py_utils/py_utils/binary_manager.py | 2 +- .../common/py_utils/py_utils/chrome_binaries.json | 67 +-- catapult/common/py_utils/py_utils/cloud_storage.py | 5 + .../py_utils/py_utils/cloud_storage_unittest.py | 13 + catapult/common/py_utils/py_utils/file_util.py | 23 + .../common/py_utils/py_utils/file_util_unittest.py | 66 +++ catapult/common/py_utils/py_utils/lock.py | 12 +- catapult/common/py_utils/py_utils/memory_debug.py | 14 +- .../refactor/annotated_symbol/class_definition.py | 2 - .../annotated_symbol/function_definition.py | 2 - .../refactor/annotated_symbol/import_statement.py | 7 +- .../refactor/annotated_symbol/reference.py | 1 + .../py_utils/py_utils/retry_util_unittest.py | 3 +- catapult/dependency_manager/PRESUBMIT.py | 1 + .../dependency_manager/archive_info.py | 2 +- .../dependency_manager/base_config.py | 28 ++ .../dependency_manager/base_config_unittest.py | 43 +- catapult/devil/devil/android/apk_helper.py | 28 ++ catapult/devil/devil/android/apk_helper_test.py | 26 ++ catapult/devil/devil/android/device_errors.py | 30 +- catapult/devil/devil/android/device_utils.py | 258 +++++++++++- .../devil/devil/android/device_utils_devicetest.py | 33 ++ catapult/devil/devil/android/device_utils_test.py | 171 +++++++- catapult/devil/devil/android/flag_changer.py | 22 +- catapult/devil/devil/android/flag_changer_test.py | 13 +- catapult/devil/devil/android/forwarder.py | 2 +- catapult/devil/devil/android/perf/perf_control.py | 6 +- .../devil/devil/android/perf/thermal_throttle.py | 1 + catapult/devil/devil/android/sdk/adb_wrapper.py | 31 +- catapult/devil/devil/android/sdk/fastboot.py | 4 +- catapult/devil/devil/android/sdk/shared_prefs.py | 57 ++- .../devil/devil/android/sdk/shared_prefs_test.py | 13 + catapult/devil/devil/android/sdk/version_codes.py | 4 +- .../devil/devil/android/tools/device_monitor.py | 7 +- .../devil/devil/android/tools/device_recovery.py | 5 +- .../devil/devil/android/tools/provision_devices.py | 2 + .../devil/android/tools/script_common_test.py | 2 +- catapult/devil/devil/utils/__init__.py | 23 - .../devil/devil/utils/battor_device_mapping.py | 309 -------------- catapult/devil/devil/utils/cmd_helper.py | 22 + catapult/devil/devil/utils/find_usb_devices.py | 4 +- .../devil/devil/utils/find_usb_devices_test.py | 161 ++----- catapult/devil/devil/utils/lazy/weak_constant.py | 23 +- .../devil/devil/utils/lazy/weak_constant_test.py | 70 +++ catapult/devil/devil/utils/markdown.py | 8 +- catapult/devil/devil/utils/reraiser_thread.py | 9 +- .../devil/utils/test/data/test_serial_map.json | 1 - catapult/devil/devil/utils/timeout_retry.py | 3 +- catapult/devil/devil/utils/update_mapping.py | 47 --- catapult/devil/pylintrc | 1 + catapult/systrace/PRESUBMIT.py | 1 - .../atrace_helper/jni/atrace_process_dump.cc | 36 +- catapult/systrace/atrace_helper/jni/logging.h | 1 + catapult/systrace/atrace_helper/jni/main.cc | 3 +- .../atrace_helper/jni/process_memory_stats.cc | 4 +- catapult/systrace/atrace_helper/jni/time_utils.cc | 1 + .../profile_chrome/chrome_startup_tracing_agent.py | 3 +- .../profile_chrome/chrome_tracing_agent.py | 5 +- .../chrome_tracing_agent_unittest.py | 4 +- .../systrace/profile_chrome/ddms_tracing_agent.py | 3 +- .../systrace/profile_chrome/perf_tracing_agent.py | 6 +- catapult/systrace/systrace/__init__.py | 1 - catapult/systrace/systrace/output_generator.py | 1 - .../systrace/systrace/output_generator_unittest.py | 15 +- catapult/systrace/systrace/run_systrace.py | 10 +- catapult/systrace/systrace/systrace_runner.py | 3 +- .../systrace/systrace/systrace_trace_viewer.html | 469 +++++++++++++-------- .../systrace/test_data/battor_test_data.txt | 16 - .../systrace/tracing_agents/agents_unittest.py | 2 +- .../systrace/tracing_agents/atrace_agent.py | 8 +- .../tracing_agents/atrace_from_file_agent.py | 3 +- .../systrace/tracing_agents/battor_trace_agent.py | 165 -------- .../tracing_agents/battor_trace_agent_unittest.py | 182 -------- .../systrace/systrace/tracing_agents/walt_agent.py | 3 +- .../systrace/tracing_agents/walt_agent_unittest.py | 6 +- catapult/systrace/systrace/tracing_controller.py | 3 +- .../systrace/update_systrace_trace_viewer.py | 2 + catapult/systrace/systrace/util.py | 76 +--- catapult/systrace/systrace/util_unittest.py | 30 -- catapult/tracing/tracing/trace_data/trace_data.py | 14 +- .../tracing/trace_data/trace_data_unittest.py | 6 +- 94 files changed, 1540 insertions(+), 2506 deletions(-) delete mode 100644 catapult/common/battor/battor/__init__.py delete mode 100644 catapult/common/battor/battor/battor_binary_dependencies.json delete mode 100644 catapult/common/battor/battor/battor_error.py delete mode 100644 catapult/common/battor/battor/battor_wrapper.py delete mode 100644 catapult/common/battor/battor/battor_wrapper_devicetest.py delete mode 100644 catapult/common/battor/battor/battor_wrapper_unittest.py delete mode 100755 catapult/common/battor/bin/run_py_tests delete mode 100755 catapult/common/battor/bin/upload_battor_binaries.py create mode 100755 catapult/common/node_runner/node_runner/minify create mode 100644 catapult/common/py_utils/py_utils/file_util.py create mode 100644 catapult/common/py_utils/py_utils/file_util_unittest.py delete mode 100755 catapult/devil/devil/utils/battor_device_mapping.py create mode 100644 catapult/devil/devil/utils/lazy/weak_constant_test.py delete mode 100644 catapult/devil/devil/utils/test/data/test_serial_map.json delete mode 100755 catapult/devil/devil/utils/update_mapping.py delete mode 100644 catapult/systrace/systrace/test_data/battor_test_data.txt delete mode 100644 catapult/systrace/systrace/tracing_agents/battor_trace_agent.py delete mode 100755 catapult/systrace/systrace/tracing_agents/battor_trace_agent_unittest.py delete mode 100644 catapult/systrace/systrace/util_unittest.py diff --git a/UPSTREAM_REVISION b/UPSTREAM_REVISION index 872cc65e..ef75e069 100644 --- a/UPSTREAM_REVISION +++ b/UPSTREAM_REVISION @@ -1 +1 @@ -3fe65c60e0314212730f3e5363a58dfda949d761 +eae13a4b34ccf54974dfacf7d72effd99729f543 diff --git a/catapult/common/battor/battor/__init__.py b/catapult/common/battor/battor/__init__.py deleted file mode 100644 index f18f3304..00000000 --- a/catapult/common/battor/battor/__init__.py +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. -import os -import sys - -def _JoinPath(*path_parts): - return os.path.abspath(os.path.join(*path_parts)) - - -def _AddDirToPythonPath(*path_parts): - path = _JoinPath(*path_parts) - if os.path.isdir(path) and path not in sys.path: - # Some call sites that use Telemetry assume that sys.path[0] is the - # directory containing the script, so we add these extra paths to right - # after sys.path[0]. - sys.path.insert(1, path) - -_CATAPULT_DIR = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - os.path.pardir, os.path.pardir, os.path.pardir) - -_AddDirToPythonPath(_CATAPULT_DIR, 'common', 'battor') -_AddDirToPythonPath(_CATAPULT_DIR, 'common', 'py_utils') -_AddDirToPythonPath(_CATAPULT_DIR, 'dependency_manager') -_AddDirToPythonPath(_CATAPULT_DIR, 'devil') -_AddDirToPythonPath(_CATAPULT_DIR, 'third_party', 'pyserial') diff --git a/catapult/common/battor/battor/battor_binary_dependencies.json b/catapult/common/battor/battor/battor_binary_dependencies.json deleted file mode 100644 index 23be1a30..00000000 --- a/catapult/common/battor/battor/battor_binary_dependencies.json +++ /dev/null @@ -1,100 +0,0 @@ -{ - "config_type": "BaseConfig", - "dependencies": { - "avrdude_binary": { - "cloud_storage_base_folder": "binary_dependencies/battor", - "cloud_storage_bucket": "chromium-telemetry", - "file_info": { - "mac_x86_64": { - "cloud_storage_hash": "6de6324c279ea75c79c68cab4c2ddcc68da1b286", - "download_path": "../bin/darwin/x86_64/avrdude", - "local_paths": [ - "../bin/override/avrdude" - ] - }, - "linux_x86_64": { - "cloud_storage_hash": "db29526605f6f95a75ab33f4060b8c330152de69", - "download_path": "../bin/linux/x86_64/avrdude", - "local_paths": [ - "../bin/override/avrdude" - ] - }, - "win_AMD64": { - "cloud_storage_hash": "517aa73b093e254007076cf5ac7afb94151df2ed", - "download_path": "../bin/win/x86_64/avrdude.exe", - "local_paths": [ - "../bin/override/avrdude.exe" - ] - }, - "win_x86": { - "cloud_storage_hash": "517aa73b093e254007076cf5ac7afb94151df2ed", - "download_path": "../bin/win/x86_64/avrdude.exe", - "local_paths": [ - "../bin/override/avrdude.exe" - ] - } - } - }, - "avrdude_config": { - "cloud_storage_base_folder": "binary_dependencies/battor", - "cloud_storage_bucket": "chromium-telemetry", - "file_info": { - "default": { - "cloud_storage_hash": "ccdfa12743429b8b92b61a20163d6311ab55a4fa", - "download_path": "../bin/battor/avrdude.conf", - "local_paths": [ - "../bin/override/avrdude.conf" - ] - } - } - }, - "battor_agent_binary": { - "cloud_storage_base_folder": "binary_dependencies/battor", - "cloud_storage_bucket": "chromium-telemetry", - "file_info": { - "mac_x86_64": { - "cloud_storage_hash": "154613804a1855a4871422bf99aa8e0dd0d7f62e", - "download_path": "../bin/darwin/x86_64/battor_agent", - "local_paths": [ - "../bin/override/battor_agent" - ] - }, - "linux_x86_64": { - "cloud_storage_hash": "f53e15b97301e9dab00e893800ec67a98e9f13ea", - "download_path": "../bin/linux/x86_64/battor_agent", - "local_paths": [ - "../bin/override/battor_agent" - ] - }, - "win_AMD64": { - "cloud_storage_hash": "4095a9159dd189ec591679e43b14d34ae30335eb", - "download_path": "../bin/win/x86_64/battor_agent.exe", - "local_paths": [ - "../bin/override/battor_agent.exe" - ] - }, - "win_x86": { - "cloud_storage_hash": "4095a9159dd189ec591679e43b14d34ae30335eb", - "download_path": "../bin/win/x86_64/battor_agent.exe", - "local_paths": [ - "../bin/override/battor_agent.exe" - ] - } - } - }, - "battor_firmware": { - "cloud_storage_base_folder": "binary_dependencies/battor", - "cloud_storage_bucket": "chromium-telemetry", - "file_info": { - "default": { - "cloud_storage_hash": "0649655f78698368a16a4e9ec37967b80528fdbf", - "download_path": "../bin/battor/battor_firmware.hex", - "local_paths": [ - "../bin/override/battor_firmware.hex" - ], - "version_in_cs": "de05458" - } - } - } - } -} diff --git a/catapult/common/battor/battor/battor_error.py b/catapult/common/battor/battor/battor_error.py deleted file mode 100644 index 3ea6efc6..00000000 --- a/catapult/common/battor/battor/battor_error.py +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright 2016 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. - -from devil import base_error - -class BattOrError(base_error.BaseError): - pass diff --git a/catapult/common/battor/battor/battor_wrapper.py b/catapult/common/battor/battor/battor_wrapper.py deleted file mode 100644 index 93aed8b0..00000000 --- a/catapult/common/battor/battor/battor_wrapper.py +++ /dev/null @@ -1,436 +0,0 @@ -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import datetime -import os -import logging -import platform -import random -import subprocess -import sys -import tempfile -import time - -from battor import battor_error -import py_utils -from py_utils import atexit_with_log -from py_utils import cloud_storage -import dependency_manager -from devil.utils import battor_device_mapping -from devil.utils import find_usb_devices - -import serial -from serial.tools import list_ports - - -DEFAULT_SHELL_CLOSE_TIMEOUT_S = 60 - - -def IsBattOrConnected(*args, **kwargs): - """Returns True if BattOr is detected. - - See _IsBattOrConnected below for arguments. - """ - is_connected = _IsBattOrConnected(*args, **kwargs) - if is_connected: - logging.info('BattOr power monitor is connected.') - else: - logging.info('BattOr power monitor is not connected.') - return is_connected - - -def _IsBattOrConnected(test_platform, android_device=None, - android_device_map=None, android_device_file=None): - """Returns True if BattOr is detected.""" - if test_platform == 'android': - if not android_device: - raise ValueError('Must pass android device serial when determining ' - 'support on android platform') - - if not android_device_map: - device_tree = find_usb_devices.GetBusNumberToDeviceTreeMap() - if device_tree: - for _, node in sorted(device_tree.iteritems()): - node.Display() - if len(battor_device_mapping.GetBattOrList(device_tree)) == 1: - return True - if android_device_file: - android_device_map = battor_device_mapping.ReadSerialMapFile( - android_device_file) - else: - try: - android_device_map = battor_device_mapping.GenerateSerialMap() - except battor_error.BattOrError: - return False - - # If neither if statement above is triggered, it means that an - # android_device_map was passed in and will be used. - return str(android_device) in android_device_map - - elif test_platform == 'win': - for (_1, desc, _2) in serial.tools.list_ports.comports(): - if 'USB Serial Port' in desc: - return True - logging.info('No usb serial port discovered. Available ones are: %s' % - list(serial.tools.list_ports.comports())) - return False - - elif test_platform == 'mac': - for (_1, desc, _2) in serial.tools.list_ports.comports(): - if 'BattOr' in desc: - return True - return False - - elif test_platform == 'linux': - device_tree = find_usb_devices.GetBusNumberToDeviceTreeMap(fast=True) - return bool(battor_device_mapping.GetBattOrList(device_tree)) - - return False - - -class BattOrWrapper(object): - """A class for communicating with a BattOr in python.""" - _EXIT_CMD = 'Exit' - _GET_FIRMWARE_GIT_HASH_CMD = 'GetFirmwareGitHash' - _START_TRACING_CMD = 'StartTracing' - _STOP_TRACING_CMD = 'StopTracing' - _SUPPORTS_CLOCKSYNC_CMD = 'SupportsExplicitClockSync' - _RECORD_CLOCKSYNC_CMD = 'RecordClockSyncMarker' - _SUPPORTED_PLATFORMS = ['android', 'chromeos', 'linux', 'mac', 'win'] - - _BATTOR_PARTNO = 'x192a3u' - _BATTOR_PROGRAMMER = 'avr109' - _BATTOR_BAUDRATE = '115200' - - def __init__(self, target_platform, android_device=None, battor_path=None, - battor_map_file=None, battor_map=None, serial_log_bucket=None, - autoflash=True): - """Constructor. - - Args: - target_platform: Platform BattOr is attached to. - android_device: Serial number of Android device. - battor_path: Path to BattOr device. - battor_map_file: File giving map of [device serial: BattOr path] - battor_map: Map of [device serial: BattOr path] - serial_log_bucket: The cloud storage bucket to which BattOr agent serial - logs are uploaded on failure. - - Attributes: - _battor_path: Path to BattOr. Typically similar to /tty/USB0. - _battor_agent_binary: Path to the BattOr agent binary used to communicate - with the BattOr. - _tracing: A bool saying if tracing has been started. - _battor_shell: A subprocess running the battor_agent_binary - _trace_results_path: Path to BattOr trace results file. - _serial_log_bucket: Cloud storage bucket to which BattOr agent serial logs - are uploaded on failure. - _serial_log_file: Temp file for the BattOr agent serial log. - """ - self._battor_path = self._GetBattOrPath(target_platform, android_device, - battor_path, battor_map_file, battor_map) - config = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - 'battor_binary_dependencies.json') - - self._dm = dependency_manager.DependencyManager( - [dependency_manager.BaseConfig(config)]) - self._battor_agent_binary = self._dm.FetchPath( - 'battor_agent_binary', - '%s_%s' % (py_utils.GetHostOsName(), py_utils.GetHostArchName())) - - self._autoflash = autoflash - self._serial_log_bucket = serial_log_bucket - self._tracing = False - self._battor_shell = None - self._trace_results_path = None - self._start_tracing_time = None - self._stop_tracing_time = None - self._trace_results = None - self._serial_log_file = None - self._target_platform = target_platform - self._git_hash = None - - atexit_with_log.Register(self.KillBattOrShell) - - def _FlashBattOr(self): - assert self._battor_shell, ( - 'Must start shell before attempting to flash BattOr') - - try: - device_git_hash = self.GetFirmwareGitHash() - battor_firmware, cs_git_hash = self._dm.FetchPathWithVersion( - 'battor_firmware', 'default') - if cs_git_hash != device_git_hash: - logging.info( - 'Flashing BattOr with old firmware version <%s> with new ' - 'version <%s>.', device_git_hash, cs_git_hash) - avrdude_config = self._dm.FetchPath('avrdude_config', 'default') - self.StopShell() - return self.FlashFirmware(battor_firmware, avrdude_config) - return False - except ValueError: - logging.exception('Git hash returned from BattOr was not as expected: %s' - % self._git_hash) - self.StopShell() - - finally: - if not self._battor_shell: - # TODO(charliea): Once we understand why BattOrs are crashing, remove - # this log. - # http://crbug.com/699581 - logging.info('_FlashBattOr serial log:') - self._UploadSerialLogToCloudStorage() - self._serial_log_file = None - - self.StartShell() - - def KillBattOrShell(self): - if self._battor_shell: - logging.critical('BattOr shell was not properly closed. Killing now.') - self._battor_shell.kill() - - def GetShellReturnCode(self): - """Gets the return code of the BattOr agent shell.""" - rc = self._battor_shell.poll() - return rc - - def StartShell(self): - """Start BattOr binary shell.""" - assert not self._battor_shell, 'Attempting to start running BattOr shell.' - - battor_cmd = [self._battor_agent_binary] - if self._serial_log_bucket: - # Create and immediately close a temp file in order to get a filename - # for the serial log. - self._serial_log_file = tempfile.NamedTemporaryFile(delete=False) - self._serial_log_file.close() - battor_cmd.append('--battor-serial-log=%s' % self._serial_log_file.name) - if self._battor_path: - battor_cmd.append('--battor-path=%s' % self._battor_path) - self._battor_shell = self._StartShellImpl(battor_cmd) - assert self.GetShellReturnCode() is None, 'Shell failed to start.' - - def StopShell(self, timeout=None): - """Stop BattOr binary shell.""" - assert self._battor_shell, 'Attempting to stop a non-running BattOr shell.' - assert not self._tracing, 'Attempting to stop a BattOr shell while tracing.' - timeout = timeout if timeout else DEFAULT_SHELL_CLOSE_TIMEOUT_S - - try: - self._SendBattOrCommand(self._EXIT_CMD, check_return=False) - py_utils.WaitFor(lambda: self.GetShellReturnCode() != None, timeout) - except: - # If graceful shutdown failed, resort to a simple kill command. - self.KillBattOrShell() - finally: - self._battor_shell = None - - def StartTracing(self): - """Start tracing on the BattOr.""" - assert self._battor_shell, 'Must start shell before tracing' - assert not self._tracing, 'Tracing already started.' - self._FlashBattOr() - self._SendBattOrCommand(self._START_TRACING_CMD) - self._tracing = True - self._start_tracing_time = int(time.time()) - - def StopTracing(self): - """Stop tracing on the BattOr.""" - assert self._tracing, 'Must run StartTracing before StopTracing' - # Create temp file to reserve location for saving results. - temp_file = tempfile.NamedTemporaryFile(delete=False) - self._trace_results_path = temp_file.name - temp_file.close() - self._SendBattOrCommand( - '%s %s' % (self._STOP_TRACING_CMD, self._trace_results_path)) - self._tracing = False - self._stop_tracing_time = int(time.time()) - - def CollectTraceData(self, timeout=None): - """Collect trace data from battor. - Args: - timeout: timeout for waiting on the BattOr process to terminate in - seconds. - Returns: Trace data in form of a list. - """ - if not self._stop_tracing_time or not self._start_tracing_time: - raise battor_error.BattOrError( - 'No start or stop time detected when collecting BattOr trace.\n' - 'Start: %s \n Stop: %s' % (self._start_tracing_time, - self._stop_tracing_time)) - - # The BattOr shell terminates after returning the results. - if timeout is None: - timeout = self._stop_tracing_time - self._start_tracing_time - py_utils.WaitFor(lambda: self.GetShellReturnCode() != None, timeout) - - # TODO(charliea): Once we understand why BattOrs are crashing, only do - # this on failure. - # http://crbug.com/699581 - logging.info('CollectTraceData serial log:') - self._UploadSerialLogToCloudStorage() - - with open(self._trace_results_path) as results: - self._trace_results = results.read() - self._battor_shell = None - self._serial_log_file = None - return self._trace_results - - def SupportsExplicitClockSync(self): - """Returns if BattOr supports Clock Sync events.""" - return bool(int(self._SendBattOrCommand(self._SUPPORTS_CLOCKSYNC_CMD, - check_return=False))) - - def RecordClockSyncMarker(self, sync_id): - """Record clock sync event on BattOr.""" - if not isinstance(sync_id, basestring): - raise TypeError('sync_id must be a string.') - self._SendBattOrCommand('%s %s' % (self._RECORD_CLOCKSYNC_CMD, sync_id)) - - def _GetBattOrPath(self, target_platform, android_device=None, - battor_path=None, battor_map_file=None, battor_map=None): - """Determines most likely path to the correct BattOr.""" - if target_platform not in self._SUPPORTED_PLATFORMS: - raise battor_error.BattOrError( - '%s is an unsupported platform.' % target_platform) - if target_platform in ['win']: - # Right now, the BattOr agent binary isn't able to automatically detect - # the BattOr port on Windows. To get around this, we know that the BattOr - # shows up with a name of 'USB Serial Port', so use the COM port that - # corresponds to a device with that name. - for (port, desc, _) in serial.tools.list_ports.comports(): - if 'USB Serial Port' in desc: - return port - raise battor_error.BattOrError( - 'Could not find BattOr attached to machine.') - if target_platform in ['mac']: - for (port, desc, _) in serial.tools.list_ports.comports(): - if 'BattOr' in desc: - return port - - if target_platform in ['android', 'linux']: - if battor_path: - if not isinstance(battor_path, basestring): - raise battor_error.BattOrError( - 'An invalid BattOr path was specified.') - return battor_path - - if target_platform == 'android': - if not android_device: - raise battor_error.BattOrError( - 'Must specify device for Android platform.') - if not battor_map_file and not battor_map: - # No map was passed, so must create one. - battor_map = battor_device_mapping.GenerateSerialMap() - - return battor_device_mapping.GetBattOrPathFromPhoneSerial( - str(android_device), serial_map_file=battor_map_file, - serial_map=battor_map) - - # Not Android and no explicitly passed BattOr. - device_tree = find_usb_devices.GetBusNumberToDeviceTreeMap(fast=True) - battors = battor_device_mapping.GetBattOrList(device_tree) - if len(battors) != 1: - raise battor_error.BattOrError( - 'For non-Android platforms, exactly one BattOr must be ' - 'attached unless address is explicitly given.') - return '/dev/%s' % battors.pop() - - raise NotImplementedError( - 'BattOr Wrapper not implemented for given platform') - - def _SendBattOrCommandImpl(self, cmd): - """Sends command to the BattOr.""" - self._battor_shell.stdin.write('%s\n' % cmd) - self._battor_shell.stdin.flush() - return self._battor_shell.stdout.readline() - - def _SendBattOrCommand(self, cmd, check_return=True): - status = self._SendBattOrCommandImpl(cmd) - - if check_return and not 'Done.' in status: - self.KillBattOrShell() - self._UploadSerialLogToCloudStorage() - self._serial_log_file = None - raise battor_error.BattOrError( - 'BattOr did not complete command \'%s\' correctly.\n' - 'Outputted: %s' % (cmd, status)) - return status - - def _StartShellImpl(self, battor_cmd): - return subprocess.Popen( - battor_cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, shell=False) - - def _UploadSerialLogToCloudStorage(self): - """Uploads the BattOr serial log to cloud storage.""" - if not self._serial_log_file or not cloud_storage.IsNetworkIOEnabled(): - return - - remote_path = ('battor-serial-log-%s-%d.txt' % ( - datetime.datetime.now().strftime('%Y-%m-%d_%H-%M.txt'), - random.randint(1, 100000))) - - try: - cloud_url = cloud_storage.Insert( - self._serial_log_bucket, remote_path, self._serial_log_file.name) - sys.stderr.write('View BattOr serial log at %s\n' % cloud_url) - except cloud_storage.PermissionError as e: - logging.error('Cannot upload BattOr serial log file to cloud storage due ' - 'to permission error: %s' % e.message) - - def GetFirmwareGitHash(self): - """Gets the git hash for the BattOr firmware. - - Returns: Git hash for firmware currently on the BattOr. - Also sets self._git_hash to this value. - - Raises: ValueException if the git hash is not in hex. - """ - assert self._battor_shell, ('Must start shell before getting firmware git ' - 'hash') - self._git_hash = self._SendBattOrCommand(self._GET_FIRMWARE_GIT_HASH_CMD, - check_return=False).strip() - # We expect the git hash to be a valid 6 character hexstring. This will - # throw a ValueError exception otherwise. - int(self._git_hash, 16) - return self._git_hash - - def FlashFirmware(self, hex_path, avrdude_config_path): - """Flashes the BattOr using an avrdude config at config_path with the new - firmware at hex_path. - """ - assert not self._battor_shell, 'Cannot flash BattOr with open shell' - - avrdude_binary = self._dm.FetchPath( - 'avrdude_binary', '%s_%s' % (sys.platform, platform.machine())) - # Sanitize hex file path for windows. It contains :/ which avrdude - # is not capable of handling. - _, hex_path = os.path.splitdrive(hex_path) - avr_cmd = [ - avrdude_binary, - '-e', # Specify to erase data on chip. - '-p', self._BATTOR_PARTNO, # Specify AVR device. - # Specify which microcontroller programmer to use. - '-c', self._BATTOR_PROGRAMMER, - '-b', self._BATTOR_BAUDRATE, # Specify the baud rate to communicate at. - '-P', self._battor_path, # Serial path to the battor. - # Command to execute with hex file and path to hex file. - '-U', 'flash:w:%s' % hex_path, - '-C', avrdude_config_path, # AVRdude config file path. - '2>&1' # All output goes to stderr for some reason. - ] - try: - subprocess.check_output(avr_cmd) - except subprocess.CalledProcessError as e: - raise BattOrFlashError('BattOr flash failed with return code %s.' - % e.returncode) - - self._git_hash = None - return True - - -class BattOrFlashError(Exception): - pass diff --git a/catapult/common/battor/battor/battor_wrapper_devicetest.py b/catapult/common/battor/battor/battor_wrapper_devicetest.py deleted file mode 100644 index 29c36e7e..00000000 --- a/catapult/common/battor/battor/battor_wrapper_devicetest.py +++ /dev/null @@ -1,103 +0,0 @@ -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import logging -import platform -import os -import sys -import time -import unittest - -if __name__ == '__main__': - sys.path.append( - os.path.join(os.path.dirname(__file__), '..')) - -from battor import battor_wrapper -from devil.utils import battor_device_mapping -from devil.utils import find_usb_devices -import py_utils -from py_utils import cloud_storage - - -_SUPPORTED_CQ_PLATFORMS = ['win', 'linux', 'mac'] - -class BattOrWrapperDeviceTest(unittest.TestCase): - def setUp(self): - self._platform = py_utils.GetHostOsName() - self._battor_list = None - - if self._platform == 'linux': - device_tree = find_usb_devices.GetBusNumberToDeviceTreeMap() - self._battor_list = battor_device_mapping.GetBattOrList(device_tree) - - if not battor_wrapper.IsBattOrConnected(self._platform): - self._battor_list = [] - - def testFullRun(self): - # If battor_list is an empty list, a BattOr was expected but not found. - if self._battor_list is not None and not self._battor_list: - logging.critical('No BattOrs attached. Cannot run tests.') - return - - if self._platform not in _SUPPORTED_CQ_PLATFORMS: - logging.critical('Platform %s is not supported on CQ.' % self._platform) - return - - - battor_path = (None if not self._battor_list - else '/dev/%s' % self._battor_list[0]) - battor = battor_wrapper.BattOrWrapper( - self._platform, battor_path=battor_path, - serial_log_bucket=cloud_storage.TELEMETRY_OUTPUT) - try: - battor.StartShell() - self.assertTrue(isinstance(battor.GetFirmwareGitHash(), basestring)) - # We expect the git hash to be a valid 6 character hexstring. This will - # throw a ValueError exception otherwise. - int(battor.GetFirmwareGitHash(), 16) - self.assertTrue(len(battor.GetFirmwareGitHash()) == 7) - battor.StopShell() - - battor.StartShell() - battor.StartTracing() - # TODO(rnephew): This sleep is required for now because crbug.com/602266 - # causes the BattOr to crash when the trace time is too short. Once that - # bug is fixed, we should remove this delay. - time.sleep(1) - battor.RecordClockSyncMarker('abc') - # Sleep here because clock sync marker will be flaky if not. - time.sleep(1) - battor.StopTracing() - - # Below is a work around for crbug.com/603309. On this short of a trace, 5 - # seconds is enough to ensure that the trace will finish flushing to the - # file. The process is then killed so that BattOrWrapper knows that the - # process has been closed after tracing stops. - if self._platform == 'win': - time.sleep(5) - battor._battor_shell.kill() - results = battor.CollectTraceData().splitlines() - except: - if battor._battor_shell is not None: - battor._battor_shell.kill() - battor._battor_shell = None - raise - - self.assertTrue('# BattOr' in results[0]) - self.assertTrue('# voltage_range' in results[1]) - self.assertTrue('# current_range' in results[2]) - self.assertTrue('# sample_rate' in results[3]) - # First line with results. Should be 3 'words'. - self.assertTrue(len(results[4].split()) == 3) - clock_sync_found = False - for entry in results: - if '' in entry: - clock_sync_found = True - break - self.assertTrue(clock_sync_found, 'BattOr Data:%s\n' % repr(results)) - - -if __name__ == '__main__': - logging.getLogger().setLevel(logging.DEBUG) - unittest.main(verbosity=2) diff --git a/catapult/common/battor/battor/battor_wrapper_unittest.py b/catapult/common/battor/battor/battor_wrapper_unittest.py deleted file mode 100644 index 7e14c349..00000000 --- a/catapult/common/battor/battor/battor_wrapper_unittest.py +++ /dev/null @@ -1,387 +0,0 @@ -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import dependency_manager -import logging -import mock -import subprocess -import unittest - -from battor import battor_error -from battor import battor_wrapper -from devil.utils import battor_device_mapping -from devil.utils import find_usb_devices - -import serial -from serial.tools import list_ports - - -class DependencyManagerMock(object): - def __init__(self, _): - self._fetch_return = 'path' - self._version_return = 'cbaa843' - - def FetchPath(self, _, *unused): - del unused - return self._fetch_return - - def FetchPathWithVersion(self, _, *unused): - del unused - return self._fetch_return, self._version_return - -class PopenMock(object): - def __init__(self, *unused): - pass - - def poll(self): - pass - - def kill(self): - pass - - -class IsBattOrConnectedTest(unittest.TestCase): - def setUp(self): - # Windows monkey patches. - self._serial_tools_return = [] - self._comports = serial.tools.list_ports.comports - serial.tools.list_ports.comports = lambda: self._serial_tools_return - - # Linux/Android monkey patches. - self._generate_serial_map_return = {} - self._generate_serial_map = battor_device_mapping.GenerateSerialMap - battor_device_mapping.GenerateSerialMap = ( - lambda: self._generate_serial_map_return) - - self._read_serial_map_file_return = {} - self._read_serial_map_file = battor_device_mapping.ReadSerialMapFile - battor_device_mapping.ReadSerialMapFile = ( - lambda f: self._read_serial_map_file_return) - - self._get_bus_number_to_device_tree_map = ( - find_usb_devices.GetBusNumberToDeviceTreeMap) - find_usb_devices.GetBusNumberToDeviceTreeMap = lambda fast=None: {} - - self._get_battor_list_return = [] - self._get_battor_list = battor_device_mapping.GetBattOrList - battor_device_mapping.GetBattOrList = lambda x: self._get_battor_list_return - - def tearDown(self): - serial.tools.list_ports.comports = self._comports - battor_device_mapping.GenerateSerialMap = self._generate_serial_map - battor_device_mapping.ReadSerialMapFile = self._read_serial_map_file - find_usb_devices.GetBusNumberToDeviceTreeMap = ( - self._get_bus_number_to_device_tree_map) - battor_device_mapping.GetBattOrList = self._get_battor_list - - def forceException(self): - raise NotImplementedError - - def testAndroidWithBattOr(self): - self._generate_serial_map_return = {'abc': '123'} - self.assertTrue(battor_wrapper.IsBattOrConnected('android', 'abc')) - - def testAndroidWithoutMatchingBattOr(self): - self._generate_serial_map_return = {'notabc': 'not123'} - self.assertFalse(battor_wrapper.IsBattOrConnected('android', 'abc')) - - def testAndroidNoDevicePassed(self): - with self.assertRaises(ValueError): - battor_wrapper.IsBattOrConnected('android') - - def testAndroidWithMapAndFile(self): - device_map = {'abc': '123'} - battor_device_mapping.ReadSerialMapFile = self.forceException - self.assertTrue( - battor_wrapper.IsBattOrConnected('android', android_device='abc', - android_device_map=device_map, - android_device_file='file')) - - def testAndroidWithMap(self): - self.assertTrue( - battor_wrapper.IsBattOrConnected('android', android_device='abc', - android_device_map={'abc', '123'})) - - def testAndroidWithFile(self): - self._read_serial_map_file_return = {'abc': '123'} - self.assertTrue( - battor_wrapper.IsBattOrConnected('android', android_device='abc', - android_device_file='file')) - - def testLinuxWithBattOr(self): - self._get_battor_list_return = ['battor'] - self.assertTrue(battor_wrapper.IsBattOrConnected('linux')) - - def testLinuxWithoutBattOr(self): - self._get_battor_list_return = [] - self.assertFalse(battor_wrapper.IsBattOrConnected('linux')) - - def testMacWithBattOr(self): - self._serial_tools_return = [('/dev/tty.usbserial-MAA', 'BattOr v3.3', '')] - self.assertTrue(battor_wrapper.IsBattOrConnected('mac')) - - def testMacWithoutBattOr(self): - self._serial_tools_return = [('/dev/tty.usbserial-MAA', 'not_one', '')] - self.assertFalse(battor_wrapper.IsBattOrConnected('mac')) - - def testWinWithBattOr(self): - self._serial_tools_return = [('COM4', 'USB Serial Port', '')] - self.assertTrue(battor_wrapper.IsBattOrConnected('win')) - - def testWinWithoutBattOr(self): - self._get_battor_list_return = [] - self.assertFalse(battor_wrapper.IsBattOrConnected('win')) - - -class BattOrWrapperTest(unittest.TestCase): - def setUp(self): - self._battor = None - self._is_battor = True - self._battor_list = ['battor1'] - self._should_pass = True - self._fake_map = {'battor1': 'device1'} - self._fake_return_code = None - self._fake_battor_return = 'Done.\n' - - self._get_battor_path_from_phone_serial = ( - battor_device_mapping.GetBattOrPathFromPhoneSerial) - self._get_bus_number_to_device_tree_map = ( - find_usb_devices.GetBusNumberToDeviceTreeMap) - self._dependency_manager = dependency_manager.DependencyManager - self._get_battor_list = battor_device_mapping.GetBattOrList - self._is_battor = battor_device_mapping.IsBattOr - self._generate_serial_map = battor_device_mapping.GenerateSerialMap - self._serial_tools = serial.tools.list_ports.comports - - battor_device_mapping.GetBattOrPathFromPhoneSerial = ( - lambda x, serial_map_file=None, serial_map=None: x + '_battor') - find_usb_devices.GetBusNumberToDeviceTreeMap = lambda fast=False: True - dependency_manager.DependencyManager = DependencyManagerMock - battor_device_mapping.GetBattOrList = lambda x: self._battor_list - battor_device_mapping.IsBattOr = lambda x, y: self._is_battor - battor_device_mapping.GenerateSerialMap = lambda: self._fake_map - serial.tools.list_ports.comports = lambda: [('COM4', 'USB Serial Port', '')] - - self._subprocess_check_output_code = 0 - def subprocess_check_output_mock(*unused): - if self._subprocess_check_output_code != 0: - raise subprocess.CalledProcessError(None, None) - return 0 - self._subprocess_check_output = subprocess.check_output - subprocess.check_output = subprocess_check_output_mock - - def tearDown(self): - battor_device_mapping.GetBattOrPathFromPhoneSerial = ( - self._get_battor_path_from_phone_serial) - find_usb_devices.GetBusNumberToDeviceTreeMap = ( - self._get_bus_number_to_device_tree_map) - dependency_manager.DependencyManager = self._dependency_manager - battor_device_mapping.GetBattOrList = self._get_battor_list - battor_device_mapping.IsBattOr = self._is_battor - battor_device_mapping.GenerateSerialMap = self._generate_serial_map - serial.tools.list_ports.comports = self._serial_tools - subprocess.check_output = self._subprocess_check_output - - def _DefaultBattOrReplacements(self): - battor_wrapper.DEFAULT_SHELL_CLOSE_TIMEOUT_S = .1 - self._battor._StartShellImpl = lambda *unused: PopenMock() - self._battor.GetShellReturnCode = lambda *unused: self._fake_return_code - self._battor._SendBattOrCommandImpl = lambda x: self._fake_battor_return - self._battor._StopTracingImpl = lambda *unused: (self._fake_battor_return, - None) - - def testBadPlatform(self): - with self.assertRaises(battor_error.BattOrError): - self._battor = battor_wrapper.BattOrWrapper('unknown') - - def testInitAndroidWithBattOr(self): - self._battor = battor_wrapper.BattOrWrapper('android', android_device='abc') - self.assertEquals(self._battor._battor_path, 'abc_battor') - - def testInitAndroidWithoutBattOr(self): - self._battor_list = [] - self._fake_map = {} - battor_device_mapping.GetBattOrPathFromPhoneSerial = ( - self._get_battor_path_from_phone_serial) - with self.assertRaises(battor_error.BattOrError): - self._battor = battor_wrapper.BattOrWrapper('android', - android_device='abc') - - def testInitBattOrPathIsBattOr(self): - battor_path = 'battor/path/here' - self._battor = battor_wrapper.BattOrWrapper( - 'android', android_device='abc', battor_path=battor_path) - self.assertEquals(self._battor._battor_path, battor_path) - - def testInitNonAndroidWithBattOr(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self.assertEquals(self._battor._battor_path, 'COM4') - - def testInitNonAndroidWithMultipleBattOr(self): - self._battor_list.append('battor2') - with self.assertRaises(battor_error.BattOrError): - self._battor = battor_wrapper.BattOrWrapper('linux') - - def testInitNonAndroidWithoutBattOr(self): - self._battor_list = [] - serial.tools.list_ports.comports = lambda: [('COM4', 'None', '')] - with self.assertRaises(battor_error.BattOrError): - self._battor = battor_wrapper.BattOrWrapper('win') - - def testStartShellPass(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self.assertIsNotNone(self._battor._battor_shell) - - def testStartShellDoubleStart(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - with self.assertRaises(AssertionError): - self._battor.StartShell() - - def testStartShellFail(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.GetShellReturnCode = lambda *unused: 1 - with self.assertRaises(AssertionError): - self._battor.StartShell() - - def testStartTracingPass(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.StartTracing() - self.assertTrue(self._battor._tracing) - - def testStartTracingDoubleStart(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.StartTracing() - with self.assertRaises(AssertionError): - self._battor.StartTracing() - - def testStartTracingCommandFails(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor._SendBattOrCommandImpl = lambda *unused: 'Fail.\n' - self._battor.StartShell() - with self.assertRaises(battor_error.BattOrError): - self._battor.StartTracing() - - def testStopTracingPass(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.StartTracing() - self._battor.GetShellReturnCode = lambda *unused: 0 - self._battor.StopTracing() - self.assertFalse(self._battor._tracing) - - def testStopTracingNotRunning(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - with self.assertRaises(AssertionError): - self._battor.StopTracing() - - def testFlashFirmwarePass(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self.assertTrue(self._battor.FlashFirmware('hex_path', 'config_path')) - - def testFlashFirmwareFail(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self._subprocess_check_output_code = 1 - with self.assertRaises(battor_wrapper.BattOrFlashError): - self._battor.FlashFirmware('hex_path', 'config_path') - - def testFlashFirmwareShellRunning(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self._battor.StartShell() - with self.assertRaises(AssertionError): - self._battor.FlashFirmware('hex_path', 'config_path') - - def testGetFirmwareGitHashNotRunning(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - with self.assertRaises(AssertionError): - self._battor.GetFirmwareGitHash() - - def testGetFirmwareGitHashPass(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.GetFirmwareGitHash = lambda: 'cbaa843' - self.assertTrue(isinstance(self._battor.GetFirmwareGitHash(), basestring)) - - def testStopShellPass(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._fake_return_code = 0 - self._battor.StopShell() - self.assertIsNone(self._battor._battor_shell) - - @mock.patch('time.sleep', mock.Mock) - def testStopShellTimeOutAndKill(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.StopShell() - self.assertIsNone(self._battor._battor_shell) - - def testStopShellNotStarted(self): - self._battor = battor_wrapper.BattOrWrapper('win') - self._DefaultBattOrReplacements() - with self.assertRaises(AssertionError): - self._battor.StopShell() - - @mock.patch('time.sleep', mock.Mock) - def testFlashBattOrSameGitHash(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.GetFirmwareGitHash = lambda: 'cbaa843' - dependency_manager.DependencyManager._version_return = 'cbaa843' - self.assertFalse(self._battor._FlashBattOr()) - - @mock.patch('time.sleep', mock.Mock) - def testFlashBattOrDifferentGitHash(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.GetFirmwareGitHash = lambda: 'bazz732' - dependency_manager.DependencyManager._version_return = 'cbaa843' - self.assertTrue(self._battor._FlashBattOr()) - - def testCollectTraceDataNoStartTime(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.StartTracing() - self._battor.GetShellReturnCode = lambda *unused: 0 - self._battor.StopTracing() - self._battor._start_tracing_time = None - with self.assertRaises(battor_error.BattOrError): - self._battor.CollectTraceData() - - def testCollectTraceDataNoStopTime(self): - self._battor = battor_wrapper.BattOrWrapper('linux') - self._DefaultBattOrReplacements() - self._battor.StartShell() - self._battor.StartTracing() - self._battor.GetShellReturnCode = lambda *unused: 0 - self._battor.StopTracing() - self._battor._stop_tracing_time = None - with self.assertRaises(battor_error.BattOrError): - self._battor.CollectTraceData() - - -if __name__ == '__main__': - logging.getLogger().setLevel(logging.DEBUG) - unittest.main(verbosity=2) diff --git a/catapult/common/battor/bin/run_py_tests b/catapult/common/battor/bin/run_py_tests deleted file mode 100755 index 61103bd9..00000000 --- a/catapult/common/battor/bin/run_py_tests +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env python -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import os -import sys - -_CATAPULT_PATH = os.path.abspath(os.path.join( - os.path.dirname(__file__), '..', '..', '..')) -_BATTOR_PATH = os.path.abspath(os.path.join( - os.path.dirname(__file__), '..')) - -sys.path.append(_CATAPULT_PATH) -from catapult_build import run_with_typ - - -def main(): - return run_with_typ.Run(top_level_dir=_BATTOR_PATH) - - -if __name__ == '__main__': - sys.exit(main()) diff --git a/catapult/common/battor/bin/upload_battor_binaries.py b/catapult/common/battor/bin/upload_battor_binaries.py deleted file mode 100755 index 8901cfca..00000000 --- a/catapult/common/battor/bin/upload_battor_binaries.py +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env python -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import argparse -import os -import sys - -sys.path.append( - os.path.abspath(os.path.join(os.path.dirname(__file__), - '..', '..', '..', 'dependency_manager'))) -from dependency_manager import base_config # pylint: disable=import-error - - -_SUPPORTED_ARCHS = [ - 'linux2_x86_64', 'darwin_x86_64', 'win_AMD64', 'win32_AMD64', 'win32_x86', - 'default' -] -_DEFAULT_DEP = 'battor_agent_binary' -_DEFAULT_CONFIG = os.path.join(os.path.dirname(__file__), '..', 'battor', - 'battor_binary_dependencies.json') - - -def UploadBinary(arch, path, config, dep): - print 'Uploading binary:' - print ' arch: %s' % arch - print ' path: %s' % path - print ' config: %s' % config - print ' dep: %s' % dep - c = base_config.BaseConfig(config, writable=True) - c.AddCloudStorageDependencyUpdateJob( - dep, arch, path, version=None, execute_job=True) - print 'Upload complete.' - - -def main(): - parser = argparse.ArgumentParser() - parser.add_argument('--arch', '--architecture', required=True, - help='Architecture binary is built for.') - parser.add_argument('--path', required=True, help='Path to binary.') - parser.add_argument('--config', default=_DEFAULT_CONFIG, - help='Path to dependency manager config') - parser.add_argument('--dep', default=_DEFAULT_DEP, - help='Name of dependency to update.') - args = parser.parse_args() - if args.arch not in _SUPPORTED_ARCHS: - print 'Arch must be one of: %s' % _SUPPORTED_ARCHS - return 1 - UploadBinary(args.arch, args.path, args.config, args.dep) - return 0 - -if __name__ == '__main__': - sys.exit(main()) diff --git a/catapult/common/bin/update_chrome_reference_binaries b/catapult/common/bin/update_chrome_reference_binaries index 641fdc41..02070f08 100755 --- a/catapult/common/bin/update_chrome_reference_binaries +++ b/catapult/common/bin/update_chrome_reference_binaries @@ -42,11 +42,11 @@ CHROME_GS_BUCKET = 'chrome-unsigned' # Add one to enable updating it. (Must also update _PLATFORM_MAP.) _PLATFORMS_TO_UPDATE = ['mac_x86_64', 'win_x86', 'win_AMD64', 'linux_x86_64', 'android_k_armeabi-v7a', 'android_l_arm64-v8a', - 'android_l_armeabi-v7a'] + 'android_l_armeabi-v7a', 'android_n_armeabi-v7a'] # Remove a channal name from this list to disable updating it. # Add one to enable updating it. -_CHANNELS_TO_UPDATE = ['stable'] # 'canary', 'dev' +_CHANNELS_TO_UPDATE = ['stable', 'canary', 'dev'] # Omaha is Chrome's autoupdate server. It reports the current versions used @@ -62,34 +62,39 @@ _OMAHA_PLATFORMS = { 'stable': ['mac', 'linux', 'win', 'android'], # destination: Name of the folder to download the reference build to. UpdateInfo = collections.namedtuple('UpdateInfo', 'omaha, gs_folder, gs_build, zip_name') -_PLATFORM_MAP = { 'mac_x86_64': UpdateInfo(omaha='mac', - gs_folder='desktop-*', - gs_build='mac64', - zip_name='chrome-mac.zip'), - 'win_x86': UpdateInfo(omaha='win', - gs_folder='desktop-*', - gs_build='win', - zip_name='chrome-win.zip'), - 'win_AMD64': UpdateInfo(omaha='win', +_PLATFORM_MAP = {'mac_x86_64': UpdateInfo(omaha='mac', gs_folder='desktop-*', - gs_build='win64', - zip_name='chrome-win64.zip'), - 'linux_x86_64': UpdateInfo(omaha='linux', - gs_folder='desktop-*', - gs_build='linux64', - zip_name='chrome-linux64.zip'), - 'android_k_armeabi-v7a': UpdateInfo(omaha='android', - gs_folder='android-*', - gs_build='arm', - zip_name='Chrome.apk'), - 'android_l_arm64-v8a': UpdateInfo(omaha='android', - gs_folder='android-*', - gs_build='arm_64', - zip_name='ChromeModern.apk'), - 'android_l_armeabi-v7a': UpdateInfo(omaha='android', - gs_folder='android-*', - gs_build='arm', - zip_name='Chrome.apk'), + gs_build='mac64', + zip_name='chrome-mac.zip'), + 'win_x86': UpdateInfo(omaha='win', + gs_folder='desktop-*', + gs_build='win-clang', + zip_name='chrome-win-clang.zip'), + 'win_AMD64': UpdateInfo(omaha='win', + gs_folder='desktop-*', + gs_build='win64-clang', + zip_name='chrome-win64-clang.zip'), + 'linux_x86_64': UpdateInfo(omaha='linux', + gs_folder='desktop-*', + gs_build='linux64', + zip_name='chrome-linux64.zip'), + 'android_k_armeabi-v7a': UpdateInfo(omaha='android', + gs_folder='android-*', + gs_build='arm', + zip_name='Chrome.apk'), + 'android_l_arm64-v8a': UpdateInfo(omaha='android', + gs_folder='android-*', + gs_build='arm_64', + zip_name='ChromeModern.apk'), + 'android_l_armeabi-v7a': UpdateInfo(omaha='android', + gs_folder='android-*', + gs_build='arm', + zip_name='Chrome.apk'), + 'android_n_armeabi-v7a': UpdateInfo(omaha='android', + gs_folder='android-*', + gs_build='arm', + zip_name='Monochrome.apk'), + } @@ -139,8 +144,10 @@ def _QueuePlatformUpdate(platform, version, config, channel): remote_path = '%s/%s/%s/%s' % ( platform_info.gs_folder, version, platform_info.gs_build, filename) if not cloud_storage.Exists(CHROME_GS_BUCKET, remote_path): + cloud_storage_path = 'gs://%s/%s' % (CHROME_GS_BUCKET, remote_path) raise BuildNotFoundError( - 'Failed to find %s build for version %s at path %s.' % (platform, version, remote_path)) + 'Failed to find %s build for version %s at path %s.' % ( + platform, version, cloud_storage_path)) reference_builds_folder = os.path.join( os.path.dirname(os.path.abspath(__file__)), 'chrome_telemetry_build', 'reference_builds', channel) diff --git a/catapult/common/node_runner/node_runner/minify b/catapult/common/node_runner/node_runner/minify new file mode 100755 index 00000000..9d5bb2f1 --- /dev/null +++ b/catapult/common/node_runner/node_runner/minify @@ -0,0 +1,58 @@ +#!/usr/bin/env node +'use strict'; +/* +Copyright 2018 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. + +This script wraps common HTML transformations including stripping whitespace and +comments from HTML, CSS, and Javascript. +*/ +const dom5 = require('dom5'); +const escodegen = require('escodegen'); +const espree = require('espree'); +const fs = require('fs'); +const nopt = require('nopt'); + +const ESPREE_OPTIONS = { + attachComment: false, + comments: false, + ecmaVersion: 2018, +}; + +const args = nopt(); +const filename = args.argv.remain[0]; + +let html = fs.readFileSync(filename).toString('utf8'); +let parsedHtml = dom5.parse(html); +// First, collapse text nodes around comments (by removing comment nodes, +// re-serializing, and re-parsing) in order to prevent multiple extraneous +// newlines. +for (const node of dom5.nodeWalkAll(parsedHtml, () => true)) { + if (dom5.isCommentNode(node)) { + dom5.remove(node); + } +} +html = dom5.serialize(parsedHtml); +parsedHtml = dom5.parse(html); +// Some of these transformations are based on polyclean: +// https://github.com/googlearchive/polyclean +for (const node of dom5.nodeWalkAll(parsedHtml, () => true)) { + if (dom5.isTextNode(node)) { + dom5.setTextContent(node, dom5.getTextContent(node) + .replace(/ *\n+ */g, '\n') + .replace(/\n+/g, '\n')); + } else if (dom5.predicates.hasTagName('script')(node) && + !dom5.predicates.hasAttr('src')(node)) { + dom5.setTextContent(node, escodegen.generate( + espree.parse(dom5.getTextContent(node), ESPREE_OPTIONS), + {format: {indent: {style: ''}}})); + } else if (dom5.predicates.hasTagName('style')(node)) { + dom5.setTextContent(node, dom5.getTextContent(node) + .replace(/[\r\n]/g, '') + .replace(/ {2,}/g, ' ') + .replace(/(^|[;,\:\{\}]) /g, '$1') + .replace(/ ($|[;,\{\}])/g, '$1')); + } +} +fs.writeFileSync(filename, dom5.serialize(parsedHtml)); diff --git a/catapult/common/node_runner/node_runner/package.json b/catapult/common/node_runner/node_runner/package.json index 27d0325b..9a92270d 100644 --- a/catapult/common/node_runner/node_runner/package.json +++ b/catapult/common/node_runner/node_runner/package.json @@ -15,8 +15,12 @@ "gypfile": false, "private": true, "dependencies": { - "eslint": "^3.14.1", + "dom5": "^1.0.0", + "escodegen": "^1.0.0", + "eslint": "^4.0.0", "eslint-config-google": "^0.6.0", - "eslint-plugin-html": "^2.0.0" + "eslint-plugin-html": "^4.0.0", + "espree": "^3.0.0", + "vulcanize": "^1.16.0" } } diff --git a/catapult/common/py_utils/py_utils/__init__.py b/catapult/common/py_utils/py_utils/__init__.py index fba0897c..dcec4ed2 100644 --- a/catapult/common/py_utils/py_utils/__init__.py +++ b/catapult/common/py_utils/py_utils/__init__.py @@ -74,8 +74,8 @@ _AddDirToPythonPath(os.path.join(GetCatapultDir(), 'third_party', 'mox3')) _AddDirToPythonPath( os.path.join(GetCatapultDir(), 'third_party', 'pyfakefs')) -from devil.utils import timeout_retry -from devil.utils import reraiser_thread +from devil.utils import timeout_retry # pylint: disable=wrong-import-position +from devil.utils import reraiser_thread # pylint: disable=wrong-import-position # Decorator that adds timeout functionality to a function. diff --git a/catapult/common/py_utils/py_utils/binary_manager.py b/catapult/common/py_utils/py_utils/binary_manager.py index 8af08cf9..2d3ac8a6 100644 --- a/catapult/common/py_utils/py_utils/binary_manager.py +++ b/catapult/common/py_utils/py_utils/binary_manager.py @@ -13,7 +13,7 @@ class BinaryManager(object): """ def __init__(self, config_files): - if not config_files or type(config_files) != list: + if not config_files or not isinstance(config_files, list): raise ValueError( 'Must supply a list of config files to the BinaryManager') configs = [dependency_manager.BaseConfig(config) for config in config_files] diff --git a/catapult/common/py_utils/py_utils/chrome_binaries.json b/catapult/common/py_utils/py_utils/chrome_binaries.json index ce357c7e..8a9b6bfe 100644 --- a/catapult/common/py_utils/py_utils/chrome_binaries.json +++ b/catapult/common/py_utils/py_utils/chrome_binaries.json @@ -6,22 +6,22 @@ "cloud_storage_bucket": "chrome-telemetry", "file_info": { "mac_x86_64": { - "cloud_storage_hash": "b321a01b2c98fe62b1876655b10436c2226b1b76", + "cloud_storage_hash": "6278cf24b700076fd17ae8616fd980d18f33ed5d", "download_path": "bin/reference_builds/chrome-mac64.zip", "path_within_archive": "chrome-mac/Google Chrome.app/Contents/MacOS/Google Chrome", - "version_in_cs": "62.0.3194.0" + "version_in_cs": "70.0.3509.0" }, "win_AMD64": { - "cloud_storage_hash": "2da1c7861745ab0e8f666f119eeb58c1410710cc", - "download_path": "bin\\reference_build\\chrome-win64-pgo.zip", - "path_within_archive": "chrome-win64-pgo\\chrome.exe", - "version_in_cs": "62.0.3194.0" + "cloud_storage_hash": "a78facdb295d2ee36aaf5af89e54b5c5fcd48f7c", + "download_path": "bin\\reference_build\\chrome-win64-clang.zip", + "path_within_archive": "chrome-win64-clang\\chrome.exe", + "version_in_cs": "70.0.3509.0" }, "win_x86": { - "cloud_storage_hash": "270abd11621386be612af02b707844cba06c0dbd", - "download_path": "bin\\reference_build\\chrome-win32-pgo.zip", - "path_within_archive": "chrome-win32-pgo\\chrome.exe", - "version_in_cs": "62.0.3194.0" + "cloud_storage_hash": "348e8133c5fa687864a3d8eff13ed5be6852e95d", + "download_path": "bin\\reference_build\\chrome-win32-clang.zip", + "path_within_archive": "chrome-win32-clang\\chrome.exe", + "version_in_cs": "70.0.3509.0" } } }, @@ -30,10 +30,10 @@ "cloud_storage_bucket": "chrome-telemetry", "file_info": { "linux_x86_64": { - "cloud_storage_hash": "2592ec6f8dd56227c3c281e3cccecd6c9ba72cad", + "cloud_storage_hash": "1ef8d8ebf114b47aecf11a36c21377376ced3794", "download_path": "bin/reference_build/chrome-linux64.zip", "path_within_archive": "chrome-linux64/chrome", - "version_in_cs": "62.0.3192.0" + "version_in_cs": "69.0.3497.23" } } }, @@ -42,45 +42,50 @@ "cloud_storage_bucket": "chrome-telemetry", "file_info": { "android_k_armeabi-v7a": { - "cloud_storage_hash": "948c776335d3a38d6e8de0dba576e109c6b5724c", + "cloud_storage_hash": "5a4cc68b2ef5e6073f9f8f42987155d5fc8a3c48", "download_path": "bin/reference_build/android_k_armeabi-v7a/ChromeStable.apk", - "version_in_cs": "63.0.3239.111" + "version_in_cs": "68.0.3440.85" }, "android_l_arm64-v8a": { - "cloud_storage_hash": "a25663aad7397002f6dfe44fb97087fdd77df119", + "cloud_storage_hash": "42d527ca74e99fb9398826204db09c8740df7fd4", "download_path": "bin/reference_build/android_l_arm64-v8a/ChromeStable.apk", - "version_in_cs": "63.0.3239.111" + "version_in_cs": "68.0.3440.85" }, "android_l_armeabi-v7a": { - "cloud_storage_hash": "948c776335d3a38d6e8de0dba576e109c6b5724c", + "cloud_storage_hash": "5a4cc68b2ef5e6073f9f8f42987155d5fc8a3c48", "download_path": "bin/reference_build/android_l_armeabi-v7a/ChromeStable.apk", - "version_in_cs": "63.0.3239.111" + "version_in_cs": "68.0.3440.85" + }, + "android_n_armeabi-v7a": { + "cloud_storage_hash": "d37f47a804e815daf001f65d0c13a2cf38641f3e", + "download_path": "bin/reference_build/android_n_armeabi-v7a/Monochrome.apk", + "version_in_cs": "68.0.3440.85" }, "linux_x86_64": { - "cloud_storage_hash": "b0506e43d268eadb887ccc847695674f9d2e51a5", + "cloud_storage_hash": "aab60e4a4ee4f3d638aa6a33e52ffb6423fa7080", "download_path": "bin/reference_build/chrome-linux64.zip", "path_within_archive": "chrome-linux64/chrome", - "version_in_cs": "63.0.3239.108" + "version_in_cs": "68.0.3440.84" }, "mac_x86_64": { - "cloud_storage_hash": "56a3de45b37b7eb563006c30a548a48928cffb39", + "cloud_storage_hash": "8a020bc9caa2526408dc23044e8dcfaaf6b6948e", "download_path": "bin/reference_builds/chrome-mac64.zip", "path_within_archive": "chrome-mac/Google Chrome.app/Contents/MacOS/Google Chrome", - "version_in_cs": "63.0.3239.108" + "version_in_cs": "68.0.3440.84" }, "win_AMD64": { - "cloud_storage_hash": "d1511334055c88fd9fa5e6e63fee666d9be8c433", - "download_path": "bin\\reference_build\\chrome-win64.zip", - "path_within_archive": "chrome-win64\\chrome.exe", - "version_in_cs": "63.0.3239.108" + "cloud_storage_hash": "19da10346662d8e791076a0ddcfbf2a435b6915a", + "download_path": "bin\\reference_build\\chrome-win64-clang.zip", + "path_within_archive": "chrome-win64-clang\\chrome.exe", + "version_in_cs": "68.0.3440.84" }, "win_x86": { - "cloud_storage_hash": "9e869b3b25ee7b682712cde6eaddc2d7fa84cc90", - "download_path": "bin\\reference_build\\chrome-win32.zip", - "path_within_archive": "chrome-win32\\chrome.exe", - "version_in_cs": "63.0.3239.108" + "cloud_storage_hash": "760ff8661550f6aebadedba99075efe6adae3414", + "download_path": "bin\\reference_build\\chrome-win-clang.zip", + "path_within_archive": "chrome-win-clang\\chrome.exe", + "version_in_cs": "68.0.3440.84" } } } } -} +} \ No newline at end of file diff --git a/catapult/common/py_utils/py_utils/cloud_storage.py b/catapult/common/py_utils/py_utils/cloud_storage.py index f6013806..df5589b7 100644 --- a/catapult/common/py_utils/py_utils/cloud_storage.py +++ b/catapult/common/py_utils/py_utils/cloud_storage.py @@ -120,6 +120,9 @@ def _EnsureExecutable(gsutil): os.chmod(gsutil, st.st_mode | stat.S_IEXEC) +def _IsRunningOnSwarming(): + return os.environ.get('SWARMING_HEADLESS') is not None + 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 @@ -132,6 +135,8 @@ def _RunCommand(args): if py_utils.IsRunningOnCrosDevice(): gsutil_env = os.environ.copy() gsutil_env['HOME'] = _CROS_GSUTIL_HOME_WAR + elif _IsRunningOnSwarming(): + gsutil_env = os.environ.copy() if os.name == 'nt': # If Windows, prepend python. Python scripts aren't directly executable. diff --git a/catapult/common/py_utils/py_utils/cloud_storage_unittest.py b/catapult/common/py_utils/py_utils/cloud_storage_unittest.py index ae2f7482..7648db6b 100644 --- a/catapult/common/py_utils/py_utils/cloud_storage_unittest.py +++ b/catapult/common/py_utils/py_utils/cloud_storage_unittest.py @@ -154,6 +154,19 @@ class CloudStorageFakeFsUnitTest(BaseFakeFsUnitTest): finally: cloud_storage._RunCommand = orig_run_command + @mock.patch('py_utils.cloud_storage.subprocess.Popen') + def testSwarmingUsesExistingEnv(self, mock_popen): + os.environ['SWARMING_HEADLESS'] = '1' + + mock_gsutil = mock_popen() + mock_gsutil.communicate = mock.MagicMock(return_value=('a', 'b')) + mock_gsutil.returncode = None + + cloud_storage.Copy('bucket1', 'bucket2', 'remote_path1', 'remote_path2') + + mock_popen.assert_called_with( + mock.ANY, stderr=-1, env=os.environ, stdout=-1) + @mock.patch('py_utils.cloud_storage._FileLock') def testDisableCloudStorageIo(self, unused_lock_mock): os.environ['DISABLE_CLOUD_STORAGE_IO'] = '1' diff --git a/catapult/common/py_utils/py_utils/file_util.py b/catapult/common/py_utils/py_utils/file_util.py new file mode 100644 index 00000000..36cc42f2 --- /dev/null +++ b/catapult/common/py_utils/py_utils/file_util.py @@ -0,0 +1,23 @@ +# Copyright 2018 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import errno +import os +import shutil + + +def CopyFileWithIntermediateDirectories(source_path, dest_path): + """Copies a file and creates intermediate directories as needed. + + Args: + source_path: Path to the source file. + dest_path: Path to the destination where the source file should be copied. + """ + assert os.path.exists(source_path) + try: + os.makedirs(os.path.dirname(dest_path)) + except OSError, e: + if e.errno != errno.EEXIST: + raise + shutil.copy(source_path, dest_path) diff --git a/catapult/common/py_utils/py_utils/file_util_unittest.py b/catapult/common/py_utils/py_utils/file_util_unittest.py new file mode 100644 index 00000000..4bb19a14 --- /dev/null +++ b/catapult/common/py_utils/py_utils/file_util_unittest.py @@ -0,0 +1,66 @@ +# Copyright 2018 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import errno +import os +import shutil +import tempfile +import unittest + +from py_utils import file_util + + +class FileUtilTest(unittest.TestCase): + + def setUp(self): + self._tempdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self._tempdir) + + def testCopySimple(self): + source_path = os.path.join(self._tempdir, 'source') + with open(source_path, 'w') as f: + f.write('data') + + dest_path = os.path.join(self._tempdir, 'dest') + + self.assertFalse(os.path.exists(dest_path)) + file_util.CopyFileWithIntermediateDirectories(source_path, dest_path) + self.assertTrue(os.path.exists(dest_path)) + self.assertEqual('data', open(dest_path, 'r').read()) + + def testCopyMakeDirectories(self): + source_path = os.path.join(self._tempdir, 'source') + with open(source_path, 'w') as f: + f.write('data') + + dest_path = os.path.join(self._tempdir, 'path', 'to', 'dest') + + self.assertFalse(os.path.exists(dest_path)) + file_util.CopyFileWithIntermediateDirectories(source_path, dest_path) + self.assertTrue(os.path.exists(dest_path)) + self.assertEqual('data', open(dest_path, 'r').read()) + + def testCopyOverwrites(self): + source_path = os.path.join(self._tempdir, 'source') + with open(source_path, 'w') as f: + f.write('source_data') + + dest_path = os.path.join(self._tempdir, 'dest') + with open(dest_path, 'w') as f: + f.write('existing_data') + + file_util.CopyFileWithIntermediateDirectories(source_path, dest_path) + self.assertEqual('source_data', open(dest_path, 'r').read()) + + def testRaisesError(self): + source_path = os.path.join(self._tempdir, 'source') + with open(source_path, 'w') as f: + f.write('data') + + dest_path = "" + with self.assertRaises(OSError) as cm: + file_util.CopyFileWithIntermediateDirectories(source_path, dest_path) + self.assertEqual(errno.ENOENT, cm.exception.error_code) diff --git a/catapult/common/py_utils/py_utils/lock.py b/catapult/common/py_utils/py_utils/lock.py index aa9a0959..f6556183 100644 --- a/catapult/common/py_utils/py_utils/lock.py +++ b/catapult/common/py_utils/py_utils/lock.py @@ -14,19 +14,23 @@ class LockException(Exception): pass +# pylint: disable=import-error +# pylint: disable=wrong-import-position if os.name == 'nt': - import win32con # pylint: disable=import-error - import win32file # pylint: disable=import-error - import pywintypes # pylint: disable=import-error + import win32con + import win32file + import pywintypes LOCK_EX = win32con.LOCKFILE_EXCLUSIVE_LOCK LOCK_SH = 0 # the default LOCK_NB = win32con.LOCKFILE_FAIL_IMMEDIATELY _OVERLAPPED = pywintypes.OVERLAPPED() elif os.name == 'posix': - import fcntl # pylint: disable=import-error + import fcntl LOCK_EX = fcntl.LOCK_EX LOCK_SH = fcntl.LOCK_SH LOCK_NB = fcntl.LOCK_NB +# pylint: enable=import-error +# pylint: enable=wrong-import-position @contextlib.contextmanager diff --git a/catapult/common/py_utils/py_utils/memory_debug.py b/catapult/common/py_utils/py_utils/memory_debug.py index 864725d0..e63938f3 100755 --- a/catapult/common/py_utils/py_utils/memory_debug.py +++ b/catapult/common/py_utils/py_utils/memory_debug.py @@ -6,8 +6,11 @@ import heapq import logging import os -import psutil import sys +try: + import psutil +except ImportError: + psutil = None BYTE_UNITS = ['B', 'KiB', 'MiB', 'GiB'] @@ -39,6 +42,9 @@ def _LogProcessInfo(pinfo, level): def LogHostMemoryUsage(top_n=10, level=logging.INFO): + if not psutil: + logging.warning('psutil module is not found, skipping logging memory info') + return if psutil.version_info < (2, 0): logging.warning('psutil %s too old, upgrade to version 2.0 or higher' ' for memory usage information.', psutil.__version__) @@ -55,7 +61,11 @@ def LogHostMemoryUsage(top_n=10, level=logging.INFO): logging.log(level, 'Memory usage of top %i processes groups', top_n) pinfos_by_names = {} for p in psutil.process_iter(): - pinfo = _GetProcessInfo(p) + try: + pinfo = _GetProcessInfo(p) + except psutil.NoSuchProcess: + logging.exception('process %s no longer exists', p) + continue pname = pinfo['name'] if pname not in pinfos_by_names: pinfos_by_names[pname] = {'name': pname, 'total_mem_rss': 0, 'pids': []} diff --git a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/class_definition.py b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/class_definition.py index 814958f3..a83ac96d 100644 --- a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/class_definition.py +++ b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/class_definition.py @@ -13,8 +13,6 @@ __all__ = [ class Class(base_symbol.AnnotatedSymbol): - # pylint: disable=abstract-class-not-used - @classmethod def Annotate(cls, symbol_type, children): if symbol_type != symbol.stmt: diff --git a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/function_definition.py b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/function_definition.py index 50a16729..384d3cf1 100644 --- a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/function_definition.py +++ b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/function_definition.py @@ -13,8 +13,6 @@ __all__ = [ class Function(base_symbol.AnnotatedSymbol): - # pylint: disable=abstract-class-not-used - @classmethod def Annotate(cls, symbol_type, children): if symbol_type != symbol.stmt: diff --git a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/import_statement.py b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/import_statement.py index 5c38c104..94e608cc 100644 --- a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/import_statement.py +++ b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/import_statement.py @@ -39,6 +39,7 @@ class DottedName(base_symbol.AnnotatedSymbol): raise ValueError('%s is a reserved keyword.' % value_part) # If we have too many children, cut the list down to size. + # pylint: disable=attribute-defined-outside-init self._children = self._children[:len(value_parts)*2-1] # Update child nodes. @@ -82,18 +83,22 @@ class AsName(base_symbol.AnnotatedSymbol): raise ValueError('%s is a reserved keyword.' % value) if value: + # pylint: disable=access-member-before-definition if len(self.children) < 3: # If we currently have no alias, add one. + # pylint: disable=access-member-before-definition self.children.append( snippet.TokenSnippet.Create(token.NAME, 'as', (0, 1))) + # pylint: disable=access-member-before-definition self.children.append( snippet.TokenSnippet.Create(token.NAME, value, (0, 1))) else: # We already have an alias. Just update the value. + # pylint: disable=access-member-before-definition self.children[2].value = value else: # Removing the alias. Strip the "as foo". - self.children = [self.children[0]] + self.children = [self.children[0]] # pylint: disable=line-too-long, attribute-defined-outside-init class Import(base_symbol.AnnotatedSymbol): diff --git a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/reference.py b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/reference.py index 757c57fb..9102c860 100644 --- a/catapult/common/py_utils/py_utils/refactor/annotated_symbol/reference.py +++ b/catapult/common/py_utils/py_utils/refactor/annotated_symbol/reference.py @@ -58,6 +58,7 @@ class Reference(base_symbol.AnnotatedSymbol): value_parts = value.split('.') # If we have too many children, cut the list down to size. + # pylint: disable=attribute-defined-outside-init self._children = self._children[:len(value_parts)] # Update child nodes. diff --git a/catapult/common/py_utils/py_utils/retry_util_unittest.py b/catapult/common/py_utils/py_utils/retry_util_unittest.py index 151f88ed..f24577f0 100644 --- a/catapult/common/py_utils/py_utils/retry_util_unittest.py +++ b/catapult/common/py_utils/py_utils/retry_util_unittest.py @@ -1,9 +1,10 @@ # 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. -import mock import unittest +import mock + from py_utils import retry_util diff --git a/catapult/dependency_manager/PRESUBMIT.py b/catapult/dependency_manager/PRESUBMIT.py index a34480c7..04039d54 100644 --- a/catapult/dependency_manager/PRESUBMIT.py +++ b/catapult/dependency_manager/PRESUBMIT.py @@ -29,4 +29,5 @@ def _GetPathsToPrepend(input_api): input_api.os_path.join(catapult_dir, 'third_party', 'mock'), input_api.os_path.join(catapult_dir, 'third_party', 'pyfakefs'), + input_api.os_path.join(catapult_dir, 'third_party', 'zipfile'), ] diff --git a/catapult/dependency_manager/dependency_manager/archive_info.py b/catapult/dependency_manager/dependency_manager/archive_info.py index ff13f907..ff80b63c 100644 --- a/catapult/dependency_manager/dependency_manager/archive_info.py +++ b/catapult/dependency_manager/dependency_manager/archive_info.py @@ -48,7 +48,7 @@ class ArchiveInfo(object): # Remove stale unzip results if self._stale_unzip_path_glob: for path in glob.glob(self._stale_unzip_path_glob): - shutil.rmtree(path) + shutil.rmtree(path, ignore_errors=True) # TODO(aiolos): Replace UnzipFile with zipfile.extractall once python # version 2.7.4 or later can safely be assumed. dependency_manager_util.UnzipArchive( diff --git a/catapult/dependency_manager/dependency_manager/base_config.py b/catapult/dependency_manager/dependency_manager/base_config.py index c735688f..a23d00ae 100644 --- a/catapult/dependency_manager/dependency_manager/base_config.py +++ b/catapult/dependency_manager/dependency_manager/base_config.py @@ -190,6 +190,26 @@ class BaseConfig(object): def config_path(self): return self._config_path + def AddNewDependency( + self, dependency, cloud_storage_base_folder, cloud_storage_bucket): + self._ValidateIsConfigWritable() + if dependency in self: + raise ValueError('Config already contains dependency %s' % dependency) + self._config_data[dependency] = { + 'cloud_storage_base_folder': cloud_storage_base_folder, + 'cloud_storage_bucket': cloud_storage_bucket, + 'file_info': {}, + } + + def SetDownloadPath(self, dependency, platform, download_path): + self._ValidateIsConfigWritable() + if not dependency in self: + raise ValueError('Config does not contain dependency %s' % dependency) + platform_dicts = self._config_data[dependency]['file_info'] + if platform not in platform_dicts: + platform_dicts[platform] = {} + platform_dicts[platform]['download_path'] = download_path + def AddCloudStorageDependencyUpdateJob( self, dependency, platform, dependency_path, version=None, execute_job=True): @@ -293,6 +313,14 @@ class BaseConfig(object): return self._GetPlatformData( dependency, platform, data_type='version_in_cs') + def __contains__(self, dependency): + """ Returns whether this config contains |dependency| + + Args: + dependency: the string name of dependency + """ + return dependency in self._config_data + def _IsDirty(self): with open(self._config_path, 'r') as fstream: curr_config_data = json.load(fstream) diff --git a/catapult/dependency_manager/dependency_manager/base_config_unittest.py b/catapult/dependency_manager/dependency_manager/base_config_unittest.py index 0dc775bd..c10d2a78 100755 --- a/catapult/dependency_manager/dependency_manager/base_config_unittest.py +++ b/catapult/dependency_manager/dependency_manager/base_config_unittest.py @@ -1119,6 +1119,48 @@ class BaseConfigDataManipulationUnittests(fake_filesystem_unittest.TestCase): self.fs.CreateFile(self.file_path, contents='\n'.join(self.expected_file_lines)) + def testContaining(self): + config = dependency_manager.BaseConfig(self.file_path) + self.assertTrue('dep1' in config) + self.assertTrue('dep2' in config) + self.assertFalse('dep3' in config) + + def testAddNewDependencyNotWriteable(self): + config = dependency_manager.BaseConfig(self.file_path) + with self.assertRaises(dependency_manager.ReadWriteError): + config.AddNewDependency('dep4', 'foo', 'bar') + + def testAddNewDependencyWriteableButDependencyAlreadyExists(self): + config = dependency_manager.BaseConfig(self.file_path, writable=True) + with self.assertRaises(ValueError): + config.AddNewDependency('dep2', 'foo', 'bar') + + def testAddNewDependencySuccessfully(self): + config = dependency_manager.BaseConfig(self.file_path, writable=True) + config.AddNewDependency('dep3', 'foo', 'bar') + self.assertTrue('dep3' in config) + + def testSetDownloadPathNotWritable(self): + config = dependency_manager.BaseConfig(self.file_path) + with self.assertRaises(dependency_manager.ReadWriteError): + config.SetDownloadPath('dep2', 'plat1', '../../relative/dep1/path1') + + def testSetDownloadPathOnExistingPlatformSuccesfully(self): + config = dependency_manager.BaseConfig(self.file_path, writable=True) + download_path = '../../relative/dep1/foo.bar' + config.SetDownloadPath('dep2', 'plat1', download_path) + self.assertEqual( + download_path, + config._GetPlatformData('dep2', 'plat1', 'download_path')) + + def testSetDownloadPathOnNewPlatformSuccesfully(self): + config = dependency_manager.BaseConfig(self.file_path, writable=True) + download_path = '../../relative/dep1/foo.bar' + config.SetDownloadPath('dep2', 'newplat', download_path) + self.assertEqual( + download_path, + config._GetPlatformData('dep2', 'newplat', 'download_path')) + def testSetPlatformDataFailureNotWritable(self): config = dependency_manager.BaseConfig(self.file_path) @@ -1410,7 +1452,6 @@ class BaseConfigTest(unittest.TestCase): self.assertEqual(self.GetConfigDataFromDict(self.empty_dict), config._config_data) - @mock.patch('dependency_manager.dependency_info.DependencyInfo') @mock.patch('os.path') @mock.patch('__builtin__.open') diff --git a/catapult/devil/devil/android/apk_helper.py b/catapult/devil/devil/android/apk_helper.py index 8acb41e6..ab7649f8 100644 --- a/catapult/devil/devil/android/apk_helper.py +++ b/catapult/devil/devil/android/apk_helper.py @@ -5,6 +5,7 @@ """Module containing utilities for apk packages.""" import re +import zipfile from devil import base_error from devil.android.sdk import aapt @@ -242,3 +243,30 @@ class ApkHelper(object): if '.' not in name: return '%s.%s' % (self.GetPackageName(), name) return name + + def _ListApkPaths(self): + with zipfile.ZipFile(self._apk_path) as z: + return z.namelist() + + def GetAbis(self): + """Returns a list of ABIs in the apk (empty list if no native code).""" + # Use lib/* to determine the compatible ABIs. + libs = set() + for path in self._ListApkPaths(): + path_tokens = path.split('/') + if len(path_tokens) >= 2 and path_tokens[0] == 'lib': + libs.add(path_tokens[1]) + lib_to_abi = { + 'armeabi-v7a': ['armeabi-v7a', 'arm64-v8a'], + 'arm64-v8a': ['arm64-v8a'], + 'x86': ['x86', 'x64'], + 'x64': ['x64'] + } + try: + output = set() + for lib in libs: + for abi in lib_to_abi[lib]: + output.add(abi) + return sorted(output) + except KeyError: + raise base_error.BaseError('Unexpected ABI in lib/* folder.') diff --git a/catapult/devil/devil/android/apk_helper_test.py b/catapult/devil/devil/android/apk_helper_test.py index 12137db0..3be9d819 100755 --- a/catapult/devil/devil/android/apk_helper_test.py +++ b/catapult/devil/devil/android/apk_helper_test.py @@ -3,6 +3,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import collections +import os import unittest from devil import base_error @@ -125,6 +127,11 @@ def _MockAaptDump(manifest_dump): 'devil.android.sdk.aapt.Dump', mock.Mock(side_effect=None, return_value=manifest_dump.split('\n'))) +def _MockListApkPaths(files): + return mock.patch( + 'devil.android.apk_helper.ApkHelper._ListApkPaths', + mock.Mock(side_effect=None, return_value=files)) + class ApkHelperTest(mock_calls.TestCase): def testGetInstrumentationName(self): @@ -220,6 +227,25 @@ class ApkHelperTest(mock_calls.TestCase): self.assertEquals('org.chromium.RandomTestRunner', helper.GetInstrumentationName()) + def testGetArchitectures(self): + AbiPair = collections.namedtuple('AbiPair', ['abi32bit', 'abi64bit']) + for abi_pair in [AbiPair('lib/armeabi-v7a', 'lib/arm64-v8a'), + AbiPair('lib/x86', 'lib/x64')]: + with _MockListApkPaths([abi_pair.abi32bit]): + helper = apk_helper.ApkHelper('') + self.assertEquals(set([os.path.basename(abi_pair.abi32bit), + os.path.basename(abi_pair.abi64bit)]), + set(helper.GetAbis())) + with _MockListApkPaths([abi_pair.abi32bit, abi_pair.abi64bit]): + helper = apk_helper.ApkHelper('') + self.assertEquals(set([os.path.basename(abi_pair.abi32bit), + os.path.basename(abi_pair.abi64bit)]), + set(helper.GetAbis())) + with _MockListApkPaths([abi_pair.abi64bit]): + helper = apk_helper.ApkHelper('') + self.assertEquals(set([os.path.basename(abi_pair.abi64bit)]), + set(helper.GetAbis())) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/catapult/devil/devil/android/device_errors.py b/catapult/devil/devil/android/device_errors.py index 57f36150..cd0266c8 100644 --- a/catapult/devil/devil/android/device_errors.py +++ b/catapult/devil/devil/android/device_errors.py @@ -55,15 +55,15 @@ class _BaseCommandFailedError(CommandFailedError): self.status = status if not message: adb_cmd = ' '.join(cmd_helper.SingleQuote(arg) for arg in self.args) - message = ['adb %s: failed ' % adb_cmd] + segments = ['adb %s: failed ' % adb_cmd] if status: - message.append('with exit status %s ' % self.status) + segments.append('with exit status %s ' % self.status) if output: - message.append('and output:\n') - message.extend('- %s\n' % line for line in output.splitlines()) + segments.append('and output:\n') + segments.extend('- %s\n' % line for line in output.splitlines()) else: - message.append('and no output.') - message = ''.join(message) + segments.append('and no output.') + message = ''.join(segments) super(_BaseCommandFailedError, self).__init__(message, device_serial) def __eq__(self, other): @@ -79,8 +79,7 @@ class _BaseCommandFailedError(CommandFailedError): """Support pickling.""" result = [None, None, None, None, None] super_result = super(_BaseCommandFailedError, self).__reduce__() - for i in range(len(super_result)): - result[i] = super_result[i] + result[:len(super_result)] = super_result # Update the args used to reconstruct this exception. result[1] = ( @@ -120,19 +119,19 @@ class AdbShellCommandFailedError(AdbCommandFailedError): def __init__(self, command, output, status, device_serial=None): self.command = command - message = ['shell command run via adb failed on the device:\n', + segments = ['shell command run via adb failed on the device:\n', ' command: %s\n' % command] - message.append(' exit status: %s\n' % status) + segments.append(' exit status: %s\n' % status) if output: - message.append(' output:\n') + segments.append(' output:\n') if isinstance(output, basestring): output_lines = output.splitlines() else: output_lines = output - message.extend(' - %s\n' % line for line in output_lines) + segments.extend(' - %s\n' % line for line in output_lines) else: - message.append(" output: ''\n") - message = ''.join(message) + segments.append(" output: ''\n") + message = ''.join(segments) super(AdbShellCommandFailedError, self).__init__( ['shell', command], output, status, device_serial, message) @@ -140,8 +139,7 @@ class AdbShellCommandFailedError(AdbCommandFailedError): """Support pickling.""" result = [None, None, None, None, None] super_result = super(AdbShellCommandFailedError, self).__reduce__() - for i in range(len(super_result)): - result[i] = super_result[i] + result[:len(super_result)] = super_result # Update the args used to reconstruct this exception. result[1] = (self.command, self.output, self.status, self.device_serial) diff --git a/catapult/devil/devil/android/device_utils.py b/catapult/devil/devil/android/device_utils.py index 5a3db413..518e4393 100644 --- a/catapult/devil/devil/android/device_utils.py +++ b/catapult/devil/devil/android/device_utils.py @@ -59,6 +59,29 @@ _DEFAULT_RETRIES = 3 # the timeout_retry decorators. DEFAULT = object() +# A sentinel object to require that calls to RunShellCommand force running the +# command with su even if the device has been rooted. To use, pass into the +# as_root param. +_FORCE_SU = object() + +_RECURSIVE_DIRECTORY_LIST_SCRIPT = """ + function list_subdirs() { + for f in "$1"/* ; + do + if [ -d "$f" ] ; + then + if [ "$f" == "." ] || [ "$f" == ".." ] ; + then + continue ; + fi ; + echo "$f" ; + list_subdirs "$f" ; + fi ; + done ; + } ; + list_subdirs %s +""" + _RESTART_ADBD_SCRIPT = """ trap '' HUP trap '' TERM @@ -88,6 +111,7 @@ _PERMISSIONS_BLACKLIST_RE = re.compile('|'.join(fnmatch.translate(p) for p in [ 'android.permission.DISABLE_KEYGUARD', 'android.permission.DOWNLOAD_WITHOUT_NOTIFICATION', 'android.permission.EXPAND_STATUS_BAR', + 'android.permission.FOREGROUND_SERVICE', 'android.permission.GET_PACKAGE_SIZE', 'android.permission.INSTALL_SHORTCUT', 'android.permission.INJECT_EVENTS', @@ -648,6 +672,22 @@ class DeviceUtils(object): raise device_errors.CommandFailedError( 'Version name for %s not found on dumpsys output' % package, str(self)) + @decorators.WithTimeoutAndRetriesFromInstance() + def GetPackageArchitecture(self, package, timeout=None, retries=None): + """Get the architecture of a package installed on the device. + + Args: + package: Name of the package. + + Returns: + A string with the architecture, or None if the package is missing. + """ + lines = self._GetDumpsysOutput(['package', package], 'primaryCpuAbi') + if lines: + _, _, package_arch = lines[-1].partition('=') + return package_arch.strip() + return None + @decorators.WithTimeoutAndRetriesFromInstance() def GetApplicationDataDirectory(self, package, timeout=None, retries=None): """Get the data directory on the device for the given package. @@ -670,6 +710,31 @@ class DeviceUtils(object): raise device_errors.CommandFailedError( 'Could not find data directory for %s', package) + @decorators.WithTimeoutAndRetriesFromInstance() + def GetSecurityContextForPackage(self, package, encrypted=False, timeout=None, + retries=None): + """Gets the SELinux security context for the given package. + + Args: + package: Name of the package. + encrypted: Whether to check in the encrypted data directory + (/data/user_de/0/) or the unencrypted data directory (/data/data/). + + Returns: + The package's security context as a string, or None if not found. + """ + directory = '/data/user_de/0/' if encrypted else '/data/data/' + for line in self.RunShellCommand(['ls', '-Z', directory], + as_root=True, check_return=True): + split_line = line.split() + # ls -Z output differs between Android versions, but the package is + # always last and the context always starts with "u:object" + if split_line[-1] == package: + for column in split_line: + if column.startswith('u:object'): + return column + return None + def TakeBugReport(self, path, timeout=60*5, retries=None): """Takes a bug report and dumps it to the specified path. @@ -1064,7 +1129,7 @@ class DeviceUtils(object): if run_as: cmd = 'run-as %s sh -c %s' % (cmd_helper.SingleQuote(run_as), cmd_helper.SingleQuote(cmd)) - if as_root and self.NeedsSU(): + if (as_root is _FORCE_SU) or (as_root and self.NeedsSU()): # "su -c sh -c" allows using shell features in |cmd| cmd = self._Su('sh -c %s' % cmd_helper.SingleQuote(cmd)) @@ -1201,6 +1266,33 @@ class DeviceUtils(object): if line.startswith('Error:'): raise device_errors.CommandFailedError(line, str(self)) + @decorators.WithTimeoutAndRetriesFromInstance() + def StartService(self, intent_obj, user_id=None, timeout=None, retries=None): + """Start a service on the device. + + Args: + intent_obj: An Intent object to send describing the service to start. + user_id: A specific user to start the service as, defaults to current. + timeout: Timeout in seconds. + retries: Number of retries + + Raises: + CommandFailedError if the service could not be started. + CommandTimeoutError on timeout. + DeviceUnreachableError on missing device. + """ + # For whatever reason, startservice was changed to start-service on O and + # above. + cmd = ['am', 'startservice'] + if self.build_version_sdk >= version_codes.OREO: + cmd[1] = 'start-service' + if user_id: + cmd.extend(['--user', str(user_id)]) + cmd.extend(intent_obj.am_args) + for line in self.RunShellCommand(cmd, check_return=True): + if line.startswith('Error:'): + raise device_errors.CommandFailedError(line, str(self)) + @decorators.WithTimeoutAndRetriesFromInstance() def StartInstrumentation(self, component, finish=True, raw=False, extras=None, timeout=None, retries=None): @@ -1383,7 +1475,7 @@ class DeviceUtils(object): missing_dirs.add(posixpath.dirname(d)) if delete_device_stale and all_stale_files: - self.RunShellCommand(['rm', '-f'] + all_stale_files, check_return=True) + self.RemovePath(all_stale_files, force=True, recursive=True) if all_changed_files: if missing_dirs: @@ -1483,15 +1575,66 @@ class DeviceUtils(object): else: to_push.append((host_abs_path, device_abs_path)) to_delete = device_checksums.keys() + # We can't rely solely on the checksum approach since it does not catch + # stale directories, which can result in empty directories that cause issues + # during copying in efficient_android_directory_copy.sh. So, find any stale + # directories here so they can be removed in addition to stale files. + if track_stale: + to_delete.extend(self._GetStaleDirectories(host_path, device_path)) def cache_commit_func(): - new_sums = {posixpath.join(device_path, path[len(host_path) + 1:]): val - for path, val in host_checksums.iteritems()} + # When host_path is a not a directory, the path.join() call below would + # have an '' as the second argument, causing an unwanted / to be appended. + if os.path.isfile(host_path): + assert len(host_checksums) == 1 + new_sums = {device_path: host_checksums[host_path]} + else: + new_sums = {posixpath.join(device_path, path[len(host_path) + 1:]): val + for path, val in host_checksums.iteritems()} cache_entry = [ignore_other_files, new_sums] self._cache['device_path_checksums'][device_path] = cache_entry return (to_push, up_to_date, to_delete, cache_commit_func) + def _GetStaleDirectories(self, host_path, device_path): + """Gets a list of stale directories on the device. + + Args: + host_path: an absolute path of a directory on the host + device_path: an absolute path of a directory on the device + + Returns: + A list containing absolute paths to directories on the device that are + considered stale. + """ + def get_device_dirs(path): + directories = set() + command = _RECURSIVE_DIRECTORY_LIST_SCRIPT % cmd_helper.SingleQuote(path) + # We use shell=True to evaluate the command as a script through the shell, + # otherwise RunShellCommand tries to interpret it as the name of a (non + # existent) command to run. + for line in self.RunShellCommand( + command, shell=True, check_return=True): + directories.add(posixpath.relpath(posixpath.normpath(line), path)) + return directories + + def get_host_dirs(path): + directories = set() + if not os.path.isdir(path): + return directories + for root, _, _ in os.walk(path): + if root != path: + # Strip off the top level directory so we can compare the device and + # host. + directories.add( + os.path.relpath(root, path).replace(os.sep, posixpath.sep)) + return directories + + host_dirs = get_host_dirs(host_path) + device_dirs = get_device_dirs(device_path) + stale_dirs = device_dirs - host_dirs + return [posixpath.join(device_path, d) for d in stale_dirs] + def _ComputeDeviceChecksumsForApks(self, package_name): ret = self._cache['package_apk_checksums'].get(package_name) if ret is None: @@ -1608,6 +1751,8 @@ class DeviceUtils(object): except zip_utils.ZipFailedError: return False + logger.info('Pushing %d files via .zip of size %d', len(files), + os.path.getsize(zip_path)) self.NeedsSU() with device_temp_file.DeviceTempFile( self.adb, suffix='.zip') as device_temp: @@ -2279,9 +2424,8 @@ class DeviceUtils(object): """ try: ps_cmd = 'ps' - # ps behavior was changed in Android above N, http://crbug.com/686716 - if (self.build_version_sdk >= version_codes.NOUGAT_MR1 - and self.build_id[0] > 'N'): + # ps behavior was changed in Android O and above, http://crbug.com/686716 + if self.build_version_sdk >= version_codes.OREO: ps_cmd = 'ps -e' if pattern: return self._RunPipedShellCommand( @@ -2331,6 +2475,29 @@ class DeviceUtils(object): processes.append(ProcessInfo(**row)) return processes + def _GetDumpsysOutput(self, extra_args, pattern=None): + """Runs |dumpsys| command on the device and returns its output. + + This private method implements support for filtering the output by a given + |pattern|, but does not do any output parsing. + """ + try: + cmd = ['dumpsys'] + extra_args + if pattern: + cmd = ' '.join(cmd_helper.SingleQuote(s) for s in cmd) + return self._RunPipedShellCommand( + '%s | grep -F %s' % (cmd, cmd_helper.SingleQuote(pattern))) + else: + cmd = ['dumpsys'] + extra_args + return self.RunShellCommand(cmd, check_return=True, large_output=True) + except device_errors.AdbShellCommandFailedError as e: + if e.status and isinstance(e.status, list) and not e.status[0]: + # If dumpsys succeeded but grep failed, there were no lines matching + # the given pattern. + return [] + else: + raise + # TODO(#4103): Remove after migrating clients to ListProcesses. @decorators.WithTimeoutAndRetriesFromInstance() def GetPids(self, process_name=None, timeout=None, retries=None): @@ -2437,6 +2604,30 @@ class DeviceUtils(object): ['setenforce', '1' if int(enabled) else '0'], as_root=True, check_return=True) + @decorators.WithTimeoutAndRetriesFromInstance() + def SetWebViewImplementation(self, package_name, timeout=None, retries=None): + """Select the WebView implementation to the specified package. + + Args: + package_name: The package name of a WebView implementation. The package + must be already installed on the device. + timeout: timeout in seconds + retries: number of retries + + Raises: + CommandFailedError on failure. + CommandTimeoutError on timeout. + DeviceUnreachableError on missing device. + """ + output = self.RunShellCommand( + ['cmd', 'webviewupdate', 'set-webview-implementation', package_name], + single_line=True, check_return=True) + if output == 'Success': + logging.info('WebView provider set to: %s', package_name) + else: + raise device_errors.CommandFailedError( + 'Error setting WebView provider: %s' % output, str(self)) + @decorators.WithTimeoutAndRetriesFromInstance() def TakeScreenshot(self, host_path=None, timeout=None, retries=None): """Takes a screenshot of the device. @@ -2615,7 +2806,7 @@ class DeviceUtils(object): @classmethod def HealthyDevices(cls, blacklist=None, device_arg='default', retry=True, - **kwargs): + abis=None, **kwargs): """Returns a list of DeviceUtils instances. Returns a list of DeviceUtils instances that are attached, not blacklisted, @@ -2639,6 +2830,8 @@ class DeviceUtils(object): blacklisted. retry: If true, will attempt to restart adb server and query it again if no devices are found. + abis: A list of ABIs for which the device needs to support at least one of + (optional). A device serial, or a list of device serials (optional). Returns: @@ -2674,14 +2867,23 @@ class DeviceUtils(object): return True return False + def supports_abi(abi, serial): + if abis and abi not in abis: + logger.warning("Device %s doesn't support required ABIs.", serial) + return False + return True + def _get_devices(): if device_arg: devices = [cls(x, **kwargs) for x in device_arg if not blacklisted(x)] else: devices = [] for adb in adb_wrapper.AdbWrapper.Devices(): - if not blacklisted(adb.GetDeviceSerial()): - devices.append(cls(_CreateAdbWrapper(adb), **kwargs)) + serial = adb.GetDeviceSerial() + if not blacklisted(serial): + device = cls(_CreateAdbWrapper(adb), **kwargs) + if supports_abi(device.GetABI(), serial): + devices.append(device) if len(devices) == 0 and not allow_no_devices: raise device_errors.NoDevicesError() @@ -2806,3 +3008,39 @@ class DeviceUtils(object): return self.SendKeyEvent(keyevent.KEYCODE_POWER) timeout_retry.WaitFor(screen_test, wait_period=1) + + @decorators.WithTimeoutAndRetriesFromInstance() + def ChangeOwner(self, owner_group, paths, timeout=None, retries=None): + """Changes file system ownership for permissions. + + Args: + owner_group: New owner and group to assign. Note that this should be a + string in the form user[.group] where the group is option. + paths: Paths to change ownership of. + + Note that the -R recursive option is not supported by all Android + versions. + """ + if not paths: + return + self.RunShellCommand(['chown', owner_group] + paths, check_return=True) + + @decorators.WithTimeoutAndRetriesFromInstance() + def ChangeSecurityContext(self, security_context, paths, timeout=None, + retries=None): + """Changes the SELinux security context for files. + + Args: + security_context: The new security context as a string + paths: Paths to change the security context of. + + Note that the -R recursive option is not supported by all Android + versions. + """ + if not paths: + return + command = ['chcon', security_context] + paths + + # Note, need to force su because chcon can fail with permission errors even + # if the device is rooted. + self.RunShellCommand(command, as_root=_FORCE_SU, check_return=True) diff --git a/catapult/devil/devil/android/device_utils_devicetest.py b/catapult/devil/devil/android/device_utils_devicetest.py index 173094b9..0836f3ea 100755 --- a/catapult/devil/devil/android/device_utils_devicetest.py +++ b/catapult/devil/devil/android/device_utils_devicetest.py @@ -210,6 +210,39 @@ class DeviceUtilsPushDeleteFilesTest(device_test_case.DeviceTestCase): cmd_helper.RunCmd(['rm', '-rf', host_tmp_dir]) self.device.RemovePath(_DEVICE_DIR, recursive=True, force=True) + def testPushWithStaleDirectories(self): + # Make a few files and directories to push. + host_tmp_dir = tempfile.mkdtemp() + host_sub_dir1 = '%s/%s' % (host_tmp_dir, _SUB_DIR1) + host_sub_dir2 = "%s/%s/%s" % (host_tmp_dir, _SUB_DIR, _SUB_DIR2) + os.makedirs(host_sub_dir1) + os.makedirs(host_sub_dir2) + + self._MakeTempFileGivenDir(host_sub_dir1, _OLD_CONTENTS) + self._MakeTempFileGivenDir(host_sub_dir2, _OLD_CONTENTS) + + # Push all our created files/directories and verify they're on the device. + self.device.PushChangedFiles([(host_tmp_dir, _DEVICE_DIR)], + delete_device_stale=True) + top_level_dirs = self.device.ListDirectory(_DEVICE_DIR) + self.assertIn(_SUB_DIR1, top_level_dirs) + self.assertIn(_SUB_DIR, top_level_dirs) + sub_dir = self.device.ListDirectory('%s/%s' % (_DEVICE_DIR, _SUB_DIR)) + self.assertIn(_SUB_DIR2, sub_dir) + + # Remove one of the directories on the host and push again. + cmd_helper.RunCmd(['rm', '-rf', host_sub_dir2]) + self.device.PushChangedFiles([(host_tmp_dir, _DEVICE_DIR)], + delete_device_stale=True) + + # Verify that the directory we removed is no longer on the device, but the + # other directories still are. + top_level_dirs = self.device.ListDirectory(_DEVICE_DIR) + self.assertIn(_SUB_DIR1, top_level_dirs) + self.assertIn(_SUB_DIR, top_level_dirs) + sub_dir = self.device.ListDirectory('%s/%s' % (_DEVICE_DIR, _SUB_DIR)) + self.assertEqual([], sub_dir) + def testRestartAdbd(self): def get_adbd_pid(): try: diff --git a/catapult/devil/devil/android/device_utils_test.py b/catapult/devil/devil/android/device_utils_test.py index b5660ac4..88c91b51 100755 --- a/catapult/devil/devil/android/device_utils_test.py +++ b/catapult/devil/devil/android/device_utils_test.py @@ -31,6 +31,8 @@ from devil.utils import mock_calls with devil_env.SysPath(devil_env.PYMOCK_PATH): import mock # pylint: disable=import-error +ARM32_ABI = 'armeabi-v7a' +ARM64_ABI = 'arm64-v8a' def Process(name, pid, ppid='1'): return device_utils.ProcessInfo(name=name, pid=pid, ppid=ppid) @@ -57,6 +59,7 @@ class _MockApkHelper(object): self.path = path self.package_name = package_name self.perms = perms + self.abis = [ARM32_ABI] def GetPackageName(self): return self.package_name @@ -64,6 +67,9 @@ class _MockApkHelper(object): def GetPermissions(self): return self.perms + def GetAbis(self): + return self.abis + class _MockMultipleDevicesError(Exception): pass @@ -465,6 +471,27 @@ class DeviceUtils_GetApplicationVersionTest(DeviceUtilsTest): self.device.GetApplicationVersion('com.android.chrome') +class DeviceUtils_GetPackageArchitectureTest(DeviceUtilsTest): + + def test_GetPackageArchitecture_exists(self): + with self.assertCall( + self.call.device._RunPipedShellCommand( + 'dumpsys package com.android.chrome | grep -F primaryCpuAbi'), + [' primaryCpuAbi=armeabi-v7a']): + self.assertEquals( + ARM32_ABI, + self.device.GetPackageArchitecture('com.android.chrome')) + + def test_GetPackageArchitecture_notExists(self): + with self.assertCall( + self.call.device._RunPipedShellCommand( + 'dumpsys package com.android.chrome | grep -F primaryCpuAbi'), + []): + self.assertEquals( + None, + self.device.GetPackageArchitecture('com.android.chrome')) + + class DeviceUtilsGetApplicationDataDirectoryTest(DeviceUtilsTest): def testGetApplicationDataDirectory_exists(self): @@ -1415,6 +1442,62 @@ class DeviceUtilsStartActivityTest(DeviceUtilsTest): self.device.StartActivity(test_intent) +class DeviceUtilsStartServiceTest(DeviceUtilsTest): + def testStartService_success(self): + test_intent = intent.Intent(action='android.intent.action.START', + package='test.package', + activity='.Main') + with self.patch_call(self.call.device.build_version_sdk, + return_value=version_codes.NOUGAT): + with self.assertCall( + self.call.adb.Shell('am startservice ' + '-a android.intent.action.START ' + '-n test.package/.Main'), + 'Starting service: Intent { act=android.intent.action.START }'): + self.device.StartService(test_intent) + + def testStartService_failure(self): + test_intent = intent.Intent(action='android.intent.action.START', + package='test.package', + activity='.Main') + with self.patch_call(self.call.device.build_version_sdk, + return_value=version_codes.NOUGAT): + with self.assertCall( + self.call.adb.Shell('am startservice ' + '-a android.intent.action.START ' + '-n test.package/.Main'), + 'Error: Failed to start test service'): + with self.assertRaises(device_errors.CommandFailedError): + self.device.StartService(test_intent) + + def testStartService_withUser(self): + test_intent = intent.Intent(action='android.intent.action.START', + package='test.package', + activity='.Main') + with self.patch_call(self.call.device.build_version_sdk, + return_value=version_codes.NOUGAT): + with self.assertCall( + self.call.adb.Shell('am startservice ' + '--user TestUser ' + '-a android.intent.action.START ' + '-n test.package/.Main'), + 'Starting service: Intent { act=android.intent.action.START }'): + self.device.StartService(test_intent, user_id='TestUser') + + def testStartService_onOreo(self): + test_intent = intent.Intent(action='android.intent.action.START', + package='test.package', + activity='.Main') + with self.patch_call(self.call.device.build_version_sdk, + return_value=version_codes.OREO): + with self.assertCall( + self.call.adb.Shell('am start-service ' + '-a android.intent.action.START ' + '-n test.package/.Main'), + 'Starting service: Intent { act=android.intent.action.START }'): + self.device.StartService(test_intent) + + class DeviceUtilsStartInstrumentationTest(DeviceUtilsTest): def testStartInstrumentation_nothing(self): @@ -1691,6 +1774,8 @@ class DeviceUtilsPushChangedFilesZippedTest(DeviceUtilsTest): mock_zip_temp_dir), (mock.call.devil.utils.zip_utils.WriteZipFile( '/test/temp/dir/tmp.zip', test_files)), + (mock.call.os.path.getsize( + '/test/temp/dir/tmp.zip'), 123), (self.call.device.NeedsSU(), True), (mock.call.devil.android.device_temp_file.DeviceTempFile(self.adb, suffix='.zip'), @@ -2525,6 +2610,20 @@ class DeviceUtilsGetSetEnforce(DeviceUtilsTest): self.device.SetEnforce(enabled='0') # Not recommended but it works! +class DeviceUtilsSetWebViewImplementationTest(DeviceUtilsTest): + + def testSetWebViewImplementation_success(self): + with self.assertCall(self.call.adb.Shell( + 'cmd webviewupdate set-webview-implementation foo.org'), 'Success'): + self.device.SetWebViewImplementation('foo.org') + + def testSetWebViewImplementation_failure(self): + with self.assertCall(self.call.adb.Shell( + 'cmd webviewupdate set-webview-implementation foo.org'), 'Oops!'): + with self.assertRaises(device_errors.CommandFailedError): + self.device.SetWebViewImplementation('foo.org') + + class DeviceUtilsTakeScreenshotTest(DeviceUtilsTest): def testTakeScreenshot_fileNameProvided(self): @@ -2621,7 +2720,11 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): test_serials = ['0123456789abcdef', 'fedcba9876543210'] with self.assertCalls( (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), - [_AdbWrapperMock(s) for s in test_serials])): + [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI)): blacklist = mock.NonCallableMock(**{'Read.return_value': []}) devices = device_utils.DeviceUtils.HealthyDevices(blacklist) for serial, device in zip(test_serials, devices): @@ -2632,7 +2735,9 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): test_serials = ['0123456789abcdef', 'fedcba9876543210'] with self.assertCalls( (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), - [_AdbWrapperMock(s) for s in test_serials])): + [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI)): blacklist = mock.NonCallableMock( **{'Read.return_value': ['fedcba9876543210']}) devices = device_utils.DeviceUtils.HealthyDevices(blacklist) @@ -2645,6 +2750,10 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): with self.assertCalls( (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI), (mock.call.devil.android.device_errors.MultipleDevicesError(mock.ANY), _MockMultipleDevicesError())): with self.assertRaises(_MockMultipleDevicesError): @@ -2654,7 +2763,9 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): test_serials = ['0123456789abcdef'] with self.assertCalls( (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), - [_AdbWrapperMock(s) for s in test_serials])): + [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI)): devices = device_utils.DeviceUtils.HealthyDevices(device_arg=None) self.assertEquals(1, len(devices)) @@ -2684,7 +2795,11 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): test_serials = ['0123456789abcdef', 'fedcba9876543210'] with self.assertCalls( (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), - [_AdbWrapperMock(s) for s in test_serials])): + [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI)): devices = device_utils.DeviceUtils.HealthyDevices(device_arg=()) self.assertEquals(2, len(devices)) @@ -2725,6 +2840,33 @@ class DeviceUtilsHealthyDevicesTest(mock_calls.TestCase): del os.environ['ANDROID_SERIAL'] self.assertEquals(2, len(devices)) + def testHealthyDevices_abisArg_no_matching_abi(self): + test_serials = ['0123456789abcdef', 'fedcba9876543210'] + with self.assertCalls( + (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), + [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI)): + with self.assertRaises(device_errors.NoDevicesError): + device_utils.DeviceUtils.HealthyDevices(device_arg=[], retry=False, + abis=[ARM64_ABI]) + + def testHealthyDevices_abisArg_filter_on_abi(self): + test_serials = ['0123456789abcdef', 'fedcba9876543210'] + with self.assertCalls( + (mock.call.devil.android.sdk.adb_wrapper.AdbWrapper.Devices(), + [_AdbWrapperMock(s) for s in test_serials]), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM64_ABI), + (mock.call.devil.android.device_utils.DeviceUtils.GetABI(), + ARM32_ABI)): + devices = device_utils.DeviceUtils.HealthyDevices(device_arg=[], + retry=False, + abis=[ARM64_ABI]) + self.assertEquals(1, len(devices)) + class DeviceUtilsRestartAdbdTest(DeviceUtilsTest): @@ -2967,6 +3109,27 @@ class DeviceUtilsGetIMEITest(DeviceUtilsTest): self.device.GetIMEI() +class DeviceUtilsChangeOwner(DeviceUtilsTest): + + def testChangeOwner(self): + with self.assertCalls( + (self.call.device.RunShellCommand( + ['chown', 'user.group', '/path/to/file1', 'file2'], + check_return=True))): + self.device.ChangeOwner('user.group', ['/path/to/file1', 'file2']) + + +class DeviceUtilsChangeSecurityContext(DeviceUtilsTest): + + + def testChangeSecurityContext(self): + with self.assertCalls( + (self.call.device.RunShellCommand( + ['chcon', 'u:object_r:system_data_file:s0', '/path', '/path2'], + as_root=device_utils._FORCE_SU, check_return=True))): + self.device.ChangeSecurityContext('u:object_r:system_data_file:s0', + ['/path', '/path2']) + if __name__ == '__main__': logging.getLogger().setLevel(logging.DEBUG) unittest.main(verbosity=2) diff --git a/catapult/devil/devil/android/flag_changer.py b/catapult/devil/devil/android/flag_changer.py index 0055e233..9cb1c02e 100644 --- a/catapult/devil/devil/android/flag_changer.py +++ b/catapult/devil/devil/android/flag_changer.py @@ -52,12 +52,14 @@ class FlagChanger(object): once the tests have completed. """ - def __init__(self, device, cmdline_file): + def __init__(self, device, cmdline_file, use_legacy_path=False): """Initializes the FlagChanger and records the original arguments. Args: device: A DeviceUtils instance. cmdline_file: Name of the command line file where to store flags. + use_legacy_path: Whether to use the legacy commandline path (needed for + M54 and earlier) """ self._device = device self._should_reset_enforce = False @@ -66,13 +68,18 @@ class FlagChanger(object): raise ValueError( 'cmdline_file should be a file name only, do not include path' ' separators in: %s' % cmdline_file) - self._cmdline_path = posixpath.join(_CMDLINE_DIR, cmdline_file) + cmdline_path = posixpath.join(_CMDLINE_DIR, cmdline_file) + alternate_cmdline_path = posixpath.join(_CMDLINE_DIR_LEGACY, cmdline_file) - cmdline_path_legacy = posixpath.join(_CMDLINE_DIR_LEGACY, cmdline_file) - if self._device.PathExists(cmdline_path_legacy): + if use_legacy_path: + cmdline_path, alternate_cmdline_path = ( + alternate_cmdline_path, cmdline_path) + self._cmdline_path = cmdline_path + + if self._device.PathExists(alternate_cmdline_path): logger.warning( - 'Removing legacy command line file %r.', cmdline_path_legacy) - self._device.RemovePath(cmdline_path_legacy, as_root=True) + 'Removing alternate command line file %r.', alternate_cmdline_path) + self._device.RemovePath(alternate_cmdline_path, as_root=True) self._state_stack = [None] # Actual state is set by GetCurrentFlags(). self.GetCurrentFlags() @@ -195,7 +202,7 @@ class FlagChanger(object): """ # The initial state must always remain on the stack. assert len(self._state_stack) > 1, ( - "Mismatch between calls to Add/RemoveFlags and Restore") + 'Mismatch between calls to Add/RemoveFlags and Restore') self._state_stack.pop() if len(self._state_stack) == 1: self._ResetEnforce() @@ -237,6 +244,7 @@ def _ParseFlags(line): current_quote = None current_flag = None + # pylint: disable=unsubscriptable-object for c in line: # Detect start or end of quote block. if (current_quote is None and c in _QUOTES) or c == current_quote: diff --git a/catapult/devil/devil/android/flag_changer_test.py b/catapult/devil/devil/android/flag_changer_test.py index 5342cf44..dbe6facc 100755 --- a/catapult/devil/devil/android/flag_changer_test.py +++ b/catapult/devil/devil/android/flag_changer_test.py @@ -42,7 +42,7 @@ class FlagChangerTest(unittest.TestCase): self.cmdline_path_legacy = posixpath.join( flag_changer._CMDLINE_DIR_LEGACY, _CMDLINE_FILE) - def testFlagChanger_removeLegacyCmdLine(self): + def testFlagChanger_removeAlternateCmdLine(self): self.device.WriteFile(self.cmdline_path_legacy, 'chrome --old --stuff') self.assertTrue(self.device.PathExists(self.cmdline_path_legacy)) @@ -52,6 +52,17 @@ class FlagChangerTest(unittest.TestCase): self.cmdline_path) self.assertFalse(self.device.PathExists(self.cmdline_path_legacy)) + def testFlagChanger_removeAlternateCmdLineLegacyPath(self): + self.device.WriteFile(self.cmdline_path, 'chrome --old --stuff') + self.assertTrue(self.device.PathExists(self.cmdline_path)) + + changer = flag_changer.FlagChanger(self.device, 'chrome-command-line', + use_legacy_path=True) + self.assertEquals( + changer._cmdline_path, # pylint: disable=protected-access + self.cmdline_path_legacy) + self.assertFalse(self.device.PathExists(self.cmdline_path)) + def testFlagChanger_mustBeFileName(self): with self.assertRaises(ValueError): flag_changer.FlagChanger(self.device, '/data/local/chrome-command-line') diff --git a/catapult/devil/devil/android/forwarder.py b/catapult/devil/devil/android/forwarder.py index cf1fbe14..6be46516 100644 --- a/catapult/devil/devil/android/forwarder.py +++ b/catapult/devil/devil/android/forwarder.py @@ -306,7 +306,7 @@ class Forwarder(object): instance = Forwarder._GetInstanceLocked(None) serial = str(device) serial_with_port = (serial, device_port) - if not serial_with_port in instance._device_to_host_port_map: + if serial_with_port not in instance._device_to_host_port_map: logger.error('Trying to unmap non-forwarded port %d', device_port) return diff --git a/catapult/devil/devil/android/perf/perf_control.py b/catapult/devil/devil/android/perf/perf_control.py index 06a5db61..9ac85ebc 100644 --- a/catapult/devil/devil/android/perf/perf_control.py +++ b/catapult/devil/devil/android/perf/perf_control.py @@ -50,12 +50,12 @@ class PerfControl(object): product_model = self._device.product_model # TODO(epenner): Enable on all devices (http://crbug.com/383566) - if 'Nexus 4' == product_model: + if product_model == 'Nexus 4': self._ForceAllCpusOnline(True) if not self._AllCpusAreOnline(): logger.warning('Failed to force CPUs online. Results may be NOISY!') self.SetScalingGovernor('performance') - elif 'Nexus 5' == product_model: + elif product_model == 'Nexus 5': self._ForceAllCpusOnline(True) if not self._AllCpusAreOnline(): logger.warning('Failed to force CPUs online. Results may be NOISY!') @@ -79,7 +79,7 @@ class PerfControl(object): if not self._device.HasRoot(): return product_model = self._device.product_model - if 'Nexus 5' == product_model: + if product_model == 'Nexus 5': if self._AllCpusAreOnline(): self._SetScalingMaxFreq(2265600) self._SetMaxGpuClock(450000000) diff --git a/catapult/devil/devil/android/perf/thermal_throttle.py b/catapult/devil/devil/android/perf/thermal_throttle.py index 546a92e0..fd6b08f3 100644 --- a/catapult/devil/devil/android/perf/thermal_throttle.py +++ b/catapult/devil/devil/android/perf/thermal_throttle.py @@ -76,6 +76,7 @@ class ThermalThrottle(object): self._device = device self._throttled = False self._detector = None + # pylint: disable=redefined-variable-type if OmapThrottlingDetector.IsSupported(device): self._detector = OmapThrottlingDetector(device) elif ExynosThrottlingDetector.IsSupported(device): diff --git a/catapult/devil/devil/android/sdk/adb_wrapper.py b/catapult/devil/devil/android/sdk/adb_wrapper.py index 5d24d470..099a0f8a 100644 --- a/catapult/devil/devil/android/sdk/adb_wrapper.py +++ b/catapult/devil/devil/android/sdk/adb_wrapper.py @@ -9,7 +9,9 @@ should be delegated to a higher level (ex. DeviceUtils). """ import collections -import distutils.version +# pylint: disable=import-error +# pylint: disable=no-name-in-module +import distutils.version as du_version import errno import logging import os @@ -39,8 +41,10 @@ _ADB_VERSION_RE = re.compile(r'Android Debug Bridge version (\d+\.\d+\.\d+)') _EMULATOR_RE = re.compile(r'^emulator-[0-9]+$') _DEVICE_NOT_FOUND_RE = re.compile(r"error: device '(?P.+)' not found") _READY_STATE = 'device' -_VERITY_DISABLE_RE = re.compile(r'Verity (already )?disabled') -_VERITY_ENABLE_RE = re.compile(r'Verity (already )?enabled') +_VERITY_DISABLE_RE = re.compile(r'(V|v)erity (is )?(already )?disabled' + r'|Successfully disabled verity') +_VERITY_ENABLE_RE = re.compile(r'(V|v)erity (is )?(already )?enabled' + r'|Successfully enabled verity') _WAITING_FOR_DEVICE_RE = re.compile(r'- waiting for device -') @@ -422,8 +426,8 @@ class AdbWrapper(object): """ VerifyLocalFileExists(local) - if (distutils.version.LooseVersion(self.Version()) < - distutils.version.LooseVersion('1.0.36')): + if (du_version.LooseVersion(self.Version()) < + du_version.LooseVersion('1.0.36')): # Different versions of adb handle pushing a directory to an existing # directory differently. @@ -480,6 +484,19 @@ class AdbWrapper(object): 'File pulled from the device did not arrive on the host: %s' % local, device_serial=str(self)) + def StartShell(self, cmd): + """Starts a subprocess on the device and returns a handle to the process. + + Args: + args: A sequence of program arguments. The executable to run is the first + item in the sequence. + + Returns: + An instance of subprocess.Popen associated with the live process. + """ + return cmd_helper.StartCmd( + self._BuildAdbCmd(['shell'] + cmd, self._device_serial)) + def Shell(self, command, expect_status=0, timeout=DEFAULT_TIMEOUT, retries=DEFAULT_RETRIES): """Runs a shell command on the device. @@ -671,8 +688,8 @@ class AdbWrapper(object): Returns: The output of adb forward --list as a string. """ - if (distutils.version.LooseVersion(self.Version()) >= - distutils.version.LooseVersion('1.0.36')): + if (du_version.LooseVersion(self.Version()) >= + du_version.LooseVersion('1.0.36')): # Starting in 1.0.36, this can occasionally fail with a protocol fault. # As this interrupts all connections with all devices, we instead just # return an empty list. This may give clients an inaccurate result, but diff --git a/catapult/devil/devil/android/sdk/fastboot.py b/catapult/devil/devil/android/sdk/fastboot.py index ae99d398..47f4167f 100644 --- a/catapult/devil/devil/android/sdk/fastboot.py +++ b/catapult/devil/devil/android/sdk/fastboot.py @@ -54,7 +54,7 @@ class Fastboot(object): Raises: TypeError: If cmd is not of type list. """ - if type(cmd) == list: + if isinstance(cmd, list): cmd = [cls._fastboot_path.read()] + cmd else: raise TypeError( @@ -76,7 +76,7 @@ class Fastboot(object): Raises: TypeError: If cmd is not of type list. """ - if type(cmd) == list: + if isinstance(cmd, list): cmd = ['-s', self._device_serial] + cmd return self._RunFastbootCommand(cmd) diff --git a/catapult/devil/devil/android/sdk/shared_prefs.py b/catapult/devil/devil/android/sdk/shared_prefs.py index 2fa2e6a1..c985cacc 100644 --- a/catapult/devil/devil/android/sdk/shared_prefs.py +++ b/catapult/devil/devil/android/sdk/shared_prefs.py @@ -10,10 +10,10 @@ See e.g.: import logging import posixpath +from xml.etree import ElementTree from devil.android import device_errors from devil.android.sdk import version_codes -from xml.etree import ElementTree logger = logging.getLogger(__name__) @@ -167,7 +167,7 @@ _PREF_TYPES = {c.tag_name: c for c in [BooleanPref, FloatPref, IntPref, class SharedPrefs(object): - def __init__(self, device, package, filename): + def __init__(self, device, package, filename, use_encrypted_path=False): """Helper object to read and update "Shared Prefs" of Android apps. Such files typically look like, e.g.: @@ -196,12 +196,29 @@ class SharedPrefs(object): package: A string with the package name of the app that owns the shared preferences file. filename: A string with the name of the preferences file to read/write. + use_encrypted_path: Whether to read and write to the shared prefs location + in the device-encrypted path (/data/user_de) instead of the older, + unencrypted path (/data/data). Only supported on N+, but falls back to + the unencrypted path if the encrypted path is not supported on the given + device. """ self._device = device self._xml = None self._package = package self._filename = filename - self._path = '/data/data/%s/shared_prefs/%s' % (package, filename) + self._unencrypted_path = '/data/data/%s/shared_prefs/%s' % (package, + filename) + self._encrypted_path = '/data/user_de/0/%s/shared_prefs/%s' % (package, + filename) + self._path = self._unencrypted_path + self._encrypted = use_encrypted_path + if use_encrypted_path: + if self._device.build_version_sdk < version_codes.NOUGAT: + logging.info('SharedPrefs set to use encrypted path, but given device ' + 'is not running N+. Falling back to unencrypted path') + self._encrypted = False + else: + self._path = self._encrypted_path self._changed = False def __repr__(self): @@ -277,14 +294,24 @@ class SharedPrefs(object): # to the shared_prefs directory, which mimics the behavior of a file # created by the app itself if self._device.build_version_sdk >= version_codes.MARSHMALLOW: - security_context = self._GetSecurityContext(self.package) - if security_context == None: + security_context = self._device.GetSecurityContextForPackage(self.package, + encrypted=self._encrypted) + if security_context is None: raise device_errors.CommandFailedError( 'Failed to get security context for %s' % self.package) - self._device.RunShellCommand( - ['chcon', '-R', security_context, - '/data/data/%s/shared_prefs' % self.package], - as_root=True, check_return=True) + paths = [posixpath.dirname(self.path), self.path] + self._device.ChangeSecurityContext(security_context, paths) + + # Ensure that there isn't both an encrypted and unencrypted version of the + # file on the device at the same time. + if self._device.build_version_sdk >= version_codes.NOUGAT: + remove_path = (self._unencrypted_path if self._encrypted + else self._encrypted_path) + if self._device.PathExists(remove_path, as_root=True): + logging.warning('Found an equivalent shared prefs file at %s, removing', + remove_path) + self._device.RemovePath(remove_path, as_root=True) + self._device.KillAll(self.package, exact=True, as_root=True, quiet=True) self._changed = False @@ -406,15 +433,3 @@ class SharedPrefs(object): pref.set(value) self._changed = True logger.info('Setting property: %s', pref) - - def _GetSecurityContext(self, package): - for line in self._device.RunShellCommand(['ls', '-Z', '/data/data/'], - as_root=True, check_return=True): - split_line = line.split() - # ls -Z output differs between Android versions, but the package is - # always last and the context always starts with "u:object" - if split_line[-1] == package: - for column in split_line: - if column.startswith('u:object'): - return column - return None diff --git a/catapult/devil/devil/android/sdk/shared_prefs_test.py b/catapult/devil/devil/android/sdk/shared_prefs_test.py index 4c31c569..49587c89 100755 --- a/catapult/devil/devil/android/sdk/shared_prefs_test.py +++ b/catapult/devil/devil/android/sdk/shared_prefs_test.py @@ -166,6 +166,19 @@ class SharedPrefsTest(unittest.TestCase): # contents were not modified self.assertEquals(prefs.AsDict(), self.expected_data) + def testEncryptedPath(self): + type(self.device).build_version_sdk = mock.PropertyMock( + return_value=version_codes.MARSHMALLOW) + with shared_prefs.SharedPrefs(self.device, 'com.some.package', + 'prefs.xml', use_encrypted_path=True) as prefs: + self.assertTrue(prefs.path.startswith('/data/data')) + + type(self.device).build_version_sdk = mock.PropertyMock( + return_value=version_codes.NOUGAT) + with shared_prefs.SharedPrefs(self.device, 'com.some.package', + 'prefs.xml', use_encrypted_path=True) as prefs: + self.assertTrue(prefs.path.startswith('/data/user_de/0')) + if __name__ == '__main__': logging.getLogger().setLevel(logging.DEBUG) unittest.main(verbosity=2) diff --git a/catapult/devil/devil/android/sdk/version_codes.py b/catapult/devil/devil/android/sdk/version_codes.py index ec14359a..1750f00d 100644 --- a/catapult/devil/devil/android/sdk/version_codes.py +++ b/catapult/devil/devil/android/sdk/version_codes.py @@ -17,5 +17,5 @@ LOLLIPOP_MR1 = 22 MARSHMALLOW = 23 NOUGAT = 24 NOUGAT_MR1 = 25 -O = 26 -O_MR1 = 27 +OREO = 26 +OREO_MR1 = 27 diff --git a/catapult/devil/devil/android/tools/device_monitor.py b/catapult/devil/devil/android/tools/device_monitor.py index 10e0333a..565f8658 100755 --- a/catapult/devil/devil/android/tools/device_monitor.py +++ b/catapult/devil/devil/android/tools/device_monitor.py @@ -112,9 +112,9 @@ def get_device_status_unsafe(device): except ValueError: continue key = line.split(':')[0].strip() - if 'MemTotal' == key: + if key == 'MemTotal': status['mem']['total'] = value - elif 'MemFree' == key: + elif key == 'MemFree': status['mem']['free'] = value # Process @@ -162,7 +162,8 @@ def get_device_status(device): try: status = get_device_status_unsafe(device) except device_errors.DeviceUnreachableError: - status = {'state': 'offline'} + status = collections.defaultdict(dict) + status['state'] = 'offline' return status diff --git a/catapult/devil/devil/android/tools/device_recovery.py b/catapult/devil/devil/android/tools/device_recovery.py index 0925aaef..e8b9ba3f 100755 --- a/catapult/devil/devil/android/tools/device_recovery.py +++ b/catapult/devil/devil/android/tools/device_recovery.py @@ -8,10 +8,11 @@ import argparse import logging import os -import psutil import signal import sys +import psutil + if __name__ == '__main__': sys.path.append( os.path.abspath(os.path.join(os.path.dirname(__file__), @@ -38,7 +39,7 @@ def KillAllAdb(): # as newer (v2 and over) versions of psutil. # See: http://grodola.blogspot.com/2014/01/psutil-20-porting.html pinfo = p.as_dict(attrs=['pid', 'name', 'cmdline']) - if 'adb' == pinfo['name']: + if pinfo['name'] == 'adb': pinfo['cmdline'] = ' '.join(pinfo['cmdline']) yield p, pinfo except (psutil.NoSuchProcess, psutil.AccessDenied): diff --git a/catapult/devil/devil/android/tools/provision_devices.py b/catapult/devil/devil/android/tools/provision_devices.py index 68aca3b9..3a726e59 100755 --- a/catapult/devil/devil/android/tools/provision_devices.py +++ b/catapult/devil/devil/android/tools/provision_devices.py @@ -509,6 +509,8 @@ def LogDeviceProperties(device): logger.info(' %s', prop) +# TODO(jbudorick): Relocate this either to device_utils or a separate +# and more intentionally reusable layer on top of device_utils. def CheckExternalStorage(device): """Checks that storage is writable and if not makes it writable. diff --git a/catapult/devil/devil/android/tools/script_common_test.py b/catapult/devil/devil/android/tools/script_common_test.py index 3ddb1c16..30f9aaf9 100755 --- a/catapult/devil/devil/android/tools/script_common_test.py +++ b/catapult/devil/devil/android/tools/script_common_test.py @@ -18,6 +18,7 @@ with devil_env.SysPath(devil_env.PYMOCK_PATH): import mock # pylint: disable=import-error with devil_env.SysPath(devil_env.DEPENDENCY_MANAGER_PATH): + # pylint: disable=wrong-import-order from dependency_manager import exceptions @@ -90,4 +91,3 @@ class InitializeEnvironmentTest(unittest.TestCase): if __name__ == '__main__': sys.exit(unittest.main()) - diff --git a/catapult/devil/devil/utils/__init__.py b/catapult/devil/devil/utils/__init__.py index ff84988d..e69de29b 100644 --- a/catapult/devil/devil/utils/__init__.py +++ b/catapult/devil/devil/utils/__init__.py @@ -1,23 +0,0 @@ -# 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. - -import os -import sys - -def _JoinPath(*path_parts): - return os.path.abspath(os.path.join(*path_parts)) - - -def _AddDirToPythonPath(*path_parts): - path = _JoinPath(*path_parts) - if os.path.isdir(path) and path not in sys.path: - # Some call sites that use Telemetry assume that sys.path[0] is the - # directory containing the script, so we add these extra paths to right - # after sys.path[0]. - sys.path.insert(1, path) - -_CATAPULT_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), - os.path.pardir, os.path.pardir, os.path.pardir) - -_AddDirToPythonPath(_CATAPULT_DIR, 'common', 'battor') diff --git a/catapult/devil/devil/utils/battor_device_mapping.py b/catapult/devil/devil/utils/battor_device_mapping.py deleted file mode 100755 index 8cabb830..00000000 --- a/catapult/devil/devil/utils/battor_device_mapping.py +++ /dev/null @@ -1,309 +0,0 @@ -#!/usr/bin/python -# Copyright 2016 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. - - -''' -This script provides tools to map BattOrs to phones. - -Phones are identified by the following string: - -"Phone serial number" - Serial number of the phone. This can be -obtained via 'adb devices' or 'usb-devices', and is not expected -to change for a given phone. - -BattOrs are identified by the following two strings: - -"BattOr serial number" - Serial number of the BattOr. This can be -obtained via 'usb-devices', and is not expected to change for -a given BattOr. - -"BattOr path" - The path of the form '/dev/ttyUSB*' that is used -to communicate with the BattOr (the battor_agent binary takes -this BattOr path as a parameter). The BattOr path is frequently -reassigned by the OS, most often when the device is disconnected -and then reconnected. Thus, the BattOr path cannot be expected -to be stable. - -In a typical application, the user will require the BattOr path -for the BattOr that is plugged into a given phone. For instance, -the user will be running tracing on a particular phone, and will -need to know which BattOr path to use to communicate with the BattOr -to get the corresponding power trace. - -Getting this mapping requires two steps: (1) determining the -mapping between phone serial numbers and BattOr serial numbers, and -(2) getting the BattOr path corresponding to a given BattOr serial -number. - -For step (1), we generate a JSON file giving this mapping. This -JSON file consists of a list of items of the following form: -[{'phone': , 'battor': }, -{'phone': , 'battor': }, ...] - -The default way to generate this JSON file is using the function -GenerateSerialMapFile, which generates a mapping based on assuming -that the system has two identical USB hubs connected to it, and -the phone plugged into physical port number 1 on one hub corresponds -to the BattOr plugged into physical port number 1 on the other hub, -and similarly with physical port numbers 2, 3, etc. This generates -the map file based on the structure at the time GenerateSerialMapFile called. -Note that after the map file is generated, port numbers are no longer used; -the user could move around the devices in the ports without affecting -which phone goes with which BattOr. (Thus, if the user wanted to update the -mapping to match the new port connections, the user would have to -re-generate this file.) - -The script update_mapping.py will do this updating from the command line. - -If the user wanted to specify a custom mapping, the user could instead -create the JSON file manually. (In this case, hubs would not be necessary -and the physical ports connected would be irrelevant.) - -Step (2) is conducted through the function GetBattOrPathFromPhoneSerial, -which takes a serial number mapping generated via step (1) and a phone -serial number, then gets the corresponding BattOr serial number from the -map and determines its BattOr path (e.g. /dev/ttyUSB0). Since BattOr paths -can change if devices are connected and disconnected (even if connected -or disconnected via the same port) this function should be called to -determine the BattOr path every time before connecting to the BattOr. - -Note that if there is only one BattOr connected to the system, then -GetBattOrPathFromPhoneSerial will always return that BattOr and will ignore -the mapping file. Thus, if the user never has more than one BattOr connected -to the system, the user will not need to generate mapping files. -''' - - -import json -import collections - -from battor import battor_error -from devil.utils import find_usb_devices -from devil.utils import usb_hubs - - -def GetBattOrList(device_tree_map): - return [x for x in find_usb_devices.GetTTYList() - if IsBattOr(x, device_tree_map)] - - -def IsBattOr(tty_string, device_tree_map): - (bus, device) = find_usb_devices.GetBusDeviceFromTTY(tty_string) - node = device_tree_map[bus].FindDeviceNumber(device) - return '0403:6001' in node.desc - - -def GetBattOrSerialNumbers(device_tree_map): - for x in find_usb_devices.GetTTYList(): - if IsBattOr(x, device_tree_map): - (bus, device) = find_usb_devices.GetBusDeviceFromTTY(x) - devnode = device_tree_map[bus].FindDeviceNumber(device) - yield devnode.serial - - -def ReadSerialMapFile(filename): - """Reads JSON file giving phone-to-battor serial number map. - - Parses a JSON file consisting of a list of items of the following form: - [{'phone': , 'battor': }, - {'phone': , 'battor': }, ...] - - indicating which phone serial numbers should be matched with - which BattOr serial numbers. Returns dictionary of the form: - - {: , - : } - - Args: - filename: Name of file to read. - """ - result = {} - with open(filename, 'r') as infile: - in_dict = json.load(infile) - for x in in_dict: - result[x['phone']] = x['battor'] - return result - -def WriteSerialMapFile(filename, serial_map): - """Writes a map of phone serial numbers to BattOr serial numbers to file. - - Writes a JSON file consisting of a list of items of the following form: - [{'phone': , 'battor': }, - {'phone': , 'battor': }, ...] - - indicating which phone serial numbers should be matched with - which BattOr serial numbers. Mapping is based on the physical port numbers - of the hubs that the BattOrs and phones are connected to. - - Args: - filename: Name of file to write. - serial_map: Serial map {phone: battor} - """ - result = [] - for (phone, battor) in serial_map.iteritems(): - result.append({'phone': phone, 'battor': battor}) - with open(filename, 'w') as outfile: - json.dump(result, outfile) - -def GenerateSerialMap(hub_types=None): - """Generates a map of phone serial numbers to BattOr serial numbers. - - Generates a dict of: - {: , - : } - indicating which phone serial numbers should be matched with - which BattOr serial numbers. Mapping is based on the physical port numbers - of the hubs that the BattOrs and phones are connected to. - - Args: - hub_types: List of hub types to check for. If not specified, checks - for all defined hub types. (see usb_hubs.py for details) - """ - if hub_types: - hub_types = [usb_hubs.GetHubType(x) for x in hub_types] - else: - hub_types = usb_hubs.ALL_HUBS - - devtree = find_usb_devices.GetBusNumberToDeviceTreeMap() - - # List of serial numbers in the system that represent BattOrs. - battor_serials = list(GetBattOrSerialNumbers(devtree)) - - # If there's only one BattOr in the system, then a serial number ma - # is not necessary. - if len(battor_serials) == 1: - return {} - - # List of dictionaries, one for each hub, that maps the physical - # port number to the serial number of that hub. For instance, in a 2 - # hub system, this could return [{1:'ab', 2:'cd'}, {1:'jkl', 2:'xyz'}] - # where 'ab' and 'cd' are the phone serial numbers and 'jkl' and 'xyz' - # are the BattOr serial numbers. - port_to_serial = find_usb_devices.GetAllPhysicalPortToSerialMaps( - hub_types, device_tree_map=devtree) - - class serials(object): - def __init__(self): - self.phone = None - self.battor = None - - # Map of {physical port number: [phone serial #, BattOr serial #]. This - # map is populated by executing the code below. For instance, in the above - # example, after the code below is executed, port_to_devices would equal - # {1: ['ab', 'jkl'], 2: ['cd', 'xyz']} - port_to_devices = collections.defaultdict(serials) - for hub in port_to_serial: - for (port, serial) in hub.iteritems(): - if serial in battor_serials: - if port_to_devices[port].battor is not None: - raise battor_error.BattOrError('Multiple BattOrs on same port number') - else: - port_to_devices[port].battor = serial - else: - if port_to_devices[port].phone is not None: - raise battor_error.BattOrError('Multiple phones on same port number') - else: - port_to_devices[port].phone = serial - - # Turn the port_to_devices map into a map of the form - # {phone serial number: BattOr serial number}. - result = {} - for pair in port_to_devices.values(): - if pair.phone is None: - continue - if pair.battor is None: - raise battor_error.BattOrError( - 'Phone detected with no corresponding BattOr') - result[pair.phone] = pair.battor - return result - -def GenerateSerialMapFile(filename, hub_types=None): - """Generates a serial map file and writes it.""" - WriteSerialMapFile(filename, GenerateSerialMap(hub_types)) - -def _PhoneToPathMap(serial, serial_map, devtree): - """Maps phone serial number to TTY path, assuming serial map is provided.""" - try: - battor_serial = serial_map[serial] - except KeyError: - raise battor_error.BattOrError('Serial number not found in serial map.') - for tree in devtree.values(): - for node in tree.AllNodes(): - if isinstance(node, find_usb_devices.USBDeviceNode): - if node.serial == battor_serial: - bus_device_to_tty = find_usb_devices.GetBusDeviceToTTYMap() - bus_device = (node.bus_num, node.device_num) - try: - return bus_device_to_tty[bus_device] - except KeyError: - raise battor_error.BattOrError( - 'Device with given serial number not a BattOr ' - '(does not have TTY path)') - - -def GetBattOrPathFromPhoneSerial(serial, serial_map=None, - serial_map_file=None): - """Gets the TTY path (e.g. '/dev/ttyUSB0') to communicate with the BattOr. - - (1) If serial_map is given, it is treated as a dictionary mapping - phone serial numbers to BattOr serial numbers. This function will get the - TTY path for the given BattOr serial number. - - (2) If serial_map_file is given, it is treated as the name of a - phone-to-BattOr mapping file (generated with GenerateSerialMapFile) - and this will be loaded and used as the dict to map port numbers to - BattOr serial numbers. - - You can only give one of serial_map and serial_map_file. - - Args: - serial: Serial number of phone connected on the same physical port that - the BattOr is connected to. - serial_map: Map of phone serial numbers to BattOr serial numbers, given - as a dictionary. - serial_map_file: Map of phone serial numbers to BattOr serial numbers, - given as a file. - hub_types: List of hub types to check for. Used only if serial_map_file - is None. - - Returns: - Device string used to communicate with device. - - Raises: - ValueError: If serial number is not given. - BattOrError: If BattOr not found or unexpected USB topology. - """ - # If there's only one BattOr connected to the system, just use that one. - # This allows for use on, e.g., a developer's workstation with no hubs. - devtree = find_usb_devices.GetBusNumberToDeviceTreeMap() - all_battors = GetBattOrList(devtree) - if len(all_battors) == 1: - return '/dev/' + all_battors[0] - - if not serial: - raise battor_error.BattOrError( - 'Two or more BattOrs connected, no serial provided') - - if serial_map and serial_map_file: - raise ValueError('Cannot specify both serial_map and serial_map_file') - - if serial_map_file: - serial_map = ReadSerialMapFile(serial_map_file) - - tty_string = _PhoneToPathMap(serial, serial_map, devtree) - - if not tty_string: - raise battor_error.BattOrError( - 'No device with given serial number detected.') - - if IsBattOr(tty_string, devtree): - return '/dev/' + tty_string - else: - raise battor_error.BattOrError( - 'Device with given serial number is not a BattOr.') - -if __name__ == '__main__': - # Main function for testing purposes - print GenerateSerialMap() diff --git a/catapult/devil/devil/utils/cmd_helper.py b/catapult/devil/devil/utils/cmd_helper.py index b477c700..5ea85d85 100644 --- a/catapult/devil/devil/utils/cmd_helper.py +++ b/catapult/devil/devil/utils/cmd_helper.py @@ -187,6 +187,27 @@ def GetCmdStatusAndOutput(args, cwd=None, shell=False, env=None): return (status, stdout) +def StartCmd(args, cwd=None, shell=False, env=None): + """Starts a subprocess and returns a handle to the process. + + Args: + args: A string or a sequence of program arguments. The program to execute is + the string or the first item in the args sequence. + cwd: If not None, the subprocess's current directory will be changed to + |cwd| before it's executed. + shell: Whether to execute args as a shell command. Must be True if args + is a string and False if args is a sequence. + env: If not None, a mapping that defines environment variables for the + subprocess. + + Returns: + A process handle from subprocess.Popen. + """ + _ValidateAndLogCommand(args, cwd, shell) + return Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + shell=shell, cwd=cwd, env=env) + + def GetCmdStatusOutputAndError(args, cwd=None, shell=False, env=None): """Executes a subprocess and returns its exit code, output, and errors. @@ -226,6 +247,7 @@ def _IterProcessStdoutFcntl( process, iter_timeout=None, timeout=None, buffer_size=4096, poll_interval=1): """An fcntl-based implementation of _IterProcessStdout.""" + # pylint: disable=too-many-nested-blocks import fcntl try: # Enable non-blocking reads from the child's stdout. diff --git a/catapult/devil/devil/utils/find_usb_devices.py b/catapult/devil/devil/utils/find_usb_devices.py index 74b888dd..b6dcc702 100755 --- a/catapult/devil/devil/utils/find_usb_devices.py +++ b/catapult/devil/devil/utils/find_usb_devices.py @@ -452,9 +452,9 @@ def GetBusDeviceFromTTY(tty_string): for line in _GetTtyUSBInfo(tty_string).splitlines(): bus_match = _BUS_NUM_REGEX.match(line) device_match = _DEVICE_NUM_REGEX.match(line) - if bus_match and bus_num == None: + if bus_match and bus_num is None: bus_num = int(bus_match.group(1)) - if device_match and device_num == None: + if device_match and device_num is None: device_num = int(device_match.group(1)) if bus_num is None or device_num is None: raise ValueError('Info not found') diff --git a/catapult/devil/devil/utils/find_usb_devices_test.py b/catapult/devil/devil/utils/find_usb_devices_test.py index e8b00c85..c22f21b8 100755 --- a/catapult/devil/devil/utils/find_usb_devices_test.py +++ b/catapult/devil/devil/utils/find_usb_devices_test.py @@ -17,23 +17,21 @@ Bus 001: Bus 002: 1: Device 011 "quux" 2: Device 020 "My Test HUB" #hub 1 -2:1: Device 021 "battor_p7_h1_t0" #physical port 7 on hub 1, on ttyUSB0 -2:3: Device 022 "battor_p5_h1_t1" #physical port 5 on hub 1, on ttyUSB1 +2:1: Device 021 "usb_device_p7_h1_t0" #physical port 7 on hub 1, on ttyUSB0 +2:3: Device 022 "usb_device_p5_h1_t1" #physical port 5 on hub 1, on ttyUSB1 2:4: Device 023 "My Test Internal HUB" #internal section of hub 1 -2:4:2: Device 024 "battor_p3_h1_t2" #physical port 3 on hub 1, on ttyUSB2 +2:4:2: Device 024 "usb_device_p3_h1_t2" #physical port 3 on hub 1, on ttyUSB2 2:4:3: Device 026 "Not a Battery Monitor" #physical port 1 on hub 1, on ttyUSB3 -2:4:4: Device 025 "battor_p1_h1_t3" #physical port 1 on hub 1, on ttyUSB3 +2:4:4: Device 025 "usb_device_p1_h1_t3" #physical port 1 on hub 1, on ttyUSB3 3: Device 100 "My Test HUB" #hub 2 3:4: Device 101 "My Test Internal HUB" #internal section of hub 2 -3:4:4: Device 102 "battor_p1_h2_t4" #physical port 1 on hub 2, on ttyusb4 +3:4:4: Device 102 "usb_device_p1_h2_t4" #physical port 1 on hub 2, on ttyusb4 """ import logging -import os import unittest from devil import devil_env -from devil.utils import battor_device_mapping from devil.utils import find_usb_devices from devil.utils import lsusb from devil.utils import usb_hubs @@ -51,15 +49,15 @@ DEVLIST = [(1, 11, 'foo'), (1, 13, 'baz'), (2, 11, 'quux'), (2, 20, 'My Test HUB'), - (2, 21, 'ID 0403:6001 battor_p7_h1_t0'), - (2, 22, 'ID 0403:6001 battor_p5_h1_t1'), + (2, 21, 'ID 0403:6001 usb_device_p7_h1_t0'), + (2, 22, 'ID 0403:6001 usb_device_p5_h1_t1'), (2, 23, 'My Test Internal HUB'), - (2, 24, 'ID 0403:6001 battor_p3_h1_t2'), - (2, 25, 'ID 0403:6001 battor_p1_h1_t3'), + (2, 24, 'ID 0403:6001 usb_device_p3_h1_t2'), + (2, 25, 'ID 0403:6001 usb_device_p1_h1_t3'), (2, 26, 'Not a Battery Monitor'), (2, 100, 'My Test HUB'), (2, 101, 'My Test Internal HUB'), - (2, 102, 'ID 0403:6001 battor_p1_h1_t4')] + (2, 102, 'ID 0403:6001 usb_device_p1_h1_t4')] LSUSB_OUTPUT = [ {'bus': b, 'device': d, 'desc': t, 'id': (1000*b)+d} @@ -82,14 +80,14 @@ T: Bus=02 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 11 Spd=000 MxCh=00 T: Bus=02 Lev=00 Prnt=00 Port=01 Cnt=00 Dev#= 20 Spd=000 MxCh=00 T: Bus=02 Lev=00 Prnt=20 Port=00 Cnt=00 Dev#= 21 Spd=000 MxCh=00 -S: SerialNumber=BattOr0 +S: SerialNumber=UsbDevice0 T: Bus=02 Lev=00 Prnt=20 Port=02 Cnt=00 Dev#= 22 Spd=000 MxCh=00 -S: SerialNumber=BattOr1 +S: SerialNumber=UsbDevice1 T: Bus=02 Lev=00 Prnt=20 Port=03 Cnt=00 Dev#= 23 Spd=000 MxCh=00 T: Bus=02 Lev=00 Prnt=23 Port=01 Cnt=00 Dev#= 24 Spd=000 MxCh=00 -S: SerialNumber=BattOr2 +S: SerialNumber=UsbDevice2 T: Bus=02 Lev=00 Prnt=23 Port=03 Cnt=00 Dev#= 25 Spd=000 MxCh=00 -S: SerialNumber=BattOr3 +S: SerialNumber=UsbDevice3 T: Bus=02 Lev=00 Prnt=23 Port=02 Cnt=00 Dev#= 26 Spd=000 MxCh=00 T: Bus=02 Lev=00 Prnt=00 Port=02 Cnt=00 Dev#=100 Spd=000 MxCh=00 @@ -103,15 +101,15 @@ Bus 001 Device 012: FAST bar Bus 001 Device 013: baz Bus 002 Device 011: quux Bus 002 Device 020: My Test HUB -Bus 002 Device 021: ID 0403:6001 battor_p7_h1_t0 -Bus 002 Device 022: ID 0403:6001 battor_p5_h1_t1 +Bus 002 Device 021: ID 0403:6001 usb_device_p7_h1_t0 +Bus 002 Device 022: ID 0403:6001 usb_device_p5_h1_t1 Bus 002 Device 023: My Test Internal HUB -Bus 002 Device 024: ID 0403:6001 battor_p3_h1_t2 -Bus 002 Device 025: ID 0403:6001 battor_p1_h1_t3 +Bus 002 Device 024: ID 0403:6001 usb_device_p3_h1_t2 +Bus 002 Device 025: ID 0403:6001 usb_device_p1_h1_t3 Bus 002 Device 026: Not a Battery Monitor Bus 002 Device 100: My Test HUB Bus 002 Device 101: My Test Internal HUB -Bus 002 Device 102: ID 0403:6001 battor_p1_h1_t4 +Bus 002 Device 102: ID 0403:6001 usb_device_p1_h1_t4 ''' LIST_TTY_OUTPUT = ''' @@ -213,17 +211,6 @@ class USBScriptTest(unittest.TestCase): lsusb.raw_lsusb = mock.Mock( return_value=RAW_LSUSB_OUTPUT) - def testIsBattOr(self): - bd = find_usb_devices.GetBusNumberToDeviceTreeMap() - self.assertTrue(battor_device_mapping.IsBattOr('ttyUSB3', bd)) - self.assertFalse(battor_device_mapping.IsBattOr('ttyUSB5', bd)) - - def testGetBattOrs(self): - bd = find_usb_devices.GetBusNumberToDeviceTreeMap() - self.assertEquals(battor_device_mapping.GetBattOrList(bd), - ['ttyUSB0', 'ttyUSB1', 'ttyUSB2', - 'ttyUSB3', 'ttyUSB4']) - def testGetTTYDevices(self): pp = find_usb_devices.GetAllPhysicalPortToTTYMaps([TEST_HUB]) result = list(pp) @@ -247,131 +234,49 @@ class USBScriptTest(unittest.TestCase): def testGetSerialMapping(self): pp = find_usb_devices.GetAllPhysicalPortToSerialMaps([TEST_HUB]) result = list(pp) - self.assertEquals(result[0], {7:'BattOr0', - 5:'BattOr1', - 3:'BattOr2', - 1:'BattOr3'}) + self.assertEquals(result[0], {7:'UsbDevice0', + 5:'UsbDevice1', + 3:'UsbDevice2', + 1:'UsbDevice3'}) self.assertEquals(result[1], {}) def testFastDeviceDescriptions(self): bd = find_usb_devices.GetBusNumberToDeviceTreeMap() dev_foo = bd[1].FindDeviceNumber(11) dev_bar = bd[1].FindDeviceNumber(12) - dev_battor_p7_h1_t0 = bd[2].FindDeviceNumber(21) + dev_usb_device_p7_h1_t0 = bd[2].FindDeviceNumber(21) self.assertEquals(dev_foo.desc, 'FAST foo') self.assertEquals(dev_bar.desc, 'FAST bar') - self.assertEquals(dev_battor_p7_h1_t0.desc, - 'ID 0403:6001 battor_p7_h1_t0') + self.assertEquals(dev_usb_device_p7_h1_t0.desc, + 'ID 0403:6001 usb_device_p7_h1_t0') def testDeviceDescriptions(self): bd = find_usb_devices.GetBusNumberToDeviceTreeMap(fast=False) dev_foo = bd[1].FindDeviceNumber(11) dev_bar = bd[1].FindDeviceNumber(12) - dev_battor_p7_h1_t0 = bd[2].FindDeviceNumber(21) + dev_usb_device_p7_h1_t0 = bd[2].FindDeviceNumber(21) self.assertEquals(dev_foo.desc, 'foo') self.assertEquals(dev_bar.desc, 'bar') - self.assertEquals(dev_battor_p7_h1_t0.desc, - 'ID 0403:6001 battor_p7_h1_t0') + self.assertEquals(dev_usb_device_p7_h1_t0.desc, + 'ID 0403:6001 usb_device_p7_h1_t0') def testDeviceInformation(self): bd = find_usb_devices.GetBusNumberToDeviceTreeMap(fast=False) dev_foo = bd[1].FindDeviceNumber(11) dev_bar = bd[1].FindDeviceNumber(12) - dev_battor_p7_h1_t0 = bd[2].FindDeviceNumber(21) + dev_usb_device_p7_h1_t0 = bd[2].FindDeviceNumber(21) self.assertEquals(dev_foo.info['id'], 1011) self.assertEquals(dev_bar.info['id'], 1012) - self.assertEquals(dev_battor_p7_h1_t0.info['id'], 2021) + self.assertEquals(dev_usb_device_p7_h1_t0.info['id'], 2021) def testSerialNumber(self): bd = find_usb_devices.GetBusNumberToDeviceTreeMap(fast=False) dev_foo = bd[1].FindDeviceNumber(11) dev_bar = bd[1].FindDeviceNumber(12) - dev_battor_p7_h1_t0 = bd[2].FindDeviceNumber(21) + dev_usb_device_p7_h1_t0 = bd[2].FindDeviceNumber(21) self.assertEquals(dev_foo.serial, 'FooSerial') self.assertEquals(dev_bar.serial, 'BarSerial') - self.assertEquals(dev_battor_p7_h1_t0.serial, 'BattOr0') - - def testBattOrDictMapping(self): - map_dict = {'Phone1':'BattOr1', 'Phone2':'BattOr2', 'Phone3':'BattOr3'} - a1 = battor_device_mapping.GetBattOrPathFromPhoneSerial( - 'Phone1', serial_map=map_dict) - a2 = battor_device_mapping.GetBattOrPathFromPhoneSerial( - 'Phone2', serial_map=map_dict) - a3 = battor_device_mapping.GetBattOrPathFromPhoneSerial( - 'Phone3', serial_map=map_dict) - self.assertEquals(a1, '/dev/ttyUSB1') - self.assertEquals(a2, '/dev/ttyUSB2') - self.assertEquals(a3, '/dev/ttyUSB3') - - def testBattOrDictFromFileMapping(self): - try: - map_dict = {'Phone1':'BattOr1', 'Phone2':'BattOr2', 'Phone3':'BattOr3'} - curr_dir = os.path.dirname(os.path.realpath(__file__)) - filename = os.path.join(curr_dir, 'test', 'data', 'test_write_map.json') - battor_device_mapping.WriteSerialMapFile(filename, map_dict) - a1 = battor_device_mapping.GetBattOrPathFromPhoneSerial( - 'Phone1', serial_map_file=filename) - a2 = battor_device_mapping.GetBattOrPathFromPhoneSerial( - 'Phone2', serial_map_file=filename) - a3 = battor_device_mapping.GetBattOrPathFromPhoneSerial( - 'Phone3', serial_map_file=filename) - finally: - os.remove(filename) - self.assertEquals(a1, '/dev/ttyUSB1') - self.assertEquals(a2, '/dev/ttyUSB2') - self.assertEquals(a3, '/dev/ttyUSB3') - - def testReadSerialMapFile(self): - curr_dir = os.path.dirname(os.path.realpath(__file__)) - map_dict = battor_device_mapping.ReadSerialMapFile( - os.path.join(curr_dir, 'test', 'data', 'test_serial_map.json')) - self.assertEquals(len(map_dict.keys()), 3) - self.assertEquals(map_dict['Phone1'], 'BattOr1') - self.assertEquals(map_dict['Phone2'], 'BattOr2') - self.assertEquals(map_dict['Phone3'], 'BattOr3') - -original_PPTSM = find_usb_devices.GetAllPhysicalPortToSerialMaps -original_PPTTM = find_usb_devices.GetAllPhysicalPortToTTYMaps -original_GBL = battor_device_mapping.GetBattOrList -original_GBNDM = find_usb_devices.GetBusNumberToDeviceTreeMap -original_IB = battor_device_mapping.IsBattOr -original_GBSM = battor_device_mapping.GetBattOrSerialNumbers - -def setup_battor_test(serial, tty, battor, bser=None): - serial_mapper = mock.Mock(return_value=serial) - tty_mapper = mock.Mock(return_value=tty) - battor_lister = mock.Mock(return_value=battor) - devtree = mock.Mock(return_value=None) - is_battor = mock.Mock(side_effect=lambda x, y: x in battor) - battor_serials = mock.Mock(return_value=bser) - find_usb_devices.GetAllPhysicalPortToSerialMaps = serial_mapper - find_usb_devices.GetAllPhysicalPortToTTYMaps = tty_mapper - battor_device_mapping.GetBattOrList = battor_lister - find_usb_devices.GetBusNumberToDeviceTreeMap = devtree - battor_device_mapping.IsBattOr = is_battor - battor_device_mapping.GetBattOrSerialNumbers = battor_serials - -class BattOrMappingTest(unittest.TestCase): - def tearDown(self): - find_usb_devices.GetAllPhysicalPortToSerialMaps = original_PPTSM - find_usb_devices.GetAllPhysicalPortToTTYMaps = original_PPTTM - battor_device_mapping.GetBattOrList = original_GBL - find_usb_devices.GetBusNumberToDeviceTreeMap = original_GBNDM - battor_device_mapping.IsBattOr = original_IB - battor_device_mapping.GetBattOrSerialNumbers = original_GBSM - - def test_generate_serial_map(self): - setup_battor_test([{1:'Phn1', 2:'Phn2', 3:'Phn3'}, - {1:'Bat1', 2:'Bat2', 3:'Bat3'}], - [{}, - {1:'ttyUSB0', 2:'ttyUSB1', 3:'ttyUSB2'}], - ['ttyUSB0', 'ttyUSB1', 'ttyUSB2'], - ['Bat1', 'Bat2', 'Bat3']) - result = battor_device_mapping.GenerateSerialMap() - self.assertEqual(len(result), 3) - self.assertEqual(result['Phn1'], 'Bat1') - self.assertEqual(result['Phn2'], 'Bat2') - self.assertEqual(result['Phn3'], 'Bat3') + self.assertEquals(dev_usb_device_p7_h1_t0.serial, 'UsbDevice0') if __name__ == "__main__": diff --git a/catapult/devil/devil/utils/lazy/weak_constant.py b/catapult/devil/devil/utils/lazy/weak_constant.py index 3558f29a..24ad9406 100644 --- a/catapult/devil/devil/utils/lazy/weak_constant.py +++ b/catapult/devil/devil/utils/lazy/weak_constant.py @@ -4,6 +4,9 @@ import threading +from devil.utils import reraiser_thread +from devil.utils import timeout_retry + class WeakConstant(object): """A thread-safe, lazily initialized object. @@ -13,17 +16,27 @@ class WeakConstant(object): """ def __init__(self, initializer): - self._initialized = False + self._initialized = threading.Event() self._initializer = initializer self._lock = threading.Lock() self._val = None def read(self): """Get the object, creating it if necessary.""" - if self._initialized: + if self._initialized.is_set(): return self._val with self._lock: - if not self._initialized: - self._val = self._initializer() - self._initialized = True + if not self._initialized.is_set(): + # We initialize the value on a separate thread to protect + # from holding self._lock indefinitely in the event that + # self._initializer hangs. + initializer_thread = reraiser_thread.ReraiserThread( + self._initializer) + initializer_thread.start() + timeout_retry.WaitFor( + lambda: initializer_thread.join(1) or not initializer_thread.isAlive(), + wait_period=0) + self._val = initializer_thread.GetReturnValue() + self._initialized.set() + return self._val diff --git a/catapult/devil/devil/utils/lazy/weak_constant_test.py b/catapult/devil/devil/utils/lazy/weak_constant_test.py new file mode 100644 index 00000000..95191501 --- /dev/null +++ b/catapult/devil/devil/utils/lazy/weak_constant_test.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python +# Copyright 2018 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. + +# pylint: disable=protected-access + +import time +import unittest + +from devil import devil_env +from devil.utils import lazy +from devil.utils import timeout_retry + +with devil_env.SysPath(devil_env.PYMOCK_PATH): + import mock + + +class DynamicSideEffect(object): + """A helper object for handling a sequence of single-use side effects.""" + + def __init__(self, side_effects): + self._side_effects = iter(side_effects or []) + + def __call__(self): + val = next(self._side_effects)() + if isinstance(val, Exception): + raise val + return val + + +class WeakConstantTest(unittest.TestCase): + + def testUninitialized(self): + """Ensure that the first read calls the initializer.""" + initializer = mock.Mock(return_value='initializer called') + test_constant = lazy.WeakConstant(initializer) + self.assertEquals( + 'initializer called', + test_constant.read()) + initializer.assert_called_once() + + def testInitialized(self): + """Ensure that reading doesn't reinitialize the value.""" + initializer = mock.Mock(return_value='initializer called') + test_constant = lazy.WeakConstant(initializer) + test_constant._initialized.set() + test_constant._val = 'initializer not called' + self.assertEquals( + 'initializer not called', + test_constant.read()) + initializer.assert_not_called() + + def testFirstCallHangs(self): + """Ensure that reading works even if the first initializer call hangs.""" + dyn = DynamicSideEffect([ + lambda: time.sleep(10), + lambda: 'second try worked!' + ]) + + initializer = mock.Mock(side_effect=dyn) + test_constant = lazy.WeakConstant(initializer) + self.assertEquals( + 'second try worked!', + timeout_retry.Run(test_constant.read, 1, 1)) + initializer.assert_has_calls([mock.call(), mock.call()]) + + +if __name__ == '__main__': + unittest.main() diff --git a/catapult/devil/devil/utils/markdown.py b/catapult/devil/devil/utils/markdown.py index 54e7ed56..6867e9db 100755 --- a/catapult/devil/devil/utils/markdown.py +++ b/catapult/devil/devil/utils/markdown.py @@ -182,7 +182,7 @@ def md_module(module_obj, module_path=None, module_link=None): A list of markdown-formatted lines. """ def should_doc(name): - return (type(module_obj.__dict__[name]) != types.ModuleType + return (not isinstance(module_obj.__dict__[name], types.ModuleType) and not name.startswith('_')) stuff_to_doc = sorted( @@ -193,9 +193,9 @@ def md_module(module_obj, module_path=None, module_link=None): functions_to_doc = [] for s in stuff_to_doc: - if type(s) == types.TypeType: + if isinstance(s, types.TypeType): classes_to_doc.append(s) - elif type(s) == types.FunctionType: + elif isinstance(s, types.FunctionType): functions_to_doc.append(s) command = ['devil/utils/markdown.py'] @@ -243,7 +243,7 @@ def md_class(class_obj): content.extend(md_docstring(class_obj.__doc__)) def should_doc(name, obj): - return (type(obj) == types.FunctionType + return (isinstance(obj, types.FunctionType) and (name.startswith('__') or not name.startswith('_'))) methods_to_doc = sorted( diff --git a/catapult/devil/devil/utils/reraiser_thread.py b/catapult/devil/devil/utils/reraiser_thread.py index 56d95f39..cb9764ef 100644 --- a/catapult/devil/devil/utils/reraiser_thread.py +++ b/catapult/devil/devil/utils/reraiser_thread.py @@ -47,10 +47,13 @@ class ReraiserThread(threading.Thread): func: callable to call on a new thread. args: list of positional arguments for callable, defaults to empty. kwargs: dictionary of keyword arguments for callable, defaults to empty. - name: thread name, defaults to Thread-N. + name: thread name, defaults to the function name. """ - if not name and func.__name__ != '': - name = func.__name__ + if not name: + if hasattr(func, '__name__') and func.__name__ != '': + name = func.__name__ + else: + name = 'anonymous' super(ReraiserThread, self).__init__(name=name) if not args: args = [] diff --git a/catapult/devil/devil/utils/test/data/test_serial_map.json b/catapult/devil/devil/utils/test/data/test_serial_map.json deleted file mode 100644 index f0682816..00000000 --- a/catapult/devil/devil/utils/test/data/test_serial_map.json +++ /dev/null @@ -1 +0,0 @@ -[{"phone": "Phone1", "battor": "BattOr1"}, {"phone": "Phone2", "battor": "BattOr2"}, {"phone": "Phone3", "battor": "BattOr3"}] diff --git a/catapult/devil/devil/utils/timeout_retry.py b/catapult/devil/devil/utils/timeout_retry.py index 2327b6bf..d662c1d2 100644 --- a/catapult/devil/devil/utils/timeout_retry.py +++ b/catapult/devil/devil/utils/timeout_retry.py @@ -109,7 +109,8 @@ def WaitFor(condition, wait_period=5, max_tries=None): # pylint: disable=no-member timeout_thread_group.GetRemainingTime(wait_period, suffix=' waiting for condition %r' % condition_name) - time.sleep(wait_period) + if wait_period: + time.sleep(wait_period) return None diff --git a/catapult/devil/devil/utils/update_mapping.py b/catapult/devil/devil/utils/update_mapping.py deleted file mode 100755 index 6666b9b0..00000000 --- a/catapult/devil/devil/utils/update_mapping.py +++ /dev/null @@ -1,47 +0,0 @@ -#!/usr/bin/env python - -# Copyright 2016 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -import argparse -import sys - -from devil.utils import battor_device_mapping - -def parse_options(): - """Parses and checks the command-line options. - - Returns: - A tuple containing the options structure. - """ - usage = 'Usage: ./update_mapping.py [options]' - desc = ('Example: ./update_mapping.py -o mapping.json.\n' - 'This script generates and stores a file that gives the\n' - 'mapping between phone serial numbers and BattOr serial numbers\n' - 'Mapping is based on which physical ports on the USB hubs the\n' - 'devices are plugged in to. For instance, if there are two hubs,\n' - 'the phone connected to port N on the first hub is mapped to the\n' - 'BattOr connected to port N on the second hub, for each N.') - parser = argparse.ArgumentParser(usage=usage, description=desc) - parser.add_argument('-o', '--output', dest='out_file', - default='mapping.json', type=str, - action='store', help='mapping file name') - parser.add_argument('-u', '--hub', dest='hub_types', - action='append', choices=['plugable_7port', - 'plugable_7port_usb3_part2', - 'plugable_7port_usb3_part3'], - help='USB hub types.') - options = parser.parse_args() - if not options.hub_types: - options.hub_types = ['plugable_7port', 'plugable_7port_usb3_part2', - 'plugable_7port_usb3_part3'] - return options - -def main(): - options = parse_options() - battor_device_mapping.GenerateSerialMapFile(options.out_file, - options.hub_types) - -if __name__ == "__main__": - sys.exit(main()) diff --git a/catapult/devil/pylintrc b/catapult/devil/pylintrc index 7e024a25..1a059cf2 100644 --- a/catapult/devil/pylintrc +++ b/catapult/devil/pylintrc @@ -14,6 +14,7 @@ disable= locally-enabled, missing-docstring, star-args, + wrong-import-position, [REPORTS] diff --git a/catapult/systrace/PRESUBMIT.py b/catapult/systrace/PRESUBMIT.py index 1234a8d8..275dfdbe 100644 --- a/catapult/systrace/PRESUBMIT.py +++ b/catapult/systrace/PRESUBMIT.py @@ -25,7 +25,6 @@ def _GetPathsToPrepend(input_api): return [ project_dir, - input_api.os_path.join(catapult_dir, 'common', 'battor'), input_api.os_path.join(catapult_dir, 'common', 'py_trace_event'), input_api.os_path.join(catapult_dir, 'common', 'py_utils'), input_api.os_path.join(catapult_dir, 'devil'), diff --git a/catapult/systrace/atrace_helper/jni/atrace_process_dump.cc b/catapult/systrace/atrace_helper/jni/atrace_process_dump.cc index ef72f789..d4dada02 100644 --- a/catapult/systrace/atrace_helper/jni/atrace_process_dump.cc +++ b/catapult/systrace/atrace_helper/jni/atrace_process_dump.cc @@ -4,6 +4,7 @@ #include "atrace_process_dump.h" +#include #include #include @@ -37,7 +38,7 @@ void AtraceProcessDump::SetDumpInterval(int interval_ms) { void AtraceProcessDump::RunAndPrintJson(FILE* stream) { out_ = stream; - fprintf(out_, "{\"start_ts\": \"%llu\", \"snapshots\":[\n", + fprintf(out_, "{\"start_ts\": \"%" PRIu64 "\", \"snapshots\":[\n", time_utils::GetTimestamp()); CHECK(snapshot_timer_); @@ -132,13 +133,14 @@ bool AtraceProcessDump::ShouldTakeFullDump(const ProcessInfo* process) { } void AtraceProcessDump::SerializeSnapshot() { - fprintf(out_, "{\"ts\":\"%llu\",\"memdump\":{\n", snapshot_timestamp_); + fprintf(out_, "{\"ts\":\"%" PRIu64 "\",\"memdump\":{\n", + snapshot_timestamp_); for (auto it = snapshot_.begin(); it != snapshot_.end();) { const ProcessSnapshot* process = it->second.get(); const ProcessMemoryStats* mem = &process->memory; fprintf(out_, "\"%d\":{", process->pid); - fprintf(out_, "\"vm\":%llu,\"rss\":%llu", + fprintf(out_, "\"vm\":%" PRIu64 ",\"rss\":%" PRIu64, mem->virt_kb(), mem->rss_kb()); fprintf(out_, ",\"oom_sc\":%d,\"oom_sc_adj\":%d" @@ -149,17 +151,18 @@ void AtraceProcessDump::SerializeSnapshot() { process->utime, process->stime); if (mem->full_stats_available()) { - fprintf(out_, ",\"pss\":%llu,\"swp\":%llu" - ",\"pc\":%llu,\"pd\":%llu,\"sc\":%llu,\"sd\":%llu", + fprintf(out_, ",\"pss\":%" PRIu64 ",\"swp\":%" PRIu64 + ",\"pc\":%" PRIu64 ",\"pd\":%" PRIu64 + ",\"sc\":%" PRIu64 ",\"sd\":%" PRIu64, mem->pss_kb(), mem->swapped_kb(), mem->private_clean_kb(), mem->private_dirty_kb(), mem->shared_clean_kb(), mem->shared_dirty_kb()); } if (mem->gpu_stats_available()) { - fprintf(out_, ",\"gpu_egl\":%llu,\"gpu_egl_pss\":%llu" - ",\"gpu_gl\":%llu,\"gpu_gl_pss\":%llu" - ",\"gpu_etc\":%llu,\"gpu_etc_pss\":%llu", + fprintf(out_, ",\"gpu_egl\":%" PRIu64 ",\"gpu_egl_pss\":%" PRIu64 + ",\"gpu_gl\":%" PRIu64 ",\"gpu_gl_pss\":%" PRIu64 + ",\"gpu_etc\":%" PRIu64 ",\"gpu_etc_pss\":%" PRIu64, mem->gpu_graphics_kb(), mem->gpu_graphics_pss_kb(), mem->gpu_gl_kb(), mem->gpu_gl_pss_kb(), mem->gpu_other_kb(), mem->gpu_other_pss_kb()); @@ -176,11 +179,13 @@ void AtraceProcessDump::SerializeSnapshot() { for (size_t k = 0; k < n_mmaps; ++k) { const ProcessMemoryStats::MmapInfo* mm = mem->mmap(k); fprintf(out_, - "{\"vm\":\"%llx-%llx\",\"file\":\"%s\",\"flags\":\"%s\"," - "\"pss\":%llu,\"rss\":%llu,\"swp\":%llu," - "\"pc\":%llu,\"pd\":%llu," - "\"sc\":%llu,\"sd\":%llu}", - mm->start_addr, mm->end_addr, mm->mapped_file, mm->prot_flags, + "{\"vm\":\"%" PRIx64 "-%" PRIx64 "\"," + "\"file\":\"%s\",\"flags\":\"%s\"," + "\"pss\":%" PRIu64 ",\"rss\":%" PRIu64 ",\"swp\":%" PRIu64 "," + "\"pc\":%" PRIu64 ",\"pd\":%" PRIu64 "," + "\"sc\":%" PRIu64 ",\"sd\":%" PRIu64 "}", + mm->start_addr, mm->end_addr, + mm->mapped_file, mm->prot_flags, mm->pss_kb, mm->rss_kb, mm->swapped_kb, mm->private_clean_kb, mm->private_dirty_kb, mm->shared_clean_kb, mm->shared_dirty_kb); @@ -233,11 +238,12 @@ void AtraceProcessDump::SerializePersistentProcessInfo() { void AtraceProcessDump::TakeAndSerializeMemInfo() { std::map mem_info; CHECK(procfs_utils::ReadMemInfoStats(&mem_info)); - fprintf(out_, "{\"ts\":\"%llu\",\"meminfo\":{\n", time_utils::GetTimestamp()); + fprintf(out_, "{\"ts\":\"%" PRIu64 "\",\"meminfo\":{\n", + time_utils::GetTimestamp()); for (auto it = mem_info.begin(); it != mem_info.end(); ++it) { if (it != mem_info.begin()) fprintf(out_, ","); - fprintf(out_, "\"%s\":%llu", it->first.c_str(), it->second); + fprintf(out_, "\"%s\":%" PRIu64, it->first.c_str(), it->second); } fprintf(out_, "}}"); } diff --git a/catapult/systrace/atrace_helper/jni/logging.h b/catapult/systrace/atrace_helper/jni/logging.h index c1320512..824fddd0 100644 --- a/catapult/systrace/atrace_helper/jni/logging.h +++ b/catapult/systrace/atrace_helper/jni/logging.h @@ -9,6 +9,7 @@ #include #include #include +#include #define CHECK_ARGS(COND, ERR) \ "FAILED CHECK(%s) @ %s:%d (errno: %s)\n", #COND, __FILE__, __LINE__, \ diff --git a/catapult/systrace/atrace_helper/jni/main.cc b/catapult/systrace/atrace_helper/jni/main.cc index 764b573a..68593b4c 100644 --- a/catapult/systrace/atrace_helper/jni/main.cc +++ b/catapult/systrace/atrace_helper/jni/main.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include #include #include @@ -45,7 +46,7 @@ int main(int argc, char** argv) { if (argc == 2 && !strcmp(argv[1], "--echo-ts")) { // Used by clock sync marker to correct the difference between // Linux monotonic clocks on the device and host. - printf("%llu\n", time_utils::GetTimestamp()); + printf("%" PRIu64 "\n", time_utils::GetTimestamp()); return 0; } diff --git a/catapult/systrace/atrace_helper/jni/process_memory_stats.cc b/catapult/systrace/atrace_helper/jni/process_memory_stats.cc index e13dc225..fbad23b9 100644 --- a/catapult/systrace/atrace_helper/jni/process_memory_stats.cc +++ b/catapult/systrace/atrace_helper/jni/process_memory_stats.cc @@ -4,6 +4,7 @@ #include "process_memory_stats.h" +#include #include #include @@ -73,7 +74,8 @@ bool ProcessMemoryStats::ReadFullStats(int pid) { // Note that the mapped file name ([stack]) is optional and won't be // present on anonymous memory maps (hence res >= 3 below). int res = sscanf(line, - "%llx-%llx %4s %*llx %*[:0-9a-f] %*[0-9a-f]%*[ \t]%127[^\n]", + "%" PRIx64 "-%" PRIx64 " %4s %*" PRIx64 " %*[:0-9a-f] " + "%*[0-9a-f]%*[ \t]%127[^\n]", &new_mmap->start_addr, &new_mmap->end_addr, new_mmap->prot_flags, new_mmap->mapped_file); last_mmap_entry = new_mmap.get(); diff --git a/catapult/systrace/atrace_helper/jni/time_utils.cc b/catapult/systrace/atrace_helper/jni/time_utils.cc index 2da7d793..3c8aa16e 100644 --- a/catapult/systrace/atrace_helper/jni/time_utils.cc +++ b/catapult/systrace/atrace_helper/jni/time_utils.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include "logging.h" diff --git a/catapult/systrace/profile_chrome/chrome_startup_tracing_agent.py b/catapult/systrace/profile_chrome/chrome_startup_tracing_agent.py index e389c153..c1456c11 100644 --- a/catapult/systrace/profile_chrome/chrome_startup_tracing_agent.py +++ b/catapult/systrace/profile_chrome/chrome_startup_tracing_agent.py @@ -5,9 +5,10 @@ import logging import optparse import os -import py_utils import re +import py_utils + from devil.android import flag_changer from devil.android.constants import webapk from devil.android.perf import cache_control diff --git a/catapult/systrace/profile_chrome/chrome_tracing_agent.py b/catapult/systrace/profile_chrome/chrome_tracing_agent.py index 1e8895bd..285d2996 100644 --- a/catapult/systrace/profile_chrome/chrome_tracing_agent.py +++ b/catapult/systrace/profile_chrome/chrome_tracing_agent.py @@ -5,9 +5,10 @@ import json import optparse import os -import py_utils import re +import py_utils + from devil.android import device_errors from devil.android.sdk import intent from systrace import trace_result @@ -209,6 +210,8 @@ def _ComputeChromeCategories(config): categories.append('disabled-by-default-blink.scheduler') categories.append('disabled-by-default-cc.debug.scheduler') categories.append('disabled-by-default-renderer.scheduler') + categories.append('disabled-by-default-sequence_manager') + categories.append('sequence_manager') if config.chrome_categories: categories += config.chrome_categories.split(',') return categories diff --git a/catapult/systrace/profile_chrome/chrome_tracing_agent_unittest.py b/catapult/systrace/profile_chrome/chrome_tracing_agent_unittest.py index 25c701ae..1556762b 100644 --- a/catapult/systrace/profile_chrome/chrome_tracing_agent_unittest.py +++ b/catapult/systrace/profile_chrome/chrome_tracing_agent_unittest.py @@ -17,7 +17,7 @@ class ChromeAgentTest(agents_unittest.BaseAgentTest): @decorators.Disabled def testGetCategories(self): curr_browser = self.GetChromeProcessID() - if curr_browser == None: + if curr_browser is None: self.StartBrowser() categories = \ @@ -35,7 +35,7 @@ class ChromeAgentTest(agents_unittest.BaseAgentTest): @decorators.Disabled def testTracing(self): curr_browser = self.GetChromeProcessID() - if curr_browser == None: + if curr_browser is None: self.StartBrowser() categories = '*' diff --git a/catapult/systrace/profile_chrome/ddms_tracing_agent.py b/catapult/systrace/profile_chrome/ddms_tracing_agent.py index 9d041b91..d9789391 100644 --- a/catapult/systrace/profile_chrome/ddms_tracing_agent.py +++ b/catapult/systrace/profile_chrome/ddms_tracing_agent.py @@ -4,9 +4,10 @@ import optparse import os -import py_utils import re +import py_utils + from profile_chrome import util from systrace import trace_result from systrace import tracing_agents diff --git a/catapult/systrace/profile_chrome/perf_tracing_agent.py b/catapult/systrace/profile_chrome/perf_tracing_agent.py index 65831dfd..c7ad9468 100644 --- a/catapult/systrace/profile_chrome/perf_tracing_agent.py +++ b/catapult/systrace/profile_chrome/perf_tracing_agent.py @@ -5,12 +5,13 @@ import logging import optparse import os -import py_utils import signal import subprocess import sys import tempfile +import py_utils + from devil.android import device_temp_file from devil.android.perf import perf_control @@ -22,9 +23,10 @@ _CATAPULT_DIR = os.path.join( os.path.dirname(os.path.abspath(__file__)), '..', '..') sys.path.append(os.path.join(_CATAPULT_DIR, 'telemetry')) try: - # pylint: disable=F0401 + # pylint: disable=F0401,no-name-in-module,wrong-import-position from telemetry.internal.platform.profiler import android_profiling_helper from telemetry.internal.util import binary_manager + # pylint: enable=wrong-import-position except ImportError: android_profiling_helper = None binary_manager = None diff --git a/catapult/systrace/systrace/__init__.py b/catapult/systrace/systrace/__init__.py index 831f7743..730c1642 100644 --- a/catapult/systrace/systrace/__init__.py +++ b/catapult/systrace/systrace/__init__.py @@ -19,7 +19,6 @@ def _AddDirToPythonPath(*path_parts): _CATAPULT_DIR = os.path.join( os.path.dirname(os.path.abspath(__file__)), os.path.pardir, os.path.pardir) -_AddDirToPythonPath(_CATAPULT_DIR, 'common', 'battor') _AddDirToPythonPath(_CATAPULT_DIR, 'common', 'py_utils') _AddDirToPythonPath(_CATAPULT_DIR, 'common', 'py_trace_event') _AddDirToPythonPath(_CATAPULT_DIR, 'common', 'py_trace_event', 'py_trace_event') diff --git a/catapult/systrace/systrace/output_generator.py b/catapult/systrace/systrace/output_generator.py index 6b63ac8e..9b2666e1 100644 --- a/catapult/systrace/systrace/output_generator.py +++ b/catapult/systrace/systrace/output_generator.py @@ -25,7 +25,6 @@ _SYSTRACE_TO_TRACE_DATA_NAME_MAPPING = { 'androidProcessDump': trace_data.ANDROID_PROCESS_DATA_PART, 'atraceProcessDump': trace_data.ATRACE_PROCESS_DUMP_PART, 'systemTraceEvents': trace_data.ATRACE_PART, - 'powerTraceAsString': trace_data.BATTOR_TRACE_PART, 'systraceController': trace_data.TELEMETRY_PART, 'traceEvents': trace_data.CHROME_TRACE_PART, 'waltTrace': trace_data.WALT_TRACE_PART, diff --git a/catapult/systrace/systrace/output_generator_unittest.py b/catapult/systrace/systrace/output_generator_unittest.py index 58e11ac8..dba47712 100644 --- a/catapult/systrace/systrace/output_generator_unittest.py +++ b/catapult/systrace/systrace/output_generator_unittest.py @@ -19,7 +19,7 @@ from tracing.trace_data import trace_data as trace_data_module TEST_DIR = os.path.join(os.path.dirname(__file__), 'test_data') ATRACE_DATA = os.path.join(TEST_DIR, 'atrace_data') -BATTOR_DATA = os.path.join(TEST_DIR, 'battor_test_data.txt') +ATRACE_PROCESS_DUMP_DATA = os.path.join(TEST_DIR, 'atrace_procfs_dump') COMBINED_PROFILE_CHROME_DATA = os.path.join( TEST_DIR, 'profile-chrome_systrace_perf_chrome_data') @@ -76,19 +76,18 @@ class OutputGeneratorTest(unittest.TestCase): trace_results = [] trace_data_builder = trace_data_module.TraceDataBuilder() - with open(BATTOR_DATA) as fp: - battor_data = fp.read() - trace_results.append( - trace_result.TraceResult('powerTraceAsString', battor_data)) - trace_data_builder.AddTraceFor( - trace_data_module.BATTOR_TRACE_PART, battor_data) - with open(ATRACE_DATA) as fp: atrace_data = fp.read() trace_results.append( trace_result.TraceResult('systemTraceEvents', atrace_data)) trace_data_builder.AddTraceFor(trace_data_module.ATRACE_PART, atrace_data) + with open(ATRACE_PROCESS_DUMP_DATA) as fp: + atrace_process_dump_data = fp.read() + trace_results.append( + trace_result.TraceResult('atraceProcessDump', atrace_process_dump_data)) + trace_data_builder.AddTraceFor(trace_data_module.ATRACE_PROCESS_DUMP_PART, + atrace_process_dump_data) with open(COMBINED_PROFILE_CHROME_DATA) as fp: chrome_data = fp.read() diff --git a/catapult/systrace/systrace/run_systrace.py b/catapult/systrace/systrace/run_systrace.py index cb320366..2ca85046 100755 --- a/catapult/systrace/systrace/run_systrace.py +++ b/catapult/systrace/systrace/run_systrace.py @@ -14,6 +14,7 @@ the kernel. It creates an HTML file for visualizing the trace. # The flags= parameter of re.sub() is new in Python 2.7. And Systrace does not # support Python 3 yet. +# pylint: disable=wrong-import-position import sys version = sys.version_info[:2] @@ -44,13 +45,13 @@ from systrace import util from systrace.tracing_agents import atrace_agent from systrace.tracing_agents import atrace_from_file_agent from systrace.tracing_agents import atrace_process_dump -from systrace.tracing_agents import battor_trace_agent from systrace.tracing_agents import ftrace_agent from systrace.tracing_agents import walt_agent +# pylint: enable=wrong-import-position ALL_MODULES = [atrace_agent, atrace_from_file_agent, atrace_process_dump, - battor_trace_agent, ftrace_agent, walt_agent] + ftrace_agent, walt_agent] def parse_options(argv): @@ -158,13 +159,16 @@ def main_impl(arguments): if options.target == 'android' and not options.from_file: initialize_devil() + devices = [a.GetDeviceSerial() for a in adb_wrapper.AdbWrapper.Devices()] if not options.device_serial_number: - devices = [a.GetDeviceSerial() for a in adb_wrapper.AdbWrapper.Devices()] if len(devices) == 0: raise RuntimeError('No ADB devices connected.') elif len(devices) >= 2: raise RuntimeError('Multiple devices connected, serial number required') options.device_serial_number = devices[0] + elif options.device_serial_number not in devices: + raise RuntimeError('Device with the serial number "%s" is not connected.' + % options.device_serial_number) # If list_categories is selected, just print the list of categories. # In this case, use of the tracing controller is not necessary. diff --git a/catapult/systrace/systrace/systrace_runner.py b/catapult/systrace/systrace/systrace_runner.py index b3d93247..514f5bb0 100644 --- a/catapult/systrace/systrace/systrace_runner.py +++ b/catapult/systrace/systrace/systrace_runner.py @@ -14,13 +14,12 @@ from systrace.tracing_agents import android_process_data_agent from systrace.tracing_agents import atrace_agent from systrace.tracing_agents import atrace_from_file_agent from systrace.tracing_agents import atrace_process_dump -from systrace.tracing_agents import battor_trace_agent from systrace.tracing_agents import ftrace_agent from systrace.tracing_agents import walt_agent AGENT_MODULES = [android_process_data_agent, atrace_agent, atrace_from_file_agent, atrace_process_dump, - battor_trace_agent, ftrace_agent, walt_agent] + ftrace_agent, walt_agent] class SystraceRunner(object): def __init__(self, script_dir, options): diff --git a/catapult/systrace/systrace/systrace_trace_viewer.html b/catapult/systrace/systrace/systrace_trace_viewer.html index 1b7fb390..29f8d59a 100644 --- a/catapult/systrace/systrace/systrace_trace_viewer.html +++ b/catapult/systrace/systrace/systrace_trace_viewer.html @@ -614,23 +614,21 @@
{{name}} -