[lacros] Run Lacros test targets with ash_chrome if needed

This CL makes two changes:
1. Make use_xvfb imply use_ash_chrome.
2. Mark test targets to use xvfb and ash_chrome if needed.

Bug: 1104318
Change-Id: I7128b31b83e2389e92e5507093396aaf537dfdf6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324765Reviewed-by: Sven Zheng <svenzheng@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792730}

build/lacros/test_runner.py

@@ -39,6 +39,7 @@
 import argparse
 import os
 import logging
+import re
 import shutil
 import signal
 import subprocess
@@ -71,10 +72,23 @@ _PREBUILT_ASH_CHROME_DIR = os.path.join(os.path.dirname(__file__),
 # running all the test targets.
 # List of targets that require ash-chrome as a Wayland server in order to run.
 _TARGETS_REQUIRE_ASH_CHROME = [
+    'app_shell_unittests',
+    'aura_unittests',
     'browser_tests',
     'components_unittests',
+    'compositor_unittests',
+    'content_unittests',
+    'dbus_unittests',
+    'extensions_unittests',
+    'message_center_unittests',
+    'snapshot_unittests',
     'sync_integration_tests',
     'unit_tests',
+    'views_unittests',
+    'wm_unittests',
+
+    # regex patters.
+    '.*_browsertests',
 ]
 
 def _GetAshChromeDirPath(version):
@@ -304,8 +318,10 @@ def _RunTest(args, forward_args):
   # 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):
+  requires_ash_chrome = any(
+      re.match(t, os.path.basename(args.command))
+      for t in _TARGETS_REQUIRE_ASH_CHROME)
+  if not requires_ash_chrome and not args.ash_chrome_version:
     return _RunTestDirectly(args, forward_args)
 
   return _RunTestWithAshChrome(args, forward_args)

build/lacros/test_runner_test.py

@@ -55,6 +55,11 @@ class TestRunnerTest(unittest.TestCase):
       mock_popen.assert_called_with([command])
       self.assertFalse(mock_download.called)
 
+  @parameterized.expand([
+      'browser_tests',
+      'components_browsertests',
+      'content_browsertests',
+  ])
   @mock.patch.object(os,
                      'listdir',
                      return_value=['wayland-0', 'wayland-0.lock'])
@@ -70,8 +75,8 @@ class TestRunnerTest(unittest.TestCase):
   @mock.patch.object(subprocess, 'Popen', return_value=mock.Mock())
   # Tests that the test runner downloads and spawns ash-chrome if ash-chrome is
   # required.
-  def test_require_ash_chrome(self, mock_popen, mock_download, *_):
-    args = ['script_name', 'test', 'browser_tests']
+  def test_require_ash_chrome(self, command, mock_popen, mock_download, *_):
+    args = ['script_name', 'test', command]
     with mock.patch.object(sys, 'argv', args):
       test_runner.Main()
       mock_download.assert_called_with('86.0.4187.0')
@@ -88,7 +93,7 @@ class TestRunnerTest(unittest.TestCase):
       self.assertDictEqual({'XDG_RUNTIME_DIR': '/tmp/xdg'}, ash_chrome_env)
 
       test_args = mock_popen.call_args_list[1][0][0]
-      self.assertListEqual(['browser_tests'], test_args)
+      self.assertListEqual([command], test_args)
       test_env = mock_popen.call_args_list[1][1].get('env', {})
       self.assertDictEqual(
           {

chrome/test/BUILD.gn

@@ -631,15 +631,15 @@ static_library("browser_tests_runner") {
 
 if (!is_android) {
   test("browser_tests") {
+    if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+      use_xvfb = true
+    }
+
     configs += [ "//build/config:precompiled_headers" ]
     defines = [
       "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",
@@ -3058,8 +3058,8 @@ if (is_mac) {
 }
 
 test("unit_tests") {
-  if (chromeos_is_browser_only) {
-    use_ash_chrome = true
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
   }
 
   inputs = [
@@ -6518,8 +6518,7 @@ if (!is_fuchsia) {
 
 if (!is_fuchsia && !is_android) {
   test("sync_integration_tests") {
-    if (chromeos_is_browser_only) {
-      use_ash_chrome = true
+    if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
       use_xvfb = true
     }
 

components/BUILD.gn

@@ -45,8 +45,8 @@ if (is_ios) {
 # 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_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
   }
 
   if (is_android || is_linux || is_mac || is_win) {
@@ -540,6 +540,10 @@ if (is_android) {
 # no-op under Fuchsia.
 if (!is_ios && !is_fuchsia) {
   test("components_browsertests") {
+    if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+      use_xvfb = true
+    }
+
     defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
 
     sources = [

content/test/BUILD.gn

@@ -874,6 +874,10 @@ if (is_android) {
 }
 
 test("content_browsertests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   # See comment at the top of //content/BUILD.gn for why this is disabled in
   # component builds.
   if (is_component_build) {
@@ -1600,6 +1604,10 @@ static_library("run_all_unittests") {
 }
 
 test("content_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   # See comment at the top of //content/BUILD.gn for why this is disabled in
   # component builds.
   if (is_component_build) {

dbus/BUILD.gn

@@ -69,6 +69,10 @@ static_library("test_support") {
 }
 
 test("dbus_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "bus_unittest.cc",
     "dbus_statistics_unittest.cc",

extensions/BUILD.gn

@@ -208,6 +208,10 @@ repack("shell_and_test_pak") {
 }
 
 test("extensions_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "test/extensions_unittests_main.cc",
     "test/logging_timer_unittest.cc",
@@ -242,6 +246,10 @@ test("extensions_unittests") {
 }
 
 test("extensions_browsertests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   data = [
     "//extensions/test/data/",
     "//net/tools/testserver/",

extensions/shell/BUILD.gn

@@ -276,6 +276,10 @@ executable("app_shell") {
 }
 
 test("app_shell_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "../test/extensions_unittests_main.cc",
     "browser/api/identity/identity_api_unittest.cc",

testing/test.gni

@@ -43,7 +43,6 @@ if ((is_linux && !is_chromeos) || is_mac || is_win ||
 #   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) &&
@@ -406,7 +405,13 @@ template("test") {
         "--test-launcher-bot-mode",
       ]
 
-      if (defined(invoker.use_ash_chrome) && invoker.use_ash_chrome) {
+      # When use_xvfb is set by the invoker, it indicates that running this test
+      # target requires a window, and in lacros build, ash-chrome serves as the
+      # display server. Note that even though the tests themselves do not
+      # require xvfb anymore, xvfb.py is still needed to invoke the lacros test
+      # runner because ash-chrome is based on x11.
+      _use_ash_chrome = _use_xvfb
+      if (_use_ash_chrome) {
         executable_args += [ "--ash-chrome-version=for_bots" ]
       }
 
@@ -427,7 +432,7 @@ template("test") {
         "//build/lacros/test_runner.py",
         "//.vpython",
       ]
-      if (defined(invoker.use_ash_chrome) && invoker.use_ash_chrome) {
+      if (_use_ash_chrome) {
         data += [ "//build/lacros/prebuilt_ash_chrome/for_bots" ]
       }
     }

ui/aura/BUILD.gn

@@ -306,6 +306,10 @@ executable("aura_demo") {
 }
 
 test("aura_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "../compositor_extra/shadow_unittest.cc",
     "gestures/gesture_recognizer_unittest.cc",

ui/compositor/BUILD.gn

@@ -221,6 +221,10 @@ jumbo_static_library("test_support") {
 }
 
 test("compositor_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "animation_throughput_reporter_unittest.cc",
     "callback_layer_animation_observer_unittest.cc",

ui/message_center/BUILD.gn

@@ -155,6 +155,10 @@ if (enable_message_center) {
   }
 
   test("message_center_unittests") {
+    if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+      use_xvfb = true
+    }
+
     sources = [
       "lock_screen/fake_lock_screen_controller.cc",
       "lock_screen/fake_lock_screen_controller.h",

ui/snapshot/BUILD.gn

@@ -92,6 +92,10 @@ jumbo_source_set("snapshot_export") {
 }
 
 test("snapshot_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [ "test/run_all_unittests.cc" ]
 
   if (is_mac) {

ui/views/BUILD.gn

@@ -1029,6 +1029,10 @@ jumbo_source_set("test_support") {
 }
 
 test("views_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "accessible_pane_view_unittest.cc",
     "animation/bounds_animator_unittest.cc",

ui/wm/BUILD.gn

@@ -119,6 +119,10 @@ jumbo_static_library("test_support") {
 }
 
 test("wm_unittests") {
+  if ((is_linux && !is_chromeos) || chromeos_is_browser_only) {
+    use_xvfb = true
+  }
+
   sources = [
     "core/capture_controller_unittest.cc",
     "core/compound_event_filter_unittest.cc",