Clean up logging and add device randomization.

Clean up logging and add device randomization.

If no device_name is picked and os_version is, will pick random device with
given os; and vice versa. If neither is picked, both are randomly picked.

Clean up logging. Disable appurify/requests logging if verbosity is
less than 2.

BUG=428729
Committed: https://crrev.com/b74dd0630e0b7b24bc83eb27c9945e1c819c3550
Cr-Commit-Position: refs/heads/master@{#310337}

[rnephew (Reviews Here), 2014-12-19 22:30:34]

rnephew@chromium.org changed reviewers:
+ jbudorick@chromium.org, klundberg@chromium.org

[rnephew (Reviews Here), 2014-12-19 22:30:35]

I know the disabling of the logging might look a little hacky, but I think its
worth it for how much cleaner the logs are with it.

With disabling:

I    0.000s Main  Generating remote service access token
I    4.080s Main  Finding device to run tests on.
I    9.966s Main  Found device: Evita 4.0.4
W    9.966s Main  No isolate file provided. No data deps will be pushed.
I    9.966s Main  Triggering test run.
I    9.966s Main  Upload
/usr/local/google/home/rnephew/chromium/m38/src/out/Debug/events_unittests_apk/events_unittests-debug.apk
to remote service.
I   15.967s Main  Using default runner type: robotium
I   15.967s Main  Using default runner package: robotium
I   15.967s Main  Uploading
/usr/local/google/home/rnephew/chromium/m38/src/out/Debug/events_unittests_apk/events_unittests-debug.apk
to remote service.
I   22.082s Main  Generating config file for test.
I   37.080s Main  Test run id: 5f8e2a5b2dbc46d6983e91d7966ed3f5
I   43.079s Main  Revoking remote service access token


Without disabling:
I    0.000s Main  Generating remote service access token
W    0.003s Main  HTTP POST
http://172.22.21.180:80/resource/access_token/generate/
I    0.008s Main  Starting new HTTP connection (1): 172.22.21.180
I    3.310s Main  Finding device to run tests on.
W    3.310s Main  HTTP GET http://172.22.21.180:80/resource/devices/list/
I    3.311s Main  Starting new HTTP connection (1): 172.22.21.180
I   10.368s Main  Found device: Galaxy S4 4.4.2
W   10.368s Main  No isolate file provided. No data deps will be pushed.
I   10.369s Main  Triggering test run.
I   10.369s Main  Upload
/usr/local/google/home/rnephew/chromium/m38/src/out/Debug/events_unittests_apk/events_unittests-debug.apk
to remote service.
W   10.369s Main  HTTP POST http://172.22.21.180:80/resource/apps/upload/
I   10.382s Main  Starting new HTTP connection (1): 172.22.21.180
I   15.307s Main  Using default runner type: robotium
I   15.308s Main  Using default runner package: robotium
I   15.308s Main  Uploading
/usr/local/google/home/rnephew/chromium/m38/src/out/Debug/events_unittests_apk/events_unittests-debug.apk
to remote service.
W   15.308s Main  HTTP POST http://172.22.21.180:80/resource/tests/upload/
I   15.313s Main  Starting new HTTP connection (1): 172.22.21.180
I   19.373s Main  Generating config file for test.
W   19.375s Main  HTTP POST
http://172.22.21.180:80/resource/tests/config/upload/
I   19.379s Main  Starting new HTTP connection (1): 172.22.21.180
W   24.305s Main  HTTP POST http://172.22.21.180:80/resource/tests/run/
I   24.307s Main  Starting new HTTP connection (1): 172.22.21.180
I   34.367s Main  Test run id: 2a5e383d006f463dbbc266681af713de
W   34.367s Main  HTTP GET http://172.22.21.180:80/resource/tests/check/
I   34.368s Main  Starting new HTTP connection (1): 172.22.21.180
I   40.367s Main  Revoking remote service access token
W   40.367s Main  HTTP POST
http://172.22.21.180:80/resource/access_token/revoke/
I   40.369s Main  Starting new HTTP connection (1): 172.22.21.180

[jbudorick, 2015-01-05 16:52:30]

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_environment.py:102:
remote_device_helper.SetLogging(self._verbose_count, logging.WARNING)
Could we have remote_device_helper provide this functionality as a context
manager? That way, we'd remove the override even if appurify raises an
exception.

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_environment.py:134: if
device['name'] != self._remote_device:
Can't this just be

  if self._remote_device and device['name'] != self._remote_device:

?

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_environment.py:137: if
device['os_version'] != self._remote_device_os:
(similar)

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
File build/android/pylib/remote/device/remote_device_helper.py (right):

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_helper.py:27:
logging.disable(level)
I'm concerned with how this interacts with threading. logging.disable appears to
apply globally -- what if another thread starts running while waiting for a
response from appurify?

[rnephew (Wrong account), 2015-01-05 19:31:53]

rnephew@google.com changed reviewers:
+ rnephew@google.com

[rnephew (Wrong account), 2015-01-05 19:31:54]

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_environment.py:102:
remote_device_helper.SetLogging(self._verbose_count, logging.WARNING)
On 2015/01/05 16:52:29, jbudorick wrote:
> Could we have remote_device_helper provide this functionality as a context
> manager? That way, we'd remove the override even if appurify raises an
> exception.

Done, except I moved it to appurify_sanitize because it seems like a better
place for it.

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_environment.py:134: if
device['name'] != self._remote_device:
On 2015/01/05 16:52:29, jbudorick wrote:
> Can't this just be
> 
>   if self._remote_device and device['name'] != self._remote_device:
> 
> ?

Done.

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_environment.py:137: if
device['os_version'] != self._remote_device_os:
On 2015/01/05 16:52:29, jbudorick wrote:
> (similar)

Done.

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
File build/android/pylib/remote/device/remote_device_helper.py (right):

https://codereview.chromium.org/814653003/diff/1/build/android/pylib/remote/d...
build/android/pylib/remote/device/remote_device_helper.py:27:
logging.disable(level)
On 2015/01/05 16:52:29, jbudorick wrote:
> I'm concerned with how this interacts with threading. logging.disable appears
to
> apply globally -- what if another thread starts running while waiting for a
> response from appurify?

Added warning comment for now. A better solution will need to be investigated if
threading is ever supported with appurify.

[jbudorick, 2015-01-06 18:02:17]

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/appurify_sanitized.py (right):

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/appurify_sanitized.py:34: try:
do you need the try/finally in here? can this not just be

  if verbose_count < 2:
    logging.disable(level)
    yield
    logging.disable(logging.NOTSET)

?

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:104:
access_token_results = appurify_sanitized.api.access_token_generate(
Are we wrapping all of our calls to the appurify api with SanitizeLogging? I
wonder if there's a good way to do that automagically in appurify_sanitized...

[rnephew (Wrong account), 2015-01-06 18:15:46]

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/appurify_sanitized.py (right):

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/appurify_sanitized.py:34: try:
On 2015/01/06 18:02:17, jbudorick wrote:
> do you need the try/finally in here? can this not just be
> 
>   if verbose_count < 2:
>     logging.disable(level)
>     yield
>     logging.disable(logging.NOTSET)
> 
> ?

Done.

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:104:
access_token_results = appurify_sanitized.api.access_token_generate(
On 2015/01/06 18:02:17, jbudorick wrote:
> Are we wrapping all of our calls to the appurify api with SanitizeLogging? I
> wonder if there's a good way to do that automagically in appurify_sanitized...

I've been thinking / looking into this and will switch to a way to do this as
soon as I find a way to do it.

[jbudorick, 2015-01-06 23:47:43]

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:104:
access_token_results = appurify_sanitized.api.access_token_generate(
On 2015/01/06 18:15:45, rnephew wrote:
> On 2015/01/06 18:02:17, jbudorick wrote:
> > Are we wrapping all of our calls to the appurify api with SanitizeLogging? I
> > wonder if there's a good way to do that automagically in
appurify_sanitized...
> 
> I've been thinking / looking into this and will switch to a way to do this as
> soon as I find a way to do it.

Do you intend to do this in this CL or in a subsequent one?

[rnephew (Wrong account), 2015-01-06 23:48:47]

On 2015/01/06 23:47:43, jbudorick wrote:
>
https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
> File build/android/pylib/remote/device/remote_device_environment.py (right):
> 
>
https://codereview.chromium.org/814653003/diff/40001/build/android/pylib/remo...
> build/android/pylib/remote/device/remote_device_environment.py:104:
> access_token_results = appurify_sanitized.api.access_token_generate(
> On 2015/01/06 18:15:45, rnephew wrote:
> > On 2015/01/06 18:02:17, jbudorick wrote:
> > > Are we wrapping all of our calls to the appurify api with SanitizeLogging?
I
> > > wonder if there's a good way to do that automagically in
> appurify_sanitized...
> > 
> > I've been thinking / looking into this and will switch to a way to do this
as
> > soon as I find a way to do it.
> 
> Do you intend to do this in this CL or in a subsequent one?

A subsequent one unless I can figure out a good way of doing it before this gets
LGTM'd.

[jbudorick, 2015-01-07 18:02:37]

Style nits

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
File build/android/pylib/remote/device/appurify_sanitized.py (right):

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/appurify_sanitized.py:34: if verbose_count <
2:
nit: this whole block is indented too deep

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:139:
logging.info('Found device: %s %s' %
the linter should catch this... I need to check on why it isn't

anyhow, this should be

  logging.info('Found device: %s %s', device['name'], device['os_version'])

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_test_run.py (right):

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_test_run.py:152:
logging.info('Test status: %s' % self._results['detailed_status'])
Same w.r.t. lint error: this should be

  logging.info('Test status: %s', self._results['detailed_status'])

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_test_run.py:186:
logging.info('Uploading %s to remote service.' % app_path)
same

[rnephew (Wrong account), 2015-01-07 18:11:25]

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
File build/android/pylib/remote/device/appurify_sanitized.py (right):

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/appurify_sanitized.py:34: if verbose_count <
2:
On 2015/01/07 18:02:37, jbudorick wrote:
> nit: this whole block is indented too deep

Done.

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_environment.py (right):

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_environment.py:139:
logging.info('Found device: %s %s' %
On 2015/01/07 18:02:37, jbudorick wrote:
> the linter should catch this... I need to check on why it isn't
> 
> anyhow, this should be
> 
>   logging.info('Found device: %s %s', device['name'], device['os_version'])

Done.

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
File build/android/pylib/remote/device/remote_device_test_run.py (right):

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_test_run.py:152:
logging.info('Test status: %s' % self._results['detailed_status'])
On 2015/01/07 18:02:37, jbudorick wrote:
> Same w.r.t. lint error: this should be
> 
>   logging.info('Test status: %s', self._results['detailed_status'])

Done.

https://codereview.chromium.org/814653003/diff/80001/build/android/pylib/remo...
build/android/pylib/remote/device/remote_device_test_run.py:186:
logging.info('Uploading %s to remote service.' % app_path)
On 2015/01/07 18:02:37, jbudorick wrote:
> same

Done.

[jbudorick, 2015-01-07 18:15:30]

lgtm w/ one last nit

https://codereview.chromium.org/814653003/diff/100001/build/android/pylib/rem...
File build/android/pylib/remote/device/remote_device_helper.py (right):

https://codereview.chromium.org/814653003/diff/100001/build/android/pylib/rem...
build/android/pylib/remote/device/remote_device_helper.py:7: import logging
nit: remove this, it's no longer necessary

[rnephew (Wrong account), 2015-01-07 18:18:42]

https://codereview.chromium.org/814653003/diff/100001/build/android/pylib/rem...
File build/android/pylib/remote/device/remote_device_helper.py (right):

https://codereview.chromium.org/814653003/diff/100001/build/android/pylib/rem...
build/android/pylib/remote/device/remote_device_helper.py:7: import logging
On 2015/01/07 18:15:30, jbudorick wrote:
> nit: remove this, it's no longer necessary

Done.

[rnephew (Wrong account), 2015-01-07 18:18:47]

The CQ bit was checked by rnephew@google.com

[commit-bot: I haz the power, 2015-01-07 18:20:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/814653003/120001

[commit-bot: I haz the power, 2015-01-07 19:35:27]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-07 19:36:24]

Patchset 7 (id:??) landed as
https://crrev.com/b74dd0630e0b7b24bc83eb27c9945e1c819c3550
Cr-Commit-Position: refs/heads/master@{#310337}