Revert "[test] Add logic to run tests on Android"

This reverts commit 4c094342.

Reason for revert: Unfortunately this broke all perf builders.

Original change's description:
> [test] Add logic to run tests on Android
> 
> This adds a new command abstraction for running commands on Android
> using dockered devices on swarming.
> 
> The new abstraction handles pushing all required files to the device.
> The logic used for pushing and running is reused from the perf runner.
> 
> This adds only the mjsunit test suite. Others will be handled in
> follow up CLs. The suite logic is enhanced with auto-detection of files
> to be pushed to devices, for e.g. load or import statements.
> 
> Some test cases need an extra resource section for specifying required
> files.
> 
> Remaining failing tests are marked in the status files for later
> triage.
> 
> Bug: chromium:866862
> Change-Id: I2b957559f07fdcd8c1bd2f7034f5ba7754a31fb7
> Reviewed-on: https://chromium-review.googlesource.com/1150153
> Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55041}

TBR=machenbach@chromium.org,yangguo@chromium.org,sergiyb@chromium.org

Change-Id: If80129810586b709dab762c9b5724888e15daec2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:866862
Reviewed-on: https://chromium-review.googlesource.com/1170962Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55058}

infra/mb/mb_config.pyl

@@ -205,7 +205,6 @@
     'tryserver.v8': {
       'v8_android_arm_compile_rel': 'release_android_arm',
       'v8_android_arm64_compile_dbg': 'debug_android_arm64',
-      'v8_android_arm64_n5x_rel_ng': 'release_android_arm64',
       'v8_fuchsia_rel_ng': 'release_x64_fuchsia_trybot',
       'v8_linux_rel_ng': 'release_x86_gcmole_trybot',
       'v8_linux_verify_csa_rel_ng': 'release_x86_verify_csa',

infra/testing/PRESUBMIT.py

@@ -23,8 +23,6 @@ SUPPORTED_BUILDER_SPEC_KEYS = [
 SUPPORTED_SWARMING_DIMENSIONS = [
   'cores',
   'cpu',
-  'device_os',
-  'device_type',
   'os',
 ]
 

infra/testing/builders.pyl

@@ -31,18 +31,6 @@
   ##############################################################################
   ### luci.v8.try
   ##############################################################################
-  # Android
-  'v8_android_arm64_n5x_rel_ng_triggered': {
-    'swarming_dimensions' : {
-      'device_os': 'MMB29Q',
-      'device_type': 'bullhead',
-      'os': 'Android',
-    },
-    'tests': [
-      {'name': 'mjsunit', 'variant': 'default', 'shards': 2},
-    ],
-  },
-  ##############################################################################
   # Linux32
   'v8_linux_dbg_ng_triggered': {
     'swarming_dimensions' : {

test/BUILD.gn

@@ -45,7 +45,6 @@ group("v8_perf") {
   data_deps = [
     "cctest:cctest",
     "..:d8",
-    "../tools:v8_android_test_runner_deps",
   ]
 
   data = [
@@ -64,6 +63,15 @@ group("v8_perf") {
     "js-perf-test/",
     "memory/",
   ]
+
+  if (is_android && !build_with_chromium) {
+    data_deps += [ "../build/android:test_runner_py" ]
+
+    data += [
+      # This is used by run_perf.py, but not included by test_runner_py above.
+      "../third_party/catapult/devil/devil/android/perf/",
+    ]
+  }
 }
 
 group("v8_bot_default") {

test/mjsunit/mjsunit.status

@@ -641,31 +641,6 @@
   'tzoffset-seoul-noi18n': [SKIP],
 }],  # 'system == windows'
 
-##############################################################################
-['system == android', {
-  # Tests consistently failing on Android.
-  # Precision:
-  'es6/math-log2-log10': [FAIL],
-  # Timezone issues:
-  'icu-date-lord-howe': [FAIL],
-  'icu-date-to-string': [FAIL],
-  'regress/regress-6288': [FAIL],
-  'tzoffset-seoul': [FAIL],
-  'tzoffset-seoul-noi18n': [FAIL],
-  'tzoffset-transition-apia': [FAIL],
-  'tzoffset-transition-lord-howe': [FAIL],
-  'tzoffset-transition-moscow': [FAIL],
-  'tzoffset-transition-new-york': [FAIL],
-  'tzoffset-transition-new-york-noi18n': [FAIL],
-  # OOM:
-  'regress/regress-599414-array-concat-fast-path': [FAIL],
-  'regress/regress-748069': [FAIL],
-  'regress/regress-752764': [FAIL],
-  'regress/regress-779407': [FAIL],
-  # Issue with module import:
-  'regress/regress-797581': [FAIL],
-}],  # 'system == android'
-
 ##############################################################################
 ['system == macos', {
   # BUG(v8:5333)

test/mjsunit/testcfg.py

@@ -41,22 +41,6 @@ SELF_SCRIPT_PATTERN = re.compile(r"//\s+Env: TEST_FILE_NAME")
 MODULE_PATTERN = re.compile(r"^// MODULE$", flags=re.MULTILINE)
 NO_HARNESS_PATTERN = re.compile(r"^// NO HARNESS$", flags=re.MULTILINE)
 
-# Patterns for additional resource files on Android. Files that are not covered
-# by one of the other patterns below will be specified in the resources section.
-RESOURCES_PATTERN = re.compile(r"//\s+Resources:(.*)")
-# Pattern to auto-detect files to push on Android for statements like:
-# load("path/to/file.js")
-LOAD_PATTERN = re.compile(
-    r"(?:load|readbuffer|read)\((?:'|\")([^'\"]*)(?:'|\")\)")
-# Pattern to auto-detect files to push on Android for statements like:
-# import "path/to/file.js"
-MODULE_RESOURCES_PATTERN_1 = re.compile(
-    r"(?:import|export)(?:\(| )(?:'|\")([^'\"]*)(?:'|\")")
-# Pattern to auto-detect files to push on Android for statements like:
-# import foobar from "path/to/file.js"
-MODULE_RESOURCES_PATTERN_2 = re.compile(
-    r"(?:import|export).*from (?:'|\")([^'\"]*)(?:'|\")")
-
 # Flags known to misbehave when combining arbitrary mjsunit tests. Can also
 # be compiled regular expressions.
 COMBINE_TESTS_FLAGS_BLACKLIST = [
@@ -140,47 +124,6 @@ class TestCase(testcase.TestCase):
     self._files_suffix = files_suffix
     self._env = self._parse_source_env(source)
 
-  def _get_resources_for_file(self, file):
-    """Returns for a given file a list of absolute paths of files needed by the
-    given file.
-    """
-    with open(file) as f:
-      source = f.read()
-    result = []
-    def add_path(path):
-      result.append(os.path.abspath(path.replace('/', os.path.sep)))
-    for match in RESOURCES_PATTERN.finditer(source):
-      # There are several resources per line. Relative to base dir.
-      for path in match.group(1).strip().split():
-        add_path(path)
-    for match in LOAD_PATTERN.finditer(source):
-      # Files in load statements are relative to base dir.
-      add_path(match.group(1))
-    for match in MODULE_RESOURCES_PATTERN_1.finditer(source):
-      # Imported files are side by side with the test case.
-      add_path(os.path.join(
-          self.suite.root, os.path.dirname(self.path), match.group(1)))
-    for match in MODULE_RESOURCES_PATTERN_2.finditer(source):
-      # Imported files are side by side with the test case.
-      add_path(os.path.join(
-          self.suite.root, os.path.dirname(self.path), match.group(1)))
-    return result
-
-  def _get_resources(self):
-    """Returns the list of files needed by a test case."""
-    result = set()
-    to_check = [self._get_source_path()]
-    # Recurse over all files until reaching a fixpoint.
-    while to_check:
-      next_resource = to_check.pop()
-      result.add(next_resource)
-      for resource in self._get_resources_for_file(next_resource):
-        # Only add files that exist on disc. The pattens we check for give some
-        # false positives otherwise.
-        if resource not in result and os.path.exists(resource):
-          to_check.append(resource)
-    return sorted(list(result))
-
   def _parse_source_env(self, source):
     env_match = ENV_PATTERN.search(source)
     env = {}

test/mjsunit/tools/profviz.js

@@ -30,8 +30,6 @@
 // Files: tools/consarray.js tools/profile.js tools/profile_view.js
 // Files: tools/logreader.js tools/arguments.js tools/tickprocessor.js
 // Files: tools/profviz/composer.js
-// Resources: test/mjsunit/tools/profviz-test.log
-// Resources: test/mjsunit/tools/profviz-test.default
 // Env: TEST_FILE_NAME
 
 assertEquals('string', typeof TEST_FILE_NAME);

test/mjsunit/tools/tickprocessor.js

@@ -29,14 +29,6 @@
 // Files: tools/splaytree.js tools/codemap.js tools/csvparser.js
 // Files: tools/consarray.js tools/profile.js tools/profile_view.js
 // Files: tools/logreader.js tools/arguments.js tools/tickprocessor.js
-// Resources: test/mjsunit/tools/tickprocessor-test-func-info.log
-// Resources: test/mjsunit/tools/tickprocessor-test.default
-// Resources: test/mjsunit/tools/tickprocessor-test.func-info
-// Resources: test/mjsunit/tools/tickprocessor-test.gc-state
-// Resources: test/mjsunit/tools/tickprocessor-test.ignore-unknown
-// Resources: test/mjsunit/tools/tickprocessor-test.log
-// Resources: test/mjsunit/tools/tickprocessor-test.only-summary
-// Resources: test/mjsunit/tools/tickprocessor-test.separate-ic
 // Env: TEST_FILE_NAME
 
 

tools/BUILD.gn

@@ -25,26 +25,9 @@ group("v8_check_static_initializers") {
   ]
 }
 
-group("v8_android_test_runner_deps") {
-  testonly = true
-
-  if (is_android && !build_with_chromium) {
-    data_deps = [
-      "../build/android:test_runner_py",
-    ]
-    data = [
-      # This is used by android.py, but not included by test_runner_py above.
-      "../third_party/catapult/devil/devil/android/perf/",
-    ]
-  }
-}
-
 group("v8_testrunner") {
-  testonly = true
-
   data_deps = [
     "..:v8_dump_build_config",
-    ":v8_android_test_runner_deps",
   ]
 
   data = [

tools/predictable_wrapper.py

@@ -17,14 +17,10 @@ compared. Differences are reported as errors.
 import sys
 
 from testrunner.local import command
-from testrunner.local import utils
 
 MAX_TRIES = 3
 TIMEOUT = 120
 
-# Predictable mode works only when run on the host os.
-command.setup(utils.GuessOS())
-
 def main(args):
   def allocation_str(stdout):
     for line in reversed((stdout or '').splitlines()):

tools/testrunner/base_runner.py

@@ -19,7 +19,6 @@ sys.path.insert(
     os.path.dirname(os.path.abspath(__file__))))
 
 
-from testrunner.local import command
 from testrunner.local import testsuite
 from testrunner.local import utils
 from testrunner.test_config import TestConfig
@@ -172,12 +171,11 @@ class BuildConfig(object):
     else:
       self.arch = build_config['v8_target_cpu']
 
+    self.is_debug = build_config['is_debug']
     self.asan = build_config['is_asan']
     self.cfi_vptr = build_config['is_cfi']
     self.dcheck_always_on = build_config['dcheck_always_on']
     self.gcov_coverage = build_config['is_gcov_coverage']
-    self.is_android = build_config['is_android']
-    self.is_debug = build_config['is_debug']
     self.msan = build_config['is_msan']
     self.no_i18n = not build_config['v8_enable_i18n_support']
     self.no_snap = not build_config['v8_use_snapshot']
@@ -223,7 +221,6 @@ class BaseTestRunner(object):
     self.build_config = None
     self.mode_name = None
     self.mode_options = None
-    self.target_os = None
 
   def execute(self, sys_args=None):
     if sys_args is None:  # pragma: no cover
@@ -237,7 +234,6 @@ class BaseTestRunner(object):
         print ' '.join(sys.argv)
 
       self._load_build_config(options)
-      command.setup(self.target_os)
 
       try:
         self._process_default_options(options)
@@ -260,8 +256,6 @@ class BaseTestRunner(object):
       return utils.EXIT_CODE_INTERNAL_ERROR
     except KeyboardInterrupt:
       return utils.EXIT_CODE_INTERRUPTED
-    finally:
-      command.tear_down()
 
   def _create_parser(self):
     parser = optparse.OptionParser()
@@ -375,13 +369,6 @@ class BaseTestRunner(object):
       print '>>> Autodetected:'
       print self.build_config
 
-    # Represents the OS where tests are run on. Same as host OS except for
-    # Android, which is determined by build output.
-    if self.build_config.is_android:
-      self.target_os = 'android'
-    else:
-      self.target_os = utils.GuessOS()
-
   # Returns possible build paths in order:
   # gn
   # outdir
@@ -476,11 +463,7 @@ class BaseTestRunner(object):
             'build directory (%s) instead.' % self.outdir)
 
     if options.j == 0:
-      if self.build_config.is_android:
-        # Adb isn't happy about multi-processed file pushing.
-        options.j = 1
-      else:
-        options.j = multiprocessing.cpu_count()
+      options.j = multiprocessing.cpu_count()
 
     options.command_prefix = shlex.split(options.command_prefix)
     options.extra_flags = sum(map(shlex.split, options.extra_flags), [])
@@ -647,7 +630,7 @@ class BaseTestRunner(object):
       "simd_mips": simd_mips,
       "simulator": utils.UseSimulator(self.build_config.arch),
       "simulator_run": False,
-      "system": self.target_os,
+      "system": utils.GuessOS(),
       "tsan": self.build_config.tsan,
       "ubsan_vptr": self.build_config.ubsan_vptr,
     }

tools/testrunner/local/command.py

@@ -4,22 +4,16 @@
 
 
 import os
-import re
 import signal
 import subprocess
 import sys
 import threading
 import time
 
-from ..local.android import (
-    android_driver, CommandFailedException, TimeoutException)
 from ..local import utils
 from ..objects import output
 
 
-BASE_DIR = os.path.normpath(
-    os.path.join(os.path.dirname(os.path.abspath(__file__)), '..' , '..', '..'))
-
 SEM_INVALID_VALUE = -1
 SEM_NOGPFAULTERRORBOX = 0x0002  # Microsoft Platform SDK WinBase.h
 
@@ -39,18 +33,7 @@ class AbortException(Exception):
 
 class BaseCommand(object):
   def __init__(self, shell, args=None, cmd_prefix=None, timeout=60, env=None,
-               verbose=False, resources_func=None):
-    """Initialize the command.
-
-    Args:
-      shell: The name of the executable (e.g. d8).
-      args: List of args to pass to the executable.
-      cmd_prefix: Prefix of command (e.g. a wrapper script).
-      timeout: Timeout in seconds.
-      env: Environment dict for execution.
-      verbose: Print additional output.
-      resources_func: Callable, returning all test files needed by this command.
-    """
+               verbose=False):
     assert(timeout > 0)
 
     self.shell = shell
@@ -60,11 +43,11 @@ class BaseCommand(object):
     self.env = env or {}
     self.verbose = verbose
 
-  def execute(self):
+  def execute(self, **additional_popen_kwargs):
     if self.verbose:
       print '# %s' % self
 
-    process = self._start_process()
+    process = self._start_process(**additional_popen_kwargs)
 
     # Variable to communicate with the signal handler.
     abort_occured = [False]
@@ -96,13 +79,14 @@ class BaseCommand(object):
       duration
     )
 
-  def _start_process(self):
+  def _start_process(self, **additional_popen_kwargs):
     try:
       return subprocess.Popen(
         args=self._get_popen_args(),
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         env=self._get_env(),
+        **additional_popen_kwargs
       )
     except Exception as e:
       sys.stderr.write('Error executing: %s\n' % self)
@@ -203,85 +187,8 @@ class WindowsCommand(BaseCommand):
       sys.stdout.flush()
 
 
-class AndroidCommand(BaseCommand):
-  def __init__(self, shell, args=None, cmd_prefix=None, timeout=60, env=None,
-               verbose=False, resources_func=None):
-    """Initialize the command and all files that need to be pushed to the
-    Android device.
-    """
-    self.shell_name = os.path.basename(shell)
-    self.shell_dir = os.path.dirname(shell)
-    self.files_to_push = resources_func()
-
-    # Make all paths in arguments relative and also prepare files from arguments
-    # for pushing to the device.
-    rel_args = []
-    find_path_re = re.compile(r'.*(%s/[^\'"]+).*' % re.escape(BASE_DIR))
-    for arg in (args or []):
-      match = find_path_re.match(arg)
-      if match:
-        self.files_to_push.append(match.group(1))
-      rel_args.append(
-          re.sub(r'(.*)%s/(.*)' % re.escape(BASE_DIR), r'\1\2', arg))
-
-    super(AndroidCommand, self).__init__(
-        shell, args=rel_args, cmd_prefix=cmd_prefix, timeout=timeout, env=env,
-        verbose=verbose)
-
-  def execute(self, **additional_popen_kwargs):
-    """Execute the command on the device.
-
-    This pushes all required files to the device and then runs the command.
-    """
-    if self.verbose:
-      print '# %s' % self
-
-    android_driver().push_executable(self.shell_dir, 'bin', self.shell_name)
-
-    for abs_file in self.files_to_push:
-      abs_dir = os.path.dirname(abs_file)
-      file_name = os.path.basename(abs_file)
-      rel_dir = os.path.relpath(abs_dir, BASE_DIR)
-      android_driver().push_file(abs_dir, file_name, rel_dir)
-
-    start_time = time.time()
-    return_code = 0
-    timed_out = False
-    try:
-      stdout = android_driver().run(
-          'bin', self.shell_name, self.args, '.', self.timeout)
-    except CommandFailedException as e:
-      return_code = e.status
-      stdout = e.output
-    except TimeoutException as e:
-      return_code = 1
-      timed_out = True
-      # Sadly the Android driver doesn't provide output on timeout.
-      stdout = ''
-
-    duration = time.time() - start_time
-    return output.Output(
-        return_code,
-        timed_out,
-        stdout,
-        '',  # No stderr available.
-        -1,  # No pid available.
-        duration,
-    )
-
-
-Command = None
-def setup(target_os):
-  """Set the Command class to the OS-specific version."""
-  global Command
-  if target_os == 'android':
-    Command = AndroidCommand
-  elif target_os == 'windows':
-    Command = WindowsCommand
-  else:
-    Command = PosixCommand
-
-def tear_down():
-  """Clean up after using commands."""
-  if Command == AndroidCommand:
-    android_driver().tear_down()
+# Set the Command class to the OS-specific version.
+if utils.IsWindows():
+  Command = WindowsCommand
+else:
+  Command = PosixCommand

tools/testrunner/local/statusfile.py

@@ -55,7 +55,7 @@ for key in [SKIP, FAIL, PASS, CRASH, SLOW, FAIL_OK, NO_VARIANTS, FAIL_SLOPPY,
 
 # Support arches, modes to be written as keywords instead of strings.
 VARIABLES = {ALWAYS: True}
-for var in ["debug", "release", "big", "little", "android",
+for var in ["debug", "release", "big", "little",
             "android_arm", "android_arm64", "android_ia32", "android_x64",
             "arm", "arm64", "ia32", "mips", "mipsel", "mips64", "mips64el",
             "x64", "ppc", "ppc64", "s390", "s390x", "macos", "windows",

tools/testrunner/objects/testcase.py

@@ -240,8 +240,7 @@ class TestCase(object):
       args=params,
       env=env,
       timeout=timeout,
-      verbose=self._test_config.verbose,
-      resources_func=self._get_resources,
+      verbose=self._test_config.verbose
     )
 
   def _parse_source_flags(self, source=None):
@@ -261,14 +260,6 @@ class TestCase(object):
   def _get_source_path(self):
     return None
 
-  def _get_resources(self):
-    """Returns a list of absolute paths with additional files needed by the
-    test case.
-
-    Used to push additional files to Android devices.
-    """
-    return []
-
   @property
   def output_proc(self):
     if self.expected_outcomes is outproc.OUTCOMES_PASS:

tools/unittests/testdata/testroot1/v8_build_config.json

 {
   "current_cpu": "x64",
   "dcheck_always_on": false,
-  "is_android": false,
   "is_asan": false,
   "is_cfi": false,
   "is_component_build": false,

tools/unittests/testdata/testroot2/v8_build_config.json

 {
   "current_cpu": "x64",
   "dcheck_always_on": false,
-  "is_android": false,
   "is_asan": false,
   "is_cfi": false,
   "is_component_build": false,