Skip to content

Commit 4e7dc25

Browse files
authored
Fix passing MIN_x_VERSION in EMCC_CFLAGS when running with a skip env. var. set. (#25753)
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.
1 parent 35482d3 commit 4e7dc25

File tree

1 file changed

+8
-0
lines changed

1 file changed

+8
-0
lines changed

test/test_browser.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import argparse
77
import os
88
import random
9+
import re
910
import shlex
1011
import shutil
1112
import subprocess
@@ -236,8 +237,15 @@ def decorator(f):
236237
def decorated(self, *args, **kwargs):
237238
if should_skip == 'error':
238239
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.')
240+
elif should_skip and bool(re.search(r"MIN_.*_VERSION", os.getenv('EMCC_CFLAGS', ''))):
241+
# should_skip=True, so we should skip running this test. However, user has specified a MIN_x_VERSION
242+
# directive in EMCC_CFLAGS, so we cannot even try to compile this test, or otherwise emcc can
243+
# error out on the MIN_x_VERSION being too old. So skip both compiling+running this test.
244+
self.skipTest(message)
239245
elif should_skip:
246+
# Skip running this test in a browser, but do test compiling it, to get partial coverage.
240247
self.skip_exec = message
248+
241249
f(self, *args, **kwargs)
242250

243251
return decorated

0 commit comments

Comments
 (0)