From e44f3827ac4ff09e41f32ff7f1d0157ced0045d6 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Mon, 30 Sep 2024 23:08:19 -0700 Subject: [PATCH] Fix combining of test parametization. NFC (#22637) As far as I can tell the only test that was relying on this features is `browser.test_sdl_image_prepare` which uses `@also_with_proxied` in addition to `@also_with_wasmfs` and it was getting the names and values backwards. I confirmed manually that all 4 test combination now do the right thing. --- test/common.py | 20 +++++++++++++++++++- test/test_browser.py | 20 ++++++++++++-------- test/test_core.py | 4 ++-- test/test_other.py | 16 ++++++++-------- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/test/common.py b/test/common.py index 3f366c2d5e53..b861530c027d 100644 --- a/test/common.py +++ b/test/common.py @@ -376,6 +376,8 @@ def also_with_wasmfs(f): @wraps(f) def metafunc(self, wasmfs, *args, **kwargs): + if DEBUG: + print('parameterize:wasmfs=%d' % wasmfs) if wasmfs: self.set_setting('WASMFS') self.emcc_args.append('-DWASMFS') @@ -393,6 +395,8 @@ def also_with_noderawfs(func): @wraps(func) def metafunc(self, rawfs, *args, **kwargs): + if DEBUG: + print('parameterize:rawfs=%d' % rawfs) if rawfs: self.require_node() self.emcc_args += ['-DNODERAWFS'] @@ -410,6 +414,8 @@ def also_with_env_modify(name_updates_mapping): def decorated(f): @wraps(f) def metafunc(self, updates, *args, **kwargs): + if DEBUG: + print('parameterize:env_modify=%s' % (updates)) if updates: with env_modify(updates): return f(self, *args, **kwargs) @@ -432,6 +438,8 @@ def also_with_minimal_runtime(f): @wraps(f) def metafunc(self, with_minimal_runtime, *args, **kwargs): + if DEBUG: + print('parameterize:minimal_runtime=%s' % with_minimal_runtime) assert self.get_setting('MINIMAL_RUNTIME') is None if with_minimal_runtime: self.set_setting('MINIMAL_RUNTIME', 1) @@ -447,6 +455,8 @@ def also_with_wasm_bigint(f): @wraps(f) def metafunc(self, with_bigint, *args, **kwargs): + if DEBUG: + print('parameterize:bigint=%s' % with_bigint) if with_bigint: if self.is_wasm2js(): self.skipTest('wasm2js does not support WASM_BIGINT') @@ -469,6 +479,8 @@ def also_with_wasm64(f): @wraps(f) def metafunc(self, with_wasm64, *args, **kwargs): + if DEBUG: + print('parameterize:wasm64=%s' % with_wasm64) if with_wasm64: self.require_wasm64() self.set_setting('MEMORY64') @@ -488,6 +500,8 @@ def also_with_wasm2js(f): @wraps(f) def metafunc(self, with_wasm2js, *args, **kwargs): assert self.get_setting('WASM') is None + if DEBUG: + print('parameterize:wasm2js=%s' % with_wasm2js) if with_wasm2js: self.require_wasm2js() self.set_setting('WASM', 0) @@ -522,6 +536,8 @@ def also_with_standalone_wasm(impure=False): def decorated(func): @wraps(func) def metafunc(self, standalone): + if DEBUG: + print('parameterize:standalone=%s' % standalone) if not standalone: func(self) else: @@ -559,6 +575,8 @@ def with_all_eh_sjlj(f): @wraps(f) def metafunc(self, mode, *args, **kwargs): + if DEBUG: + print('parameterize:eh_mode=%s' % mode) if mode == 'wasm' or mode == 'wasm_exnref': # Wasm EH is currently supported only in wasm backend and V8 if self.is_wasm2js(): @@ -719,7 +737,7 @@ def parameterize(func, parameters): if prev: # If we're parameterizing 2nd time, construct a cartesian product for various combinations. func._parameterize = { - '_'.join(filter(None, [k1, k2])): v1 + v2 for (k1, v1), (k2, v2) in itertools.product(prev.items(), parameters.items()) + '_'.join(filter(None, [k1, k2])): v2 + v1 for (k1, v1), (k2, v2) in itertools.product(prev.items(), parameters.items()) } else: func._parameterize = parameters diff --git a/test/test_browser.py b/test/test_browser.py index e4b32d40b671..9ef63bb61453 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -27,7 +27,7 @@ from tools import shared from tools import ports from tools import utils -from tools.shared import EMCC, WINDOWS, FILE_PACKAGER, PIPE +from tools.shared import EMCC, WINDOWS, FILE_PACKAGER, PIPE, DEBUG from tools.utils import delete_dir @@ -82,6 +82,8 @@ def also_with_wasmfs(f): @wraps(f) def metafunc(self, wasmfs, *args, **kwargs): + if DEBUG: + print('parameterize:wasmfs=%d' % wasmfs) if wasmfs: self.set_setting('WASMFS') self.emcc_args = self.emcc_args.copy() + ['-DWASMFS'] @@ -100,6 +102,8 @@ def also_with_proxying(f): @wraps(f) def metafunc(self, proxied, *args, **kwargs): + if DEBUG: + print('parameterize:proxied=%d' % proxied) self.proxied = proxied f(self, *args, **kwargs) @@ -901,7 +905,7 @@ def test_sdl_canvas_alpha(self): '': (False,), 'delay': (True,) }) - def test_sdl_key(self, delay, async_, defines): + def test_sdl_key(self, defines, async_, delay): if delay: settimeout_start = 'setTimeout(function() {' settimeout_end = '}, 1);' @@ -1970,11 +1974,11 @@ def test_cubegeom_pre2(self): def test_cubegeom_pre3(self): self.reftest('third_party/cubegeom/cubegeom_pre3.c', 'third_party/cubegeom/cubegeom_pre2.png', args=['-sLEGACY_GL_EMULATION', '-lGL', '-lSDL']) + @also_with_proxying @parameterized({ '': ([],), 'tracing': (['-sTRACE_WEBGL_CALLS'],), }) - @also_with_proxying @requires_graphics_hardware def test_cubegeom(self, args): if self.proxied and args: @@ -2725,11 +2729,11 @@ def test_wget_data(self): create_file('test.txt', 'emscripten') self.btest_exit('test_wget_data.c', args=['-O2', '-g2', '-sASYNCIFY']) + @also_with_wasm2js @parameterized({ '': ([],), 'es6': (['-sEXPORT_ES6'],), }) - @also_with_wasm2js def test_locate_file(self, args): self.set_setting('EXIT_RUNTIME') create_file('src.c', r''' @@ -4106,11 +4110,11 @@ def test_utf8_textdecoder(self): def test_utf16_textdecoder(self): self.btest_exit('benchmark/benchmark_utf16.cpp', 0, args=['--embed-file', test_file('utf16_corpus.txt') + '@/utf16_corpus.txt', '-sEXPORTED_RUNTIME_METHODS=UTF16ToString,stringToUTF16,lengthBytesUTF16']) + @also_with_threads @parameterized({ '': ([],), 'closure': (['--closure=1'],), }) - @also_with_threads def test_TextDecoder(self, args): self.emcc_args += args @@ -4338,7 +4342,7 @@ def test_webgl_resize_offscreencanvas_from_main_thread(self, args1, args2, args3 'enable': (1,), 'disable': (0,), }) - def test_webgl_simple_extensions(self, simple_enable_extensions, webgl_version): + def test_webgl_simple_extensions(self, webgl_version, simple_enable_extensions): cmd = ['-DWEBGL_CONTEXT_VERSION=' + str(webgl_version), '-DWEBGL_SIMPLE_ENABLE_EXTENSION=' + str(simple_enable_extensions), '-sMAX_WEBGL_VERSION=2', @@ -4393,12 +4397,12 @@ def test_fetch_to_memory(self): self.btest_exit('fetch/test_fetch_to_memory.cpp', args=['-sFETCH_DEBUG', '-sFETCH'] + arg) + @also_with_wasm2js @parameterized({ '': ([],), 'pthread_exit': (['-DDO_PTHREAD_EXIT'],), }) @no_firefox('https://github.com/emscripten-core/emscripten/issues/16868') - @also_with_wasm2js def test_fetch_from_thread(self, args): shutil.copy(test_file('gears.png'), '.') self.btest_exit('fetch/test_fetch_from_thread.cpp', @@ -4628,11 +4632,11 @@ def test_single_file_html(self): self.assertNotExists('test.wasm') # Tests that SINGLE_FILE works as intended in generated HTML with MINIMAL_RUNTIME + @also_with_wasm2js @parameterized({ '': ([],), 'O3': (['-O3'],) }) - @also_with_wasm2js def test_minimal_runtime_single_file_html(self, opts): self.btest('single_file_static_initializer.cpp', '19', args=opts + ['-sMINIMAL_RUNTIME', '-sSINGLE_FILE']) self.assertExists('test.html') diff --git a/test/test_core.py b/test/test_core.py index a040f44a7832..e343a573fe93 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -4624,13 +4624,13 @@ def test_dylink_postsets_chunking(self): int global_var = 12345; ''', expected=['12345\n'], force_c=True) + @with_dylink_reversed @parameterized({ 'libcxx': ('libc,libc++,libmalloc,libc++abi',), 'all': ('1',), 'missing': ('libc,libmalloc,libc++abi', False, False, False), 'missing_assertions': ('libc,libmalloc,libc++abi', False, False, True), }) - @with_dylink_reversed def test_dylink_syslibs(self, syslibs, expect_pass=True, with_reversed=True, assertions=True): # one module uses libcxx, need to force its inclusion when it isn't the main if not with_reversed and self.dylink_reversed: @@ -9377,11 +9377,11 @@ def test_pthread_dylink_main_module_1(self): self.set_setting('MAIN_MODULE') self.do_runf('hello_world.c') + @with_dylink_reversed @parameterized({ '': ([],), 'pthreads': (['-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME', '-pthread', '-Wno-experimental'],) }) - @with_dylink_reversed def test_Module_dynamicLibraries(self, args): # test that Module.dynamicLibraries works with pthreads self.emcc_args += args diff --git a/test/test_other.py b/test/test_other.py index 5a7d2d9ef841..9111feb6d658 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -429,7 +429,7 @@ def test_export_es6_allows_export_in_post_js(self): # load a worker before startup to check ES6 modules there as well 'pthreads': (['-pthread', '-sPTHREAD_POOL_SIZE=1'],), }) - def test_export_es6(self, args, package_json): + def test_export_es6(self, package_json, args): self.run_process([EMCC, test_file('hello_world.c'), '-sEXPORT_ES6', '-o', 'hello.mjs'] + args) # In ES6 mode we use MODULARIZE, so we must instantiate an instance of the @@ -1646,12 +1646,12 @@ def test_export_all_and_exported_functions(self): self.emcc('lib.c', ['-sEXPORTED_FUNCTIONS=_libfunc2', '-sEXPORT_ALL', '--pre-js', 'pre.js'], output_filename='a.out.js') self.assertContained('libfunc\n', self.run_js('a.out.js')) + @also_with_wasmfs + @crossplatform @parameterized({ '': ([],), 'closure': (['-O2', '--closure=1'],), }) - @also_with_wasmfs - @crossplatform def test_stdin(self, args): create_file('in.txt', 'abcdef\nghijkl\n') self.set_setting('ENVIRONMENT', 'node,shell') @@ -2642,7 +2642,7 @@ def test_undefined_exported_js_function(self, outfile): 'error': ['ERROR'], 'ignore': [None] }) - def test_undefined_symbols(self, action, args): + def test_undefined_symbols(self, args, action): create_file('main.c', r''' #include #include @@ -4900,12 +4900,12 @@ def add_on_abort_and_verify(extra=''): os.remove('a.out.wasm') # trigger onAbort by intentionally causing startup to fail add_on_abort_and_verify() + @also_with_wasm2js @parameterized({ '': ([],), '01': (['-O1', '-g2'],), 'O2': (['-O2', '-g2', '-flto'],), }) - @also_with_wasm2js def test_no_exit_runtime(self, opts): create_file('code.cpp', r''' #include @@ -5193,7 +5193,7 @@ def test_warn_dylibs(self): 'wasm2js': [0], 'wasm2js_2': [2], }) - def test_symbol_map(self, wasm, opts): + def test_symbol_map(self, opts, wasm): def get_symbols_lines(symbols_file): self.assertTrue(os.path.isfile(symbols_file), "Symbols file %s isn't created" % symbols_file) # check that the map is correct @@ -8304,7 +8304,7 @@ def test(contents, expected, args=[], assert_returncode=0): # noqa '': ([],), 'O2': (['-O2'],), }) - def test_warn_unexported_main(self, args, filename): + def test_warn_unexported_main(self, filename, args): warning = 'emcc: warning: `main` is defined in the input files, but `_main` is not in `EXPORTED_FUNCTIONS`. Add it to this list if you want `main` to run. [-Wunused-main]' proc = self.run_process([EMCC, test_file(filename), '-sEXPORTED_FUNCTIONS=[]'] + args, stderr=PIPE) @@ -13064,10 +13064,10 @@ def test_check_undefined(self, flag): err = self.expect_fail([EMCC, flag, '-sERROR_ON_UNDEFINED_SYMBOLS', test_file('other/test_check_undefined.c')]) self.assertContained('undefined symbol: foo', err) + @also_with_wasm64 @parameterized({ 'asyncify': (['-sASYNCIFY'],), }) - @also_with_wasm64 def test_missing_symbols_at_runtime(self, args): # We deliberately pick a symbol there that takes a pointer as an argument. # We had a regression where the pointer-handling wrapper function could