Change test wrapper script arg handling.

Previously, any isolated script test that ran on a bot
required the `--isolated-script-test-output` flag to be
passed to it.

This CL makes that flag optional.

This was made mandatory originally in order to ensure that
it was being called correctly; however, making it mandatory
means that devs have to set it when using the script locally,
even if they don't care about the output. Fixing the latter
problem seems worth not catching the former at this point;
it seems unlikely we'll introduce a bug in the recipes that
the mandatory flag check would catch.

Also, if you wanted to use a generated bin/run_ script
that contained an argument that would otherwise be expanded
by swarming (something like `--system-log=$ISOLATED_OUTDIR/system.log`),
this CL will allow that to now be embedded into the bin/run_
wrapper. If ISOLATED_OUTDIR is not set locally (and it usually
won't be), then the arg will be dropped.

These two changes in conjunction should allow us to move
all of the command line arguments that are essentially
compile-time-static into the bin/run_ wrapper (thus further
simplifying what is configured in gn_isolate_map.pyl and the
//testing/buildbot/*.pyl files) while still preserving dev-friendliness.

That said, this CL itself should introduce no functional changes
other than the --isolated-script-test-output flag no longer being required.

Bug: 816629
Change-Id: Ic02d2acc44c4a305dceff1fcab50cfe48d515b8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462070
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Luke Z <lpz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821787}

build/util/generate_wrapper.py

@@ -88,10 +88,65 @@ PY_TEMPLATE = textwrap.dedent("""\
       return args
 
 
+    def FindIsolatedOutdir(raw_args):
+      outdir = None
+      i = 0
+      remaining_args = []
+      while i < len(raw_args):
+        if raw_args[i] == '--isolated-outdir' and i < len(raw_args)-1:
+          outdir = raw_args[i+1]
+          i += 2
+        elif raw_args[i].startswith('--isolated-outdir='):
+          outdir = raw_args[i][len('--isolated-outdir='):]
+          i += 1
+        else:
+          remaining_args.append(raw_args[i])
+          i += 1
+      if not outdir and 'ISOLATED_OUTDIR' in os.environ:
+        outdir = os.environ['ISOLATED_OUTDIR']
+      return outdir, remaining_args
+
+
+    def FilterIsolatedOutdirBasedArgs(outdir, args):
+      rargs = []
+      i = 0
+      while i < len(args):
+        if 'ISOLATED_OUTDIR' in args[i]:
+          if outdir:
+            # Rewrite the arg.
+            rargs.append(args[i].replace('${{ISOLATED_OUTDIR}}',
+                                         outdir).replace(
+              '$ISOLATED_OUTDIR', outdir))
+            i += 1
+          else:
+            # Simply drop the arg.
+            i += 1
+        elif (not outdir and
+              args[i].startswith('-') and
+              '=' not in args[i] and
+              i < len(args) - 1 and
+              'ISOLATED_OUTDIR' in args[i+1]):
+          # Parsing this case is ambiguous; if we're given
+          # `--foo $ISOLATED_OUTDIR` we can't tell if $ISOLATED_OUTDIR
+          # is meant to be the value of foo, or if foo takes no argument
+          # and $ISOLATED_OUTDIR is the first positional arg.
+          #
+          # We assume the former will be much more common, and so we
+          # need to drop --foo and $ISOLATED_OUTDIR.
+          i += 2
+        else:
+          rargs.append(args[i])
+          i += 1
+      return rargs
+
+
     def main(raw_args):
       executable_path = ExpandWrappedPath('{executable_path}')
-      executable_args = ExpandWrappedPaths({executable_args})
-      cmd = [executable_path] + executable_args + raw_args
+      outdir, remaining_args = FindIsolatedOutdir(raw_args)
+      args = {executable_args}
+      args = FilterIsolatedOutdirBasedArgs(outdir, args)
+      executable_args = ExpandWrappedPaths(args)
+      cmd = [executable_path] + args + remaining_args
       if executable_path.endswith('.py'):
         cmd = [sys.executable] + cmd
       return subprocess.call(cmd)

testing/scripts/common.py

@@ -210,11 +210,11 @@ class BaseIsolatedScriptArgsAdapter(object):
     self._rest_args = None
     self._parser.add_argument(
         '--isolated-outdir', type=str,
-        required=False, # TODO(crbug.com/816629): Make this be required.
+        required=False,
         help='value of $ISOLATED_OUTDIR from swarming task')
     self._parser.add_argument(
         '--isolated-script-test-output', type=str,
-        required=True,
+        required=False,
         help='path to write test results JSON object to')
     self._parser.add_argument(
         '--isolated-script-test-filter', type=str,