[test] Add audio worklet parameter tests (and tidy other interactive tests) #23659
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cwoffenden
added a commit
to cwoffenden/emscripten
that referenced
this pull request
Feb 12, 2025
This was referenced Feb 12, 2025
cwoffenden
force-pushed
the
cw-audio-param-tests
branch
from
a041945 to
0f5e291
Compare
cwoffenden
changed the title
cwoffenden
changed the title
sbc100
reviewed
Feb 20, 2025
sbc100
reviewed
Feb 20, 2025
sbc100
reviewed
Feb 20, 2025
This was actually a fun one to write! |
sbc100
reviewed
Feb 20, 2025
|
All review suggestions done:
|
This mostly adds an Audio Worklet parameter test, but also tidies a little the related tests and shared code.
cwoffenden
force-pushed
the
cw-audio-param-tests
branch
from
0588d01 to
a55981e
Compare
sbc100
pushed a commit
that referenced
this pull request
Feb 20, 2025
All the audio tests in `test_browser.py` (except the mixer) were updated now the CI has audio, specifically: - Tests that require audio were flagged as so (tests that don't are documented) - Tests that currently fail for 2GB and wasm64 were flagged (only tests that play audio fail) - All tests were migrated to `btest_exit()` as previously discussed - (Tests moved `printf` calls to Emscripten's API and not stdio) - All tests were run manually and the audio output verified - All tests were run with `emscripten_force_exit` returning non-zero to verify they failed - All of this was also verified that they're running on a Debian VM without audio to manually check the console for `"Test success"` and other outputs for tests without `requires_sound_hardware` The mixer/AW struct test is a standalone PR in #23659. Fixes: #23131
sbc100
reviewed
Feb 21, 2025
| } | ||
|
|
||
| // This implementation has no custom start-up requirements | ||
| EmscriptenStartWebAudioWorkletCallback getStartCallback(void) { |
There was a problem hiding this comment.
Why does this need to be a function that returns a function? Why not just declare a function called startCallback?
There was a problem hiding this comment.
It was because most tests return and use this default function in the shared code:
Whereas one needed a custom implementation, so this was the cleanest way to keep most of the code shared.
There was a problem hiding this comment.
(Cleanest way and still keeping it C)
sbc100
approved these changes
Feb 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is splits out the test changes from #23508 adding an interactive parameter test:
This test is also added to the browser tests now #23665 has merged, offering a single test that touches all the various structs and their offsets. When run as part of the
browsertests it plays for 1 second, for interactive without theTEST_AND_EXITmacro it will play continuously.It also tidies a little the shared code (and related tests) in the process. Interactive tests had
_2gband_4gboptions added (which will fail until #23508 lands, but they're not part of the CI). All features of these tests as interactive now work fully on Chrome, Firefox and Safari (Safari has issues with looping, but it doesn't affect the spirit of the test).