Add CommandLineArgumentParameter and use it for WebView tests

build/android/pylib/instrumentation/test_runner.py

 # Copyright (c) 2012 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.
 
 """Class for running instrumentation tests on a single device."""
 
 import logging
 import os
 import re
 import sys
 import time
 
 from devil.android import device_errors
 from pylib import constants
 from pylib import flag_changer
 from pylib import valgrind_tools
 from pylib.base import base_test_result
 from pylib.base import base_test_runner
 from pylib.instrumentation import instrumentation_test_instance
 from pylib.instrumentation import json_perf_parser
 from pylib.instrumentation import test_result
 from pylib.local.device import local_device_instrumentation_test_run
 
 sys.path.append(os.path.join(constants.DIR_SOURCE_ROOT, 'build', 'util', 'lib',
                              'common'))
 import perf_tests_results_helper # pylint: disable=F0401
 
 
 _PERF_TEST_ANNOTATION = 'PerfTest'
+_PARAMETERIZED_TEST_ANNOTATION = 'ParameterizedTest'
+_PARAMETERIZED_TEST_SET_ANNOTATION = 'ParameterizedTest$Set'
+_COMMAND_LINE_PARAMETER = 'cmdlinearg-parameter'
 
 
 class TestRunner(base_test_runner.BaseTestRunner):

[jbudorick, 2015/10/27 16:14:37]

beware that I'm actively working on deprecating this entire class in favor of
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...

[mnaganov (inactive), 2015/10/27 23:53:30]

No problem, how do you run it?

[jbudorick, 2015/10/28 16:36:35]

It's a bit verbose at the moment, but I'm working on switching our wrapper
scripts to it:

./build/android/test_runner.py instrumentation --enable-platform-mode -e local
--apk-under-test out/Debug/apks/AndroidWebView.apk --test-apk
out/Debug/apks/AndroidWebViewTest.apk --isolate-file-path
android_webview/android_webview_test_apk.isolate

   """Responsible for running a series of tests connected to a single device."""
 
   _DEVICE_COVERAGE_DIR = 'chrome/test/coverage'
   _HOSTMACHINE_PERF_OUTPUT_FILE = '/tmp/chrome-profile'
   _DEVICE_PERF_OUTPUT_SEARCH_PREFIX = (constants.DEVICE_PERF_OUTPUT_DIR +
                                        '/chrome-profile*')
 
   def __init__(self, test_options, device, shard_index, test_pkg,
                additional_flags=None):
     """Create a new TestRunner.
@@ skipping 73 matching lines @@
           stripped_flags = (l.strip() for l in device_flags_file)
           flags_to_add.extend([flag for flag in stripped_flags if flag])
       self.flags.AddFlags(flags_to_add)
 
   def TearDown(self):
     """Cleans up the test harness and saves outstanding data from test run."""
     if self.flags:
       self.flags.Restore()
     super(TestRunner, self).TearDown()
 
-  def TestSetup(self, test):
+  def TestSetup(self, test, flag_modifiers):
     """Sets up the test harness for running a particular test.
 
     Args:
       test: The name of the test that will be run.
     """
     self.SetupPerfMonitoringIfNeeded(test)
     self._SetupIndividualTestTimeoutScale(test)
     self.tool.SetupEnvironment()
 
-    if self.flags and self._IsFreTest(test):
-      self.flags.RemoveFlags(['--disable-fre'])
+    if self.flags:
+      self.flags.PushFlags(add=flag_modifiers[0], remove=flag_modifiers[1])
 
     # Make sure the forwarder is still running.
     self._RestartHttpServerForwarderIfNecessary()
 
     if self.coverage_dir:
       coverage_basename = '%s.ec' % test
       self.coverage_device_file = '%s/%s/%s' % (
           self.device.GetExternalStoragePath(),
           TestRunner._DEVICE_COVERAGE_DIR, coverage_basename)
       self.coverage_host_file = os.path.join(
@@ skipping 16 matching lines @@
     """Determines whether a test is a performance test.
 
     Args:
       test: The name of the test to be checked.
 
     Returns:
       Whether the test is annotated as a performance test.
     """
     return _PERF_TEST_ANNOTATION in self.test_pkg.GetTestAnnotations(test)
 
+  def _GetTestCmdlineParameters(self, test):
+    """Determines whether the test is parameterized to be run with different
+       command-line flags.
+
+    Args:
+      test: The name of the test to be checked.
+
+    Returns:
+      If the test is parameterized, returns a list of add/remove tuples
+      with lists of flags, e.g.:
+
+        [(["--flag-to-add"], []), ([], ["--flag-to-remove"]), ([], [])]

