Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2419023003:
[Android] Check list length before printing forwarder information. (Closed)
|
Created:
4 years, 2 months ago by rnephew (Reviews Here) Modified:
4 years, 2 months ago CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org Target Ref:
refs/heads/master Project:
catapult Visibility:
Public. |
Description[Android] Check list length before printing forwarder information.
BUG=chromium:655653
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/baf2eab473e1b1ddcefd5b601a6e66e2dde8cbc4
Patch Set 1 #
Total comments: 2
Patch Set 2 : [Android] Check line length before printing forwarder information. #Messages
Total messages: 19 (7 generated)
rnephew@chromium.org changed reviewers: + mikecase@chromium.org
Description was changed from ========== [Android] Check list length before printing forwarder information. BUG=chromium:655662 ========== to ========== [Android] Check list length before printing forwarder information. BUG=chromium:655653 ==========
Any idea what hte output looks like when this fails? Is this bad that we aren't calling ForwardRemove here? https://codereview.chromium.org/2419023003/diff/1/telemetry/telemetry/interna... File telemetry/telemetry/internal/platform/android_platform_backend.py (right): https://codereview.chromium.org/2419023003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/platform/android_platform_backend.py:493: 'but outcome was unexpected:\n%s' % line) nit: I would reword this error message to something like... 'ADB forward list output unexpected. Expected format "<device id> tcp:<host port>" but got %s' % line Up to you though
On 2016/10/14 16:58:13, mikecase wrote: > Any idea what hte output looks like when this fails? Is this bad that we aren't > calling ForwardRemove here? > > https://codereview.chromium.org/2419023003/diff/1/telemetry/telemetry/interna... > File telemetry/telemetry/internal/platform/android_platform_backend.py (right): > > https://codereview.chromium.org/2419023003/diff/1/telemetry/telemetry/interna... > telemetry/telemetry/internal/platform/android_platform_backend.py:493: 'but > outcome was unexpected:\n%s' % line) > nit: I would reword this error message to something like... > > 'ADB forward list output unexpected. Expected format "<device id> tcp:<host > port>" but got %s' % line > > Up to you though I dont know what the output looks like unfortunatly. I dont think its bad that we dont call it since the output is not as expected. If it is bad, I assume/hope the logging will show that.
https://codereview.chromium.org/2419023003/diff/1/telemetry/telemetry/interna... File telemetry/telemetry/internal/platform/android_platform_backend.py (right): https://codereview.chromium.org/2419023003/diff/1/telemetry/telemetry/interna... telemetry/telemetry/internal/platform/android_platform_backend.py:493: 'but outcome was unexpected:\n%s' % line) On 2016/10/14 16:58:13, mikecase wrote: > nit: I would reword this error message to something like... > > 'ADB forward list output unexpected. Expected format "<device id> tcp:<host > port>" but got %s' % line > > Up to you though Done.
lgtm
The CQ bit was checked by rnephew@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Catapult Presubmit on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Pr...)
rnephew@chromium.org changed reviewers: + nednguyen@google.com, zhenw@chromium.org
The CQ bit was checked by nednguyen@google.com
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Android] Check list length before printing forwarder information. BUG=chromium:655653 ========== to ========== [Android] Check list length before printing forwarder information. BUG=chromium:655653 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2425473002/ by nednguyen@google.com. The reason for reverting is: Suspect causing massive Android failure BUG=chromium:656224. |
