Add --render-results-dir arg to store render results locally.

Add --render-results-dir arg to store render results locally.

If --render-results-dir is specified, the render result images
will get pulled from the device and stored there locally.

BUG=722779
Review-Url: https://codereview.chromium.org/2889663003
Cr-Commit-Position: refs/heads/master@{#472670}
Committed: https://chromium.googlesource.com/chromium/src/+/39e3f9e3bdd92088ebf9c48bfd9395ebe38d7801

[mikecase (-- gone --), 2017-05-16 19:08:39]

mikecase@chromium.org changed reviewers:
+ jbudorick@chromium.org, peconn@chromium.org

[jbudorick, 2017-05-16 19:59:58]

Going to talk to you about this offline later today.

[PEConn, 2017-05-17 08:34:34]

Thanks for acting on the bug so quickly!

https://codereview.chromium.org/2889663003/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2889663003/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:557:
if self._test_instance.gs_results_bucket:
Can you invert this if statement to reduce nesting?

if not self._test_instance.gs_results_bucket:
  return

for failure_filename in os.listdir(
...

[mikecase (-- gone --), 2017-05-17 21:10:29]

@ john, would like to commit this and then make the changes to the test_runner
output files we talked about later.

https://codereview.chromium.org/2889663003/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2889663003/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:557:
if self._test_instance.gs_results_bucket:
On 2017/05/17 at 08:34:34, PEConn wrote:
> Can you invert this if statement to reduce nesting?
> 
> if not self._test_instance.gs_results_bucket:
>   return
> 
> for failure_filename in os.listdir(
> ...

Done

[jbudorick, 2017-05-17 21:26:45]

lgtm w/ nits

I'm ok with not blocking this on reworking test runner output.

https://codereview.chromium.org/2889663003/diff/20001/build/android/pylib/loc...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2889663003/diff/20001/build/android/pylib/loc...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:536:
self._test_instance.render_results_dir is None) as render_host_dir:
nit: Can you name the context manager something other than render_host_dir?
Reusing it immediately below is a bit awkward.

Also, not bool(self._test_instance.render_results_dir)?

https://codereview.chromium.org/2889663003/diff/20001/build/android/pylib/loc...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:556: #
Upload results to Google Storage.
Since we no longer always do this, I think it'd be more clear to have it in its
own function.

[mikecase (-- gone --), 2017-05-17 21:46:05]

https://codereview.chromium.org/2889663003/diff/20001/build/android/pylib/loc...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2889663003/diff/20001/build/android/pylib/loc...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:536:
self._test_instance.render_results_dir is None) as render_host_dir:
On 2017/05/17 at 21:26:45, jbudorick wrote:
> nit: Can you name the context manager something other than render_host_dir?
Reusing it immediately below is a bit awkward.
> 
> Also, not bool(self._test_instance.render_results_dir)?

Done

https://codereview.chromium.org/2889663003/diff/20001/build/android/pylib/loc...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:556: #
Upload results to Google Storage.
On 2017/05/17 at 21:26:45, jbudorick wrote:
> Since we no longer always do this, I think it'd be more clear to have it in
its own function.

Done, added _UploadRenderTestResults function.

[mikecase (-- gone --), 2017-05-17 21:46:36]

The CQ bit was checked by mikecase@chromium.org

[mikecase (-- gone --), 2017-05-17 21:46:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/2889663003/#ps60001
(title: "Add --render-results-dir arg to store render results locally.")

[commit-bot: I haz the power, 2017-05-17 21:48:09]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 04:57:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495057596847560,
"parent_rev": "518dccbd3d3c5f9d1655d1b516f2682d653fcbbb", "commit_rev":
"39e3f9e3bdd92088ebf9c48bfd9395ebe38d7801"}

[commit-bot: I haz the power, 2017-05-18 04:57:59]

Description was changed from

==========
Add --render-results-dir arg to store render results locally.

If --render-results-dir is specified, the render result images
will get pulled from the device and stored there locally.

BUG=722779
==========

to

==========
Add --render-results-dir arg to store render results locally.

If --render-results-dir is specified, the render result images
will get pulled from the device and stored there locally.

BUG=722779

Review-Url: https://codereview.chromium.org/2889663003
Cr-Commit-Position: refs/heads/master@{#472670}
Committed:
https://chromium.googlesource.com/chromium/src/+/39e3f9e3bdd92088ebf9c48bfd93...
==========

[commit-bot: I haz the power, 2017-05-18 04:58:00]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/39e3f9e3bdd92088ebf9c48bfd93...