-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Enable support for running the test harness in Safari. #25499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
5d0adb5
66a595d
8d9b47a
0da91d1
fa3de84
30b2eeb
2b41554
f0c11e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
import json | ||
import logging | ||
import os | ||
import plistlib | ||
import psutil | ||
import re | ||
import shlex | ||
|
@@ -52,6 +53,11 @@ | |
# used in CI. To use a custom start command specify the executable and command | ||
# line flags. | ||
# | ||
# Note that when specifying EMTEST_BROWSER to run tests on a Safari browser: | ||
# the command line must point to the root of the app bundle, and not to the | ||
# Safari executable inside the bundle. I.e. pass EMTEST_BROWSER=/Applications/Safari.app | ||
# instead of EMTEST_BROWSER=/Applications/Safari.app/Contents/MacOS/Safari | ||
# | ||
# There are two special values that can be used here if running in an actual | ||
# browser is not desired: | ||
# EMTEST_BROWSER=0 : This will disable the actual running of the test and simply | ||
|
@@ -130,6 +136,15 @@ def configure(data_dir): | |
shutil.copy(test_file('firefox_user.js'), os.path.join(data_dir, 'user.js')) | ||
|
||
|
||
class SafariConfig: | ||
default_flags = ('', ) | ||
executable_name = 'Safari' | ||
|
||
@staticmethod | ||
def configure(data_dir): | ||
""" Safari has no special configuration step.""" | ||
|
||
|
||
# Special value for passing to assert_returncode which means we expect that program | ||
# to fail with non-zero return code, but we don't care about specifically which one. | ||
NON_ZERO = -1 | ||
|
@@ -202,6 +217,21 @@ def is_firefox(): | |
return EMTEST_BROWSER and 'firefox' in EMTEST_BROWSER.lower() | ||
|
||
|
||
def is_safari(): | ||
return EMTEST_BROWSER and 'safari' in EMTEST_BROWSER.lower() | ||
|
||
|
||
def get_safari_version(): | ||
plist_path = os.path.join(EMTEST_BROWSER.strip(), 'Contents', 'version.plist') | ||
version_str = plistlib.load(open(plist_path, 'rb')).get('CFBundleShortVersionString') | ||
# Split into parts (major.minor.patch) | ||
parts = (version_str.split('.') + ['0', '0', '0'])[:3] | ||
# Convert each part into integers, discarding any trailing string, e.g. '13a' -> 13. | ||
parts = [int(re.match(r"\d+", s).group()) if re.match(r"\d+", s) else 0 for s in parts] | ||
# Return version as XXYYZZ | ||
return parts[0] * 10000 + parts[1] * 100 + parts[2] | ||
|
||
|
||
def compiler_for(filename, force_c=False): | ||
if shared.suffix(filename) in ('.cc', '.cxx', '.cpp') and not force_c: | ||
return EMXX | ||
|
@@ -2418,6 +2448,8 @@ def configure_test_browser(): | |
config = ChromeConfig() | ||
elif is_firefox(): | ||
config = FirefoxConfig() | ||
elif is_safari(): | ||
config = SafariConfig() | ||
if config: | ||
EMTEST_BROWSER += ' ' + ' '.join(config.default_flags) | ||
if EMTEST_HEADLESS == 1: | ||
|
@@ -2548,11 +2580,21 @@ def browser_restart(cls): | |
def browser_open(cls, url): | ||
assert has_browser() | ||
browser_args = EMTEST_BROWSER | ||
parallel_harness = worker_id is not None | ||
|
||
config = None | ||
if is_chrome(): | ||
config = ChromeConfig() | ||
elif is_firefox(): | ||
config = FirefoxConfig() | ||
elif is_safari(): | ||
config = SafariConfig() | ||
|
||
if EMTEST_BROWSER_AUTO_CONFIG: | ||
# Prepare the browser data directory, if it uses one. | ||
if EMTEST_BROWSER_AUTO_CONFIG and config and hasattr(config, 'data_dir_flag'): | ||
logger.info('Using default CI configuration.') | ||
browser_data_dir = DEFAULT_BROWSER_DATA_DIR | ||
if worker_id is not None: | ||
if parallel_harness: | ||
# Running in parallel mode, give each browser its own profile dir. | ||
browser_data_dir += '-' + str(worker_id) | ||
|
||
|
@@ -2567,52 +2609,65 @@ def browser_open(cls, url): | |
# Recreate the new data directory. | ||
os.mkdir(browser_data_dir) | ||
|
||
if is_chrome(): | ||
config = ChromeConfig() | ||
elif is_firefox(): | ||
config = FirefoxConfig() | ||
else: | ||
exit_with_error(f'EMTEST_BROWSER_AUTO_CONFIG only currently works with firefox or chrome. EMTEST_BROWSER was "{EMTEST_BROWSER}"') | ||
if not config: | ||
exit_with_error(f'EMTEST_BROWSER_AUTO_CONFIG only currently works with firefox, chrome and safari. EMTEST_BROWSER was "{EMTEST_BROWSER}"') | ||
if WINDOWS: | ||
# Escape directory delimiter backslashes for shlex.split. | ||
browser_data_dir = browser_data_dir.replace('\\', '\\\\') | ||
config.configure(browser_data_dir) | ||
browser_args += f' {config.data_dir_flag}"{browser_data_dir}"' | ||
|
||
browser_args = shlex.split(browser_args) | ||
if is_safari(): | ||
# For the macOS 'open' command, pass | ||
# --new: to make a new Safari app be launched, rather than add a tab to an existing Safari process/window | ||
# --fresh: do not restore old tabs (e.g. if user had old navigated windows open) | ||
# --background: Open the new Safari window behind the current Terminal window, to make following the test run more pleasing (this is for convenience only) | ||
# -a <exe_name>: The path to the executable to open, in this case Safari | ||
browser_args = ['open', '--new', '--fresh', '--background', '-a'] + browser_args | ||
|
||
|
||
logger.info('Launching browser: %s', str(browser_args)) | ||
|
||
if WINDOWS and is_firefox(): | ||
cls.launch_browser_harness_windows_firefox(worker_id, config, browser_args, url) | ||
if (WINDOWS and is_firefox()) or is_safari(): | ||
cls.launch_browser_harness_with_proc_snapshot_workaround(parallel_harness, config, browser_args, url) | ||
else: | ||
cls.browser_procs = [subprocess.Popen(browser_args + [url])] | ||
|
||
@classmethod | ||
def launch_browser_harness_windows_firefox(cls, worker_id, config, browser_args, url): | ||
''' Dedicated function for launching browser harness on Firefox on Windows, | ||
which requires extra care for window positioning and process tracking.''' | ||
def launch_browser_harness_with_proc_snapshot_workaround(cls, parallel_harness, config, browser_args, url): | ||
''' Dedicated function for launching browser harness in scenarios where | ||
we need to identify the launched browser processes via a before-after | ||
subprocess snapshotting delta workaround.''' | ||
|
||
# In order for this to work, each browser needs to be launched one at a time | ||
# so that we know which process belongs to which browser. | ||
with FileLock(browser_spawn_lock_filename) as count: | ||
# Firefox is a multiprocess browser. On Windows, killing the spawned | ||
# process will not bring down the whole browser, but only one browser tab. | ||
# So take a delta snapshot before->after spawning the browser to find | ||
# which subprocesses we launched. | ||
if worker_id is not None: | ||
# Take a snapshot before spawning the browser to find which processes | ||
# existed before launching the browser. | ||
if parallel_harness or is_safari(): | ||
procs_before = list_processes_by_name(config.executable_name) | ||
|
||
# Browser launch | ||
cls.browser_procs = [subprocess.Popen(browser_args + [url])] | ||
# Give Firefox time to spawn its subprocesses. Use an increasing timeout | ||
# as a crude way to account for system load. | ||
if worker_id is not None: | ||
|
||
# Give the browser time to spawn its subprocesses. Use an increasing | ||
# timeout as a crude way to account for system load. | ||
if parallel_harness or is_safari(): | ||
time.sleep(2 + count * 0.3) | ||
procs_after = list_processes_by_name(config.executable_name) | ||
|
||
# Take a snapshot again to find which processes exist after launching | ||
# the browser. Then the newly launched browser processes are determined | ||
# by the delta before->after. | ||
cls.browser_procs = list(set(procs_after).difference(set(procs_before))) | ||
if len(cls.browser_procs) == 0: | ||
logger.warning('Could not detect the launched browser subprocesses. The test harness may not be able to close browser windows if a test hangs, and at harness exit.') | ||
|
||
|
||
# Firefox on Windows quirk: | ||
# Make sure that each browser window is visible on the desktop. Otherwise | ||
# browser might decide that the tab is backgrounded, and not load a test, | ||
# or it might not tick rAF()s forward, causing tests to hang. | ||
if worker_id is not None and not EMTEST_HEADLESS: | ||
# On Firefox on Windows we needs to track subprocesses that got created | ||
# by Firefox. Other setups can use 'browser_proc' directly to terminate | ||
# the browser. | ||
cls.browser_procs = list(set(procs_after).difference(set(procs_before))) | ||
if WINDOWS and parallel_harness and not EMTEST_HEADLESS: | ||
# Wrap window positions on a Full HD desktop area modulo primes. | ||
for proc in cls.browser_procs: | ||
move_browser_window(proc.pid, (300 + count * 47) % 1901, (10 + count * 37) % 997) | ||
|
@@ -2686,6 +2741,15 @@ def run_browser(self, html_file, expected=None, message=None, timeout=None, extr | |
if DEBUG: | ||
print('[browser launch:', html_file, ']') | ||
assert not (message and expected), 'run_browser expects `expected` or `message`, but not both' | ||
|
||
# Accurate version cutoff is not known. | ||
# Needed at least for version Safari Version 17.6 (17618.3.11.11.7, 17618) | ||
# Also needed for Safari Version 18.5 (20621.2.5.11.8) | ||
if is_safari() and get_safari_version() < 190000: | ||
# Old Safari cannot handle running multiple browser pages in the same browser instance | ||
# So restart the browser between each browser test. | ||
self.browser_restart() | ||
|
||
if expected is not None: | ||
try: | ||
self.harness_in_queue.put(( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error seems like its unreachable since its inside the
if EMTEST_BROWSER_AUTO_CONFIG and config
block?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good catch.