Skip to content

[CI] Enable audio tests in Chrome #23665

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
merged 16 commits into from
Feb 18, 2025
Merged

[CI] Enable audio tests in Chrome #23665

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
42e032e
Minor: Firefox now supports wasm64
cwoffenden Feb 12, 2025
c858dea
The AW test should now fail, waiting on audio playback
cwoffenden Feb 13, 2025
29451a4
Confirmed failure, now flagging as needing audio
cwoffenden Feb 13, 2025
b7fb25a
Correctly halt for btest_exit()
cwoffenden Feb 14, 2025
a9132c8
Attempt to run CI with autoplay enabled
cwoffenden Feb 14, 2025
92c8b5c
Retrying without wasm64
cwoffenden Feb 14, 2025
a42c6d4
Try adding a dummy sound device...
cwoffenden Feb 14, 2025
4b80dd2
Try with a full path...
cwoffenden Feb 14, 2025
9bcf181
Try with Chrome flags for audio...
cwoffenden Feb 14, 2025
f9a781b
Try dummy audio for FF; remove LACKS_SOUND_HARDWARE
cwoffenden Feb 15, 2025
e8ca2c4
Removed FF flags (they don't work)
cwoffenden Feb 18, 2025
9fd83f7
Flagged tests as requiring sound hardware
cwoffenden Feb 18, 2025
9e63502
Revert back test (changes moved to #23695)
cwoffenden Feb 18, 2025
e9f37db
Finish moving test code to other PR
cwoffenden Feb 18, 2025
5d27dc8
SDL audio tests with wasm64 disabled for now
cwoffenden Feb 18, 2025
9008192
Changed todo to URL of PR
cwoffenden Feb 18, 2025
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
7 changes: 5 additions & 2 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,16 +306,18 @@ commands:
- run:
name: run tests (<< parameters.title >>)
environment:
EMTEST_LACKS_SOUND_HARDWARE: "1"
EMTEST_DETECT_TEMPFILE_LEAKS: "0"
# --no-sandbox because we are running as root and chrome requires
# this flag for now: https://crbug.com/638180
CHROME_FLAGS_BASE: "--no-first-run -start-maximized --no-sandbox --use-gl=swiftshader --user-data-dir=/tmp/chrome-emscripten-profile --enable-experimental-web-platform-features"
CHROME_FLAGS_HEADLESS: "--headless=new --remote-debugging-port=1234"
CHROME_FLAGS_WASM: "--enable-experimental-webassembly-features --js-flags=\"--experimental-wasm-stack-switching --experimental-wasm-type-reflection\""
CHROME_FLAGS_NOCACHE: "--disk-cache-dir=/dev/null --disk-cache-size=1 --media-cache-size=1 --disable-application-cache --incognito"
# The runners lack sound hardware so fallback to a dummy device (and
# bypass the user gesture so audio tests work without interaction)
CHROME_FLAGS_AUDIO: " --use-fake-device-for-media-stream --autoplay-policy=no-user-gesture-required"
command: |
export EMTEST_BROWSER="/usr/bin/google-chrome $CHROME_FLAGS_BASE $CHROME_FLAGS_HEADLESS $CHROME_FLAGS_WASM $CHROME_FLAGS_NOCACHE"
export EMTEST_BROWSER="/usr/bin/google-chrome $CHROME_FLAGS_BASE $CHROME_FLAGS_HEADLESS $CHROME_FLAGS_WASM $CHROME_FLAGS_NOCACHE $CHROME_FLAGS_AUDIO"
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we make this change should we also remove the EMTEST_LACKS_SOUND_HARDWARE setting above?

I'm kind of inclined to make this change as its one separate PR, e.g. [CI] Enable audio testing on chrome?

Copy link
Collaborator Author

@cwoffenden cwoffenden Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't have much time today so it was mostly experimenting and coming back to it, but:

  • I'll remove theEMTEST_LACKS_SOUND_HARDWARE ✅
  • I'll rename to match its new purpose (I wasn't sure what I'd achieve when starting) ✅
  • I'll disable all the wasm64 and 2GB tests with audio for now

Is it fine to always enable the dummy sound hardware and bypass the media interaction? I can't think what this would break (and where real sound hardware is present its used as a preference, at least from my manual testing).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I imagine enabling dummy sounds hardware and bypass the interaction check should be fine for all out tests..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I limited removing EMTEST_LACKS_SOUND_HARDWARE to Chrome (FF I can't get to work, documented in the comments).

# There are tests in the browser test suite that using libraries
# that are not included by "./embuilder build ALL". For example the
# PIC version of libSDL which is used by test_sdl2_misc_main_module
Expand Down Expand Up @@ -369,6 +371,7 @@ commands:
# support in headless mode) resolves
EMTEST_LACKS_GRAPHICS_HARDWARE: "1"
EMTEST_LACKS_WEBGPU: "1"
# TODO: replicate with FF the same as CHROME_FLAGS_AUDIO
EMTEST_LACKS_SOUND_HARDWARE: "1"
# OffscreenCanvas support is not yet done in Firefox.
EMTEST_LACKS_OFFSCREEN_CANVAS: "1"
Expand Down
6 changes: 4 additions & 2 deletions test/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3231,6 +3231,7 @@ def test_sdl2_misc_via_object(self):
'': (['-sUSE_SDL=2', '-sUSE_SDL_MIXER=2'],),
'dash_l': (['-lSDL2', '-lSDL2_mixer'],),
})
@no_wasm64('TODO: SDL2 audio memory64')
@requires_sound_hardware
def test_sdl2_mixer_wav(self, flags):
shutil.copy(test_file('sounds/the_entertainer.wav'), 'sound.wav')
Expand All @@ -3244,6 +3245,7 @@ def test_sdl2_mixer_wav(self, flags):
# TODO: need to source freepats.cfg and a midi file
# 'mod': (['mid'], 'MIX_INIT_MID', 'midi.mid'),
})
@no_wasm64('TODO: SDL2 audio memory64')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just submitted a PR on the SDL side: libsdl-org/SDL#12332

Can you use that URL as the string argument to no_wasm64 here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@requires_sound_hardware
def test_sdl2_mixer_music(self, formats, flags, music_name):
shutil.copy(test_file('sounds', music_name), '.')
Expand Down Expand Up @@ -5469,9 +5471,8 @@ def test_full_js_library_strict(self):
'es6': (['-sEXPORT_ES6'],),
'strict': (['-sSTRICT'],),
})
@requires_sound_hardware
def test_audio_worklet(self, args):
if '-sMEMORY64' in args and is_firefox():
self.skipTest('https://github.com/emscripten-core/emscripten/issues/19161')
self.btest_exit('webaudio/audioworklet.c', args=['-sAUDIO_WORKLET', '-sWASM_WORKERS'] + args)

# Tests that audioworklets and workers can be used at the same time
Expand All @@ -5490,6 +5491,7 @@ def test_audio_worklet_post_function(self, args):
'': ([],),
'closure': (['--closure', '1', '-Oz'],),
})
@requires_sound_hardware
def test_audio_worklet_modularize(self, args):
self.btest_exit('webaudio/audioworklet.c', args=['-sAUDIO_WORKLET', '-sWASM_WORKERS', '-sMODULARIZE=1', '-sEXPORT_NAME=MyModule', '--shell-file', test_file('shell_that_launches_modularize.html')] + args)

Expand Down