Skip to content

Commit 608b732

Browse files
mi-acCommit Bot
mi-ac
authored and
Commit Bot
committed
[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/+/2450016 Reviewed-by: Jakob Kummerow <[email protected]> Commit-Queue: Michael Achenbach <[email protected]> Cr-Commit-Position: refs/heads/master@{#70363}
1 parent 89af4ca commit 608b732

4 files changed

Lines changed: 51 additions & 145 deletions

File tree

‎tools/testrunner/base_runner.py‎

Lines changed: 34 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from __future__ import print_function
77
from functools import reduce
88

9-
from collections import OrderedDict
9+
from collections import OrderedDict, namedtuple
1010
import json
1111
import multiprocessing
1212
import optparse
@@ -116,52 +116,38 @@
116116
]
117117

118118

119-
class ModeConfig(object):
120-
def __init__(self, flags, timeout_scalefactor, status_mode, execution_mode):
121-
self.flags = flags
122-
self.timeout_scalefactor = timeout_scalefactor
123-
self.status_mode = status_mode
124-
self.execution_mode = execution_mode
125-
119+
ModeConfig = namedtuple(
120+
'ModeConfig', 'label flags timeout_scalefactor status_mode execution_mode')
126121

127122
DEBUG_FLAGS = ["--nohard-abort", "--enable-slow-asserts", "--verify-heap"]
128123
RELEASE_FLAGS = ["--nohard-abort"]
129-
MODES = {
130-
"debug": ModeConfig(
131-
flags=DEBUG_FLAGS,
132-
timeout_scalefactor=4,
133-
status_mode="debug",
134-
execution_mode="debug",
135-
),
136-
"optdebug": ModeConfig(
124+
125+
DEBUG_MODE = ModeConfig(
126+
label='debug',
137127
flags=DEBUG_FLAGS,
138128
timeout_scalefactor=4,
139129
status_mode="debug",
140130
execution_mode="debug",
141-
),
142-
"release": ModeConfig(
131+
)
132+
133+
RELEASE_MODE = ModeConfig(
134+
label='release',
143135
flags=RELEASE_FLAGS,
144136
timeout_scalefactor=1,
145137
status_mode="release",
146138
execution_mode="release",
147-
),
148-
# Normal trybot release configuration. There, dchecks are always on which
149-
# implies debug is set. Hence, the status file needs to assume debug-like
150-
# behavior/timeouts.
151-
"tryrelease": ModeConfig(
139+
)
140+
141+
# Normal trybot release configuration. There, dchecks are always on which
142+
# implies debug is set. Hence, the status file needs to assume debug-like
143+
# behavior/timeouts.
144+
TRY_RELEASE_MODE = ModeConfig(
145+
label='release+dchecks',
152146
flags=RELEASE_FLAGS,
153-
timeout_scalefactor=1,
154-
status_mode="debug",
155-
execution_mode="release",
156-
),
157-
# This mode requires v8 to be compiled with dchecks and slow dchecks.
158-
"slowrelease": ModeConfig(
159-
flags=RELEASE_FLAGS + ["--enable-slow-asserts"],
160-
timeout_scalefactor=2,
147+
timeout_scalefactor=4,
161148
status_mode="debug",
162149
execution_mode="release",
163-
),
164-
}
150+
)
165151

166152
PROGRESS_INDICATORS = {
167153
'verbose': progress.VerboseProgressIndicator,
@@ -268,7 +254,6 @@ def __init__(self, basedir=None):
268254
self.basedir = basedir or BASE_DIR
269255
self.outdir = None
270256
self.build_config = None
271-
self.mode_name = None
272257
self.mode_options = None
273258
self.target_os = None
274259

@@ -305,7 +290,7 @@ def execute(self, sys_args=None):
305290
tests = self._load_testsuite_generators(args, options)
306291
self._setup_env()
307292
print(">>> Running tests for %s.%s" % (self.build_config.arch,
308-
self.mode_name))
293+
self.mode_options.label))
309294
exit_code = self._do_execute(tests, args, options)
310295
if exit_code == utils.EXIT_CODE_FAILURES and options.json_test_results:
311296
print("Force exit code 0 after failures. Json test results file "
@@ -339,9 +324,6 @@ def _add_parser_default_options(self, parser):
339324
default="out")
340325
parser.add_option("--arch",
341326
help="The architecture to run tests for")
342-
parser.add_option("-m", "--mode",
343-
help="The test mode in which to run (uppercase for builds"
344-
" in CI): %s" % MODES.keys())
345327
parser.add_option("--shell-dir", help="DEPRECATED! Executables from build "
346328
"directory will be used")
347329
parser.add_option("--test-root", help="Root directory of the test suites",
@@ -427,9 +409,8 @@ def _add_parser_options(self, parser):
427409
def _parse_args(self, parser, sys_args):
428410
options, args = parser.parse_args(sys_args)
429411

430-
if any(map(lambda v: v and ',' in v,
431-
[options.arch, options.mode])): # pragma: no cover
432-
print('Multiple arch/mode are deprecated')
412+
if options.arch and ',' in options.arch: # pragma: no cover
413+
print('Multiple architectures are deprecated')
433414
raise TestRunnerError()
434415

435416
return options, args
@@ -465,8 +446,7 @@ def _load_build_config(self, options):
465446
# Returns possible build paths in order:
466447
# gn
467448
# outdir
468-
# outdir/arch.mode
469-
# Each path is provided in two versions: <path> and <path>/mode for bots.
449+
# outdir on bots
470450
def _possible_outdirs(self, options):
471451
def outdirs():
472452
if options.gn:
@@ -475,30 +455,12 @@ def outdirs():
475455

476456
yield options.outdir
477457

478-
# TODO(machenbach): Temporary fallback to legacy outdir. The
479-
# infrastructure switches to the canonical out/build location. But
480-
# bisection will keep working with out/Release or out/Debug for a
481-
# grace period.
482-
if os.path.basename(options.outdir) == 'build':
483-
base_dir = os.path.dirname(options.outdir)
484-
release_dir = os.path.join(base_dir, 'Release')
485-
debug_dir = os.path.join(base_dir, 'Debug')
486-
if os.path.exists(release_dir) and not os.path.exists(debug_dir):
487-
yield release_dir
488-
if os.path.exists(debug_dir) and not os.path.exists(release_dir):
489-
yield debug_dir
490-
491-
if options.arch and options.mode:
492-
yield os.path.join(options.outdir,
493-
'%s.%s' % (options.arch, options.mode))
458+
if os.path.basename(options.outdir) != 'build':
459+
yield os.path.join(options.outdir, 'build')
494460

495461
for outdir in outdirs():
496462
yield os.path.join(self.basedir, outdir)
497463

498-
# bot option
499-
if options.mode:
500-
yield os.path.join(self.basedir, outdir, options.mode)
501-
502464
def _get_gn_outdir(self):
503465
gn_out_dir = os.path.join(self.basedir, DEFAULT_OUT_GN)
504466
latest_timestamp = -1
@@ -515,29 +477,12 @@ def _get_gn_outdir(self):
515477
return os.path.join(DEFAULT_OUT_GN, latest_config)
516478

517479
def _process_default_options(self, options):
518-
# We don't use the mode for more path-magic.
519-
# Therefore transform the bot mode here to fix build_config value.
520-
if options.mode:
521-
options.mode = self._bot_to_v8_mode(options.mode)
522-
523-
build_config_mode = 'debug' if self.build_config.is_debug else 'release'
524-
if options.mode:
525-
if options.mode not in MODES: # pragma: no cover
526-
print('%s mode is invalid' % options.mode)
527-
raise TestRunnerError()
528-
if MODES[options.mode].execution_mode != build_config_mode:
529-
print ('execution mode (%s) for %s is inconsistent with build config '
530-
'(%s)' % (
531-
MODES[options.mode].execution_mode,
532-
options.mode,
533-
build_config_mode))
534-
raise TestRunnerError()
535-
536-
self.mode_name = options.mode
480+
if self.build_config.is_debug:
481+
self.mode_options = DEBUG_MODE
482+
elif self.build_config.dcheck_always_on:
483+
self.mode_options = TRY_RELEASE_MODE
537484
else:
538-
self.mode_name = build_config_mode
539-
540-
self.mode_options = MODES[self.mode_name]
485+
self.mode_options = RELEASE_MODE
541486

542487
if options.arch and options.arch != self.build_config.arch:
543488
print('--arch value (%s) inconsistent with build config (%s).' % (
@@ -558,15 +503,6 @@ def _process_default_options(self, options):
558503
options.command_prefix = shlex.split(options.command_prefix)
559504
options.extra_flags = sum(map(shlex.split, options.extra_flags), [])
560505

561-
def _bot_to_v8_mode(self, config):
562-
"""Convert build configs from bots to configs understood by the v8 runner.
563-
564-
V8 configs are always lower case and without the additional _x64 suffix
565-
for 64 bit builds on windows with ninja.
566-
"""
567-
mode = config[:-4] if config.endswith('_x64') else config
568-
return mode.lower()
569-
570506
def _process_options(self, options):
571507
pass
572508

@@ -714,9 +650,7 @@ def _get_statusfile_variables(self, options):
714650
"is_clang": self.build_config.is_clang,
715651
"is_full_debug": self.build_config.is_full_debug,
716652
"mips_arch_variant": mips_arch_variant,
717-
"mode": self.mode_options.status_mode
718-
if not self.build_config.dcheck_always_on
719-
else "debug",
653+
"mode": self.mode_options.status_mode,
720654
"msan": self.build_config.msan,
721655
"no_harness": options.no_harness,
722656
"no_i18n": self.build_config.no_i18n,
@@ -827,6 +761,9 @@ def _get_shard_info(self, options):
827761
def _create_progress_indicators(self, test_count, options):
828762
procs = [PROGRESS_INDICATORS[options.progress]()]
829763
if options.json_test_results:
764+
# TODO(machenbach): Deprecate the execution mode. Previously it was meant
765+
# to differentiate several records in the json output. But there is now
766+
# only one record and the mode information is redundant.
830767
procs.append(progress.JsonTestProgressIndicator(
831768
self.framework_name,
832769
self.build_config.arch,

0 commit comments

Comments
 (0)