-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix running emrun tests on Linux against the Linux system browser #26810
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
Changes from all commits
a21cc1c
c6ddb7f
d5cb0db
e0df2e0
0c632a1
8b43aae
f92bc4a
8604995
4f6d116
e7794a5
2dca116
7abb9b5
4aed2bb
61da1f9
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 |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| # Copyright 2026 The Emscripten Authors. All rights reserved. | ||
| # Emscripten is available under two separate licenses, the MIT license and the | ||
| # University of Illinois/NCSA Open Source License. Both these licenses can be | ||
| # found in the LICENSE file. | ||
|
|
||
| import argparse | ||
| import os | ||
| import shlex | ||
| import subprocess | ||
|
|
||
| from browser_common import BrowserCore, get_browser, has_browser | ||
| from common import EMRUN, RunnerCore, path_from_root, read_file, test_file | ||
| from test_browser import also_with_threads | ||
|
|
||
| from tools.shared import EMCC, PIPE | ||
|
|
||
|
|
||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| class emrun(RunnerCore): | ||
| def test_emrun_info(self): | ||
| if not has_browser(): | ||
| self.skipTest('need a browser') | ||
| result = self.run_process([EMRUN, '--system-info', '--browser_info'], stdout=PIPE).stdout | ||
| assert 'CPU' in result | ||
| assert 'Browser' in result | ||
| assert 'Traceback' not in result | ||
|
|
||
| result = self.run_process([EMRUN, '--list-browsers'], stdout=PIPE).stdout | ||
| assert 'Traceback' not in result | ||
|
|
||
| def test_no_browser(self): | ||
| # Test --no-browser mode where we have to take care of launching the browser ourselves | ||
| # and then killing emrun when we are done. | ||
| if not has_browser(): | ||
| self.skipTest('need a browser') | ||
|
|
||
| self.run_process([EMCC, test_file('test_emrun.c'), '--emrun', '-o', 'hello_world.html']) | ||
| proc = subprocess.Popen([EMRUN, '--no-browser', '.', '--port=3333'], stdout=PIPE) | ||
| try: | ||
| if get_browser(): | ||
| url = 'http://localhost:3333/hello_world.html?argv0' | ||
| print(f'Starting browser to {url}') | ||
| BrowserCore.browser_open(url) | ||
| try: | ||
| while True: | ||
| stdout = proc.stdout.read() | ||
| if b'Dumping out file' in stdout: | ||
| break | ||
| finally: | ||
| print('Terminating browser') | ||
| BrowserCore.browser_terminate() | ||
| finally: | ||
| print('Terminating emrun server') | ||
| proc.terminate() | ||
| proc.wait() | ||
|
|
||
| def test_program_arg_separator(self): | ||
| # Verify that trying to pass argument to the page without the `--` separator will | ||
| # generate an actionable error message | ||
| err = self.expect_fail([EMRUN, '--foo']) | ||
| self.assertContained('error: unrecognized arguments: --foo', err) | ||
| self.assertContained('remember to add `--` between arguments', err) | ||
|
|
||
| @also_with_threads | ||
| def test_emrun(self): | ||
| self.emcc('test_emrun.c', ['--emrun', '-o', 'test_emrun.html']) | ||
| if not has_browser(): | ||
| self.skipTest('need a browser') | ||
|
|
||
| # We cannot run emrun from the temp directory the suite will clean up afterwards, since the | ||
| # browser that is launched will have that directory as startup directory, and the browser will | ||
| # not close as part of the test, pinning down the cwd on Windows and it wouldn't be possible to | ||
| # delete it. Therefore switch away from that directory before launching. | ||
| os.chdir(path_from_root()) | ||
|
|
||
| # emrun tests may run in parallel processes, so each test case should use a unique port number | ||
| # to avoid port address already in use errors. | ||
| port = '6939' | ||
| if '-pthread' in self.cflags: | ||
| port = '6940' | ||
|
|
||
| args_base = [EMRUN, '--timeout', '30', '--safe_firefox_profile', | ||
| '--kill-exit', '--port', port, '--verbose', | ||
| '--log-stdout', self.in_dir('stdout.txt'), | ||
| '--log-stderr', self.in_dir('stderr.txt')] | ||
|
|
||
| if get_browser() is not None: | ||
| # If EMTEST_BROWSER carried command line arguments to pass to the browser, | ||
| # (e.g. "firefox -profile /path/to/foo") those can't be passed via emrun, | ||
| # so strip them out. | ||
| browser_cmd = shlex.split(get_browser()) | ||
| browser_path = browser_cmd[0] | ||
| args_base += ['--browser', browser_path] | ||
| if len(browser_cmd) > 1: | ||
| browser_args = browser_cmd[1:] | ||
| if 'firefox' in browser_path and ('-profile' in browser_args or '--profile' in browser_args): | ||
| # emrun uses its own -profile, strip it out | ||
| parser = argparse.ArgumentParser(add_help=False) # otherwise it throws with -headless | ||
| parser.add_argument('-profile') | ||
| parser.add_argument('--profile') | ||
| browser_args = parser.parse_known_args(browser_args)[1] | ||
| if browser_args: | ||
| args_base += ['--browser_args', ' ' + ' '.join(browser_args)] | ||
|
|
||
| for args in [ | ||
| [], | ||
| ['--port', '0'], | ||
| ['--private_browsing'], | ||
| ['--dump_out_directory', 'other dir/multiple'], | ||
| ['--dump_out_directory=foo_bar'], | ||
| ]: | ||
| args = args_base + args + [self.in_dir('test_emrun.html'), '--', '1', '2', '--3', 'escaped space', 'with_underscore'] | ||
| print(shlex.join(args)) | ||
| proc = self.run_process(args, check=False) | ||
| self.assertEqual(proc.returncode, 100) | ||
| dump_dir = 'dump_out' | ||
| if '--dump_out_directory' in args: | ||
| dump_dir = 'other dir/multiple' | ||
| elif '--dump_out_directory=foo_bar' in args: | ||
| dump_dir = 'foo_bar' | ||
| self.assertExists(self.in_dir(f'{dump_dir}/test.dat')) | ||
| self.assertExists(self.in_dir(f'{dump_dir}/heap.dat')) | ||
| self.assertExists(self.in_dir(f'{dump_dir}/nested/with space.dat')) | ||
| stdout = read_file(self.in_dir('stdout.txt')) | ||
| stderr = read_file(self.in_dir('stderr.txt')) | ||
| self.assertContained('argc: 6', stdout) | ||
| self.assertContained('argv[3]: --3', stdout) | ||
| self.assertContained('argv[4]: escaped space', stdout) | ||
| self.assertContained('argv[5]: with_underscore', stdout) | ||
| self.assertContained('Hello, world!', stdout) | ||
| self.assertContained('Testing ASCII characters: !"$%&\'()*+,-./:;<=>?@[\\]^_`{|}~', stdout) | ||
| self.assertContained('Testing char sequences: %20%21 ä', stdout) | ||
| self.assertContained('hello, error stream!', stderr) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you make the "move emrun into its own file" into separate PR?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Seems reasonable to combine with adding it to list of non-parall suites, since that is the motivation for moving it).
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer not, that would cause more churn at this point. |
||
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.
Have you ever run into this issue?
Maybe we should just
assert os.path.exists(init_path), 'unable to find firefox ini file'below before reading it?Uh oh!
There was an error while loading. Please reload this page.
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.
Yes, the CircleCI build-linux bot just ran into it.. that is why I added that.
It has
/usr/bin/firefoxbut noplatform.inianywhere in sight.On my own systems I do have
platform.iniin/usr/lib/firefox-esr/platform.inior/usr/lib/firefox/platform.iniUh oh!
There was an error while loading. Please reload this page.
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.
I debugged CircleCI with a search from
/usr/to find if there's a platform.ini somewhere, but that turned up nothing.8b43aae#diff-eb5e6400ad6f92fec8556d07ec1d8d14915c79814e87f354c5ab65e6163f6ad1R176-R177
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.
The firefox we use in circleci installed from tar archive in
EMTEST_BROWSER="$HOME/firefox/firefox"? Is that the one you mean?Uh oh!
There was an error while loading. Please reload this page.
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 step: https://app.circleci.com/pipelines/github/emscripten-core/emscripten/50907/workflows/6696a39d-0750-4b09-9165-349d61aa099b/jobs/1184191
has
/usr/bin/firefox(shutil.which('firefox')returned/usr/bin/firefox), but does not have/usr/.../platform.inianywhere.Uh oh!
There was an error while loading. Please reload this page.
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.
Now I see.. this task gives a clue: https://app.circleci.com/pipelines/github/emscripten-core/emscripten/50916/workflows/c9a01534-7402-4de2-aac7-028abe35de8e/jobs/1184421
It prints
so I wonder if there's a stub executable in
/usr/bin/firefoxpresent that prints out that message. So it is expected that there wouldn't exist anyplatform.inion the system.But now I'm puzzled a bit as to why this is detecting/usr/bin/firefoxin the first place...Oh that's because I wrote a silly typo.. fixed now.
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.
That CI job should not have
EMTEST_BROWSERset at all.. its not trying to run any browser tests.I guess your new code is causing that error because its now finding the
/usr/bin/firefoxeven though it doesn't needed to for this CI step.My guess is that
/usr/bin/firefoxis a symlink to somewhere like/snap/..., so maybe following the symlinks would fix it.Ideally we could fix that TODO since then it would be moot point, but I know it wasn't easy last time I tried to move that code around.
I'm happy to iterate on this stuff in followups if you want to land this change now.
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, yeah hopefully after this lands, the instability from emrun suite will be gone.