Add multiple device/os filtering and a config file.

build/android/pylib/remote/device/remote_device_environment.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Environment setup and teardown for remote devices."""
 
+import distutils.version
+import json
 import logging
 import os
 import random
 import sys
 
 from pylib import constants
 from pylib.base import environment
 from pylib.remote.device import appurify_sanitized
 from pylib.remote.device import remote_device_helper
 
 class RemoteDeviceEnvironment(environment.Environment):
   """An environment for running on remote devices."""
 
   _ENV_KEY = 'env'
   _DEVICE_KEY = 'device'
 
   def __init__(self, args, error_func):
     """Constructor.
 
     Args:
       args: Command line arguments.
       error_func: error to show when using bad command line arguments.
     """
     super(RemoteDeviceEnvironment, self).__init__()
-
-    if args.api_key_file:
-      with open(args.api_key_file) as api_key_file:
-        self._api_key = api_key_file.read().strip()
-    elif args.api_key:
-      self._api_key = args.api_key
-    else:
-      error_func('Must set api key with --api-key or --api-key-file')
-
-    if args.api_secret_file:
-      with open(args.api_secret_file) as api_secret_file:
-        self._api_secret = api_secret_file.read().strip()
-    elif args.api_secret:
-      self._api_secret = args.api_secret
-    else:
-      error_func('Must set api secret with --api-secret or --api-secret-file')
-
-    if not args.api_protocol:
-      error_func('Must set api protocol with --api-protocol. Example: http')
-    self._api_protocol = args.api_protocol
-
-    if not args.api_address:
-      error_func('Must set api address with --api-address')
-    self._api_address = args.api_address
-
-    if not args.api_port:
-      error_func('Must set api port with --api-port.')
-    self._api_port = args.api_port
-
-    self._access_token = ''
-    self._results_path = args.results_path
-    self._remote_device = args.remote_device
-    self._remote_device_os = args.remote_device_os
-    self._runner_package = args.runner_package
-    self._runner_type = args.runner_type
-    self._device = ''
+    self._access_token = None
+    self._device = None
+    self._device_type = args.device_type
     self._verbose_count = args.verbose_count
-    self._device_type = args.device_type
     self._timeouts = {
         'queueing': 60 * 10,
         'installing': 60 * 10,
         'in-progress': 60 * 30,
         'unknown': 60 * 5
     }
+    # Example config file:
+    # {
+    #   "remote_device": ["Galaxy S4", "Galaxy S3"],
+    #   "remote_device_os": ["4.4.2", "4.4.4"],
+    #   "remote_device_minimum_os": "4.4.2",
+    #   "api_address": "www.example.com",
+    #   "api_port": "80",
+    #   "api_protocol": "http",
+    #   "api_secret": "apisecret",
+    #   "api_key": "apikey",
+    #   "timeouts": {
+    #     "queueing": 600,
+    #     "installing": 600,
+    #     "in-progress": 1800,
+    #     "unknown": 300
+    #   }
+    # }
+    if args.remote_device_file:
+      with open(args.remote_device_file) as device_file:
+        device_json = json.load(device_file)
+    else:
+      device_json = {}
+
+    self._api_address = device_json.get('api_address', None)

[jbudorick, 2015/01/30 14:24:59]

Shouldn't we only do the device_json parsing if --remote-device-file is passed?
No point in doing it if device_json is {}, right?

[rnephew (Wrong account), 2015/01/30 17:51:10]

Talked offline about why Im doing this.

+    self._api_key = device_json.get('api_key', None)
+    self._api_port = device_json.get('api_port', None)
+    self._api_protocol = device_json.get('api_protocol', None)
+    self._api_secret = device_json.get('api_secret', None)
+    self._remote_device = device_json.get('remote_device', None)
+    self._remote_device_minimum_os = device_json.get(
+        'remote_device_minimum_os', None)
+    self._remote_device_os = device_json.get('remote_device_os', None)
+    self._device_oem = device_json.get('device_oem', None)
+    self._device_type = device_json.get('device_type', 'Android')
+    self._results_path = device_json.get('results_path', None)
+    self._runner_package = device_json.get('runner_package', None)
+    self._runner_type = device_json.get('runner_type', None)
+    if 'timeouts' in device_json:
+      for key in device_json['timeouts']:
+        self._timeouts[key] = device_json['timeouts'][key]
+
+    if args.api_key_file:
+      with open(args.api_key_file) as api_key_file:
+        temp_key = api_key_file.read().strip()
+        if self._api_key and temp_key != self._api_key:
+          logging.info('Overriding api key.')
+          self._api_key = temp_key
+    elif args.api_key:
+      if self._api_key and self._api_key != args.api_key:
+        logging.info('Overriding api key.')
+      self._api_key = args.api_key
+
+    if args.api_secret_file:
+      with open(args.api_secret_file) as api_secret_file:
+        temp_secret = api_secret_file.read().strip()
+        if self._api_secret and temp_secret != self._api_secret:
+          logging.info('Overriding api secret.')
+          self._api_secret = temp_secret
+    elif args.api_secret:
+      if self._api_secret and self._api_secret != args.api_secret:
+        logging.info('Overriding api secret.')
+      self._api_secret = args.api_secret
+
+    if args.api_address:
+      if self._api_address and self._api_address != args.api_address:

[jbudorick, 2015/01/30 14:24:59]

This blocok is pretty repetitive. Can we make it into a function? (Probably a
local one?) e.g. maybe something like this:

def maybe_assign_and_log_override(attr, arg, desc):
  if attr:
    if attr and attr != arg:
      logging.info('Overriding %s from %s to %s', desc, attr, arg)
    attr = arg

maybe_assign_and_log_override(self._api_address, args.api_address,
'api_address')

[jbudorick, 2015/01/30 14:26:32]

Wow, "blocok" ... yikes.

[rnephew (Wrong account), 2015/01/30 17:51:10]

Done.

+        logging.info('Overriding api address from %s to %s.',
+                     self._api_address, args.api_address)
+      self._api_address = args.api_address
+    if args.api_port:
+      if self._api_port and self._api_port != args.api_port:
+        logging.info('Overriding api port from %s to %s.',
+                     self._api_port, args.api_port)
+      self._api_port = args.api_port
+    if args.api_protocol:
+      if self._api_protocol and self._api_protocol != args.api_protocol:
+        logging.info('Overriding api protocol from %s to %s.',
+                     self._api_protocol, args.api_protocol)
+      self._api_protocol = args.api_protocol
+    if args.remote_device:
+      if self._remote_device and self._remote_device != args.remote_device:
+        logging.info('Overriding remote device from %s to %s.',
+                     self._remote_device, args.remote_device)
+      self._remote_device = args.remote_device
+    if args.remote_device_minimum_os:
+      if (self._remote_device_minimum_os
+          and self._remote_device_minimum_os != args.remote_device_minimum_os):
+        logging.info('Overriding remote device minimum os from %s '
+                     'to %s.', self._remote_device_minimum_os,
+                     args.remote_device_minimum_os)
+      self._remote_device_minimum_os = args.remote_device_minimum_os
+    if args.remote_device_os:
+      if( self._remote_device_os

[jbudorick, 2015/01/30 14:24:59]

nit: if (self._remote_device_os

[rnephew (Wrong account), 2015/01/30 17:51:10]

Done.

+          and self._remote_device_os != args.remote_device_os):
+        logging.info('Overriding remote device os from %s to %s.',
+                     self._remote_device_os, args.remote_device_os)
+      self._remote_device_os = args.remote_device_os
+    if args.device_oem:
+      if self._device_oem and self._device_oem != args.device_oem:
+        logging.info('Overriding device oem from  %s to %s.',

[jbudorick, 2015/01/30 14:24:59]

nit: one space after from

[rnephew (Wrong account), 2015/01/30 17:51:10]

Done.

+                     self._device_oem, args.device_oem)
+      self._device_oem = args.device_oem
+    if args.device_type:
+      if self._device_type and self._device_type != args.device_type:
+        logging.info('Overriding device type from %s to %s.',
+                     self._device_type, args.device_type)
+      self._device_type = args.device_type
+    if args.results_path:
+      if self._results_path and self._results_path != args.results_path:
+        logging.info('Overriding results path from %s to %s.',
+                     self._results_path, args.results_path)
+      self._results_path = args.results_path
+    if args.runner_package:
+      if self._runner_package and self._runner_package != args.runner_package:
+        logging.info('Overriding runner package from %s to %s.',
+                     self._runner_package, args.runner_package)
+      self._runner_package = args.runner_package
+    if args.runner_type:
+      if self._runner_type and self._runner_type != args.runner_type:
+        logging.info('Overriding runner type from %s to %s.',
+                     self._runner_type, args.runner_type)
+      self._runner_type = args.runner_type
+
+    if not self._api_address:
+      error_func('Must set api address with --api-address'
+                 ' or in --remote-device-file.')
+    if not self._api_key:
+      error_func('Must set api key with --api-key, --api-key-file'
+                 ' or in --remote-device-file')
+    if not self._api_port:
+      error_func('Must set api port with --api-port'
+                 ' or in --remote-device-file')
+    if not self._api_protocol:
+      error_func('Must set api protocol with --api-protocol'
+                 ' or in --remote-device-file. Example: http')
+    if not self._api_secret:
+      error_func('Must set api secret with --api-secret, --api-secret-file'
+                 ' or in --remote-device-file')
+
+    logging.info('Api address: %s', self._api_address)
+    logging.info('Api port: %s', self._api_port)
+    logging.info('Api protocol: %s', self._api_protocol)
+    logging.info('Remote device: %s', self._remote_device)
+    logging.info('Remote device minimum os: %s',

[jbudorick, 2015/01/30 14:24:59]

nit: OS

[rnephew (Wrong account), 2015/01/30 17:51:10]

Done.

+                 self._remote_device_minimum_os)
+    logging.info('Remote device os: %s', self._remote_device_os)

[jbudorick, 2015/01/30 14:24:59]

same nit

[rnephew (Wrong account), 2015/01/30 17:51:10]

Done.

+    logging.info('Remote device OEM: %s', self._device_oem)
+    logging.info('Remote device type: %s', self._device_type)
+    logging.info('Results Path: %s', self._results_path)
+    logging.info('Runner package: %s', self._runner_package)
+    logging.info('Runner type: %s', self._runner_type)
+    logging.info('Timeouts: %s', self._timeouts)
 
     if not args.trigger and not args.collect:
       self._trigger = True
       self._collect = True
     else:
       self._trigger = args.trigger
       self._collect = args.collect
 
   def SetUp(self):
     """Set up the test environment."""
@@ skipping 58 matching lines @@
     with appurify_sanitized.SanitizeLogging(self._verbose_count,
                                             logging.WARNING):
       dev_list_res = appurify_sanitized.api.devices_list(self._access_token)
     remote_device_helper.TestHttpResponse(dev_list_res,
                                           'Unable to generate access token.')
     device_list = dev_list_res.json()['response']
     random.shuffle(device_list)
     for device in device_list:
       if device['os_name'] != self._device_type:
         continue
-      if self._remote_device and device['name'] != self._remote_device:
+      if self._remote_device and device['name'] not in self._remote_device:
         continue
       if (self._remote_device_os
-          and device['os_version'] != self._remote_device_os):
+          and device['os_version'] not in self._remote_device_os):
+        continue
+      if self._device_oem and device['brand'] not in self._device_oem:
+        continue
+      if (self._remote_device_minimum_os
+          and distutils.version.LooseVersion(device['os_version'])
+          < distutils.version.LooseVersion(self._remote_device_minimum_os)):
         continue
       if ((self._remote_device and self._remote_device_os)
           or device['available_devices_count']):
         logging.info('Found device: %s %s',
                      device['name'], device['os_version'])
         return device
     self._NoDeviceFound(device_list)
 
   def _PrintAvailableDevices(self, device_list):
     def compare_devices(a,b):
@@ skipping 49 matching lines @@
   def trigger(self):
     return self._trigger
 
   @property
   def verbose_count(self):
     return self._verbose_count
 
   @property
   def device_type(self):
     return self._device_type