[test] Overhaul mode processing in test runner

This simplifies mode processing as follows:
- Passing the --mode parameter is deprecated.
- The build output is now only searched in the --outdir parameter
that was passed (previously some combinations of mode and outdir
were possible).
- The mode is deduced from the build artifacts based on the gn
arguments "is_debug" and "dcheck_always_on".
- Timeouts and status file entries in release mode with dchecks are
treated like in debug mode.

This change was prepared on the infrastructure side by deprecating
the --mode flag and passing --outdir=out/build:
https://crrev.com/c/2426643

Bug: chromium:1132088, v8:10893
Change-Id: I0f34ebc003b220f07df4ecdbf69ea6c06ac1f66a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2450016Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70363}

tools/testrunner/base_runner.py

@@ -6,7 +6,7 @@
 from __future__ import print_function
 from functools import reduce
 
-from collections import OrderedDict
+from collections import OrderedDict, namedtuple
 import json
 import multiprocessing
 import optparse
@@ -116,52 +116,38 @@ SLOW_ARCHS = [
 ]
 
 
-class ModeConfig(object):
-  def __init__(self, flags, timeout_scalefactor, status_mode, execution_mode):
-    self.flags = flags
-    self.timeout_scalefactor = timeout_scalefactor
-    self.status_mode = status_mode
-    self.execution_mode = execution_mode
-
+ModeConfig = namedtuple(
+    'ModeConfig', 'label flags timeout_scalefactor status_mode execution_mode')
 
 DEBUG_FLAGS = ["--nohard-abort", "--enable-slow-asserts", "--verify-heap"]
 RELEASE_FLAGS = ["--nohard-abort"]
-MODES = {
-  "debug": ModeConfig(
-    flags=DEBUG_FLAGS,
-    timeout_scalefactor=4,
-    status_mode="debug",
-    execution_mode="debug",
-  ),
-  "optdebug": ModeConfig(
+
+DEBUG_MODE = ModeConfig(
+    label='debug',
     flags=DEBUG_FLAGS,
     timeout_scalefactor=4,
     status_mode="debug",
     execution_mode="debug",
-  ),
-  "release": ModeConfig(
+)
+
+RELEASE_MODE = ModeConfig(
+    label='release',
     flags=RELEASE_FLAGS,
     timeout_scalefactor=1,
     status_mode="release",
     execution_mode="release",
-  ),
-  # Normal trybot release configuration. There, dchecks are always on which
-  # implies debug is set. Hence, the status file needs to assume debug-like
-  # behavior/timeouts.
-  "tryrelease": ModeConfig(
+)
+
+# Normal trybot release configuration. There, dchecks are always on which
+# implies debug is set. Hence, the status file needs to assume debug-like
+# behavior/timeouts.
+TRY_RELEASE_MODE = ModeConfig(
+    label='release+dchecks',
     flags=RELEASE_FLAGS,
-    timeout_scalefactor=1,
-    status_mode="debug",
-    execution_mode="release",
-  ),
-  # This mode requires v8 to be compiled with dchecks and slow dchecks.
-  "slowrelease": ModeConfig(
-    flags=RELEASE_FLAGS + ["--enable-slow-asserts"],
-    timeout_scalefactor=2,
+    timeout_scalefactor=4,
     status_mode="debug",
     execution_mode="release",
-  ),
-}
+)
 
 PROGRESS_INDICATORS = {
   'verbose': progress.VerboseProgressIndicator,
@@ -268,7 +254,6 @@ class BaseTestRunner(object):
     self.basedir = basedir or BASE_DIR
     self.outdir = None
     self.build_config = None
-    self.mode_name = None
     self.mode_options = None
     self.target_os = None
 
@@ -305,7 +290,7 @@ class BaseTestRunner(object):
       tests = self._load_testsuite_generators(args, options)
       self._setup_env()
       print(">>> Running tests for %s.%s" % (self.build_config.arch,
-                                            self.mode_name))
+                                             self.mode_options.label))
       exit_code = self._do_execute(tests, args, options)
       if exit_code == utils.EXIT_CODE_FAILURES and options.json_test_results:
         print("Force exit code 0 after failures. Json test results file "
@@ -339,9 +324,6 @@ class BaseTestRunner(object):
                       default="out")
     parser.add_option("--arch",
                       help="The architecture to run tests for")
