New issue
Advanced search Search tips

Issue 921672 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

_ReenableChargingIfNeeded causes unnecessary retries for ADB + android + telemetry_perf_unittests.

Project Member Reported by erikc...@chromium.org, Jan 14

Issue description

CL that is unrelated to android, ADB or telemetry_perf_unittests:
https://chromium-review.googlesource.com/c/chromium/src/+/1382912/3

android-marshmallow build from CQ:
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-marshmallow-arm64-rel/167407

telemetry_perf_unittests shard#2 has a time out failure in with_patch. Task: https://chromium-swarm.appspot.com/task?id=4265eaf4cca00c10&refresh=10&show_raw=1

Relevant lines from output:
"""
13 tests passed in 829.8s, 9 skipped, 0 failures.
...
(CRITICAL) 2019-01-14 16:18:53,359 pid=300  timeout_retry.Run:173  (TimeoutThread-1-for-Dummy-13) Exception on SetCharging(<devil.android.battery_utils.BatteryUtils object at 0x7fab76007a90>, True, retries=3, timeout=30), attempt 1 of 4: NoAdbError('[Errno 2] No such file or directory',)
(WARNING) 2019-01-14 16:18:53,363 pid=300  device_temp_file.delete_temporary_file:66  Failed to delete temporary file /data/local/tmp/temp_file-f22ed67836746: [Errno 2] No such file or directory
(CRITICAL) 2019-01-14 16:18:53,365 pid=300  timeout_retry.Run:173  (TimeoutThread-2-for-Dummy-13) Exception on SetCharging(<devil.android.battery_utils.BatteryUtils object at 0x7fab76007a90>, True, retries=3, timeout=30), attempt 2 of 4: NoAdbError('[Errno 2] No such file or directory',)
(CRITICAL) 2019-01-14 16:18:53,370 pid=300  timeout_retry.Run:173  (TimeoutThread-3-for-Dummy-13) Exception on SetCharging(<devil.android.battery_utils.BatteryUtils object at 0x7fab76007a90>, True, retries=3, timeout=30), attempt 3 of 4: NoAdbError('[Errno 2] No such file or directory',)
(ERROR) 2019-01-14 16:18:53,374 pid=300  atexit_with_log._WrappedFn:16  Exception running <function _ReenableChargingIfNeeded at 0x7fab6a1fa410>
Traceback (most recent call last):
  File "/b/swarming/w/ir/third_party/catapult/common/py_utils/py_utils/atexit_with_log.py", line 13, in _WrappedFn
  File "/b/swarming/w/ir/third_party/catapult/telemetry/telemetry/internal/platform/power_monitor/android_power_monitor_controller.py", line 13, in _ReenableChargingIfNeeded
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/android/decorators.py", line 57, in timeout_retry_wrapper
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/utils/timeout_retry.py", line 158, in Run
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/utils/reraiser_thread.py", line 198, in JoinAll
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/utils/reraiser_thread.py", line 170, in _JoinAll
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/utils/reraiser_thread.py", line 93, in run
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/utils/timeout_retry.py", line 151, in <lambda>
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/android/decorators.py", line 47, in impl
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/android/battery_utils.py", line 577, in SetCharging
  File "/b/swarming/w/ir/third_party/catapult/devil/devil/android/decorators.py", line 51, in timeout_retry_wrapper
"""

There's an ADB error/python exception that happens during teardown for code that appears unrelated to the actual tests themselves. 

The test still emits the correct output.json:
https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=b1eb365e81964e780bc92628e1c8e648b8c008db&as=output.json

But the recipe sees the failure and retries all the tests in 'without patch' retries. The recipe then notices that there are no failures in 'with patch' that were not in 'without patch', and marks the build as a test.

There are some steps [e.g. memory_desktop/load:news:reddit] that emit EXPECTED:PASS, actual:SKIP. But on further investigation, the test looks like it's intentionally skipped. In this case, the output.json should be emitting EXPECTED:SKIP:
https://cs.chromium.org/chromium/src/tools/perf/benchmarks/system_health_smoke_test.py?type=cs&q=memory_desktop/load:news:reddit+-file:json$&sq=package:chromium&g=0&l=56

There are three problems that we should fix:
1) Fix devil to not throw a python exception during teardown if ADB fails on a step that is not relevant.
2) Fix catapult to emit EXPECTED:SKIP for tests that are intentionally skipped
3) Fix recipes to not run 'without patch' retries if all tests succeeded, even if a shard itself reports an error.
 

Sign in to add a comment