[go: nahoru, domu]

[lacros] Download and isolate ash-chrome on bots

Tests on CI/CQ bots should be hermetic, so download the and isolate
ash-chrome at build time instead of test time.

Bug: 1104318
Change-Id: Ia967cbc6fef205d30ec093ea4bfaadcc0eb46d62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307646
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Sven Zheng <svenzheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790606}
diff --git a/DEPS b/DEPS
index 6c4bcb4..e002099 100644
--- a/DEPS
+++ b/DEPS
@@ -154,6 +154,9 @@
   # qemu on linux-arm64 machines.
   'checkout_fuchsia_for_arm64_host': False,
 
+  # Download prebuilt ash-chrome to test lacros build.
+  'checkout_prebuilt_ash_chrome': False,
+
   # Default to the empty board. Desktop Chrome OS builds don't need cros SDK
   # dependencies. Other Chrome OS builds should always define this explicitly.
   'cros_boards': Str(''),
@@ -4938,6 +4941,16 @@
     ],
   },
 
+  {
+    'name': 'Download prebuilt ash-chrome',
+    'pattern': '.',
+    'condition': 'checkout_prebuilt_ash_chrome',
+    'action': [ 'vpython',
+                'src/build/lacros/test_runner.py',
+                'download_for_bots',
+    ],
+  },
+
   # Download and initialize "vpython" VirtualEnv environment packages.
   {
     'name': 'vpython_common',
diff --git a/build/lacros/test_runner.py b/build/lacros/test_runner.py
index 3a25b07f..cba2f14 100755
--- a/build/lacros/test_runner.py
+++ b/build/lacros/test_runner.py
@@ -24,7 +24,6 @@
 _SRC_ROOT = os.path.abspath(
     os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir))
 sys.path.append(os.path.join(_SRC_ROOT, 'third_party', 'depot_tools'))
-import download_from_google_storage
 
 # Base GS URL to store nightly uploaded official Chrome.
 # TODO(crbug.com/1035562): This URL is created for testing purpose only.
@@ -48,6 +47,7 @@
 _TARGETS_REQUIRE_ASH_CHROME = [
     'browser_tests',
     'components_unittests',
+    'sync_integration_tests',
     'unit_tests',
 ]
 
@@ -93,6 +93,13 @@
 def _DownloadAshChromeIfNecessary(version):
   """Download a given version of ash-chrome if not already exists.
 
+  Currently, a special constant version value is support: "for_bots", the reason
+  is that version number is still not pinned to chromium/src, so a constant
+  value is needed to make sure that after the builder who downloads and isolates
+  ash-chrome, the tester knows where to look for the binary to use.
+  TODO(crbug.com/1107010): remove the support once ash-chrome version is pinned
+  to chromium/src.
+
   Args:
     version: A string representing the version, such as "86.0.4187.0".
 
@@ -110,15 +117,19 @@
         os.path.join(ash_chrome_dir, 'chrome'))
 
   ash_chrome_dir = _GetAshChromeDirPath(version)
-  if IsAshChromeDirValid(ash_chrome_dir):
+  if version != 'for_bots' and IsAshChromeDirValid(ash_chrome_dir):
     return
 
   shutil.rmtree(ash_chrome_dir, ignore_errors=True)
   os.makedirs(ash_chrome_dir)
   with tempfile.NamedTemporaryFile() as tmp:
+    import download_from_google_storage
     gsutil = download_from_google_storage.Gsutil(
         download_from_google_storage.GSUTIL_DEFAULT_PATH)
-    gs_path = _GS_URL_BASE + '/' + version + '/' + _GS_ASH_CHROME_PATH
+    gs_version = (_GetLatestVersionOfAshChrome()
+                  if version == 'for_bots' else version)
+    logging.info('Ash-chrome version: %s', gs_version)
+    gs_path = _GS_URL_BASE + '/' + gs_version + '/' + _GS_ASH_CHROME_PATH
     exit_code = gsutil.call('cp', gs_path, tmp.name)
     if exit_code:
       raise RuntimeError('Failed to download: "%s"' % gs_path)
@@ -145,6 +156,7 @@
 def _GetLatestVersionOfAshChrome():
   """Returns the latest version of uploaded official ash-chrome."""
   with tempfile.NamedTemporaryFile() as tmp:
+    import download_from_google_storage
     gsutil = download_from_google_storage.Gsutil(
         download_from_google_storage.GSUTIL_DEFAULT_PATH)
     gsutil.check_call('cp', _GS_ASH_CHROME_LATEST_VERSION_FILE, tmp.name)
@@ -152,32 +164,64 @@
       return f.read().strip()
 
 
-def _ParseArguments():
+def _RunTest(args, forward_args):
+  """Run tests with given args.
+
+  args (dict): Args for this script.
+  forward_args (dict): Args to be forwarded to the test command.
+
+  Raises:
+      RuntimeError: If the given test binary doesn't exist or the test runner
+          doesn't know how to run it.
+  """
+
+  if not os.path.isfile(args.command):
+    raise RuntimeError('Specified test command: "%s" doesn\'t exist' %
+                       args.command)
+
+  # |_TARGETS_REQUIRE_ASH_CHROME| may not always be accurate as it is updated
+  # with a best effort only, therefore, allow the invoker to override the
+  # behavior with a specified ash-chrome version, which makes sure that
+  # automated CI/CQ builders would always work correctly.
+  if (os.path.basename(args.command) not in _TARGETS_REQUIRE_ASH_CHROME
+      and not args.ash_chrome_version):
+    return subprocess.call([args.command] + forward_args)
+
+  raise RuntimeError('Run tests with ash-chrome is to be implemented')
+
+
+def Main():
+  logging.basicConfig(level=logging.INFO)
   arg_parser = argparse.ArgumentParser()
   arg_parser.usage = __doc__
 
-  arg_parser.add_argument(
+  subparsers = arg_parser.add_subparsers()
+
+  download_parser = subparsers.add_parser(
+      'download_for_bots',
+      help='Download prebuilt ash-chrome for bots so that tests are hermetic '
+      'during execution')
+  download_parser.set_defaults(
+      func=lambda *_: _DownloadAshChromeIfNecessary('for_bots'))
+
+  test_parser = subparsers.add_parser('test', help='Run tests')
+  test_parser.set_defaults(func=_RunTest)
+
+  test_parser.add_argument(
       'command',
       help='A single command to invoke the tests, for example: '
       '"./url_unittests". Any argument unknown to this test runner script will '
       'be forwarded to the command, for example: "--gtest_filter=Suite.Test"')
 
-  arg_parser.add_argument(
+  test_parser.add_argument(
       '-a',
       '--ash-chrome-version',
       type=str,
       help='Version of ash_chrome to use for testing, for example: '
-      '"86.0.4187.0". If not specified, will use the latest version')
+      '"86.0.4187.0". If not specified, will use the latest version available')
 
   args = arg_parser.parse_known_args()
-  return args[0], args[1]
-
-
-def Main():
-  logging.basicConfig(level=logging.INFO)
-  args, forward_args = _ParseArguments()
-  if os.path.basename(args.command) not in _TARGETS_REQUIRE_ASH_CHROME:
-    return subprocess.call([args.command] + forward_args)
+  return args[0].func(args[0], args[1])
 
 
 if __name__ == '__main__':
diff --git a/build/lacros/test_runner_test.py b/build/lacros/test_runner_test.py
index 0e22222..baf0581 100755
--- a/build/lacros/test_runner_test.py
+++ b/build/lacros/test_runner_test.py
@@ -15,33 +15,65 @@
 
 
 class TestRunnerTest(unittest.TestCase):
+  @mock.patch.object(test_runner, '_DownloadAshChromeIfNecessary')
+  # Tests 'download_for_bots' to download ash-chrome for bots to isolate.
+  # TODO(crbug.com/1107010): remove this test once ash-chrome version is pinned
+  # to chromium/src.
+  def test_download_for_bots(self, mock_download):
+    args = ['script_name', 'download_for_bots']
+    with mock.patch.object(sys, 'argv', args):
+      test_runner.Main()
+      mock_download.assert_called_with('for_bots')
+
   @parameterized.expand([
       'url_unittests',
       './url_unittests',
       'out/release/url_unittests',
       './out/release/url_unittests',
   ])
+  @mock.patch.object(os.path, 'isfile', return_value=True)
   @mock.patch.object(test_runner, '_DownloadAshChromeIfNecessary')
   @mock.patch.object(subprocess, 'call')
   # Tests that the test runner doesn't attempt to download ash-chrome if not
   # required.
   def test_do_not_require_ash_chrome(self, command, mock_subprocess,
-                                     mock_download):
-    args = ['script_name', command]
+                                     mock_download, _):
+    args = ['script_name', 'test', command]
     with mock.patch.object(sys, 'argv', args):
       test_runner.Main()
       mock_subprocess.assert_called_with([command])
       self.assertFalse(mock_download.called)
 
+  @mock.patch.object(os.path, 'isfile', return_value=True)
+  @mock.patch.object(test_runner, '_DownloadAshChromeIfNecessary')
   @mock.patch.object(subprocess, 'call')
   # Tests that arguments not known to the test runner are forwarded to the
   # command that invokes tests.
-  def test_command_arguments(self, mock_subprocess):
-    args = ['script_name', './url_unittests', '--gtest_filter=Suite.Test']
+  def test_command_arguments(self, mock_subprocess, mock_download, _):
+    args = [
+        'script_name', 'test', './url_unittests', '--gtest_filter=Suite.Test'
+    ]
     with mock.patch.object(sys, 'argv', args):
       test_runner.Main()
+      mock_download.called
       mock_subprocess.assert_called_with(
           ['./url_unittests', '--gtest_filter=Suite.Test'])
+      self.assertFalse(mock_download.called)
+
+  @mock.patch.object(os.path, 'isfile', return_value=True)
+  # Tests that if a ash-chrome version is specified, uses ash-chrome to run
+  # tests anyway even if |_TARGETS_REQUIRE_ASH_CHROME| indicates an ash-chrome
+  # is not required.
+  def test_overrides_do_not_require_ash_chrome(self, _):
+    args = [
+        'script_name', 'test', './url_unittests', '--ash-chrome-version',
+        '86.0.4187.0'
+    ]
+    with mock.patch.object(sys, 'argv', args):
+      with self.assertRaises(RuntimeError) as context:
+        test_runner.Main()
+        self.assertEqual('Run tests with ash-chrome is to be implemented',
+                         str(context.exception))
 
 
 if __name__ == '__main__':
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 057d1160..ca01ad9e 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -634,6 +634,11 @@
       "HAS_OUT_OF_PROC_TEST_RUNNER",
       "CHROME_VERSION_MAJOR=" + chrome_version_major,
     ]
+    if (chromeos_is_browser_only) {
+      use_ash_chrome = true
+      use_xvfb = true
+    }
+
     deps = [
       ":browser_tests_runner",
       ":policy_testserver_pyproto",
@@ -3033,6 +3038,10 @@
 }
 
 test("unit_tests") {
+  if (chromeos_is_browser_only) {
+    use_ash_chrome = true
+  }
+
   inputs = [
     # enums.xml is analyzed by AboutFlagsHistogramTest, so this
     # dependency is needed to make commit bots run unit_tests on
@@ -6462,6 +6471,11 @@
 
 if (!is_fuchsia && !is_android) {
   test("sync_integration_tests") {
+    if (chromeos_is_browser_only) {
+      use_ash_chrome = true
+      use_xvfb = true
+    }
+
     sources = [
       "../browser/sync/test/integration/enable_disable_test.cc",
       "../browser/sync/test/integration/local_sync_test.cc",
diff --git a/components/BUILD.gn b/components/BUILD.gn
index e3d8755..e34666e 100644
--- a/components/BUILD.gn
+++ b/components/BUILD.gn
@@ -45,6 +45,10 @@
 # no tests will run) and add a reference here. You can add more than one unit
 # test target if convenient.
 test("components_unittests") {
+  if (chromeos_is_browser_only) {
+    use_ash_chrome = true
+  }
+
   if (is_android || is_linux || is_mac || is_win) {
     data = [ "test/data/" ]
   }
diff --git a/testing/test.gni b/testing/test.gni
index 9d90ab3..678289b 100644
--- a/testing/test.gni
+++ b/testing/test.gni
@@ -43,6 +43,7 @@
 #   use_raw_android_executable: Use executable() rather than android_apk().
 #   use_native_activity: Test implements ANativeActivity_onCreate().
 #   win_test_enable_cfi_linker: (Win) Enable CFI linker for this test binary.
+#   use_ash_chrome: Use ash-chrome to run tests, only applicable to lacros.
 template("test") {
   if (is_android) {
     _use_raw_android_executable = defined(invoker.use_raw_android_executable) &&
@@ -393,10 +394,15 @@
 
       executable_args = [
         "@WrappedPath(../../build/lacros/test_runner.py)",
+        "test",
         "@WrappedPath(./${_executable})",
         "--test-launcher-bot-mode",
       ]
 
+      if (defined(invoker.use_ash_chrome) && invoker.use_ash_chrome) {
+        executable_args += [ "--ash-chrome-version=for_bots" ]
+      }
+
       if (is_asan) {
         executable_args += [ "--asan=true" ]
       }
@@ -411,9 +417,13 @@
       }
 
       data = [
+        "//build/lacros/test_runner.py",
         "//testing/test_env.py",
         "//.vpython",
       ]
+      if (defined(invoker.use_ash_chrome) && invoker.use_ash_chrome) {
+        data += [ "//build/lacros/prebuilt_ash_chrome/for_bots" ]
+      }
     }
 
     executable(target_name) {
diff --git a/tools/mb/mb.py b/tools/mb/mb.py
index 7c3c758..16bcc06 100755
--- a/tools/mb/mb.py
+++ b/tools/mb/mb.py
@@ -1403,10 +1403,11 @@
                     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_'):
-      if is_mac or is_linux:
+    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: