Fix parallel browser test harness to also work with Firefox on Windows. #25275

juj · 2025-09-14T19:52:41Z

This is a cleaned up version 2 of #25237.

Two problems that I run into with the parallel browser harness on Windows with Firefox:

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.
Killing a Firefox browser process with

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:

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.
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.

…owser_harness # Conflicts: # test/runner.py

juj · 2025-09-16T22:07:51Z

This is the last PR remaining that is blocking me from testing my CI directly on emscripten/main rather than my own fork. Would be good to get this in so that I can migrate to target my CI on emscripten/main.

sbc100

I'd like to find a way keep this code isolated to only running on windows? We don't need any of that FileLock stuff except on windows right?

Perhaps it could even move into its own file? test/browser_launcher.py?

sbc100 · 2025-09-15T16:45:24Z

test/common.py

+  """Deletes a directory. Returns whether deletion succeeded."""
+  if not os.path.exists(dirname):
+    return True
+  if os.path.isfile(dirname):


How about assert not os.path.isfile(dirname) since it seems like a logical error to call this on a non-directory.

sbc100 · 2025-09-16T22:11:45Z

test/common.py

+      # Firefox is a multiprocess browser. 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.


Does this comment only apply to windows?

Yes, it seems to. Updated the comment.

sbc100 · 2025-09-16T22:13:37Z

test/common.py

+      # Delete old browser data directory. If we cannot (the data dir is in use on Windows),
+      # switch to another dir.
+      while not force_delete_dir(browser_data_dir):
+        browser_data_dir += '-another'


This seems like it could generated -another-another-another-another? Should we use a counter and do _1, _2 etc?

Also, like the the rest of this PR I'd would love to not run this loop except on windows.

Indeed it can, and does. I wanted to keep the logic simple. Updated to use _number

sbc100 · 2025-09-16T22:18:29Z

I'd also love to add some windows + firefox testing on the emscripten CI as part of this so that we can feel free to refactor without breaking stuff.

juj · 2025-09-16T22:36:25Z

I'd like to find a way keep this code isolated to only running on windows? We don't need any of that FileLock stuff except on windows right?

I was thinking that the FileLock could serve as a mechanism to solve #25069 (comment) later as well.

The overhead of running the FileLock mechanism on all platforms should be trivial and negligible. I can certainly refactor it out, though I figured your review would nit on two separate/duplicate implementations to launch the browser if I did. To me the logic reads cleaner with one common launcher impl.

juj · 2025-09-16T22:52:22Z

I'd also love to add some windows + firefox testing on the emscripten CI as part of this so that we can feel free to refactor without breaking stuff.

That is a good idea. I think that is good to develop in a separate PR.

…er_harness

juj · 2025-09-17T00:38:06Z

I'd also love to add some windows + firefox testing on the emscripten CI as part of this so that we can feel free to refactor without breaking stuff.

That is a good idea. I think that is good to develop in a separate PR.

Ok, updated this PR to include Firefox on Windows testing.

juj · 2025-09-17T09:43:26Z

Looks like CircleCI now also passes here on the new Firefox on Windows tests.

sbc100

Wow, that so great having windows browser tests running! Thanks for taking care of that.

I think maybe we should split them into their own running, but I'm happy to do that as a followup.

Regarding the process tracking issue ("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."), do you know why it only effect firefox and not chrome (which is also multi-process? I'm just curious if we could maybe get this fixed and remove all this extra code one day?

sbc100 · 2025-09-17T18:06:12Z

test/common.py

+        cls.browser_procs = list(set(procs_after).difference(set(procs_before)))
+        # 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)


Instead of putting if WINDOWS in through this block can we make the entire thing windows-only:

if not WINDOWS: cls.browser_procs = [subprocess.Popen(browser_args + [url])] else: ....

Perhaps append url to browser_args first?

ok I moved to a separate function.

sbc100 · 2025-09-17T18:06:44Z

test/runner.py

@@ -564,6 +564,8 @@ def set_env(name, option_value):

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


Is it not possible to store the count in the lock file itself? (i.e. can we avoid separate counter file?)

I did that first, but got odd behavior with O_EXCL when O_CREAT was not always unconditionally passed. So reverted to this simpler two-file form

brendandahl · 2025-09-17T18:20:43Z

test/common.py

+        # the browser.
+        cls.browser_procs = list(set(procs_after).difference(set(procs_before)))
+        # Wrap window positions on a Full HD desktop area modulo primes.
+        for proc in cls.browser_procs:


This shouldn't be needed in headless mode. Headless should bypass the window active tracking stuff.

Good point, updated.

brendandahl · 2025-09-17T18:29:44Z

.circleci/config.yml

+          command: |
+            # To download Firefox, we must first figure out what the latest Firefox version name is.
+            # This is because there does not exist a stable/static URL to download latest Firefox from.
+            $html = Invoke-WebRequest "https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/"


I lean towards beta, so we're not living so near the bleeding edge.

This is pre-existing. We already use firefox-nightly elsewhere in this file. We can consider changing separately.

Linux Firefox runs also download Nightly, so I did not want to diverge there. Maybe if we want to test beta, we'd switch both Linux and Windows Firefoxes at the same time, as a separate PR?

juj · 2025-09-17T20:00:51Z

do you know why it only effect firefox and not chrome (which is also multi-process?

I haven't stress tested browser tests on Chrome on Windows yet, so not quite sure.

I'm just curious if we could maybe get this fixed and remove all this extra code one day?

Maybe. But one has to be careful.

The need to move windows is only present on Windows. On Linux Mint 22 and Raspberry Pi 5 OS at least, the Linux windowing system cascades the windows automatically for Firefox, so they won't be interpreted to be in background.

On macOS, all Firefox windows do stack up in identical locations, but curiously this is not a problem there: Firefox still thinks all those windows are visible. Maybe this is because of the active program vs active window distinction that is present in the macOS windowing system, or maybe Firefox just doesn't implement the is-visible logic there. Not sure.

So the active process tracking is needed for the purposes of moving the Firefox browser windows only on Windows OS.

On Windows, in addition to being able to move browser windows, Firefox processes need to be tracked for clean termination. Refactoring this code requires very special care, because when all tests are green in the harness, i.e. "the happy path", then the browser harness does not have a need to ever force-terminate and restart any browser instance.

So even if this process tracking was plain removed, it might not show up immediately as a problem, if every test in the browser harness is green. I.e. in green state, the browser_restart() function will never need to be run.

When refactoring or stress-testing, it is important to manually place some browser tests in a hanging state to ensure that browser_restart() is actually getting called during the test run.

sbc100 · 2025-09-17T20:11:25Z

do you know why it only effect firefox and not chrome (which is also multi-process?

I haven't stress tested browser tests on Chrome on Windows yet, so not quite sure.

I'm just curious if we could maybe get this fixed and remove all this extra code one day?

Maybe. But one has to be careful.

The need to move windows is only present on Windows. On Linux Mint 22 and Raspberry Pi 5 OS at least, the Linux windowing system cascades the windows automatically for Firefox, so they won't be interpreted to be in background.

On macOS, all Firefox windows do stack up in identical locations, but curiously this is not a problem there: Firefox still thinks all those windows are visible. Maybe this is because of the active program vs active window distinction that is present in the macOS windowing system, or maybe Firefox just doesn't implement the is-visible logic there. Not sure.

So the active process tracking is needed for the purposes of moving the Firefox browser windows only on Windows OS.

On Windows, in addition to being able to move browser windows, Firefox processes need to be tracked for clean termination. Refactoring this code requires very special care, because when all tests are green in the harness, i.e. "the happy path", then the browser harness does not have a need to ever force-terminate and restart any browser instance.

So even if this process tracking was plain removed, it might not show up immediately as a problem, if every test in the browser harness is green. I.e. in green state, the browser_restart() function will never need to be run.

When refactoring or stress-testing, it is important to manually place some browser tests in a hanging state to ensure that browser_restart() is actually getting called during the test run.

I see, so perhaps we should have some kind of stress test most the restarts the browser for each test.

Even if we did that, how would we know for sure that we didn't still have the old browsers lying around? Especially in headless mode. For that matter, how did you notice this issue in headless mode? Or are you running locally in headfull mode?

sbc100 · 2025-09-17T20:20:01Z

Just to be clear, if we are able to shut down the FF instance fully, would we still also need the window moving logic?

juj · 2025-09-17T20:41:33Z

Even if we did that, how would we know for sure that we didn't still have the old browsers lying around? Especially in headless mode.

Checking with len(list_processes_by_name('firefox.exe')) > 0 will accurately tell if there are old browsers lying around, as long as we make an assumption that user does not have unrelated Firefox browser instances open in the background. (which CI won't have)

This will work also in headless mode.

For that matter, how did you notice this issue in headless mode? Or are you running locally in headfull mode?

I currently only run browser tests in headful mode.

Just to be clear, if we are able to shut down the FF instance fully, would we still also need the window moving logic?

Yes, these two needs (tracking processes to be able to move windows) and (tracking processes to be able to shut down FF instances fully) are two orthogonal needs to do process tracking.

sbc100 · 2025-09-17T20:44:22Z

Yes, these two needs (tracking processes to be able to move windows) and (tracking processes to be able to shut down FF instances fully) are two orthogonal needs to do process tracking.

But do we still need to move the windows if we can shutdown the old FF windows? Or is the moving only needed because the old window lingers?

juj · 2025-09-17T21:24:39Z

But do we still need to move the windows if we can shutdown the old FF windows? Or is the moving only needed because the old window lingers?

The moving is needed because if the harness launches, say, 8 browser instances, then on Windows on Firefox, all those browser instances launch in the exact same (x, y, width, height) window rectangle. Seven of those Firefoxes will decide that the browser tab is not visible, so Firefox will not bother to even load up the Emscripten test page contents. Only the topmost Firefox window will load up an Emscripten test.

So the seven background Firefoxes will time out in a hang after five minutes, while only the topmost window makes progress.

End result is that most of the tests do pass in the suite, save for a random selection that got allocated into a background browser window in the first place.

sbc100 · 2025-09-17T21:40:31Z

But do we still need to move the windows if we can shutdown the old FF windows? Or is the moving only needed because the old window lingers?

The moving is needed because if the harness launches, say, 8 browser instances, then on Windows on Firefox, all those browser instances launch in the exact same (x, y, width, height) window rectangle. Seven of those Firefoxes will decide that the browser tab is not visible, so Firefox will not bother to even load up the Emscripten test page contents. Only the topmost Firefox window will load up an Emscripten test.

So the seven background Firefoxes will time out in a hang after five minutes, while only the topmost window makes progress.

End result is that most of the tests do pass in the suite, save for a random selection that got allocated into a background browser window in the first place.

Ah I see so it relates the parallel runner. Got it. Should not be needed with -j1?

juj · 2025-09-17T21:43:44Z

Yes, indeed, if running in singlethreaded mode, moving the Firefox windows is not needed. This is realized with the if worker_id is not None: checks: if running the browser harness in singlethreaded mode, worker_id = None, and so the window moving logic is not applied.

juj added 4 commits September 14, 2025 22:50

Fix parallel browser test harness to also work with Firefox on Windows.

62c4e03

ruff

90b6d7e

Merge remote-tracking branch 'origin/main' into windows_firefox_mt_br…

cac56ff

…owser_harness # Conflicts: # test/runner.py

Add newline

54d34f6

sbc100 reviewed Sep 16, 2025

View reviewed changes

juj added 3 commits September 17, 2025 01:36

Review

d571e9b

Update comment

16819f6

Use _number instead of appending a suffix

f4c1bb2

juj added 8 commits September 17, 2025 02:05

Run Firefox browser tests on Windows.

cee618b

Comment

f4abd11

Adjust title

a12d00d

Install under user home

8c7b3d3

Update download logic

982174f

Slash, also speed up test

df587ca

Add back tests

46cd393

Merge branch 'circleci_firefox_windows' into windows_firefox_mt_brows…

5cd1b57

…er_harness

juj mentioned this pull request Sep 17, 2025

Run Firefox browser tests on Windows. #25288

Closed

Add skips

82bb79b

juj added 2 commits September 17, 2025 12:46

Remove stale comment

2210afa

Update comments

b4f114c

sbc100 reviewed Sep 17, 2025

View reviewed changes

sbc100 approved these changes Sep 17, 2025

View reviewed changes

brendandahl reviewed Sep 17, 2025

View reviewed changes

Refactor

aabbba6

ruff

4846cec

juj merged commit b52308c into emscripten-core:main Sep 17, 2025
29 of 31 checks passed

Fix parallel browser test harness to also work with Firefox on Windows. #25275

Fix parallel browser test harness to also work with Firefox on Windows. #25275

Conversation

juj commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juj commented Sep 16, 2025

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 commented Sep 16, 2025

Uh oh!

juj commented Sep 16, 2025

Uh oh!

juj commented Sep 16, 2025

Uh oh!

juj commented Sep 17, 2025

Uh oh!

juj commented Sep 17, 2025

Uh oh!

sbc100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

juj commented Sep 17, 2025

Uh oh!

sbc100 commented Sep 17, 2025

Uh oh!

sbc100 commented Sep 17, 2025

Uh oh!

juj commented Sep 17, 2025

Uh oh!

sbc100 commented Sep 17, 2025

Uh oh!

juj commented Sep 17, 2025

Uh oh!

sbc100 commented Sep 17, 2025

Uh oh!

juj commented Sep 17, 2025

Uh oh!

Uh oh!

juj commented Sep 14, 2025 •

edited

Loading