Expose AppUi from devil utils in browser

Expose AppUi from devil utils in browser

BUG=chromium:708300
Review-Url: https://codereview.chromium.org/2784233006
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/d1ce047ac34c5268a4b6bc61c17ac30ce5431f5f

[ssid, 2017-03-31 18:26:16]

Patchset #1 (id:1) has been deleted

[ssid, 2017-03-31 18:26:41]

Description was changed from

==========
Add api to expose browser info in Browser and device util in platform

BUG=catapult:#
==========

to

==========
[telemetry] Add api to expose browser info in Browser

BUG=catapult:#
==========

[ssid, 2017-04-04 21:33:40]

Description was changed from

==========
[telemetry] Add api to expose browser info in Browser

BUG=catapult:#
==========

to

==========
Expose AppUi from devil utils in browser

BUG=chromium:708300
==========

[ssid, 2017-04-04 21:33:48]

Patchset #1 (id:20001) has been deleted

[ssid, 2017-04-04 21:34:49]

ssid@chromium.org changed reviewers:
+ nednguyen@google.com, perezju@chromium.org

[ssid, 2017-04-04 21:34:50]

+Juan, Ned. ptal thanks!

[perezju, 2017-04-05 08:46:44]

lgtm w/comment

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
File telemetry/telemetry/internal/backends/chrome/android_browser_backend.py
(right):

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
telemetry/telemetry/internal/backends/chrome/android_browser_backend.py:76:
return app_ui.AppUi(self.device, package=self.package)
Can you cache the value after its first use as in:
https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/...

Also make it a regular method (not property) here.

[nednguyen, 2017-04-06 16:57:29]

Can you add test coverage in browser_unittests.py?

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
File telemetry/telemetry/internal/browser/browser.py (right):

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
telemetry/telemetry/internal/browser/browser.py:310: """True if the browser
backend supports app UI interactions."""
"""True if the browser supports Android app UI interactions."""

[ssid, 2017-04-13 19:27:37]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 19:27:47]

Dry run: 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-04-13 19:51:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 19:51:47]

Dry run: This issue passed the CQ dry run.

[ssid, 2017-04-13 20:07:39]

made changes, ptal, thanks!

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
File telemetry/telemetry/internal/backends/chrome/android_browser_backend.py
(right):

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
telemetry/telemetry/internal/backends/chrome/android_browser_backend.py:76:
return app_ui.AppUi(self.device, package=self.package)
On 2017/04/05 08:46:44, perezju wrote:
> Can you cache the value after its first use as in:
>
https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/...
> 
> Also make it a regular method (not property) here. 

Done.

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
File telemetry/telemetry/internal/browser/browser.py (right):

https://codereview.chromium.org/2784233006/diff/40001/telemetry/telemetry/int...
telemetry/telemetry/internal/browser/browser.py:310: """True if the browser
backend supports app UI interactions."""
On 2017/04/06 16:57:29, nednguyen wrote:
> """True if the browser supports Android app UI interactions."""

Done.

[nednguyen, 2017-04-13 20:22:03]

lgtm

https://codereview.chromium.org/2784233006/diff/60001/telemetry/telemetry/int...
File telemetry/telemetry/internal/browser/browser_unittest.py (right):

https://codereview.chromium.org/2784233006/diff/60001/telemetry/telemetry/int...
telemetry/telemetry/internal/browser/browser_unittest.py:147:
self.assertTrue(isinstance(ui, app_ui.AppUi))
can you make the test more meaningful. Maybe asserting that a specific browser
ui node exists?

[ssid, 2017-04-13 21:01:19]

thanks, fixed

https://codereview.chromium.org/2784233006/diff/60001/telemetry/telemetry/int...
File telemetry/telemetry/internal/browser/browser_unittest.py (right):

https://codereview.chromium.org/2784233006/diff/60001/telemetry/telemetry/int...
telemetry/telemetry/internal/browser/browser_unittest.py:147:
self.assertTrue(isinstance(ui, app_ui.AppUi))
On 2017/04/13 20:22:03, nednguyen wrote:
> can you make the test more meaningful. Maybe asserting that a specific browser
> ui node exists?

Done.

[ssid, 2017-04-13 21:01:24]

The CQ bit was checked by ssid@chromium.org

[ssid, 2017-04-13 21:01:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
perezju@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2784233006/#ps80001
(title: "Add getUiNode in test")

[commit-bot: I haz the power, 2017-04-13 21:01:30]

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-04-13 21:25:20]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492117284044490,
"parent_rev": "30a617a768a1f53d88ebf9b4a55920d72f2e4570", "commit_rev":
"d1ce047ac34c5268a4b6bc61c17ac30ce5431f5f"}

[commit-bot: I haz the power, 2017-04-13 21:25:25]

Description was changed from

==========
Expose AppUi from devil utils in browser

BUG=chromium:708300
==========

to

==========
Expose AppUi from devil utils in browser

BUG=chromium:708300

Review-Url: https://codereview.chromium.org/2784233006
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-04-13 21:25:26]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...