[jbudorick, 2015/10/27 16:14:36]

This would be more readable if it returned a collections.namedtuple instead of a
tuple. A client could then refer to them via e.g. flags.to_add instead of
flags[0].

[mnaganov (inactive), 2015/10/27 23:53:30]

Thanks! I'm learning neat Python tricks with you :)

+
+      That means, the test must be run three times, the first time with
+      "--flag-to-add" added to command-line, the second time with
+      "--flag-to-remove" to be removed from command-line, and the third time
+      with default command-line args. If the same flag is listed in both
+      sections, it is left unchanged.
+
+    """
+    parameterized_tests = []
+    annotations = self.test_pkg.GetTestAnnotations(test)
+    if _PARAMETERIZED_TEST_ANNOTATION in annotations:
+      parameterized_tests = [annotations[_PARAMETERIZED_TEST_ANNOTATION]]
+    elif _PARAMETERIZED_TEST_SET_ANNOTATION in annotations:
+      if annotations[_PARAMETERIZED_TEST_SET_ANNOTATION]:
+        parameterized_tests = annotations[
+          _PARAMETERIZED_TEST_SET_ANNOTATION].get('tests', [])
+    else:
+      return [([], [])]
+
+    result = []
+    for pt in parameterized_tests:
+      if not pt:
+        continue
+      for p in pt['parameters']:
+        if p['tag'] == _COMMAND_LINE_PARAMETER:
+          to_add = []
+          to_remove = []
+          if 'arguments' in p:
+            for a in p['arguments']:
+              if a['name'] == 'add':
+                to_add = a['stringArray']
+              elif a['name'] == 'remove':
+                to_remove = a['stringArray']
+          result.append((to_add, to_remove))
+    return result if result else [([], [])]
+
   def SetupPerfMonitoringIfNeeded(self, test):
     """Sets up performance monitoring if the specified test requires it.
 
     Args:
       test: The name of the test to be run.
     """
     if not self._IsPerfTest(test):
       return
     self.device.RunShellCommand(
         ['rm', TestRunner._DEVICE_PERF_OUTPUT_SEARCH_PREFIX])
     self._logcat_monitor = self.device.GetLogcatMonitor()
     self._logcat_monitor.Start()
 
-  def TestTeardown(self, test, result):
+  def TestTeardown(self, test, results):
     """Cleans up the test harness after running a particular test.
 
     Depending on the options of this TestRunner this might handle performance
     tracking.  This method will only be called if the test passed.
 
     Args:
       test: The name of the test that was just run.
