Skip to content

Commit a4d0dbc

Browse files
authored
Fix shlex splitting when launching parallel browser test runner on Windows. (#25222)
Fix shlex splitting when launching parallel browser test runner on Windows. Before this PR, running ``` set EMTEST_BROWSER=C:\Program Files\Mozilla Firefox\firefox.exe test\runner browser ``` would fail with the parallel test harness attempting to launch browser command line `["C:\Program Files\Mozilla Firefox\firefox.exe -profile C:\emsdk\emscripten\main\out\profile-1"]` when it should instead have attempted to launch command line `["C:\Program Files\Mozilla Firefox\firefox.exe", "-profile", "C:\emsdk\emscripten\main\out\profile-1"]` Fix by having the browser config first do the shlex stuff to interpret `EMTEST_BROWSER`, and only then append the list of strings of command line args, rather than first append the command line args as a string spaghetti, and using shlex split to parse the resulting command line string.
1 parent 98aabf2 commit a4d0dbc

File tree

1 file changed

+28
-18
lines changed

1 file changed

+28
-18
lines changed

test/common.py

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -91,31 +91,40 @@
9191

9292
# Default flags used to run browsers in CI testing:
9393
class ChromeConfig:
94-
data_dir_flag = '--user-data-dir='
9594
default_flags = (
9695
# --no-sandbox because we are running as root and chrome requires
9796
# this flag for now: https://crbug.com/638180
98-
'--no-first-run -start-maximized --no-sandbox --enable-unsafe-swiftshader --use-gl=swiftshader --enable-experimental-web-platform-features --enable-features=JavaScriptSourcePhaseImports',
99-
'--enable-experimental-webassembly-features --js-flags="--experimental-wasm-stack-switching --experimental-wasm-type-reflection --experimental-wasm-rab-integration"',
97+
'--no-first-run', '-start-maximized', '--no-sandbox', '--enable-unsafe-swiftshader', '--use-gl=swiftshader',
98+
'--enable-experimental-web-platform-features', '--enable-features=JavaScriptSourcePhaseImports',
99+
'--enable-experimental-webassembly-features',
100+
# N.b. the following JS engine flags are passed to --js-flags=, and must appear as one element in this list.
101+
'--js-flags=--experimental-wasm-stack-switching --experimental-wasm-type-reflection --experimental-wasm-rab-integration',
100102
# The runners lack sound hardware so fallback to a dummy device (and
101103
# bypass the user gesture so audio tests work without interaction)
102-
'--use-fake-device-for-media-stream --autoplay-policy=no-user-gesture-required',
104+
'--use-fake-device-for-media-stream', '--autoplay-policy=no-user-gesture-required',
103105
# Cache options.
104-
'--disk-cache-size=1 --media-cache-size=1 --disable-application-cache',
106+
'--disk-cache-size=1', '--media-cache-size=1', '--disable-application-cache',
105107
# Disable various background tasks downloads (e.g. updates).
106108
'--disable-background-networking',
107109
)
108-
headless_flags = '--headless=new --window-size=1024,768'
110+
headless_flags = ['--headless=new', '--window-size=1024,768']
111+
112+
@staticmethod
113+
def data_dir_cmdline(data_dir):
114+
return [f'--user-data-dir={data_dir}']
109115

110116
@staticmethod
111117
def configure(data_dir):
112118
"""Chrome has no special configuration step."""
113119

114120

115121
class FirefoxConfig:
116-
data_dir_flag = '-profile '
117122
default_flags = ()
118-
headless_flags = '-headless'
123+
headless_flags = ['-headless']
124+
125+
@staticmethod
126+
def data_dir_cmdline(data_dir):
127+
return ['-profile', data_dir]
119128

120129
@staticmethod
121130
def configure(data_dir):
@@ -2502,6 +2511,15 @@ def browser_open(cls, url):
25022511
logger.info('No EMTEST_BROWSER set. Defaulting to `google-chrome`')
25032512
EMTEST_BROWSER = 'google-chrome'
25042513

2514+
if WINDOWS:
2515+
# On Windows env. vars canonically use backslashes as directory delimiters, e.g.
2516+
# set EMTEST_BROWSER=C:\Program Files\Mozilla Firefox\firefox.exe
2517+
# and spaces are not escaped. But make sure to also support args, e.g.
2518+
# set EMTEST_BROWSER="C:\Users\clb\AppData\Local\Google\Chrome SxS\Application\chrome.exe" --enable-unsafe-webgpu
2519+
if '"' not in EMTEST_BROWSER and "'" not in EMTEST_BROWSER:
2520+
EMTEST_BROWSER = '"' + EMTEST_BROWSER.replace("\\", "/") + '"'
2521+
browser_args = shlex.split(EMTEST_BROWSER)
2522+
25052523
if EMTEST_BROWSER_AUTO_CONFIG:
25062524
logger.info('Using default CI configuration.')
25072525
cls.browser_data_dir = DEFAULT_BROWSER_DATA_DIR
@@ -2517,19 +2535,11 @@ def browser_open(cls, url):
25172535
config = FirefoxConfig()
25182536
else:
25192537
exit_with_error("EMTEST_BROWSER_AUTO_CONFIG only currently works with firefox or chrome.")
2520-
EMTEST_BROWSER += f" {config.data_dir_flag}{cls.browser_data_dir} {' '.join(config.default_flags)}"
2538+
browser_args += config.data_dir_cmdline(cls.browser_data_dir) + list(config.default_flags)
25212539
if EMTEST_HEADLESS == 1:
2522-
EMTEST_BROWSER += f" {config.headless_flags}"
2540+
browser_args += config.headless_flags
25232541
config.configure(cls.browser_data_dir)
25242542

2525-
if WINDOWS:
2526-
# On Windows env. vars canonically use backslashes as directory delimiters, e.g.
2527-
# set EMTEST_BROWSER=C:\Program Files\Mozilla Firefox\firefox.exe
2528-
# and spaces are not escaped. But make sure to also support args, e.g.
2529-
# set EMTEST_BROWSER="C:\Users\clb\AppData\Local\Google\Chrome SxS\Application\chrome.exe" --enable-unsafe-webgpu
2530-
if '"' not in EMTEST_BROWSER and "'" not in EMTEST_BROWSER:
2531-
EMTEST_BROWSER = '"' + EMTEST_BROWSER.replace("\\", "/") + '"'
2532-
browser_args = shlex.split(EMTEST_BROWSER)
25332543
logger.info('Launching browser: %s', str(browser_args))
25342544
cls.browser_proc = subprocess.Popen(browser_args + [url])
25352545

0 commit comments

Comments
 (0)