Skip to content

Commit b52308c

Browse files
authored
Fix parallel browser test harness to also work with Firefox on Windows. (#25275)
This is a cleaned up version 2 of #25237. ------- Two problems that I run into with the parallel browser harness on Windows with Firefox: 1. On Windows, all the browser windows open up in the exact same X,Y coordinate. Firefox has "is browser in the background" detection, which causes all but one test foreground harness to hang when running any `requestAnimationFrame()` based test. 2. Killing a Firefox browser process with ```py proc = subprocess.Popen(['firefox']) proc.kill() ``` does not work. This is because Firefox is a multiprocess browser, and does not have a process hierarchy where the first launched process would be some kind of a master process. (this is an old known issue, that can be seen also in existing emrun.py code) There seems to be about 12 processes that come alive when running `subprocess.Popen(['firefox'])`. -------------- To fix these issues: 1. ask the Windows Win32 API to move each spawned process to its own X,Y location. Use a file-based multiprocessing lock + counter file to get unique counter to each process. 2. Snapshot alive processes before spawning Firefox, and after. Then acquire the delta of these two to find which process IDs correspond to the launched Firefox.
1 parent 9555253 commit b52308c

File tree

5 files changed

+239
-15
lines changed

5 files changed

+239
-15
lines changed

.circleci/config.yml

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,68 @@ commands:
395395
# "Firefox is already running, but is not responding."
396396
# TODO: find out a way to shut down and restart firefox
397397
- upload-test-results
398+
run-tests-firefox-windows:
399+
description: "Runs emscripten tests under firefox on Windows"
400+
parameters:
401+
test_targets:
402+
description: "Test suites to run"
403+
type: string
404+
title:
405+
description: "Name of given test suite"
406+
type: string
407+
default: ""
408+
steps:
409+
- run:
410+
name: download firefox
411+
shell: powershell.exe -ExecutionPolicy Bypass
412+
command: |
413+
# To download Firefox, we must first figure out what the latest Firefox version name is.
414+
# This is because there does not exist a stable/static URL to download latest Firefox from.
415+
$html = Invoke-WebRequest "https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/"
416+
$zipLink = $html.Links |
417+
Where-Object { $_.href -match "firefox-.*\.en-US\.win64\.zip$" } |
418+
Select-Object -Last 1 -ExpandProperty href
419+
420+
# Download Win64 Firefox.
421+
$nightlyUrl = "https://archive.mozilla.org$zipLink"
422+
$outZip = "$env:TEMP\firefox_nightly.zip"
423+
424+
Write-Host "Downloading latest Firefox Nightly: $nightlyUrl"
425+
Invoke-WebRequest -Uri $nightlyUrl -OutFile $outZip
426+
427+
# Extract to user home directory
428+
$installDir = Join-Path $env:USERPROFILE "firefox"
429+
if (Test-Path $installDir) { Remove-Item -Recurse -Force $installDir }
430+
Expand-Archive -LiteralPath $outZip -DestinationPath $installDir
431+
432+
# Set environment variable for tests
433+
$ffExe = Join-Path $installDir "firefox.exe"
434+
[System.Environment]::SetEnvironmentVariable("EMTEST_BROWSER", $ffExe, "User")
435+
$env:EMTEST_BROWSER = $ffExe
436+
Write-Host "EMTEST_BROWSER set to $env:EMTEST_BROWSER"
437+
- run:
438+
name: run tests (<< parameters.title >>)
439+
environment:
440+
EMTEST_LACKS_GRAPHICS_HARDWARE: "1"
441+
# TODO(https://github.com/emscripten-core/emscripten/issues/24205)
442+
EMTEST_LACKS_SOUND_HARDWARE: "1"
443+
EMTEST_LACKS_WEBGPU: "1"
444+
# OffscreenCanvas support is not yet done in Firefox.
445+
EMTEST_LACKS_OFFSCREEN_CANVAS: "1"
446+
EMTEST_DETECT_TEMPFILE_LEAKS: "0"
447+
EMTEST_HEADLESS: "1"
448+
EMTEST_CORES: "2"
449+
DISPLAY: ":0"
450+
command: |
451+
# There are tests in the browser test suite that using libraries
452+
# that are not included by "./embuilder build ALL". For example the
453+
# PIC version of libSDL which is used by test_sdl2_misc_main_module
454+
set EM_FROZEN_CACHE=
455+
echo "-----"
456+
echo "Running browser tests"
457+
echo "-----"
458+
test/runner << parameters.test_targets >>
459+
- upload-test-results
398460
test-sockets-chrome:
399461
description: "Runs emscripten sockets tests under chrome"
400462
steps:
@@ -1065,6 +1127,19 @@ jobs:
10651127
title: "sockets.test_nodejs_sockets_echo*"
10661128
test_targets: "sockets.test_nodejs_sockets_echo*"
10671129
- upload-test-results
1130+
# Run browser tests as well.
1131+
- run-tests-firefox-windows:
1132+
title: "browser on firefox on windows"
1133+
# skip browser.test_glbook, as it requires mingw32-make, which is not
1134+
# installed on CircleCI.
1135+
# skip browser.test_sdl2_mixer_wav_dash_l, fails to build on Windows
1136+
# on CircleCI (works locally)
1137+
test_targets: "
1138+
browser
1139+
skip:browser.test_glbook
1140+
skip:browser.test_sdl2_mixer_wav_dash_l
1141+
"
1142+
- upload-test-results
10681143

10691144
test-mac-arm64:
10701145
executor: mac-arm64

pyproject.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,7 @@ module = [
104104
"ply.*",
105105
]
106106
ignore_errors = true
107+
108+
[[tool.mypy.overrides]]
109+
module = ["psutil", "win32gui", "win32process"]
110+
ignore_missing_imports = true

requirements-dev.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
coverage[toml]==6.5
88
mypy==1.14
9+
psutil==7.0.0
910
ruff==0.11.7
1011
types-requests==2.32.0.20241016
1112
unittest-xml-reporting==3.2.0

test/common.py

Lines changed: 157 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import json
1919
import logging
2020
import os
21+
import psutil
2122
import re
2223
import shlex
2324
import shutil
@@ -36,7 +37,7 @@
3637
import clang_native
3738
import jsrun
3839
import line_endings
39-
from tools.shared import EMCC, EMXX, DEBUG
40+
from tools.shared import EMCC, EMXX, DEBUG, exe_suffix
4041
from tools.shared import get_canonical_temp_dir, path_from_root
4142
from tools.utils import MACOS, WINDOWS, read_file, read_binary, write_binary, exit_with_error
4243
from tools.settings import COMPILE_TIME_SETTINGS
@@ -87,6 +88,7 @@
8788
# file to track which tests were flaky so they can be graphed in orange color to
8889
# visually stand out.
8990
flaky_tests_log_filename = os.path.join(path_from_root('out/flaky_tests.txt'))
91+
browser_spawn_lock_filename = os.path.join(path_from_root('out/browser_spawn_lock'))
9092

9193

9294
# Default flags used to run browsers in CI testing:
@@ -116,6 +118,7 @@ class FirefoxConfig:
116118
data_dir_flag = '-profile '
117119
default_flags = ()
118120
headless_flags = '-headless'
121+
executable_name = exe_suffix('firefox')
119122

120123
@staticmethod
121124
def configure(data_dir):
@@ -945,8 +948,25 @@ def make_dir_writeable(dirname):
945948

946949

947950
def force_delete_dir(dirname):
948-
make_dir_writeable(dirname)
949-
utils.delete_dir(dirname)
951+
"""Deletes a directory. Returns whether deletion succeeded."""
952+
if not os.path.exists(dirname):
953+
return True
954+
assert not os.path.isfile(dirname)
955+
956+
try:
957+
make_dir_writeable(dirname)
958+
utils.delete_dir(dirname)
959+
except PermissionError as e:
960+
# This issue currently occurs on Windows when running browser tests e.g.
961+
# on Firefox browser. Killing Firefox browser is not 100% watertight, and
962+
# occassionally a Firefox browser process can be left behind, holding on
963+
# to a file handle, preventing the deletion from succeeding.
964+
# We expect this issue to only occur on Windows.
965+
if not WINDOWS:
966+
raise e
967+
print(f'Warning: Failed to delete directory "{dirname}"\n{e}')
968+
return False
969+
return True
950970

951971

952972
def force_delete_contents(dirname):
@@ -2508,6 +2528,81 @@ def configure_test_browser():
25082528
EMTEST_BROWSER += f" {config.headless_flags}"
25092529

25102530

2531+
def list_processes_by_name(exe_name):
2532+
pids = []
2533+
if exe_name:
2534+
for proc in psutil.process_iter():
2535+
try:
2536+
pinfo = proc.as_dict(attrs=['pid', 'name', 'exe'])
2537+
if pinfo['exe'] and exe_name in pinfo['exe'].replace('\\', '/').split('/'):
2538+
pids.append(psutil.Process(pinfo['pid']))
2539+
except psutil.NoSuchProcess: # E.g. "process no longer exists (pid=13132)" (code raced to acquire the iterator and process it)
2540+
pass
2541+
2542+
return pids
2543+
2544+
2545+
class FileLock:
2546+
"""Implements a filesystem-based mutex, with an additional feature that it
2547+
returns an integer counter denoting how many times the lock has been locked
2548+
before (during the current python test run instance)"""
2549+
def __init__(self, path):
2550+
self.path = path
2551+
self.counter = 0
2552+
2553+
def __enter__(self):
2554+
# Acquire the lock
2555+
while True:
2556+
try:
2557+
self.fd = os.open(self.path, os.O_CREAT | os.O_EXCL | os.O_WRONLY)
2558+
break
2559+
except FileExistsError:
2560+
time.sleep(0.1)
2561+
# Return the locking count number
2562+
try:
2563+
self.counter = int(open(f'{self.path}_counter').read())
2564+
except Exception:
2565+
pass
2566+
return self.counter
2567+
2568+
def __exit__(self, *a):
2569+
# Increment locking count number before releasing the lock
2570+
with open(f'{self.path}_counter', 'w') as f:
2571+
f.write(str(self.counter + 1))
2572+
# And release the lock
2573+
os.close(self.fd)
2574+
try:
2575+
os.remove(self.path)
2576+
except Exception:
2577+
pass # Another process has raced to acquire the lock, and will delete it.
2578+
2579+
2580+
def move_browser_window(pid, x, y):
2581+
"""Utility function to move the top-level window owned by given process to
2582+
(x,y) coordinate. Used to ensure each browser window has some visible area."""
2583+
import win32gui
2584+
import win32process
2585+
2586+
def enum_windows_callback(hwnd, _unused):
2587+
_, win_pid = win32process.GetWindowThreadProcessId(hwnd)
2588+
if win_pid == pid and win32gui.IsWindowVisible(hwnd):
2589+
rect = win32gui.GetWindowRect(hwnd)
2590+
win32gui.MoveWindow(hwnd, x, y, rect[2] - rect[0], rect[3] - rect[1], True)
2591+
return True
2592+
2593+
win32gui.EnumWindows(enum_windows_callback, None)
2594+
2595+
2596+
def increment_suffix_number(str_with_maybe_suffix):
2597+
match = re.match(r"^(.*?)(?:_(\d+))?$", str_with_maybe_suffix)
2598+
if match:
2599+
base, number = match.groups()
2600+
if number:
2601+
return f'{base}_{int(number) + 1}'
2602+
2603+
return f'{str_with_maybe_suffix}_1'
2604+
2605+
25112606
class BrowserCore(RunnerCore):
25122607
# note how many tests hang / do not send an output. if many of these
25132608
# happen, likely something is broken and it is best to abort the test
@@ -2524,15 +2619,19 @@ def __init__(self, *args, **kwargs):
25242619

25252620
@classmethod
25262621
def browser_terminate(cls):
2527-
cls.browser_proc.terminate()
2528-
# If the browser doesn't shut down gracefully (in response to SIGTERM)
2529-
# after 2 seconds kill it with force (SIGKILL).
2530-
try:
2531-
cls.browser_proc.wait(2)
2532-
except subprocess.TimeoutExpired:
2533-
logger.info('Browser did not respond to `terminate`. Using `kill`')
2534-
cls.browser_proc.kill()
2535-
cls.browser_proc.wait()
2622+
for proc in cls.browser_procs:
2623+
try:
2624+
proc.terminate()
2625+
# If the browser doesn't shut down gracefully (in response to SIGTERM)
2626+
# after 2 seconds kill it with force (SIGKILL).
2627+
try:
2628+
proc.wait(2)
2629+
except (subprocess.TimeoutExpired, psutil.TimeoutExpired):
2630+
logger.info('Browser did not respond to `terminate`. Using `kill`')
2631+
proc.kill()
2632+
proc.wait()
2633+
except (psutil.NoSuchProcess, ProcessLookupError):
2634+
pass
25362635

25372636
@classmethod
25382637
def browser_restart(cls):
@@ -2551,9 +2650,18 @@ def browser_open(cls, url):
25512650
if worker_id is not None:
25522651
# Running in parallel mode, give each browser its own profile dir.
25532652
browser_data_dir += '-' + str(worker_id)
2554-
if os.path.exists(browser_data_dir):
2555-
utils.delete_dir(browser_data_dir)
2653+
2654+
# Delete old browser data directory.
2655+
if WINDOWS:
2656+
# If we cannot (the data dir is in use on Windows), switch to another dir.
2657+
while not force_delete_dir(browser_data_dir):
2658+
browser_data_dir = increment_suffix_number(browser_data_dir)
2659+
else:
2660+
force_delete_dir(browser_data_dir)
2661+
2662+
# Recreate the new data directory.
25562663
os.mkdir(browser_data_dir)
2664+
25572665
if is_chrome():
25582666
config = ChromeConfig()
25592667
elif is_firefox():
@@ -2568,7 +2676,41 @@ def browser_open(cls, url):
25682676

25692677
browser_args = shlex.split(browser_args)
25702678
logger.info('Launching browser: %s', str(browser_args))
2571-
cls.browser_proc = subprocess.Popen(browser_args + [url])
2679+
2680+
if WINDOWS and is_firefox():
2681+
cls.launch_browser_harness_windows_firefox(worker_id, config, browser_args, url)
2682+
else:
2683+
cls.browser_procs = [subprocess.Popen(browser_args + [url])]
2684+
2685+
@classmethod
2686+
def launch_browser_harness_windows_firefox(cls, worker_id, config, browser_args, url):
2687+
''' Dedicated function for launching browser harness on Firefox on Windows,
2688+
which requires extra care for window positioning and process tracking.'''
2689+
2690+
with FileLock(browser_spawn_lock_filename) as count:
2691+
# Firefox is a multiprocess browser. On Windows, killing the spawned
2692+
# process will not bring down the whole browser, but only one browser tab.
2693+
# So take a delta snapshot before->after spawning the browser to find
2694+
# which subprocesses we launched.
2695+
if worker_id is not None:
2696+
procs_before = list_processes_by_name(config.executable_name)
2697+
cls.browser_procs = [subprocess.Popen(browser_args + [url])]
2698+
# Give Firefox time to spawn its subprocesses. Use an increasing timeout
2699+
# as a crude way to account for system load.
2700+
if worker_id is not None:
2701+
time.sleep(2 + count * 0.3)
2702+
procs_after = list_processes_by_name(config.executable_name)
2703+
# Make sure that each browser window is visible on the desktop. Otherwise
2704+
# browser might decide that the tab is backgrounded, and not load a test,
2705+
# or it might not tick rAF()s forward, causing tests to hang.
2706+
if worker_id is not None and not EMTEST_HEADLESS:
2707+
# On Firefox on Windows we needs to track subprocesses that got created
2708+
# by Firefox. Other setups can use 'browser_proc' directly to terminate
2709+
# the browser.
2710+
cls.browser_procs = list(set(procs_after).difference(set(procs_before)))
2711+
# Wrap window positions on a Full HD desktop area modulo primes.
2712+
for proc in cls.browser_procs:
2713+
move_browser_window(proc.pid, (300 + count * 47) % 1901, (10 + count * 37) % 997)
25722714

25732715
@classmethod
25742716
def setUpClass(cls):

test/runner.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,8 @@ def set_env(name, option_value):
564564

565565
# Remove any old test files before starting the run
566566
utils.delete_file(common.flaky_tests_log_filename)
567+
utils.delete_file(common.browser_spawn_lock_filename)
568+
utils.delete_file(f'{common.browser_spawn_lock_filename}_counter')
567569

568570
def prepend_default(arg):
569571
if arg.startswith('test_'):

0 commit comments

Comments
 (0)