[foozzie] Refactor argument abstraction

This adds an abstraction for command-line arguments for each of the
two comparison runs done in correctness fuzzing. No functional
changes intended.

No-Try: true
Bug: chromium:1023091
Change-Id: I9421715c4904416b9aaf53848954a5248c79ffd9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1906372
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Tamer Tas <tmrts@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64979}

tools/clusterfuzz/v8_commands.py

@@ -43,7 +43,7 @@ TIMEOUT = 3
 def _startup_files(options):
   """Default files and optional architecture-specific mock file."""
   files = DEFAULT_FILES[:]
-  if options.first_arch != options.second_arch:
+  if options.first.arch != options.second.arch:
     files.append(ARCH_MOCKS)
   return files
 

tools/clusterfuzz/v8_foozzie.py

@@ -20,6 +20,8 @@ import re
 import sys
 import traceback
 
+from collections import namedtuple
+
 import v8_commands
 import v8_suppressions
 
@@ -151,34 +153,71 @@ def infer_arch(d8):
   """
   with open(os.path.join(os.path.dirname(d8), 'v8_build_config.json')) as f:
     arch = json.load(f)['v8_current_cpu']
-  return 'ia32' if arch == 'x86' else arch
+  arch = 'ia32' if arch == 'x86' else arch
+  assert arch in SUPPORTED_ARCHS
+  return arch
+
+
+class ExecutionArgumentsConfig(object):
+  def __init__(self, label):
+    self.label = label
+
+  def add_arguments(self, parser, default_config):
+    def add_argument(flag_template, help_template, **kwargs):
+      parser.add_argument(
+          flag_template % self.label,
+          help=help_template % self.label,
+          **kwargs)
+
+    add_argument(
+        '--%s-config',
+        '%s configuration',
+        default=default_config)
+    add_argument(
+        '--%s-config-extra-flags',
+        'additional flags passed to the %s run',
+        action='append',
+        default=[])
+    add_argument(
+        '--%s-d8',
+        'optional path to %s d8 executable, '
+        'default: bundled in the directory of this script',
+        default='d8')
+
+  def make_options(self, options):
+    def get(name):
+      return getattr(options, '%s_%s' % (self.label, name))
+
+    config = get('config')
+    assert config in CONFIGS
+
+    d8 = get('d8')
+    if not os.path.isabs(d8):
+      d8 = os.path.join(BASE_PATH, d8)
+    assert os.path.exists(d8)
+
+    flags = CONFIGS[config] + get('config_extra_flags')
+
+    RunOptions = namedtuple('RunOptions', ['arch', 'config', 'd8', 'flags'])
+    return RunOptions(infer_arch(d8), config, d8, flags)
 
 
 def parse_args():
+  first_config_arguments = ExecutionArgumentsConfig('first')
+  second_config_arguments = ExecutionArgumentsConfig('second')
+
   parser = argparse.ArgumentParser()
   parser.add_argument(
     '--random-seed', type=int, required=True,
     help='random seed passed to both runs')
-  parser.add_argument(
-      '--first-config', help='first configuration', default='ignition')
-  parser.add_argument(
-      '--second-config', help='second configuration', default='ignition_turbo')
-  parser.add_argument(
-      '--first-config-extra-flags', action='append', default=[],
-      help='Additional flags to pass to the run of the first configuration')
-  parser.add_argument(
-      '--second-config-extra-flags', action='append', default=[],
-      help='Additional flags to pass to the run of the second configuration')
-  parser.add_argument(
-      '--first-d8', default='d8',
-      help='optional path to first d8 executable, '
-           'default: bundled in the same directory as this script')
-  parser.add_argument(
-      '--second-d8',
-      help='optional path to second d8 executable, default: same as first')
   parser.add_argument(
       '--skip-sanity-checks', default=False, action='store_true',
       help='skip sanity checks for testing purposes')
+
+  # Add arguments for each run configuration.
+  first_config_arguments.add_arguments(parser, 'ignition')
+  second_config_arguments.add_arguments(parser, 'ignition_turbo')
+
   parser.add_argument('testcase', help='path to test case')
   options = parser.parse_args()
 
@@ -187,33 +226,14 @@ def parse_args():
           os.path.isfile(options.testcase)), (
       'Test case %s doesn\'t exist' % options.testcase)
 
-  # Use first d8 as default for second d8.
-  options.second_d8 = options.second_d8 or options.first_d8
-
-  # Ensure absolute paths.
-  if not os.path.isabs(options.first_d8):
-    options.first_d8 = os.path.join(BASE_PATH, options.first_d8)
-  if not os.path.isabs(options.second_d8):
-    options.second_d8 = os.path.join(BASE_PATH, options.second_d8)
-
-  # Ensure executables exist.
-  assert os.path.exists(options.first_d8)
-  assert os.path.exists(options.second_d8)
+  options.first = first_config_arguments.make_options(options)
+  options.second = second_config_arguments.make_options(options)
 
   # Ensure we make a sane comparison.
-  if (options.first_d8 == options.second_d8 and
-      options.first_config == options.second_config):
+  if (options.first.d8 == options.second.d8 and
+      options.first.config == options.second.config):
     parser.error('Need either executable or config difference.')
 
-  # Infer architecture from build artifacts.
-  options.first_arch = infer_arch(options.first_d8)
-  options.second_arch = infer_arch(options.second_d8)
-
-  assert options.first_arch in SUPPORTED_ARCHS
-  assert options.second_arch in SUPPORTED_ARCHS
-  assert options.first_config in CONFIGS
-  assert options.second_config in CONFIGS
-
   return options
 
 
@@ -265,8 +285,8 @@ def print_difference(
     first_config_output, second_config_output, difference, source=None):
   # The first three entries will be parsed by clusterfuzz. Format changes
   # will require changes on the clusterfuzz side.
-  first_config_label = '%s,%s' % (options.first_arch, options.first_config)
-  second_config_label = '%s,%s' % (options.second_arch, options.second_config)
+  first_config_label = '%s,%s' % (options.first.arch, options.first.config)
+  second_config_label = '%s,%s' % (options.second.arch, options.second.config)
   source_file_text = SOURCE_FILE_TEMPLATE % source if source else ''
   print((FAILURE_TEMPLATE % dict(
       configs='%s:%s' % (first_config_label, second_config_label),
@@ -291,8 +311,8 @@ def main():
 
   # Suppressions are architecture and configuration specific.
   suppress = v8_suppressions.get_suppression(
-      options.first_arch, options.first_config,
-      options.second_arch, options.second_config,
+      options.first.arch, options.first.config,
+      options.second.arch, options.second.config,
   )
 
   # Static bailout based on test case content or metadata.
@@ -303,16 +323,10 @@ def main():
   if content_bailout(content, suppress.ignore_by_content):
     return RETURN_FAIL
 
-  # Set up runtime arguments.
-  first_config_flags = (CONFIGS[options.first_config] +
-                        options.first_config_extra_flags)
-  second_config_flags = (CONFIGS[options.second_config] +
-                         options.second_config_extra_flags)
-
   first_cmd = v8_commands.Command(
-      options,'first', options.first_d8, first_config_flags)
+      options,'first', options.first.d8, options.first.flags)
   second_cmd = v8_commands.Command(
-      options, 'second', options.second_d8, second_config_flags)
+      options, 'second', options.second.d8, options.second.flags)
 
   # Sanity checks. Run both configurations with the sanity-checks file only and
   # bail out early if different.