-    parser.add_option("-m", "--mode",
-                      help="The test mode in which to run (uppercase for builds"
-                      " in CI): %s" % MODES.keys())
     parser.add_option("--shell-dir", help="DEPRECATED! Executables from build "
                       "directory will be used")
     parser.add_option("--test-root", help="Root directory of the test suites",
@@ -427,9 +409,8 @@ class BaseTestRunner(object):
   def _parse_args(self, parser, sys_args):
     options, args = parser.parse_args(sys_args)
 
-    if any(map(lambda v: v and ',' in v,
-                [options.arch, options.mode])):  # pragma: no cover
-      print('Multiple arch/mode are deprecated')
+    if options.arch and ',' in options.arch:  # pragma: no cover
+      print('Multiple architectures are deprecated')
       raise TestRunnerError()
 
     return options, args
@@ -465,8 +446,7 @@ class BaseTestRunner(object):
   # Returns possible build paths in order:
   # gn
   # outdir
-  # outdir/arch.mode
-  # Each path is provided in two versions: <path> and <path>/mode for bots.
+  # outdir on bots
   def _possible_outdirs(self, options):
     def outdirs():
       if options.gn:
@@ -475,30 +455,12 @@ class BaseTestRunner(object):
 
       yield options.outdir
 
-      # TODO(machenbach): Temporary fallback to legacy outdir. The
-      # infrastructure switches to the canonical out/build location. But
-      # bisection will keep working with out/Release or out/Debug for a
-      # grace period.
-      if os.path.basename(options.outdir) == 'build':
-        base_dir = os.path.dirname(options.outdir)
-        release_dir = os.path.join(base_dir, 'Release')
-        debug_dir = os.path.join(base_dir, 'Debug')
-        if os.path.exists(release_dir) and not os.path.exists(debug_dir):
-          yield release_dir
-        if os.path.exists(debug_dir) and not os.path.exists(release_dir):
-          yield debug_dir
-
-      if options.arch and options.mode:
-        yield os.path.join(options.outdir,
-                          '%s.%s' % (options.arch, options.mode))
+      if os.path.basename(options.outdir) != 'build':
+        yield os.path.join(options.outdir, 'build')
 
     for outdir in outdirs():
       yield os.path.join(self.basedir, outdir)
 
-      # bot option
-      if options.mode:
-        yield os.path.join(self.basedir, outdir, options.mode)
-
   def _get_gn_outdir(self):
     gn_out_dir = os.path.join(self.basedir, DEFAULT_OUT_GN)
     latest_timestamp = -1
@@ -515,29 +477,12 @@ class BaseTestRunner(object):
       return os.path.join(DEFAULT_OUT_GN, latest_config)
 
   def _process_default_options(self, options):
-    # We don't use the mode for more path-magic.
-    # Therefore transform the bot mode here to fix build_config value.
-    if options.mode:
-      options.mode = self._bot_to_v8_mode(options.mode)
-
-    build_config_mode = 'debug' if self.build_config.is_debug else 'release'
-    if options.mode:
-      if options.mode not in MODES:  # pragma: no cover
-        print('%s mode is invalid' % options.mode)
-        raise TestRunnerError()
-      if MODES[options.mode].execution_mode != build_config_mode:
-        print ('execution mode (%s) for %s is inconsistent with build config '
-               '(%s)' % (
-            MODES[options.mode].execution_mode,
-            options.mode,
-            build_config_mode))
-        raise TestRunnerError()
-
-      self.mode_name = options.mode
+    if self.build_config.is_debug:
+      self.mode_options = DEBUG_MODE
+    elif self.build_config.dcheck_always_on:
+      self.mode_options = TRY_RELEASE_MODE
     else:
-      self.mode_name = build_config_mode
-
-    self.mode_options = MODES[self.mode_name]
+      self.mode_options = RELEASE_MODE
 
     if options.arch and options.arch != self.build_config.arch:
       print('--arch value (%s) inconsistent with build config (%s).' % (
@@ -558,15 +503,6 @@ class BaseTestRunner(object):
     options.command_prefix = shlex.split(options.command_prefix)
     options.extra_flags = sum(map(shlex.split, options.extra_flags), [])
 
-  def _bot_to_v8_mode(self, config):
-    """Convert build configs from bots to configs understood by the v8 runner.
-
-    V8 configs are always lower case and without the additional _x64 suffix
-    for 64 bit builds on windows with ninja.
-    """
-    mode = config[:-4] if config.endswith('_x64') else config
-    return mode.lower()
-
   def _process_options(self, options):
     pass
 
@@ -714,9 +650,7 @@ class BaseTestRunner(object):
       "is_clang": self.build_config.is_clang,
       "is_full_debug": self.build_config.is_full_debug,
       "mips_arch_variant": mips_arch_variant,
-      "mode": self.mode_options.status_mode
-              if not self.build_config.dcheck_always_on
-              else "debug",
+      "mode": self.mode_options.status_mode,
       "msan": self.build_config.msan,
       "no_harness": options.no_harness,
       "no_i18n": self.build_config.no_i18n,
@@ -827,6 +761,9 @@ class BaseTestRunner(object):
   def _create_progress_indicators(self, test_count, options):
     procs = [PROGRESS_INDICATORS[options.progress]()]
     if options.json_test_results:
+      # TODO(machenbach): Deprecate the execution mode. Previously it was meant
+      # to differentiate several records in the json output. But there is now
+      # only one record and the mode information is redundant.
       procs.append(progress.JsonTestProgressIndicator(
         self.framework_name,
         self.build_config.arch,

tools/unittests/run_tests_test.py

@@ -67,7 +67,7 @@ def temp_base(baseroot='testroot1'):
   """
   basedir = os.path.join(TEST_DATA_ROOT, baseroot)
   with temp_dir() as tempbase:
-    builddir = os.path.join(tempbase, 'out', 'Release')
+    builddir = os.path.join(tempbase, 'out', 'build')
     testroot = os.path.join(tempbase, 'test')
     os.makedirs(builddir)
     shutil.copy(os.path.join(basedir, 'v8_build_config.json'), builddir)
@@ -112,7 +112,7 @@ def run_tests(basedir, *args, **kwargs):
 
 def override_build_config(basedir, **kwargs):
   """Override the build config with new values provided as kwargs."""
-  path = os.path.join(basedir, 'out', 'Release', 'v8_build_config.json')
+  path = os.path.join(basedir, 'out', 'build', 'v8_build_config.json')
   with open(path) as f:
     config = json.load(f)
     config.update(kwargs)
@@ -171,7 +171,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default,stress',
           '--time',
@@ -189,7 +188,6 @@ class SystemTest(unittest.TestCase):
       for shard in [1, 2]:
         result = run_tests(
             basedir,
-            '--mode=Release',
             '--progress=verbose',
             '--variants=default,stress',
             '--shard-count=2',
@@ -218,7 +216,6 @@ class SystemTest(unittest.TestCase):
       for shard in [1, 2]:
         result = run_tests(
             basedir,
-            '--mode=Release',
             '--progress=verbose',
             '--variants=default,stress',
             '--shard-count=2',
@@ -237,7 +234,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default,stress',
           'sweet/strawberries',
@@ -283,7 +279,6 @@ class SystemTest(unittest.TestCase):
       json_path = os.path.join(basedir, 'out.json')
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           '--rerun-failures-count=2',
@@ -312,7 +307,6 @@ class SystemTest(unittest.TestCase):
       json_path = os.path.join(basedir, 'out.json')
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           '--rerun-failures-count=2',
@@ -343,7 +337,6 @@ class SystemTest(unittest.TestCase):
           v8_enable_pointer_compression=False)
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           'sweet/bananas',
@@ -368,7 +361,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=nooptimization',
           'sweet/strawberries',
@@ -382,7 +374,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=nooptimization',
           '--run-skipped',
@@ -399,7 +390,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           infra_staging=False,
       )
       self.assertIn('0 tests ran', result.stdout, result)
@@ -407,24 +397,15 @@ class SystemTest(unittest.TestCase):
 
   def testNoBuildConfig(self):
     """Test failing run when build config is not found."""
-    with temp_base() as basedir:
+    with temp_dir() as basedir:
       result = run_tests(basedir)
       self.assertIn('Failed to load build config', result.stdout, result)
       self.assertEqual(5, result.returncode, result)
 
-  def testInconsistentMode(self):
-    """Test failing run when attempting to wrongly override the mode."""
-    with temp_base() as basedir:
-      override_build_config(basedir, is_debug=True)
-      result = run_tests(basedir, '--mode=Release')
-      self.assertIn('execution mode (release) for release is inconsistent '
-                    'with build config (debug)', result.stdout, result)
-      self.assertEqual(5, result.returncode, result)
-
   def testInconsistentArch(self):
     """Test failing run when attempting to wrongly override the arch."""
     with temp_base() as basedir:
-      result = run_tests(basedir, '--mode=Release', '--arch=ia32')
+      result = run_tests(basedir, '--arch=ia32')
       self.assertIn(
           '--arch value (ia32) inconsistent with build config (x64).',
           result.stdout, result)
@@ -433,13 +414,13 @@ class SystemTest(unittest.TestCase):
   def testWrongVariant(self):
     """Test using a bogus variant."""
     with temp_base() as basedir:
-      result = run_tests(basedir, '--mode=Release', '--variants=meh')
+      result = run_tests(basedir, '--variants=meh')
       self.assertEqual(5, result.returncode, result)
 
   def testModeFromBuildConfig(self):
     """Test auto-detection of mode from build config."""
     with temp_base() as basedir:
-      result = run_tests(basedir, '--outdir=out/Release', 'sweet/bananas')
+      result = run_tests(basedir, '--outdir=out/build', 'sweet/bananas')
       self.assertIn('Running tests for x64.release', result.stdout, result)
       self.assertEqual(0, result.returncode, result)
 
@@ -452,7 +433,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--variants=default',
           'sweet',
           '--report',
@@ -468,7 +448,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--variants=default,nooptimization',
           'sweet',
           '--warn-unused',
@@ -483,7 +462,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--variants=default',
           'sweet/bananas',
           '--cat',
@@ -502,7 +480,6 @@ class SystemTest(unittest.TestCase):
       override_build_config(basedir, v8_enable_verify_predictable=True)
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           'sweet/bananas',
@@ -520,7 +497,6 @@ class SystemTest(unittest.TestCase):
       override_build_config(basedir, v8_target_cpu='arm64')
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           'sweet/bananas',
@@ -534,7 +510,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           '--random-seed-stress-count=2',
@@ -549,7 +524,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           '--random-seed-stress-count=2',
@@ -573,7 +547,6 @@ class SystemTest(unittest.TestCase):
       override_build_config(basedir, is_asan=True)
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default,stress',
           'sweet/bananas',
@@ -595,7 +568,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=dots',
           'sweet/cherries',
           'sweet/bananas',
@@ -616,7 +588,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=%s' % name,
           'sweet/cherries',
           'sweet/bananas',
@@ -637,7 +608,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--exit-after-n-failures=2',
           '-j1',
@@ -656,7 +626,7 @@ class SystemTest(unittest.TestCase):
       self.assertEqual(1, result.returncode, result)
 
   def testNumFuzzer(self):
-    sys_args = ['--command-prefix', sys.executable, '--outdir', 'out/Release']
+    sys_args = ['--command-prefix', sys.executable, '--outdir', 'out/build']
 
     with temp_base() as basedir:
       with capture() as (stdout, stderr):
@@ -670,7 +640,6 @@ class SystemTest(unittest.TestCase):
     with temp_base() as basedir:
       result = run_tests(
           basedir,
-          '--mode=Release',
           '--progress=verbose',
           '--variants=default',
           '--random-seed=42',

tools/unittests/testdata/expected_test_results1.json

@@ -4,7 +4,7 @@
   "mode": "release", 
   "results": [
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [
@@ -29,7 +29,7 @@
       "variant_flags": []
     }, 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [
@@ -54,7 +54,7 @@
       "variant_flags": []
     }, 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [
@@ -81,7 +81,7 @@
   ], 
   "slowest_tests": [ 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [
@@ -105,7 +105,7 @@
       "variant_flags": []
     }, 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [
@@ -129,7 +129,7 @@
       "variant_flags": []
     }, 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py --test strawberries --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [

tools/unittests/testdata/expected_test_results2.json

@@ -4,7 +4,7 @@
   "mode": "release", 
   "results": [
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [
@@ -28,7 +28,7 @@
       "variant_flags": []
     }, 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 0, 
       "expected": [
@@ -54,7 +54,7 @@
   ], 
   "slowest_tests": [
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 0, 
       "expected": [
@@ -77,7 +77,7 @@
       "variant_flags": []
     }, 
     {
-      "command": "/usr/bin/python out/Release/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
+      "command": "/usr/bin/python out/build/d8_mocked.py bananaflakes --random-seed=123 --nohard-abort --testing-d8-test-runner", 
       "duration": 1, 
       "exit_code": 1, 
       "expected": [