Revert "[lacros] Make test runner supports lacros_chrome_browsertests"

This reverts commit e4eacd09.

Reason for revert: Suspect for lacros test failures:
https://ci.chromium.org/p/chromium/builders/ci/linux-lacros-tester-rel/2227
The only lacros-related change in diff with the previous success run.

Original change's description:
> [lacros] Make test runner supports lacros_chrome_browsertests
> 
> This CL makes test runner supports lacros_chrome_browsertests by:
> 1. Appending necessary args to establish mojo connections.
> 2. Always use '--test-launcher-jobs=1' to run tests serially because
>    multiple clients crosapis are still not supported yet.
> 
> Bug: 1120582
> Change-Id: I865ace26aa4a86c83912c6660bc433d79e43ef76
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410350
> Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Sven Zheng <svenzheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#806903}

TBR=erikchen@chromium.org,liaoyuke@chromium.org,svenzheng@chromium.org

Change-Id: Ie630ade531428e78718127792c4f6ab6b0d55025
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1120582
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410246Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806980}

build/lacros/test_runner.py

@@ -70,9 +70,6 @@ _GS_ASH_CHROME_PATH = 'ash-chromium.zip'
 _PREBUILT_ASH_CHROME_DIR = os.path.join(os.path.dirname(__file__),
                                         'prebuilt_ash_chrome')
 
