Skip to content

Conversation

juj
Copy link
Collaborator

@juj juj commented Sep 11, 2025

Fix two problems with test browser configuring refactor:

  1. Each run of non-browser suites would print "No EMTEST_BROWSER set. Defaulting to google-chrome". Simplest way is to remove this print. There is already a print logger.info('Launching browser: %s', str(browser_args)) later down the line, so it should be fairly explicit what browser cmdline is being run.
  2. if user has no EMTEST_BROWSER set, then on Windows line 2479 would throw on the '"' not in EMTEST_BROWSER check, when EMTEST_BROWSER was still None.

if not has_browser():
return

if not EMTEST_BROWSER:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems odd that this code even runs at all then a non-browser test suite runs

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sbc100 sbc100 merged commit 317f212 into emscripten-core:main Sep 11, 2025
31 checks passed
@sbc100
Copy link
Collaborator

sbc100 commented Sep 11, 2025

I wonder why that windows CI bot doesn't failing on this?

@juj
Copy link
Collaborator Author

juj commented Sep 11, 2025

Maybe it unconditionally sets EMTEST_BROWSER? That was at least the reason my CI didn't fail on this - I set EMTEST_BROWSER on all runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants