AW: run_cts auto-determines device arch

This adds proper logic to run_cts.py to auto-determine the device
architecture, and to throw an error if the device architecture is not
supported. This also adds unit tests (and configures them to run on
presubmit).

Although this is not a significant improvement (we still only support
"arm64"), this gives fast, clear feedback when the device is unsupported
(e.g., emulators), and the logic should generalize as we support more
architectures. This also adds logging (which will be visible with
--verbose, to aid debugging).

This does minor refactoring in the script, to plumb arch where it's used
instead of reassigning to args.arch (which should help with testability
in the future).

R=aluo@chromium.org

Bug: 922854
Test: vpython android_webview/tools/run_cts_test.py
Test: android_webview/tools/run_cts.py (script still runs)
Change-Id: I6528ab836f30b5885ede59853acaeee69de26fa2
Reviewed-on: https://chromium-review.googlesource.com/c/1419840
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625376}

android_webview/tools/PRESUBMIT.py

@@ -5,14 +5,21 @@
 """Presubmit for android_webview/tools."""
 
 
+def _GetPythonUnitTests(input_api, output_api):
+  return input_api.canned_checks.GetUnitTestsRecursively(
+      input_api, output_api,
+      input_api.PresubmitLocalPath(),
+      whitelist=['.*_test\\.py$'],
+      blacklist=[])
+
+
 def CommonChecks(input_api, output_api):
   """Presubmit checks run on both upload and commit.
-
-  This is currently limited to pylint.
   """
   checks = []
   checks.extend(input_api.canned_checks.GetPylint(
       input_api, output_api, pylintrc='pylintrc'))
+  checks.extend(_GetPythonUnitTests(input_api, output_api))
   return input_api.RunTests(checks, False)
 
 

android_webview/tools/run_cts.py

@@ -47,6 +47,13 @@ _SDK_PLATFORM_DICT = {
     version_codes.OREO_MR1: 'O'
 }
 
+# TODO(aluo): support 'x86' and 'x86_64'
+_SUPPORTED_ARCH_DICT = {
+    'arm64-v8a': 'arm64',
+    # The test apks under 'arm64' support both arm and arm64 devices.
+    'armeabi-v7a': 'arm64',
+}
+
 
 def GetCtsInfo(arch, platform, item):
   """Gets contents of CTS Info for arch and platform.
@@ -145,7 +152,7 @@ def MergeTestResults(existing_results_json, additional_results_json):
             "Can't merge results field %s that is not a list or dict" % v)
 
 
-def ExtractCTSZip(args):
+def ExtractCTSZip(args, arch):
   """Extract the CTS tests for args.platform.
 
   Extract the CTS zip file from _CTS_ARCHIVE_DIR to
@@ -158,7 +165,7 @@ def ExtractCTSZip(args):
   """
   base_cts_dir = None
   delete_cts_dir = False
-  relative_cts_zip_path = GetCtsInfo(args.arch, args.platform, 'filename')
+  relative_cts_zip_path = GetCtsInfo(arch, args.platform, 'filename')
 
   if args.apk_dir:
     base_cts_dir = args.apk_dir
@@ -168,7 +175,7 @@ def ExtractCTSZip(args):
 
   cts_zip_path = os.path.join(_CTS_ARCHIVE_DIR, relative_cts_zip_path)
   local_cts_dir = os.path.join(base_cts_dir,
-                               GetCtsInfo(args.arch, args.platform,
+                               GetCtsInfo(arch, args.platform,
                                           'unzip_dir')
                               )
   zf = zipfile.ZipFile(cts_zip_path, 'r')
@@ -176,7 +183,7 @@ def ExtractCTSZip(args):
   return (local_cts_dir, base_cts_dir, delete_cts_dir)
 
 
-def RunAllCTSTests(args, test_runner_args):
+def RunAllCTSTests(args, arch, test_runner_args):
   """Run CTS tests downloaded from _CTS_BUCKET.
 
   Downloads CTS tests from bucket, runs them for the
@@ -186,11 +193,11 @@ def RunAllCTSTests(args, test_runner_args):
   returns the failure code of the last failing
   test.
   """
-  local_cts_dir, base_cts_dir, delete_cts_dir = ExtractCTSZip(args)
+  local_cts_dir, base_cts_dir, delete_cts_dir = ExtractCTSZip(args, arch)
   cts_result = 0
   json_results_file = args.json_results_file
   try:
-    cts_test_runs = GetCtsInfo(args.arch, args.platform, 'test_runs')
+    cts_test_runs = GetCtsInfo(arch, args.platform, 'test_runs')
     cts_results_json = {}
     for cts_test_run in cts_test_runs:
       iteration_cts_result = 0
@@ -227,14 +234,34 @@ def DeterminePlatform(device):
   """
   return _SDK_PLATFORM_DICT.get(device.build_version_sdk)
 
+def DetermineArch(device):
+  """Determines which architecture to use based on the device properties
+
+  Args:
+    device: The DeviceUtils instance
+  Returns:
+    The formatted arch string (as expected by CIPD)
+  Raises:
+    Exception: if device architecture is not currently supported by this script.
+  """
+  arch = _SUPPORTED_ARCH_DICT.get(device.product_cpu_abi)
+  if not arch:
+    raise Exception('Could not find CIPD bucket for your device arch (' +
+                    device.product_cpu_abi +
+                    '), please specify with --arch')
+  logging.info('Guessing arch=%s because product.cpu.abi=%s', arch,
+               device.product_cpu_abi)
+  return arch
 
 def main():
   parser = argparse.ArgumentParser()
   parser.add_argument(
       '--arch',
-      choices=['arm64'],
-      default='arm64',
-      help='Arch for CTS tests.')
+      choices=list(set(_SUPPORTED_ARCH_DICT.values())),
+      default=None,
+      type=str,
+      help=('Architecture to for CTS tests. Will auto-determine based on '
+            'the device ro.product.cpu.abi property.'))
   parser.add_argument(
       '--platform',
       choices=['L', 'M', 'N', 'O'],
@@ -265,18 +292,21 @@ def main():
   devil_chromium.Initialize()
 
   devices = script_common.GetDevices(args.devices, args.blacklist_file)
+  device = devices[0]
   if len(devices) > 1:
     logging.warning('Only single device supported, using 1st of %d devices: %s',
-                    len(devices), devices[0].serial)
-  test_runner_args.extend(['-d', devices[0].serial])
+                    len(devices), device.serial)
+  test_runner_args.extend(['-d', device.serial])
 
   if args.platform is None:
-    args.platform = DeterminePlatform(devices[0])
+    args.platform = DeterminePlatform(device)
     if args.platform is None:
       raise Exception('Could not auto-determine device platform, '
                       'please specifiy --platform')
 
-  return RunAllCTSTests(args, test_runner_args)
+  arch = args.arch if args.arch else DetermineArch(device)
+
+  return RunAllCTSTests(args, arch, test_runner_args)
 
 
 if __name__ == '__main__':

android_webview/tools/run_cts_test.py

+# Copyright 2019 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.
+
+import logging
+import unittest
+import mock
+import run_cts
+
+class _RunCtsTest(unittest.TestCase):
+  """Unittests for the run_cts module.
+  """
+
+  def testDetermineArch_arm64(self):
+    logging_mock = mock.Mock()
+    logging.info = logging_mock
+    device = mock.Mock(product_cpu_abi='arm64-v8a')
+    self.assertEqual(run_cts.DetermineArch(device), 'arm64')
+    # We should log a message to explain how we auto-determined the arch. We
+    # don't assert the message itself, since that's rather strict.
+    logging_mock.assert_called()
+
+  def testDetermineArch_unsupported(self):
+    device = mock.Mock(product_cpu_abi='madeup-abi')
+    with self.assertRaises(Exception) as _:
+      run_cts.DetermineArch(device)
+
+
+if __name__ == '__main__':
+  unittest.main()