-# Number of seconds to wait for ash-chrome to start.
-ASH_CHROME_TIMEOUT_SECONDS = 10
-
 # List of targets that require ash-chrome as a Wayland server in order to run.
 _TARGETS_REQUIRE_ASH_CHROME = [
     'app_shell_unittests',
@@ -230,49 +227,11 @@ def _GetLatestVersionOfAshChrome():
       return f.read().strip()
 
 
-def _WaitForAshChromeToStart(tmp_xdg_dir, lacros_mojo_socket_file,
-                             is_lacros_chrome_browsertests):
-  """Waits for Ash-Chrome to be up and running and returns a boolean indicator.
-
-  Determine whether ash-chrome is up and running by checking whether two files
-  (lock file + socket) have been created in the |XDG_RUNTIME_DIR| and the lacros
-  mojo socket file has been created if running lacros_chrome_browsertests.
-  TODO(crbug.com/1107966): Figure out a more reliable hook to determine the
-  status of ash-chrome, likely through mojo connection.
-
-  Args:
-    tmp_xdg_dir (str): Path to the XDG_RUNTIME_DIR.
-    lacros_mojo_socket_file (str): Path to the lacros mojo socket file.
-    is_lacros_chrome_browsertests (bool): is running lacros_chrome_browsertests.
-
-  Returns:
-    A boolean indicating whether Ash-chrome is up and running.
-  """
-
-  def IsAshChromeReady(tmp_xdg_dir, lacros_mojo_socket_file,
-                       is_lacros_chrome_browsertests):
-    return (len(list(tmp_xdg_dir)) >= 2
-            and (not is_lacros_chrome_browsertests
-                 or os.path.exists(lacros_mojo_socket_file)))
-
-  time_counter = 0
-  while not IsAshChromeReady(tmp_xdg_dir, lacros_mojo_socket_file,
-                             is_lacros_chrome_browsertests):
-    time.sleep(0.5)
-    time_counter += 0.5
-    if time_counter > ASH_CHROME_TIMEOUT_SECONDS:
-      break
-
-  return IsAshChromeReady(tmp_xdg_dir, lacros_mojo_socket_file,
-                          is_lacros_chrome_browsertests)
-
-
 def _RunTestWithAshChrome(args, forward_args):
   """Runs tests with ash-chrome.
 
-  Args:
-    args (dict): Args for this script.
-    forward_args (dict): Args to be forwarded to the test command.
+  args (dict): Args for this script.
+  forward_args (dict): Args to be forwarded to the test command.
   """
   ash_chrome_version = args.ash_chrome_version or _GetLatestVersionOfAshChrome()
   _DownloadAshChromeIfNecessary(ash_chrome_version)
@@ -281,19 +240,10 @@ def _RunTestWithAshChrome(args, forward_args):
   ash_chrome_file = os.path.join(_GetAshChromeDirPath(ash_chrome_version),
                                  'chrome')
   try:
-    # Starts Ash-Chrome.
     tmp_xdg_dir_name = tempfile.mkdtemp()
     tmp_ash_data_dir_name = tempfile.mkdtemp()
-
-    # Please refer to below file for how mojo connection is set up in testing.
-    # //chrome/browser/chromeos/crosapi/test_mojo_connection_manager.h
-    lacros_mojo_socket_file = '%s/lacros.sock' % tmp_ash_data_dir_name
-    lacros_mojo_socket_arg = ('--lacros-mojo-socket-for-testing=%s' %
-                              lacros_mojo_socket_file)
-    is_lacros_chrome_browsertests = (os.path.basename(
-        args.command) == 'lacros_chrome_browsertests')
-
     ash_process = None
+
     ash_env = os.environ.copy()
     ash_env['XDG_RUNTIME_DIR'] = tmp_xdg_dir_name
     ash_cmd = [
@@ -302,8 +252,6 @@ def _RunTestWithAshChrome(args, forward_args):
         '--enable-wayland-server',
         '--no-startup-window',
     ]
-    if is_lacros_chrome_browsertests:
-      ash_cmd.append(lacros_mojo_socket_arg)
 
     ash_process_has_started = False
     total_tries = 3
@@ -311,14 +259,24 @@ def _RunTestWithAshChrome(args, forward_args):
     while not ash_process_has_started and num_tries < total_tries:
       num_tries += 1
       ash_process = subprocess.Popen(ash_cmd, env=ash_env)
-      ash_process_has_started = _WaitForAshChromeToStart(
-          tmp_xdg_dir_name, lacros_mojo_socket_file,
-          is_lacros_chrome_browsertests)
-      if ash_process_has_started:
+
+      # Determine whether ash-chrome is up and running by checking whether two
+      # files (lock file + socket) have been created in the |XDG_RUNTIME_DIR|.
+      # TODO(crbug.com/1107966): Figure out a more reliable hook to determine
+      # the status of ash-chrome, likely through mojo connection.
+      time_to_wait = 10
+      time_counter = 0
+      while len(os.listdir(tmp_xdg_dir_name)) < 2:
+        time.sleep(0.5)
+        time_counter += 0.5
+        if time_counter > time_to_wait:
+          break
+
+      if len(os.listdir(tmp_xdg_dir_name)) >= 2:
+        ash_process_has_started = True
         break
 
-      logging.warning('Starting ash-chrome timed out after %ds',
-                      ASH_CHROME_TIMEOUT_SECONDS)
+      logging.warning('Starting ash-chrome timed out after %ds', time_to_wait)
       logging.warning('Printing the output of "ps aux" for debugging:')
       subprocess.call(['ps', 'aux'])
       if ash_process and ash_process.poll() is None:
@@ -327,26 +285,6 @@ def _RunTestWithAshChrome(args, forward_args):
     if not ash_process_has_started:
       raise RuntimeError('Timed out waiting for ash-chrome to start')
 
-    # Starts tests.
-    if is_lacros_chrome_browsertests:
-      forward_args.append(lacros_mojo_socket_arg)
-
-      reason_of_jobs_1 = (
-          'multiple clients crosapi is not supported yet (crbug.com/1124490), '
-          'lacros_chrome_browsertests has to run tests serially')
-
-      if any('--test-launcher-jobs' in arg for arg in forward_args):
-        raise RuntimeError(
-            'Specifying "--test-launcher-jobs" is not allowed because %s. '
-            'Please remove it and this script will automatically append '
-            '"--test-launcher-jobs=1"' % reason_of_jobs_1)
-
-      # TODO(crbug.com/1124490): Run lacros_chrome_browsertests in parallel once
-      # the bug is fixed.
-      logging.warning('Appending "--test-launcher-jobs=1" because %s',
-                      reason_of_jobs_1)
-      forward_args.append('--test-launcher-jobs=1')
-
     test_env = os.environ.copy()
     test_env['EGL_PLATFORM'] = 'surfaceless'
     test_env['XDG_RUNTIME_DIR'] = tmp_xdg_dir_name

build/lacros/test_runner_test.py

@@ -59,7 +59,6 @@ class TestRunnerTest(unittest.TestCase):
       'browser_tests',
       'components_browsertests',
       'content_browsertests',
-      'lacros_chrome_browsertests',
   ])
   @mock.patch.object(os,
                      'listdir',
@@ -68,7 +67,6 @@ class TestRunnerTest(unittest.TestCase):
                      'mkdtemp',
                      side_effect=['/tmp/xdg', '/tmp/ash-data'])
   @mock.patch.object(os.environ, 'copy', side_effect=[{}, {}])