-      result: result for this test.
+      results: results for this test.
     """
 
     self.tool.CleanUpEnvironment()
 
-    if self.flags and self._IsFreTest(test):
+    if self.flags:
       self.flags.Restore()
 
-    if not result:
+    if not results:
       return
-    if result.DidRunPass():
+    if results.DidRunPass():
       self.TearDownPerfMonitoring(test)
 
       if self.coverage_dir:
         self.device.PullFile(
             self.coverage_device_file, self.coverage_host_file)
         self.device.RunShellCommand(
             'rm -f %s' % self.coverage_device_file)
     elif self.package_info:
       self.device.ClearApplicationState(self.package_info.package)
 
@@ skipping 122 matching lines @@
     return test_result.InstrumentationTestResult(
         test, base_test_result.ResultType.UNKNOWN, start_ms, duration_ms)
 
   #override
   def RunTest(self, test):
     results = base_test_result.TestRunResults()
     timeout = (self._GetIndividualTestTimeoutSecs(test) *
                self._GetIndividualTestTimeoutScale(test) *
                self.tool.GetTimeoutScale())
 
-    start_ms = 0
-    duration_ms = 0
-    try:
-      self.TestSetup(test)
+    cmdline_parameters = self._GetTestCmdlineParameters(test)

[jbudorick, 2015/10/27 16:14:37]

This should be part of test list generation, not test execution.

[mnaganov (inactive), 2015/10/27 23:53:30]

But... isn't test list consist of just test names? And then the test name is
just passed directly to the instrumentation command.

I guess, implementing your suggestion will require a big change in how the
information about tests is passed around the test framework.

[jbudorick, 2015/10/28 16:36:35]

Oh right, that's true in this version. In the new one, tests are dicts:

{
  'class': '...',
  'method': '...',
  'annotations': { ... }
}

so yeah, this suggestion isn't implementable in this version.

+    for cmdline_modifiers in cmdline_parameters:
+      start_ms = 0
+      duration_ms = 0
+      try:
+        flag_modifiers = cmdline_modifiers[:]
+        if self._IsFreTest(test):

[jbudorick, 2015/10/27 16:14:37]

We should be able to remove this in light of
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/syn...

[mnaganov (inactive), 2015/10/27 23:53:30]

Oh, cool. I thought that maybe it was again important for someone that this is a
@Feature.

+          flag_modifiers[1].append('--disable-fre')
+        self.TestSetup(test, flag_modifiers)
 
-      try:
-        self.device.GoHome()
-      except device_errors.CommandTimeoutError:
-        logging.exception('Failed to focus the launcher.')
+        try:
+          self.device.GoHome()
+        except device_errors.CommandTimeoutError:
+          logging.exception('Failed to focus the launcher.')
 
-      time_ms = lambda: int(time.time() * 1000)
-      start_ms = time_ms()
-      raw_output = self._RunTest(test, timeout)
-      duration_ms = time_ms() - start_ms
+        time_ms = lambda: int(time.time() * 1000)
+        start_ms = time_ms()
+        raw_output = self._RunTest(test, timeout)
+        duration_ms = time_ms() - start_ms
 
-      # Parse the test output
-      result_code, result_bundle, statuses = (
-          instrumentation_test_instance.ParseAmInstrumentRawOutput(raw_output))
-      result = self._GenerateTestResult(
-          test, result_code, result_bundle, statuses, start_ms, duration_ms)
-      if local_device_instrumentation_test_run.DidPackageCrashOnDevice(
-          self.test_pkg.GetPackageName(), self.device):
-        result.SetType(base_test_result.ResultType.CRASH)
-      results.AddResult(result)
-    except device_errors.CommandTimeoutError as e:
-      results.AddResult(test_result.InstrumentationTestResult(
+        # Parse the test output
+        result_code, result_bundle, statuses = (
+            instrumentation_test_instance.ParseAmInstrumentRawOutput(
+              raw_output))
+        result = self._GenerateTestResult(
+            test, result_code, result_bundle, statuses, start_ms, duration_ms)
+        if local_device_instrumentation_test_run.DidPackageCrashOnDevice(
+            self.test_pkg.GetPackageName(), self.device):
+          result.SetType(base_test_result.ResultType.CRASH)
+        if len(cmdline_parameters):
+          result.SetName(
+            result.GetName() + ' with {' + ' '.join(cmdline_modifiers[0]) + '}')

[jbudorick, 2015/10/27 16:14:37]

This name should be generated once, presumably before the try block.

[mnaganov (inactive), 2015/10/27 23:53:30]

We can't change the 'test' string, because it is then used for finding some
meta-info about the test. That's why I'm changing the name of the result only.
Since we are changing it before adding to the 'results', it should be OK.

I've changed the code to update the result name and append it to results after
the 'try' block, so it's only done once now.

+        results.AddResult(result)
+      except device_errors.CommandTimeoutError as e:
+        result = test_result.InstrumentationTestResult(
           test, base_test_result.ResultType.TIMEOUT, start_ms, duration_ms,
-          log=str(e) or 'No information'))
-      if self.package_info:
-        self.device.ForceStop(self.package_info.package)
-        self.device.ForceStop(self.package_info.test_package)
-    except device_errors.DeviceUnreachableError as e:
-      results.AddResult(test_result.InstrumentationTestResult(
-          test, base_test_result.ResultType.CRASH, start_ms, duration_ms,
-          log=str(e) or 'No information'))
-    self.TestTeardown(test, results)
+          log=str(e) or 'No information')
+        if len(cmdline_parameters):
+          result.SetName(
+            result.GetName() + ' with {' + ' '.join(cmdline_modifiers[0]) + '}')
+        results.AddResult(result)
+        if self.package_info:
+          self.device.ForceStop(self.package_info.package)
+          self.device.ForceStop(self.package_info.test_package)
+      except device_errors.DeviceUnreachableError as e:
+        result = test_result.InstrumentationTestResult(
+            test, base_test_result.ResultType.CRASH, start_ms, duration_ms,
+            log=str(e) or 'No information')
+        if len(cmdline_parameters):
+          result.SetName(
+            result.GetName() + ' with {' + ' '.join(cmdline_modifiers[0]) + '}')
+        results.AddResult(result)
+
+      self.TestTeardown(test, results)
+
     return (results, None if results.DidRunPass() else test)