Skip to content

Commit

Permalink
Revert of [Android] Don't use a device blacklist if one isn't provide…
Browse files Browse the repository at this point in the history
…d. (patchset #1 id:1 of https://codereview.chromium.org/1314823011/ )

Reason for revert:
Speculative revert, some builders (Nexus 5, 6 and 9 under chromium.gpu) are failing at the device status check step after this change due to:

  File "build/android/buildbot/bb_device_status_check.py", line 154, in blacklisting_device_status
    if not serial in blacklist.Read():
AttributeError: 'NoneType' object has no attribute 'Read'

See http://build.chromium.org/p/chromium.gpu/builders/Android%20Debug%20%28Nexus%205%29/builds/2067 for example.

Original issue's description:
> [Android] Don't use a device blacklist if one isn't provided.
>
> BUG=517709
>
> Committed: https://crrev.com/be5c91849d2196f97c7a4e0b3e90597c3f2b4c40
> Cr-Commit-Position: refs/heads/master@{#347473}

TBR=rnephew@chromium.org,jbudorick@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=517709

Review URL: https://codereview.chromium.org/1306653005

Cr-Commit-Position: refs/heads/master@{#347489}
  • Loading branch information
ro-berto authored and Commit bot committed Sep 4, 2015
1 parent ab0a1f7 commit 0179090
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 36 deletions.
9 changes: 6 additions & 3 deletions build/android/adb_install_apk.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,12 @@ def main():
and helper.GetSplitName()):
splits.append(f)

blacklist = (device_blacklist.Blacklist(args.blacklist_file)
if args.blacklist_file
else None)
if args.blacklist_file:
blacklist = device_blacklist.Blacklist(args.blacklist_file)
else:
# TODO(jbudorick): Remove this once the bots are converted.
blacklist = device_blacklist.Blacklist(device_blacklist.BLACKLIST_JSON)

devices = device_utils.DeviceUtils.HealthyDevices(blacklist)

if args.device:
Expand Down
8 changes: 5 additions & 3 deletions build/android/adb_reverse_forwarder.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ def main(argv):
parser.error('Bad port number')
sys.exit(1)

blacklist = (device_blacklist.Blacklist(options.blacklist_file)
if options.blacklist_file
else None)
if options.blacklist_file:
blacklist = device_blacklist.Blacklist(options.blacklist_file)
else:
blacklist = None

devices = device_utils.DeviceUtils.HealthyDevices(blacklist)

if options.device:
Expand Down
8 changes: 5 additions & 3 deletions build/android/buildbot/bb_device_status_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,11 @@ def main():

run_tests_helper.SetLogLevel(args.verbose)

blacklist = (device_blacklist.Blacklist(args.blacklist_file)
if args.blacklist_file
else None)
if args.blacklist_file:
blacklist = device_blacklist.Blacklist(args.blacklist_file)
else:
# TODO(jbudorick): Remove this once bots pass the blacklist file.
blacklist = device_blacklist.Blacklist(device_blacklist.BLACKLIST_JSON)

last_devices_path = os.path.join(
args.out_dir, device_list.LAST_DEVICES_FILENAME)
Expand Down
27 changes: 22 additions & 5 deletions build/android/devil/android/device_blacklist.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# found in the LICENSE file.

import json
import logging
import os
import threading

Expand Down Expand Up @@ -50,16 +49,34 @@ def Extend(self, devices):
Args:
devices: list of bad devices to be added to the blacklist file.
"""
logging.info('Adding %s to blacklist %s', ','.join(devices), self._path)
with self._blacklist_lock:
blacklist = self.Read()
blacklist = ReadBlacklist()
blacklist.extend(devices)
self.Write(blacklist)
WriteBlacklist(blacklist)

def Reset(self):
"""Erases the blacklist file if it exists."""
logging.info('Resetting blacklist %s', self._path)
with self._blacklist_lock:
if os.path.exists(self._path):
os.remove(self._path)


def ReadBlacklist():
# TODO(jbudorick): Phase out once all clients have migrated.
return Blacklist(BLACKLIST_JSON).Read()


def WriteBlacklist(blacklist):
# TODO(jbudorick): Phase out once all clients have migrated.
Blacklist(BLACKLIST_JSON).Write(blacklist)


def ExtendBlacklist(devices):
# TODO(jbudorick): Phase out once all clients have migrated.
Blacklist(BLACKLIST_JSON).Extend(devices)


def ResetBlacklist():
# TODO(jbudorick): Phase out once all clients have migrated.
Blacklist(BLACKLIST_JSON).Reset()

6 changes: 5 additions & 1 deletion build/android/devil/android/device_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1922,7 +1922,11 @@ def parallel(cls, devices, async=False):

@classmethod
def HealthyDevices(cls, blacklist=None, **kwargs):
blacklisted_devices = blacklist.Read() if blacklist else []
if not blacklist:
# TODO(jbudorick): Remove once clients pass in the blacklist.
blacklist = device_blacklist.Blacklist(device_blacklist.BLACKLIST_JSON)

blacklisted_devices = blacklist.Read()
def blacklisted(adb):
if adb.GetDeviceSerial() in blacklisted_devices:
logging.warning('Device %s is blacklisted.', adb.GetDeviceSerial())
Expand Down
7 changes: 4 additions & 3 deletions build/android/enable_asserts.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ def main():

args = parser.parse_args()

blacklist = (device_blacklist.Blacklist(args.blacklist_file)
if args.blacklist_file
else None)
if args.blacklist_file:
blacklist = device_blacklist.Blacklist(args.blacklist_file)
else:
blacklist = None

# TODO(jbudorick): Accept optional serial number and run only for the
# specified device when present.
Expand Down
8 changes: 5 additions & 3 deletions build/android/provision_devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ class _PHASES(object):


def ProvisionDevices(args):
blacklist = (device_blacklist.Blacklist(args.blacklist_file)
if args.blacklist_file
else None)
if args.blacklist_file:
blacklist = device_blacklist.Blacklist(args.blacklist_file)
else:
# TODO(jbudorick): Remove once the bots have switched over.
blacklist = device_blacklist.Blacklist(device_blacklist.BLACKLIST_JSON)

devices = device_utils.DeviceUtils.HealthyDevices(blacklist)
if args.device:
Expand Down
5 changes: 2 additions & 3 deletions build/android/pylib/local/device/local_device_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ class LocalDeviceEnvironment(environment.Environment):

def __init__(self, args, _error_func):
super(LocalDeviceEnvironment, self).__init__()
self._blacklist = (device_blacklist.Blacklist(args.blacklist_file)
if args.blacklist_file
else None)
self._blacklist = device_blacklist.Blacklist(
args.blacklist_file or device_blacklist.BLACKLIST_JSON)
self._device_serial = args.test_device
self._devices = []
self._max_tries = 1 + args.num_retries
Expand Down
7 changes: 4 additions & 3 deletions build/android/screenshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,10 @@ def main():
if options.verbose:
logging.getLogger().setLevel(logging.DEBUG)

blacklist = (device_blacklist.Blacklist(options.blacklist_file)
if options.blacklist_file
else None)
if options.blacklist_file:
blacklist = device_blacklist.Blacklist(options.blacklist_file)
else:
blacklist = None

devices = device_utils.DeviceUtils.HealthyDevices(blacklist)
if options.device:
Expand Down
9 changes: 6 additions & 3 deletions build/android/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -895,10 +895,13 @@ def _GetAttachedDevices(blacklist_file, test_device):
Returns:
A list of attached devices.
"""
blacklist = (device_blacklist.Blacklist(blacklist_file)
if blacklist_file
else None)
if not blacklist_file:
# TODO(jbudorick): Remove this once bots pass the blacklist file.
blacklist_file = device_blacklist.BLACKLIST_JSON
logging.warning('Using default device blacklist %s',
device_blacklist.BLACKLIST_JSON)

blacklist = device_blacklist.Blacklist(blacklist_file)
attached_devices = device_utils.DeviceUtils.HealthyDevices(blacklist)
if test_device:
test_device = [d for d in attached_devices if d == test_device]
Expand Down
7 changes: 4 additions & 3 deletions build/android/tombstones.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,10 @@ def main():
'crash stacks.')
options, _ = parser.parse_args()

blacklist = (device_blacklist.Blacklist(options.blacklist_file)
if options.blacklist_file
else None)
if options.blacklist_file:
blacklist = device_blacklist.Blacklist(options.blacklist_file)
else:
blacklist = None

if options.device:
devices = [device_utils.DeviceUtils(options.device)]
Expand Down
7 changes: 4 additions & 3 deletions build/android/update_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ def main():
args = parser.parse_args()
run_tests_helper.SetLogLevel(args.verbose)

blacklist = (device_blacklist.Blacklist(args.blacklist_file)
if args.blacklist_file
else None)
if args.blacklist_file:
blacklist = device_blacklist.Blacklist(args.blacklist_file)
else:
blacklist = None

devices = device_utils.DeviceUtils.HealthyDevices(blacklist)
if not devices:
Expand Down

0 comments on commit 0179090

Please sign in to comment.