-  @mock.patch.object(os.path, 'exists', return_value=True)
   @mock.patch.object(os.path, 'isfile', return_value=True)
   @mock.patch.object(test_runner,
                      '_GetLatestVersionOfAshChrome',
@@ -87,28 +85,15 @@ class TestRunnerTest(unittest.TestCase):
       ash_chrome_args = mock_popen.call_args_list[0][0][0]
       self.assertTrue(ash_chrome_args[0].endswith(
           'build/lacros/prebuilt_ash_chrome/793554/chrome'))
-      expected_ash_chrome_args = [
-          '--user-data-dir=/tmp/ash-data',
-          '--enable-wayland-server',
-          '--no-startup-window',
-      ]
-      if command == 'lacros_chrome_browsertests':
-        expected_ash_chrome_args.append(
-            '--lacros-mojo-socket-for-testing=/tmp/ash-data/lacros.sock')
-      self.assertListEqual(expected_ash_chrome_args, ash_chrome_args[1:])
+      self.assertListEqual([
+          '--user-data-dir=/tmp/ash-data', '--enable-wayland-server',
+          '--no-startup-window'
+      ], ash_chrome_args[1:])
       ash_chrome_env = mock_popen.call_args_list[0][1].get('env', {})
       self.assertDictEqual({'XDG_RUNTIME_DIR': '/tmp/xdg'}, ash_chrome_env)
 
       test_args = mock_popen.call_args_list[1][0][0]
-      if command == 'lacros_chrome_browsertests':
-        self.assertListEqual([
-            command,
-            '--lacros-mojo-socket-for-testing=/tmp/ash-data/lacros.sock',
-            '--test-launcher-jobs=1'
-        ], test_args)
-      else:
-        self.assertListEqual([command], test_args)
-
+      self.assertListEqual([command], test_args)
       test_env = mock_popen.call_args_list[1][1].get('env', {})
       self.assertDictEqual(
           {
@@ -116,11 +101,9 @@ class TestRunnerTest(unittest.TestCase):
               'EGL_PLATFORM': 'surfaceless'
           }, test_env)
 
-
   @mock.patch.object(os,
                      'listdir',
                      return_value=['wayland-0', 'wayland-0.lock'])
-  @mock.patch.object(os.path, 'exists', return_value=True)
   @mock.patch.object(os.path, 'isfile', return_value=True)
   @mock.patch.object(test_runner,
                      '_GetLatestVersionOfAshChrome',
@@ -140,7 +123,6 @@ class TestRunnerTest(unittest.TestCase):
   @mock.patch.object(os,
                      'listdir',
                      return_value=['wayland-0', 'wayland-0.lock'])
-  @mock.patch.object(os.path, 'exists', return_value=True)
   @mock.patch.object(os.path, 'isfile', return_value=True)
   @mock.patch.object(test_runner, '_DownloadAshChromeIfNecessary')
   @mock.patch.object(subprocess, 'Popen', return_value=mock.Mock())