[Android] Add --blacklist-file as a command-line option.

[Android] Add --blacklist-file as a command-line option.

BUG=517709
Committed: https://crrev.com/9874769cf24b5870d9bb7c22fe5312b1cf8bcd5e
Cr-Commit-Position: refs/heads/master@{#344125}

[jbudorick, 2015-08-07 21:18:20]

jbudorick@chromium.org changed reviewers:
+ mikecase@chromium.org, nednguyen@google.com, rnephew@chromium.org,
sullivan@chromium.org

[nednguyen, 2015-08-07 21:22:21]

nednguyen@google.com changed reviewers:
+ eakuefner@chromium.org

[rnephew (Reviews Here), 2015-08-07 22:26:31]

lgtm w/ comment on argparse transition.

https://codereview.chromium.org/1281923003/diff/20001/build/android/buildbot/...
File build/android/buildbot/bb_device_status_check.py (right):

https://codereview.chromium.org/1281923003/diff/20001/build/android/buildbot/...
build/android/buildbot/bb_device_status_check.py:44: def DeviceInfo(device,
args):
Should this transition from optprase to argparse be its own cl?

[jbudorick, 2015-08-07 22:31:25]

On 2015/08/07 at 22:26:31, rnephew wrote:
> lgtm w/ comment on argparse transition.
> 
>
https://codereview.chromium.org/1281923003/diff/20001/build/android/buildbot/...
> File build/android/buildbot/bb_device_status_check.py (right):
> 
>
https://codereview.chromium.org/1281923003/diff/20001/build/android/buildbot/...
> build/android/buildbot/bb_device_status_check.py:44: def DeviceInfo(device,
args):
> Should this transition from optprase to argparse be its own cl?

Maybe, but I was already modifying arguments, so...

[nednguyen, 2015-08-08 02:13:41]

Don't you need to update the perf/core/perf_benchmark.py so that all the perf
benchmarks use the blacklist-file list?

https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
File tools/telemetry/telemetry/internal/browser/browser_options.py (right):

https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/browser/browser_options.py:53:
self.no_performance_mode = False
Do you need to add self.android_black_list_file = None here?

[eakuefner, 2015-08-11 17:58:47]

lgtm

One general comment: I'm wondering if you see any problems with making
blacklist=None the default? There seem to be quite a few places where we end up
calling functions and passing None.

[jbudorick, 2015-08-13 17:38:18]

https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:1796: if not blacklist:
Ethan: the None blacklist currently gets handled here. It's a transition state.

https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
File tools/telemetry/telemetry/internal/browser/browser_options.py (right):

https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/browser/browser_options.py:53:
self.no_performance_mode = False
On 2015/08/08 at 02:13:41, nednguyen wrote:
> Do you need to add self.android_black_list_file = None here?

Added.

[nednguyen, 2015-08-13 17:42:17]

On 2015/08/13 17:38:18, jbudorick wrote:
>
https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
> File build/android/pylib/device/device_utils.py (right):
> 
>
https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
> build/android/pylib/device/device_utils.py:1796: if not blacklist:
> Ethan: the None blacklist currently gets handled here. It's a transition
state.
> 
>
https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
> File tools/telemetry/telemetry/internal/browser/browser_options.py (right):
> 
>
https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
> tools/telemetry/telemetry/internal/browser/browser_options.py:53:
> self.no_performance_mode = False
> On 2015/08/08 at 02:13:41, nednguyen wrote:
> > Do you need to add self.android_black_list_file = None here?
> 
> Added.

What will happen to the telemetry & perf unittest after this change? Will they
still use the android_black_list file to filter out unhealthy devices?

[jbudorick, 2015-08-13 17:45:35]

On 2015/08/13 at 17:42:17, nednguyen wrote:
> On 2015/08/13 17:38:18, jbudorick wrote:
> >
https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
> > File build/android/pylib/device/device_utils.py (right):
> > 
> >
https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
> > build/android/pylib/device/device_utils.py:1796: if not blacklist:
> > Ethan: the None blacklist currently gets handled here. It's a transition
state.
> > 
> >
https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
> > File tools/telemetry/telemetry/internal/browser/browser_options.py (right):
> > 
> >
https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
> > tools/telemetry/telemetry/internal/browser/browser_options.py:53:
> > self.no_performance_mode = False
> > On 2015/08/08 at 02:13:41, nednguyen wrote:
> > > Do you need to add self.android_black_list_file = None here?
> > 
> > Added.
> 
> What will happen to the telemetry & perf unittest after this change? Will they
still use the android_black_list file to filter out unhealthy devices?

Yes. The overall goal with this sequence of CLs is to have the bots all pass a
blacklist file explicitly. With this CL, anything that isn't passing an explicit
blacklist (i.e., all of the bots) will fall back on the default blacklist.

[nednguyen, 2015-08-13 17:48:14]

On 2015/08/13 17:45:35, jbudorick wrote:
> On 2015/08/13 at 17:42:17, nednguyen wrote:
> > On 2015/08/13 17:38:18, jbudorick wrote:
> > >
>
https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
> > > File build/android/pylib/device/device_utils.py (right):
> > > 
> > >
>
https://codereview.chromium.org/1281923003/diff/20001/build/android/pylib/dev...
> > > build/android/pylib/device/device_utils.py:1796: if not blacklist:
> > > Ethan: the None blacklist currently gets handled here. It's a transition
> state.
> > > 
> > >
>
https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
> > > File tools/telemetry/telemetry/internal/browser/browser_options.py
(right):
> > > 
> > >
>
https://codereview.chromium.org/1281923003/diff/20001/tools/telemetry/telemet...
> > > tools/telemetry/telemetry/internal/browser/browser_options.py:53:
> > > self.no_performance_mode = False
> > > On 2015/08/08 at 02:13:41, nednguyen wrote:
> > > > Do you need to add self.android_black_list_file = None here?
> > > 
> > > Added.
> > 
> > What will happen to the telemetry & perf unittest after this change? Will
they
> still use the android_black_list file to filter out unhealthy devices?
> 
> Yes. The overall goal with this sequence of CLs is to have the bots all pass a
> blacklist file explicitly. With this CL, anything that isn't passing an
explicit
> blacklist (i.e., all of the bots) will fall back on the default blacklist.

lgtm

[jbudorick, 2015-08-19 01:51:07]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2015-08-19 01:51:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
eakuefner@chromium.org, rnephew@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/1281923003/#ps80001
(title: "fix telemetry_unittests")

[commit-bot: I haz the power, 2015-08-19 01:51:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1281923003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1281923003/80001

[commit-bot: I haz the power, 2015-08-19 01:56:36]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-08-19 01:57:24]

Patchset 5 (id:??) landed as
https://crrev.com/9874769cf24b5870d9bb7c22fe5312b1cf8bcd5e
Cr-Commit-Position: refs/heads/master@{#344125}

[jbudorick, 2015-08-19 04:19:44]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1292053006/ by jbudorick@chromium.org.

The reason for reverting is: broke the perf bots, will fix and reland..