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. (RELAND) (patchset chromium#3 id:40001 of https://codereview.chromium.org/1304303006/ )

Reason for revert:
broke the gpu bots again? https://code.google.com/p/chromium/issues/detail?id=530242

Original issue's description:
> [Android] Don't use a device blacklist if one isn't provided. (RELAND)
>
> This is a reland of https://codereview.chromium.org/1314823011
>
> BUG=517709
>
> Committed: https://crrev.com/7555cbb5daf4fb30cb56de9ed4dd8bcd565b5cf6
> Cr-Commit-Position: refs/heads/master@{#348158}

TBR=phajdan.jr@chromium.org,rnephew@chromium.org,rnephew@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=517709

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

Cr-Commit-Position: refs/heads/master@{#348174}
  • Loading branch information
jbudorick authored and Commit bot committed Sep 10, 2015
1 parent 6e69ae5 commit 414d29f
Show file tree
Hide file tree
Showing 14 changed files with 80 additions and 53 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 @@ -55,9 +55,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
23 changes: 10 additions & 13 deletions build/android/buildbot/bb_device_status_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ def GetAllAdb():
pass


def _IsBlacklisted(serial, blacklist):
return blacklist and serial in blacklist.Read()


def _BatteryStatus(device, blacklist):
battery_info = {}
try:
Expand Down Expand Up @@ -142,7 +138,7 @@ def blacklisting_device_status(device):
}

if adb_status == 'device':
if not _IsBlacklisted(serial, blacklist):
if not serial in blacklist.Read():
try:
build_product = device.build_product
build_id = device.build_id
Expand Down Expand Up @@ -184,7 +180,7 @@ def blacklisting_device_status(device):
elif blacklist:
blacklist.Extend([serial])

device_status['blacklisted'] = _IsBlacklisted(serial, blacklist)
device_status['blacklisted'] = serial in blacklist.Read()

return device_status

Expand Down Expand Up @@ -227,8 +223,7 @@ def RecoverDevices(devices, blacklist):
for d in should_reboot_device:
logging.debug(' %s', d)

if blacklist:
blacklist.Reset()
blacklist.Reset()

if should_restart_adb:
KillAllAdb()
Expand All @@ -241,7 +236,7 @@ def RecoverDevices(devices, blacklist):
blacklist.Extend([serial])

def blacklisting_recovery(device):
if _IsBlacklisted(device.adb.GetDeviceSerial(), blacklist):
if blacklist and device.adb.GetDeviceSerial() in blacklist.Read():
logging.debug('%s is blacklisted, skipping recovery.', str(device))
return

Expand Down Expand Up @@ -287,9 +282,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 Expand Up @@ -361,7 +358,7 @@ def main():

live_devices = [status['serial'] for status in statuses
if (status['adb_status'] == 'device'
and not _IsBlacklisted(status['serial'], blacklist))]
and status['serial'] not in blacklist.Read())]

# If all devices failed, or if there are no devices, it's an infra error.
return 0 if live_devices else constants.INFRA_EXIT_CODE
Expand Down
5 changes: 1 addition & 4 deletions build/android/buildbot/bb_device_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,7 @@ def ProvisionDevices(options):

def DeviceStatusCheck(options):
bb_annotations.PrintNamedStep('device_status_check')
cmd = [
'build/android/buildbot/bb_device_status_check.py',
'--blacklist-file', 'out/bad_devices.json',
]
cmd = ['build/android/buildbot/bb_device_status_check.py']
if options.restart_usb:
cmd.append('--restart-usb')
RunCmd(cmd, halt_on_failure=True)
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()

7 changes: 6 additions & 1 deletion build/android/devil/android/device_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from devil.android import apk_helper
from devil.android import device_signal
from devil.android import decorators
from devil.android import device_blacklist
from devil.android import device_errors
from devil.android import device_temp_file
from devil.android import logcat_monitor
Expand Down Expand Up @@ -1931,7 +1932,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 @@ -13,9 +13,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 @@ -76,9 +76,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 @@ -894,10 +894,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 @@ -235,9 +235,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 @@ -87,9 +87,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
4 changes: 1 addition & 3 deletions testing/scripts/host_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ def get_device_info(args, failures):
'android',
'buildbot',
'bb_device_status_check.py'),
'--json-output', tempfile_path,
'--blacklist-file', os.path.join(
args.paths['checkout'], 'out', 'bad_devices.json')])
'--json-output', tempfile_path])

if rc:
failures.append('bb_device_status_check')
Expand Down

0 comments on commit 414d29f

Please sign in to comment.