Reland "Reland "generate_wrapper targets for iOS' non-EG/EG2 tests, EG, and EG2 tests""

This is a reland of 4ed2234c

Original change's description:
> generate_wrapper targets for iOS' non-EG/EG2 tests, EG, and EG2 tests
>
> The existing iOS recipe utilizes an isolate template, and an
> isolated.gen.json file to fill the template in per test with
> config-variables and path-variables. This isolate template contains
> the command arguments to invoke the iOS test runner.
>
> To support the iOS recipe migration to the Chromium recipe, I use GN to
> generate a script per test target instead of the isolate template. The
> script is pre-baked with arguments to the test runner (run.py). The
> given Swarming task is expected to execute the test by invoking the
> generated script.
>
> Invoking the generated script requires changes to gn_isolate_map.pyl
> for each iOS test target, and thus, these changes are only additive.
>
> * "ios_test_runner_wrapper" template wraps the generate_wrapper target.
>  It bakes iOS specific arguments into the script, such that it can
>  launch run.py with the given args.
> * ios_eg2_test, ios_eg_test, and test templates have
>  been updated to utilize the ios_test_runner_wrapper for iOS targets.
> * mb.py to skip generated_directory check when writing isolates.
>  View crbug/ in comments.
>
> Change-Id: I941d37dfdff0bd1ead1baff70907a197fee3fe64
> Bug: 1055328,912681
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023748
> Commit-Queue: Jeff Yoon <jeffyoon@chromium.org>
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#745105}

Bug: 1055328, 912681
Change-Id: Ie7979979365b570369e6b6f750cc9f3006024564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2088345Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Jeff Yoon <jeffyoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747797}

build/config/ios/ios_test_runner_config.gni

+# Copyright 2020 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+declare_args() {
+  # Controls what version of xcode to use for iOS testers. Note that this
+  # version should be in alignment with the swarming named caches used by tests,
+  # defined under src/testing/buildbot/ to avoid performance issues.
+  test_runner_xcode_build_version = "11c29"
+}

build/config/ios/ios_test_runner_wrapper.gni

+# Copyright 2020 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import("//build/config/ios/ios_sdk.gni")
+import("//build/config/ios/ios_test_runner_config.gni")
+import("//build/util/generate_wrapper.gni")
+
+# Invokes generate_wrapper to create an executable script wrapping iOS'
+# run.py with baked in arguments. Only takes effect when test entry in
+# gn_isolate_map.pyl is updated to type="generated_script" with script
+# set to the wrapper output path.
+#
+# Arguments:
+#
+# data
+#   (optional, default [ "//ios/build/bots/scripts/" ]) list of files or
+#   directories required to run target
+#
+# data_deps
+#   (optional) list of target non-linked labels
+#
+# deps
+#   (optional) list of files or directories required to run target
+#
+# executable_args
+#   (optional) a list of string arguments to pass to run.py
+#
+# retries
+#   (optional, default 3) number of retry attempts
+#
+# shards
+#   (optional, default 1) number of shards to execute tests in parallel. not
+#   the same as swamring shards.
+#
+# wrapper_output_name
+#   (optional, default "run_${target_name}") name of the wrapper script
+#
+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
+
+    # iOS main test runner
+    executable = "//ios/build/bots/scripts/run.py"
+
+    # arguments passed to run.py
+    if (!defined(executable_args)) {
+      executable_args = []
+    }
+
+    _rebased_mac_toolchain = rebase_path("//mac_toolchain", root_build_dir)
+    _rebased_xcode_path = rebase_path("//Xcode.app", root_build_dir)
+
+    # --out-dir argument is specified in gn_isolate_map.pyl because
+    # ${ISOLATED_OUTDIR} doesn't get resolved through this wrapper.
+    executable_args += [
+      "--xcode-path",
+      "@WrappedPath(${_rebased_xcode_path})",
+      "--mac-toolchain-cmd",
+      "@WrappedPath(${_rebased_mac_toolchain})",
+    ]
+
+    executable_args += [
+      "--xcode-build-version",
+      test_runner_xcode_build_version,
+    ]
+
+    # Default retries to 3
+    if (!defined(retries)) {
+      retries = 3
+    }
+    executable_args += [
+      "--retries",
+      "${retries}",
+    ]
+
+    # Default shards to 1
+    if (!defined(shards)) {
+      shards = 1
+    }
+    executable_args += [
+      "--shards",
+      "${shards}",
+    ]
+
+    # 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 += [
+        "--iossim",
+        "@WrappedPath(${_rebased_root_build_dir}/iossim)",
+      ]
+    }
+
+    # wrapper script output name and path
+    if (!defined(wrapper_output_name)) {
+      _wrapper_output_name = "run_${target_name}"
+    } else {
+      _wrapper_output_name = wrapper_output_name
+    }
+    wrapper_script = "${root_build_dir}/bin/${_wrapper_output_name}"
+
+    # ios/build/bot/scripts/*.py needs to be present for test runner
+    if (!defined(data)) {
+      data = []
+    }
+    data += [ "//ios/build/bots/scripts/" ]
+  }
+}

