Reland "[lacros] Make test runner supports lacros_chrome_browsertests"
This reverts commit 485226400b2689de3cf3f518e60ddfc8bf3239db. Reason for revert: During the refactoring, 'os.listdir' was somehow accidentally changed to 'list', which resulted in the test failures, and this CL fixes the problem. Original change's description: > Revert "[lacros] Make test runner supports lacros_chrome_browsertests" > > This reverts commit e4eacd09b1ceded62514f3f19164696f85e7b543. > > 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/+/2410246 > Reviewed-by: Sergey Poromov <poromov@chromium.org> > Commit-Queue: Sergey Poromov <poromov@chromium.org> > Cr-Commit-Position: refs/heads/master@{#806980} TBR=erikchen@chromium.org,poromov@chromium.org,liaoyuke@chromium.org,svenzheng@chromium.org # Not skipping CQ checks because this is a reland. Bug: 1120582 Change-Id: Ifaa2235a0a4f9ce8deb9aea5011eda0924e1f051 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412253 Commit-Queue: Yuke Liao <liaoyuke@chromium.org> Reviewed-by: Yuke Liao <liaoyuke@chromium.org> Reviewed-by: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#807111} GitOrigin-RevId: 10317f04d726d90c54fcfbc250e66dd3a342874c
This commit is contained in:
Yuke Liao
Copybara-Service
Родитель
5a6fca0fff
Коммит
47af417f72
|
|
@ -70,6 +70,9 @@ _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',
|
||||
|
|
@ -227,11 +230,49 @@ 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(os.listdir(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 (dict): Args for this script.
|
||||
forward_args (dict): Args to be forwarded to the test command.
|
||||
Args:
|
||||
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)
|
||||
|
|
@ -240,10 +281,19 @@ 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()
|
||||
ash_process = None
|
||||
|
||||
# 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 = [
|
||||
|
|
@ -252,6 +302,8 @@ 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
|
||||
|
|
@ -259,24 +311,14 @@ 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)
|
||||
|
||||
# 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
|
||||
ash_process_has_started = _WaitForAshChromeToStart(
|
||||
tmp_xdg_dir_name, lacros_mojo_socket_file,
|
||||
is_lacros_chrome_browsertests)
|
||||
if ash_process_has_started:
|
||||
break
|
||||
|
||||
logging.warning('Starting ash-chrome timed out after %ds', time_to_wait)
|
||||
logging.warning('Starting ash-chrome timed out after %ds',
|
||||
ASH_CHROME_TIMEOUT_SECONDS)
|
||||
logging.warning('Printing the output of "ps aux" for debugging:')
|
||||
subprocess.call(['ps', 'aux'])
|
||||
if ash_process and ash_process.poll() is None:
|
||||
|
|
@ -285,6 +327,26 @@ 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
|
||||
|
|
|
|||
|
|
@ -59,6 +59,7 @@ class TestRunnerTest(unittest.TestCase):
|
|||
'browser_tests',
|
||||
'components_browsertests',
|
||||
'content_browsertests',
|
||||
'lacros_chrome_browsertests',
|
||||
])
|
||||
@mock.patch.object(os,
|
||||
'listdir',
|
||||
|
|
@ -67,6 +68,7 @@ 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',
|
||||
|
|
@ -85,15 +87,28 @@ 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'))
|
||||
self.assertListEqual([
|
||||
'--user-data-dir=/tmp/ash-data', '--enable-wayland-server',
|
||||
'--no-startup-window'
|
||||
], ash_chrome_args[1:])
|
||||
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:])
|
||||
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]
|
||||
self.assertListEqual([command], test_args)
|
||||
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)
|
||||
|
||||
test_env = mock_popen.call_args_list[1][1].get('env', {})
|
||||
self.assertDictEqual(
|
||||
{
|
||||
|
|
@ -101,9 +116,11 @@ 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',
|
||||
|
|
@ -123,6 +140,7 @@ 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())
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче