Skip to content

Conversation

@juj
Copy link
Collaborator

@juj juj commented Nov 8, 2025

This is a bit of awkward combination of things.

On my CI, I test old Firefox (and Safari) browsers down to the min-spec, e.g. Firefox 68, Firefox 78, Firefox 91, and so on.

But because the default MIN_x_VERSION in src/settings.js is newer than the supported min-spec, I need to pass env. var. EMCC_CFLAGS=-sMIN_x_VERSION=68 for example when running the browser test suite.

I combine that env. var. with the new EMTEST_AUTOSKIP=1 env. var. so that I can offload having to maintain a large knowledgebase of which browser supports/doesn't support which feature inside the test runner itself.

This works great - almost.

The issue is that the browser feature skipping mechanism (EMTEST_AUTOSKIP and EMTEST_LACKS_x env. vars) has an intentional feature of skipping only the browser test run part, but not the test execution/Emscripten compilation part.

The intent of this mechanism has been to be able to partially test the browser tests, even if a skip env. var. is present, but just not run the test in an old browser that's known to fail.

But this results in the problem. When one uses e.g.

EMTEST_BROWSER=/path/to/firefox-91/firefox
EMCC_CFLAGS=-sMIN_FIREFOX_VERSION=91
EMTEST_LACKS_ES6_WORKERS=1

then the test browser.test_audio_worklet_es6 will not skip, but will error out with a feature matrix check

emcc: error: MIN_FIREFOX_VERSION=91 is not compatible with EXPORT_ES6 with -sWASM_WORKERS (MIN_FIREFOX_VERSION=114 or above required) [-Wcompatibility] [-Werror]

To fix this, we could either change the semantics of the skip env. vars to always skip the whole test, not just the running part. But that might reduce test coverage that was intentional to preserve.

Or we could adjust the EMTEST_AUTOSKIP=1 env. var. to be special and have a different "power level" of skipping compared to the EMTEST_LACKS_x=1 env. vars. But that feels arbitrary and prone to future confusion.

So this PR adds a bit of an awkward test to skip the whole test if any MIN_x_VERSION directives are explicitly present in EMCC_CFLAGS env. var. This way the above combination of flags will work to skip as desired, but will not reduce the test coverage on CircleCI.

def decorated(self, *args, **kwargs):
if should_skip == 'error':
raise Exception(f'This test requires a browser that supports {feature.name} but your browser {get_browser()} does not support this. Run with {skip_env_var}=1 or EMTEST_AUTOSKIP=1 to skip this test automatically.')
elif should_skip and bool(re.search(r"MIN_.*_VERSION", os.getenv('EMCC_CFLAGS', ''))):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would maybe just simplify this and skip whenever EMCC_CFLAGS is present at all in the environment at all. i.e. just be conservative in this case.

I'm honestly not sore that injecting cflags via external environment variables like this into the test suite is a good long term stratagy, so I'm hoping we can find a better way one day.

Copy link
Collaborator Author

@juj juj Nov 8, 2025

Choose a reason for hiding this comment

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

I would maybe just simplify this and skip whenever EMCC_CFLAGS is present at all in the environment at all. i.e. just be conservative in this case.

Hmm, that sounds like being imprecise, rather than conservative? I wonder if there is a scenario that would help?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I was thinking more like "If the user sepecified EMCC_CFLAGS when all best are off".. it would be so easy to trivally break all kinds of tests using EMCC_CFLAGS injection. Thats why I think we should come up with a better way for you do set you min browser versions.

@juj juj merged commit 4e7dc25 into emscripten-core:main Nov 8, 2025
3 of 16 checks passed
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