build/config/ios/rules.gni

@@ -1723,10 +1723,14 @@ template("ios_xctest_bundle") {
         # For XCTest, Xcode requires specifying the host application path via
         # both BUNDLE_LOADER and TEST_HOST attributes.
         if (invoker.product_type == _ios_xcode_xctest_bundle_id) {
+          _xcode_app_name = invoker.xcode_test_application_name
+          if (defined(invoker.xcode_test_application_output_name)) {
+            _xcode_app_name = invoker.xcode_test_application_output_name
+          }
+
           BUNDLE_LOADER = "\$(TEST_HOST)"
           TEST_HOST =
-              "\$(BUILT_PRODUCTS_DIR)/${invoker.xcode_test_application_name}" +
-              ".app/${invoker.xcode_test_application_name}"
+              "\$(BUILT_PRODUCTS_DIR)/${_xcode_app_name}.app/${_xcode_app_name}"
         }
       }
 
@@ -1786,7 +1790,13 @@ template("ios_xctest_test") {
     output_name = _xctest_output
     product_type = _ios_xcode_xctest_bundle_id
     host_target = _host_target
-    xcode_test_application_name = _host_output
+
+    # TODO(crbug.com/1056328) The change in output name results in a mismatch
+    # between this value and the ios_app_bundle target name. To mitigate, this
+    # has been modified to _host_target. output_name is set to _host_output
+    # to mitigate the naming.
+    xcode_test_application_name = _host_target
+    xcode_test_application_output_name = _host_output
 
     deps = [ _xctest_module_target ]
   }
@@ -1975,10 +1985,14 @@ template("ios_xcuitest_test") {
          "xcode_test_application_name must be defined for $target_name")
 
   _xcuitest_target = target_name
+  if (defined(invoker.output_name)) {
+    _xcuitest_target = invoker.output_name
+  }
+
   _xcuitest_runner_target = _xcuitest_target + "_runner"
   _xcuitest_module_target = _xcuitest_target + "_module"
 
-  group(_xcuitest_target) {
+  group(target_name) {
     testonly = true
 
     deps = [ ":$_xcuitest_runner_target" ]

ios/chrome/test/earl_grey2/chrome_ios_eg2_test.gni

@@ -138,9 +138,12 @@ template("chrome_ios_eg2_test") {
     forward_variables_from(invoker,
                            [
                              "bundle_deps",
-                             "xcode_test_application_name",
                              "deps",
                              "data_deps",
+                             "executable_args",
+                             "retries",
+                             "shards",
+                             "xcode_test_application_name",
                            ])
 
     xctest_bundle_principal_class = "ChromeEGTestBundleMain"

ios/third_party/earl_grey/ios_eg_test.gni

@@ -2,14 +2,25 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/ios/ios_test_runner_wrapper.gni")
 import("//build/config/ios/rules.gni")
 import("//ios/build/chrome_build.gni")
 import("//ios/public/provider/chrome/browser/build_config.gni")
 
 # EarlGrey tests are just XCTests that also depends on EarlGrey.
 template("ios_eg_test") {
-  ios_xctest_test(target_name) {
-    forward_variables_from(invoker, "*")
+  _target_name = target_name
+  _test_target = "${target_name}_test"
+  ios_xctest_test(_test_target) {
+    forward_variables_from(invoker,
+                           "*",
+                           [
+                             "executable_args",
+                             "retries",
+                             "shards",
+                             "xctest",
+                             "xcode_parallelization",
+                           ])
     if (!defined(bundle_deps)) {
       bundle_deps = []
     }
@@ -28,6 +39,55 @@ template("ios_eg_test") {
       "//ios/third_party/earl_grey:earl_grey+link",
       "//ios/third_party/ochamcrest:ochamcrest+link",
     ]
+
+    # TODO(crbug.com/1056328) Because we change the target name, the subnodes
+    # are going to append with the _test in the naming, which won't be backwards
+    # compatible during migration from iOS recipe to Chromium.
+    output_name = "${_target_name}"
+  }
+
+  ios_test_runner_wrapper(target_name) {
+    forward_variables_from(invoker,
+                           [
+                             "data",
+                             "data_deps",
+                             "deps",
+                             "executable_args",
+                             "retries",
+                             "shards",
+                             "xctest",
+                             "xcode_parallelization",
+                           ])
+
+    _root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
+
+    # include the test target above as data_deps
+    if (!defined(data_deps)) {
+      data_deps = []
+    }
+    data_deps += [ ":${_test_target}" ]
+
+    if (!defined(executable_args)) {
+      executable_args = []
+    }
+
+    # EG test apps are *.app format. No host, but required xctest and may need
+    # xcode-parallelization
+    executable_args += [
+      "--app",
+      "@WrappedPath(${_root_build_dir}/${target_name}.app)",
+    ]
+
+    # Default xctest to true for EG tests. If set, only set arg if value = true
+    if (!defined(xctest) || (defined(xctest) && xctest)) {
+      executable_args += [ "--xctest" ]
+    }
+
+    # Default xcode_parallelization to true for EG tests.
+    if (!defined(xcode_parallelization) ||
+        (defined(xcode_parallelization) && xcode_parallelization)) {
+      executable_args += [ "--xcode-parallelization" ]
+    }
   }
 }
 

ios/third_party/earl_grey2/ios_eg2_test.gni

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/ios/ios_test_runner_wrapper.gni")
 import("//build/config/ios/rules.gni")
 import("//ios/build/chrome_build.gni")
 import("//ios/public/provider/chrome/browser/build_config.gni")
@@ -62,7 +63,9 @@ template("ios_eg2_test") {
       defined(invoker.deps),
       "deps must be defined for $target_name to include at least one earl grey test file.")
 
-  ios_xcuitest_test(target_name) {
+  _target_name = target_name
+  _test_target = "${target_name}_test"
+  ios_xcuitest_test(_test_target) {
     forward_variables_from(invoker,
                            [
                              "xcode_test_application_name",
@@ -76,6 +79,51 @@ template("ios_eg2_test") {
       deps = []
     }
     deps += [ "//ios/third_party/earl_grey2:test_lib" ]
+
+    # TODO(crbug.com/1056328) Because we change the target name, the subnodes
+    # are going to append with the _test in the naming, which won't be backwards
+    # compatible during migration from iOS recipe to Chromium.
+    output_name = "${_target_name}"
+  }
+
+  ios_test_runner_wrapper(target_name) {
+    forward_variables_from(invoker,
+                           [
+                             "data",
+                             "data_deps",
+                             "deps",
+                             "executable_args",
+                             "retries",
+                             "shards",
+                             "xcode_test_application_name",
+                           ])
+    _root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
+
+    if (!defined(data_deps)) {
+      data_deps = []
+    }
+
+    # Include the top ios_eg2_test target, and the host app
+    data_deps += [ ":${_test_target}" ]
+
+    if (!defined(executable_args)) {
+      executable_args = []
+    }
+
+    # EG2 tests app are bundled as *-Runner.app, while the host app is bundled
+    # as *.app.
+    executable_args += [
+      "--app",
+      "@WrappedPath(${_root_build_dir}/${target_name}-Runner.app)",
+    ]
+    executable_args += [
+      "--host-app",
+      "@WrappedPath(${_root_build_dir}/${xcode_test_application_name}.app)",
+    ]
+
+    # All EG2 tests require xcode-parallelization. EG2 doesn't use xctest, so we
+    # leave undefined
+    executable_args += [ "--xcode-parallelization" ]
   }
 }
 

testing/iossim/BUILD.gn

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/ios/ios_sdk.gni")
 import("//build/config/mac/mac_sdk.gni")
 
 if (current_toolchain == host_toolchain) {
@@ -16,4 +17,8 @@ if (current_toolchain == host_toolchain) {
     sources = [ get_label_info(":iossim($host_toolchain)", "root_out_dir") +
                 "/iossim" ]
   }
+} else {
+  group("iossim") {
+    public_deps = [ ":iossim($default_toolchain)" ]
+  }
 }

testing/test.gni

@@ -23,6 +23,12 @@ if (is_chromeos) {
   import("//build/config/chromeos/rules.gni")
 }
 
+if (is_ios) {
+  import("//build/config/ios/ios_sdk.gni")
+  import("//build/config/ios/ios_test_runner_wrapper.gni")
+  import("//build/config/ios/rules.gni")
+}
+
 # Define a test as an executable (or apk on Android) with the "testonly" flag
 # set.
 # Variable:
@@ -227,9 +233,6 @@ template("test") {
       output_name = _exec_target
     }
   } else if (is_ios) {
-    import("//build/config/ios/ios_sdk.gni")
-    import("//build/config/ios/rules.gni")
-
     declare_args() {
       # Keep the unittest-as-xctest functionality defaulted to off until the
       # bots are updated to handle it properly.
@@ -239,6 +242,31 @@ template("test") {
     }
 
     _test_target = target_name
+    _wrapper_output_name = "run_${target_name}"
+    ios_test_runner_wrapper(_wrapper_output_name) {
+      forward_variables_from(invoker,
+                             [
+                               "data",
+                               "data_deps",
+                               "deps",
+                               "executable_args",
+                               "retries",
+                               "shards",
+                             ])
+
+      _root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
+
+      if (!defined(executable_args)) {
+        executable_args = []
+      }
+      executable_args += [
+        "--app",
+        "@WrappedPath(${_root_build_dir}/${_test_target}.app)",
+      ]
+
+      wrapper_output_name = "${_wrapper_output_name}"
+    }
+
     _resources_bundle_data = target_name + "_resources_bundle_data"
 
     bundle_data(_resources_bundle_data) {
@@ -281,6 +309,13 @@ template("test") {
         bundle_deps = []
       }
       bundle_deps += [ ":$_resources_bundle_data" ]
+
+      if (!defined(data_deps)) {
+        data_deps = []
+      }
+
+      # Include the generate_wrapper as part of data_deps
+      data_deps += [ ":${_wrapper_output_name}" ]
     }
   } else if (is_chromeos && cros_board != "") {
     # Building for a cros board (ie: not linux-chromeos).

tools/mb/mb.py

@@ -1310,6 +1310,7 @@ class MetaBuildWrapper(object):
                vals.get('cros_passthrough', False))
     is_mac = self.platform == 'darwin'
     is_msan = 'is_msan=true' in vals['gn_args']
+    is_ios = 'target_os="ios"' in vals['gn_args']
 
     err = ''
     for f in files:
@@ -1319,6 +1320,11 @@ class MetaBuildWrapper(object):
       if is_android:
         break
 
+      # iOS has generated directories in gn data items.
+      # Skipping for iOS instead of listing all apps.
+      if is_ios:
+        break
+
       # Skip a few existing violations that need to be cleaned up. Each of
       # these will lead to incorrect incremental builds if their directory
       # contents change. Do not add to this list.