Skip to content

Fix test browser.test_audio_worklet_direct to not be attempted on too old Firefox browser versions #26761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
juj merged 2 commits into from
Apr 23, 2026
+9 −1
Merged

Fix test browser.test_audio_worklet_direct to not be attempted on too old Firefox browser versions #26761

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5ce3d1
Fix test browser.test_audio_worklet_direct to not be attempted on too…
juj Apr 23, 2026
08058d6
Remove annotation
juj Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion test/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ def webgl2_disabled():
return browser_should_skip_feature('EMTEST_LACKS_WEBGL2', Feature.WEBGL2) or browser_should_skip_feature('EMTEST_LACKS_GRAPHICS_HARDWARE', Feature.WEBGL2)


def es6_module_workers_disabled():
return browser_should_skip_feature('EMTEST_LACKS_ES6_WORKERS', Feature.WORKER_ES6_MODULES)


requires_graphics_hardware = skipIfFeatureNotAvailable('EMTEST_LACKS_GRAPHICS_HARDWARE', None, 'This test requires graphics hardware')
requires_webgl2 = skipIfFeatureNotAvailable(['EMTEST_LACKS_WEBGL2', 'EMTEST_LACKS_GRAPHICS_HARDWARE'], Feature.WEBGL2, 'This test requires WebGL2 to be available')
requires_webgpu = skipIfFeatureNotAvailable(['EMTEST_LACKS_WEBGPU', 'EMTEST_LACKS_GRAPHICS_HARDWARE'], Feature.WEBGPU, 'This test requires WebGPU to be available')
Expand Down Expand Up @@ -5476,9 +5480,10 @@ def test_full_js_library_strict(self):
'audio_params_disabled': (['-sAUDIO_WORKLET_SUPPORT_AUDIO_PARAMS=0'],),
})
@requires_sound_hardware
@requires_es6_workers
@requires_shared_array_buffer
def test_audio_worklet(self, args):
if '-sEXPORT_ES6' in args and es6_module_workers_disabled():
self.skipTest('This test requires a browser with ES6 Module Workers support')
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 23, 2026

Choose a reason for hiding this comment

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

How is this different to the existing @requires_es6_workers?

Copy link
Copy Markdown
Collaborator Author

@juj juj Apr 23, 2026

Choose a reason for hiding this comment

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

Ops, good catch. I intended to remove the @requires_es6_workers annotation. The annotation was too comprehensive, even though only few cases required it. Updated now.

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 23, 2026

Choose a reason for hiding this comment

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

Can you explain what you mean? Whats wrong with @requires_es6_workers here?

Copy link
Copy Markdown
Collaborator Author

@juj juj Apr 23, 2026

Choose a reason for hiding this comment

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

Adding @requires_es6_workers would apply the ES6 Workers requirement over to all the variants:

  @parameterized({
    '': ([],),
    'with_fs': (['--preload-file', test_file('hello_world.c') + '@/'],),
    'closure': (['--closure', '1', '-Oz'],),
    'asyncify': (['-sASYNCIFY'],),
    'pthreads': (['-pthread', '-sPTHREAD_POOL_SIZE=2'],),
    'pthreads_and_closure': (['-pthread', '--closure', '1', '-Oz'],),
    'minimal_runtime': (['-sMINIMAL_RUNTIME'],),
    'minimal_runtime_pthreads_and_closure': (['-sMINIMAL_RUNTIME', '-pthread', '--closure', '1', '-Oz'],),
    'pthreads_es6': (['-pthread', '-sPTHREAD_POOL_SIZE=2', '-sEXPORT_ES6'],),
    'es6': (['-sEXPORT_ES6'],),
    'strict': (['-sSTRICT'],),
    'audio_params_disabled': (['-sAUDIO_WORKLET_SUPPORT_AUDIO_PARAMS=0'],),
  })

even though only pthreads_es6 and es6 require it. So that requirement would then skip testing coverage in any of the other cases on older Firefoxes that do support Audio Worklets, but did not support ES6 Module Workers.

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 23, 2026

Choose a reason for hiding this comment

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

I see.

For this type of case what I normally do is have an underlying self.require_es6_workers() that can then by used both in the decorator and locally within tests too. (note the singular require and then plural requires in the decorator name).

Its a shame that for decorators based on skipIfFeatureNotAvailable we don't have any easy way to use it in a non-decorator context today.

lgtm

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 23, 2026

Choose a reason for hiding this comment

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

Hopefully this will all go away if we ever land #26677!

self.btest_exit('webaudio/audioworklet.c', cflags=['-sAUDIO_WORKLET', '-sWASM_WORKERS', '-DTEST_AND_EXIT'] + args)

# Tests that audioworklets and workers can be used at the same time
Expand Down Expand Up @@ -5529,6 +5534,9 @@ def test_audio_worklet_emscripten_locks(self):
self.btest_exit('webaudio/audioworklet_emscripten_locks.c', cflags=['-sAUDIO_WORKLET', '-sWASM_WORKERS', '-pthread'])

def test_audio_worklet_direct(self):
if es6_module_workers_disabled():
self.skipTest('This test requires a browser with ES6 Module Workers support')

self.add_browser_reporting()
self.emcc('hello_world.c', ['-o', 'hello_world.mjs', '-sEXPORT_ES6', '-sSINGLE_FILE', '-sENVIRONMENT=worklet'])
create_file('worklet.mjs', '''
Expand Down
Loading