Skip to content

Commit

Permalink
Fix combining of test parametization. NFC (#22637)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sbc100 authored Oct 1, 2024
1 parent f214d08 commit e44f382
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 19 deletions.
20 changes: 19 additions & 1 deletion test/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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']
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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')
Expand All @@ -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')
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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
Expand Down
20 changes: 12 additions & 8 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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']
Expand All @@ -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)

Expand Down Expand Up @@ -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);'
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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'''
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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')
Expand Down
4 changes: 2 additions & 2 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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 <stdio.h>
#include <SDL.h>
Expand Down Expand Up @@ -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 <stdio.h>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e44f382

Please sign in to comment.