Rework generated_script handling in MB.

It turns out the work I did on the "wrapped_*" targets in
gn_isolate_map was somewhat overlapping with the already-existing
iOS work to declare things as "generated_scripts". However, the
generated_scripts did not include test_env.py.

So, this CL rearranges things so that the generated script will call
test_env.py, and then we can share logic between the generated_script
targets (and every target on iOS and LaCrOS) and use that as the
basis for the rest of the work going forward.

Bug: 816629
Change-Id: I9d55c3f1587defcb599ddf1df838dd9ed5b6a12c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357917Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/master@{#799171}

build/config/ios/ios_test_runner_wrapper.gni

@@ -40,22 +40,23 @@ template("ios_test_runner_wrapper") {
   generate_wrapper(target_name) {
     forward_variables_from(invoker,
                            [
-                             "data",
-                             "data_deps",
                              "deps",
-                             "executable_args",
                              "retries",
                              "shards",
                              "wrapper_output_name",
                            ])
     testonly = true
+    executable = "//testing/test_env.py"
 
     # iOS main test runner
-    executable = "//ios/build/bots/scripts/run.py"
+    _runner_path =
+        rebase_path("//ios/build/bots/scripts/run.py", root_build_dir)
+
+    executable_args = [ "@WrappedPath(${_runner_path})" ]
 
     # arguments passed to run.py
-    if (!defined(executable_args)) {
-      executable_args = []
+    if (defined(invoker.executable_args)) {
+      executable_args += invoker.executable_args
     }
 
     _rebased_mac_toolchain = rebase_path("//mac_toolchain", root_build_dir)
@@ -88,12 +89,14 @@ template("ios_test_runner_wrapper") {
       "${shards}",
     ]
 
+    data_deps = []
+    if (defined(invoker.data_deps)) {
+      data_deps += invoker.data_deps
+    }
+
     # test runner relies on iossim if use_ios_simulator (defined in ios_sdk.gni)
     if (use_ios_simulator) {
       _rebased_root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
-      if (!defined(data_deps)) {
-        data_deps = []
-      }
       data_deps += [ "//testing/iossim" ]
 
       executable_args += [
@@ -123,10 +126,13 @@ template("ios_test_runner_wrapper") {
     # to ensure those wrappers are distinct.
     wrapper_script = "${root_out_dir}/bin/${_wrapper_output_name}"
 
-    # ios/build/bot/scripts/*.py needs to be present for test runner
-    if (!defined(data)) {
-      data = []
+    data = []
+    if (defined(invoker.data)) {
+      data += invoker.data
     }
-    data += [ "//ios/build/bots/scripts/" ]
+    data += [
+      "//.vpython",
+      "//ios/build/bots/scripts/",
+    ]
   }
 }

testing/buildbot/gn_isolate_map.pyl

@@ -52,17 +52,6 @@
 #  : (the default), which indicates that we don't know what the command line
 #    needs to be (this is a fatal error).
 
-# TODO(crbug.com/816629): You may also set the type field to "wrapped_" +
-# one of the above values; doing so indicates that the target conforms to
-# the test wrapper API: for a given label //X:Y and build directory Z,
-# there will be either a "Z/bin/run_Y" bash script (on Unix systems) or a
-# "Z/bin/run_Y.bat" batch script (on Windows) that can be invoked directly
-# to run the target in a swarming task, and "Z/Y.runtime_deps" file that
-# contains the list of files and directories needed to run the task, in
-# the format that GN uses with write_runtime_deps: a newline-separated list
-# of files and directories relative to Z. Ultimately every target will
-# be converted to this format, and this entire file will be deleted.
-#
 # The optional "executable" field can be used to override the name
 # of the binary to run. If the field is not specified, the binary
 # name will be assumed to be the same as the ninja build target name.
@@ -1747,7 +1736,7 @@
   },
   "url_unittests": {
     "label": "//url:url_unittests",
-    "type": "wrapped_console_test_launcher",
+    "type": "console_test_launcher",
   },
   "usage_time_limit_unittests": {
     "label": "//chrome/test:usage_time_limit_unittests",

tools/mb/mb.py

@@ -1149,25 +1149,19 @@ class MetaBuildWrapper(object):
 
     command, extra_files = self.GetIsolateCommand(target, vals)
 
-    # TODO(crbug.com/816629): Convert everything to wrapped_isolated_scripts
-    # and remove the else-branch of this.
-    if isolate_map[target]['type'] == 'wrapped_console_launcher':
-      runtime_deps = self.ReadFile(
-          self.PathJoin(build_dir, target + '.runtime_deps')).splitlines()
-    else:
-      # Any warning for an unused arg will get interleaved into the cmd's
-      # stdout. When that happens, the isolate step below will fail with an
-      # obscure error when it tries processing the lines of the warning. Fail
-      # quickly in that case to avoid confusion
-      cmd = self.GNCmd('desc', build_dir, label, 'runtime_deps',
-                       '--fail-on-unused-args')
-      ret, out, _ = self.Call(cmd)
-      if ret:
-        if out:
-          self.Print(out)
-        return ret
+    # Any warning for an unused arg will get interleaved into the cmd's
+    # stdout. When that happens, the isolate step below will fail with an
+    # obscure error when it tries processing the lines of the warning. Fail
+    # quickly in that case to avoid confusion
+    cmd = self.GNCmd('desc', build_dir, label, 'runtime_deps',
+                     '--fail-on-unused-args')
+    ret, out, _ = self.Call(cmd)
+    if ret:
+      if out:
+        self.Print(out)
+      return ret
 
-      runtime_deps = out.splitlines()
+    runtime_deps = out.splitlines()
 
     ret = self.WriteIsolateFiles(build_dir, command, target, runtime_deps, vals,
                                  extra_files)
@@ -1373,25 +1367,26 @@ class MetaBuildWrapper(object):
                or 'is_chromeos_device=true' in vals['gn_args'])
     is_cros_device = 'is_chromeos_device=true' in vals['gn_args']
     is_ios = 'target_os="ios"' in vals['gn_args']
-    is_linux = ('target_os="linux"' in vals['gn_args']
-                or (self.platform in ('linux', 'linux2') and not is_android
-                    and not is_fuchsia and not is_cros))
     is_mac = self.platform == 'darwin' and not is_ios
     is_win = self.platform == 'win32' or 'target_os="win"' in vals['gn_args']
     is_lacros = 'chromeos_is_browser_only=true' in vals['gn_args']
 
     test_type = isolate_map[target]['type']
-    if test_type.startswith('wrapped_') or is_lacros:
-      if is_mac or is_linux or is_lacros:
-        cmdline = ['bin/run_{}'.format(target)]
-        return cmdline, []
-      elif is_win:
-        cmdline = ['bin/run_{}.bat'.format(target)]
-        return cmdline, []
-      else:
-        test_type = test_type[len('wrapped_'):]
 
-    # TODO(crbug.com/816629): Convert everything to wrapped_isolated_scripts
+    if self.use_luci_auth:
+      cmdline = ['luci-auth.exe' if is_win else 'luci-auth', 'context', '--']
+    else:
+      cmdline = []
+
+    if test_type == 'generated_script' or is_ios or is_lacros:
+      script = isolate_map[target].get('script', 'bin/run_{}'.format(target))
+      if is_win:
+        script += '.bat'
+      cmdline += [script]
+      return cmdline, []
+
+
+    # TODO(crbug.com/816629): Convert all targets to generated_scripts
     # and delete the rest of this function.
 
     # This should be true if tests with type='windowed_test_launcher' are
@@ -1412,49 +1407,16 @@ class MetaBuildWrapper(object):
     clang_coverage = 'use_clang_coverage=true' in vals['gn_args']
     java_coverage = 'use_jacoco_coverage=true' in vals['gn_args']
 
-    use_python3 = isolate_map[target].get('use_python3', False)
-
     executable = isolate_map[target].get('executable', target)
     executable_suffix = isolate_map[target].get(
         'executable_suffix', '.exe' if is_win else '')
-
-    # TODO(crbug.com/1060857): Remove this once swarming task templates
-    # support command prefixes.
-    if self.use_luci_auth:
-      cmdline = ['luci-auth.exe' if is_win else 'luci-auth', 'context', '--']
-    else:
-      cmdline = []
-
-    if use_python3:
-      cmdline += ['vpython3']
-      extra_files = ['../../.vpython3']
-    else:
-      cmdline += ['vpython']
-      extra_files = ['../../.vpython']
+    cmdline += ['vpython']
+    extra_files = ['../../.vpython']
     extra_files += [
       '../../testing/test_env.py',
     ]
 
-    if test_type == 'nontest':
-      self.WriteFailureAndRaise('We should not be isolating %s.' % target,
-                                output_path=None)
-
-    if test_type == 'generated_script':
-      script = isolate_map[target]['script']
-      if self.platform == 'win32':
-        script += '.bat'
-      cmdline += [
-          '../../testing/test_env.py',
-          script,
-      ]
-    elif is_ios and test_type != "raw":
-      # iOS commands are all wrapped with generate_wrapper. Some targets
-      # shared with iOS aren't defined with generated_script (ie/ basic_
-      # unittests) so we force those to follow iOS' execution process by
-      # mimicking what generated_script would do
-      script = 'bin/run_{}'.format(target)
-      cmdline += ['../../testing/test_env.py', script]
-    elif is_android and test_type != 'script':
+    if is_android and test_type != 'script':
       if asan:
         cmdline += [os.path.join('bin', 'run_with_asan'), '--']
       cmdline += [