Update chromium.android.json

Update chromium.android.json

BUG=637213
NOTRY=True
TBR=jbudorick@chromium.org

Committed: https://crrev.com/aa986de5b33958a7f4fbf27c5d1a8a5c50ac4c0f
Cr-Commit-Position: refs/heads/master@{#411749}

[nicholaslin, 2016-08-12 18:06:38]

nicholaslin@google.com changed reviewers:
+ stip@chromium.org

[nicholaslin, 2016-08-12 18:06:38]

ptal

[ghost stip (do not use), 2016-08-12 18:08:48]

lgtm, please test on larng

[ghost stip (do not use), 2016-08-12 19:13:34]

new upload lgtm, please test before landing

[dnj, 2016-08-12 19:29:01]

dnj@chromium.org changed reviewers:
+ dnj@chromium.org

[dnj, 2016-08-12 19:29:02]

https://codereview.chromium.org/2240923002/diff/40001/build/android/test_wrap...
File build/android/test_wrapper/logdog_wrapper.py (right):

https://codereview.chromium.org/2240923002/diff/40001/build/android/test_wrap...
build/android/test_wrapper/logdog_wrapper.py:60: if
os.path.exists(args.logdog_bin_cmd):
Does this mean that if binary doesn't exist, nothing will be uploaded? Shouldn't
that be an error?

[ghost stip (do not use), 2016-08-12 19:33:18]

It'll be available in isolateserver.

[dnj, 2016-08-12 19:34:58]

https://codereview.chromium.org/2240923002/diff/40001/build/android/test_wrap...
File build/android/test_wrapper/logdog_wrapper.py (right):

https://codereview.chromium.org/2240923002/diff/40001/build/android/test_wrap...
build/android/test_wrapper/logdog_wrapper.py:60: if
os.path.exists(args.logdog_bin_cmd):
On 2016/08/12 19:29:02, dnj wrote:
> Does this mean that if binary doesn't exist, nothing will be uploaded?
Shouldn't
> that be an error?

I guess what I'm saying is:
1) We're explicitly making a point of loading this binary.
2) We hedge against that loading not actually happening.

It seems that (2) contradicts the expected success of (1). If we make a point of
having the binary on the system, we should expect that it will be there, and
fail if it's not.

[ghost stip (do not use), 2016-08-12 19:38:31]

The binary is only there if we tell it to. Two cases where that isn't true:

1) a test which hasn't enabled it yet
2) developer's machines

I think this is a good check to have. Perhaps we can put some logging there in
the future saying it's turned off but while in this transitional phase we need
this.

[dnj, 2016-08-12 19:49:01]

https://codereview.chromium.org/2240923002/diff/40001/build/android/test_wrap...
File build/android/test_wrapper/logdog_wrapper.py (right):

https://codereview.chromium.org/2240923002/diff/40001/build/android/test_wrap...
build/android/test_wrapper/logdog_wrapper.py:60: if
os.path.exists(args.logdog_bin_cmd):
On 2016/08/12 19:34:57, dnj wrote:
> On 2016/08/12 19:29:02, dnj wrote:
> > Does this mean that if binary doesn't exist, nothing will be uploaded?
> Shouldn't
> > that be an error?
> 
> I guess what I'm saying is:
> 1) We're explicitly making a point of loading this binary.
> 2) We hedge against that loading not actually happening.
> 
> It seems that (2) contradicts the expected success of (1). If we make a point
of
> having the binary on the system, we should expect that it will be there, and
> fail if it's not.

Maybe the thing to do here is surround the engire "logdog_cmd = ..." block in an
"if args.logdog_bin_cmd".

This means:
1) Don't check for binary existence. If someone specifies a binary and it's
invalid, that's an error.
2) Don't do any LogDog work when someone doesn't specify the flag.

WDYT?

[ghost stip (do not use), 2016-08-12 19:51:52]

The flag is baked in mb, so that won't work. The cipd package is in the testing
json, which is independent. We need to check for the existence of the binary.

[ghost stip (do not use), 2016-08-12 19:54:48]

Description was changed from

==========
Update chromium.android.json

BUG=637213
==========

to

==========
Update chromium.android.json

BUG=637213
NOTRY=True
==========

[ghost stip (do not use), 2016-08-12 19:55:32]

https://chromium-swarm.appspot.com/user/task/30991cfbd27f8b10 succeeded, which
shows that the check works (logdog specified but not run because the binary
isn't there). Pushing this CL through, we can discuss the overall design later.

[ghost stip (do not use), 2016-08-12 19:55:36]

The CQ bit was checked by stip@chromium.org

[commit-bot: I haz the power, 2016-08-12 19:56:11]

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

[ghost stip (do not use), 2016-08-12 19:57:17]

Description was changed from

==========
Update chromium.android.json

BUG=637213
NOTRY=True
==========

to

==========
Update chromium.android.json

BUG=637213
NOTRY=True
TBR=jbudorick@chromium.org
==========

[ghost stip (do not use), 2016-08-12 19:57:17]

stip@chromium.org changed reviewers:
+ jbudorick@chromium.org

[commit-bot: I haz the power, 2016-08-12 20:04:09]

Description was changed from

==========
Update chromium.android.json

BUG=637213
NOTRY=True
TBR=jbudorick@chromium.org
==========

to

==========
Update chromium.android.json

BUG=637213
NOTRY=True
TBR=jbudorick@chromium.org
==========

[commit-bot: I haz the power, 2016-08-12 20:04:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-12 20:07:28]

Description was changed from

==========
Update chromium.android.json

BUG=637213
NOTRY=True
TBR=jbudorick@chromium.org
==========

to

==========
Update chromium.android.json

BUG=637213
NOTRY=True
TBR=jbudorick@chromium.org

Committed: https://crrev.com/aa986de5b33958a7f4fbf27c5d1a8a5c50ac4c0f
Cr-Commit-Position: refs/heads/master@{#411749}
==========

[commit-bot: I haz the power, 2016-08-12 20:07:30]

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