Skip to content
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.

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

[AUDIO_WORKLET] tests in the CI don't appear to be run (in test_browser) #23131

Open
cwoffenden opened this issue Dec 11, 2024 · 19 comments
Open
Assignees

Comments

@cwoffenden
Copy link
Contributor

cwoffenden commented Dec 11, 2024

EDIT: short version, since I think the error is misleading: they're compiling but they're instantly exiting without running.

Testing with the latest Emscripten locally (3.1.74) and seeing the same in the result from CI. Open any of the CI runs and look at the browser tests, e.g.:

https://app.circleci.com/pipelines/github/emscripten-core/emscripten/38495/workflows/353c8dd2-db7a-4691-a83b-c1b8088f8959/jobs/859340

Search for test_audio_worklet and the parameterised tests all have the same 404 error with missing files:

  test_audio_worklet (test_browser.browser) ... cache:INFO: generating system asset: symbol_lists/c9305f6ae33005a7b005afb88a5689b698a1f55c.json... (this will be cached in "/root/cache/symbol_lists/c9305f6ae33005a7b005afb88a5689b698a1f55c.json" for subsequent builds)
cache:INFO:  - ok
127.0.0.1 - - [11/Dec/2024 12:08:43] code 404, message File not found: /root/project/test.aw.js

I noticed this locally, and thought it was just the way my system was set up, then given that I'd seen there was no way the CI was passing tests that should be failing, I dug deeper. The test runner doesn't find test.aw.js and if that is copied (or a link created) then it next fails on test.js.

The tests are all passing though, I'm guessing because the main page opens and exits (interestingly it's only the Audio Worklets missing files, so 14 404s in test_browser's log).

@cwoffenden
Copy link
Contributor Author

cwoffenden commented Dec 11, 2024

It seems to stem from the change from btest() to btest_exit(). At a guess, I'd say the server was being shut down before the file was served (if I stop the test harness from existing, via various means, there's no error). The error is therefore not the problem, it's just a side effect of the test instantly exiting.

@cwoffenden
Copy link
Contributor Author

cwoffenden commented Jan 30, 2025

@sbc100 As per the discussion here:

#23508 (comment)

I'd like to get these tests running (at least not silently failing). I've put some fixes in already (e.g. STRICT), with the remaining fixes in #23508.

My question is, if I fix these they'll break the CI until other fixes are merged. What's the best way to do this? Would you like a separate PR that breaks things, or do they want rolling into #23508?

The more I delve into it the more issues I'm finding, the latest being audio params (WebAudioParamDescriptor) don't appear to be completely wired up* (and no tests exist). I can fix the code to correctly read structs based on the correct offsets, but without any tests it's a shot in the dark.

*edit here: it is in the AudioWorkletProcessor constructor but even with the API specs in front of me it's a bit of an odd design.

@cwoffenden cwoffenden changed the title Audio Worklet tests in the CI don't appear to be run (in test_browser) [AUDIO_WORKLET] tests in the CI don't appear to be run (in test_browser) Jan 30, 2025
@sbc100
Copy link
Collaborator

sbc100 commented Jan 30, 2025

If you fix the tests so they correctly start failing you can temporarily mark them as @disabled(<bug_url>) and then re-enable them when the bug is fixed

@cwoffenden
Copy link
Contributor Author

And it is what I wrote a few comments back, it's the test being shut down with btest_exit() since the work is done on the worklet not the main thread. I'll look at how the regular worklet tests are done (and hopefully find they're not broken 😁).

@sbc100
Copy link
Collaborator

sbc100 commented Jan 30, 2025

I would love to continue to use btest_exit where possible. I though I even had PR up to convert the remaining worklet testa to btest_exit but I can't find it now.

My hope is that all browser tests would use btest_exit one day.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 30, 2025

Wait, I thought it was impossible to run any audio tests during CI because they require user-interaction to start the audio? Don't they all need to interactive?

@cwoffenden
Copy link
Contributor Author

It'd save me some time if you'd already had a sample of the main thread waiting on a worklet finishing.

As for any audio tests running, good point. Chrome at least will start the AW code, which is enough to catch most of the failures. I'll look at it.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 30, 2025

Getting btest_exit to behave nicely with AUDIO_WORKLETS (and WASM_WORKERS in general) can be tricky because IIUC you cannot called exit from a wasm worker (like many JS function it is proxied back to the main thread, which is not possible in a wasm worker, so it just one of many APIs you cannot call). So you need to use wasm worker style run-on-thread to run the exit itself back on the main thread.

Oh I found the PR I was working on: #22776

@cwoffenden
Copy link
Contributor Author

Wait, I thought it was impossible to run any audio tests during CI

I did a quick test, I built this with WEBAUDIO_DEBUG and a bunch of other options:

https://wip.numfum.com/cw/2025-01-30/index.html

It won't play audio but if you look at the console it will create the worklet and perform most of the steps. It's enough to know in CI that initialisation works (esp. struct offsets and the message passing). I'll take a detour to look at what could be done.

@cwoffenden
Copy link
Contributor Author

Wait, I thought it was impossible to run any audio tests during CI because they require user-interaction to start the audio?

If I run via test/runner browser.test_audio_worklet it needs interaction (after changing the test to not exit instantly). Same with ./emrun --browser chrome test/webaudio/index.html. Until I added --port 8000, at which point audio works in Chrome without interaction (but not Firefox or Safari). runner is always on port 8888, emrun on 6931 on my machine.

Could you try this? Build and launch audioworklet.c:

cd $EMSCRIPTEN/test/webaudio
emcc -sAUDIO_WORKLET=1 -sWASM_WORKERS=1 -o index.html audioworklet.c
emrun --browser chrome --port 8000 index.html

The default setting for port 8000 looks to be with audio:

Image

Is this a path worth going down to get the audio to more than just build?

@sbc100
Copy link
Collaborator

sbc100 commented Feb 10, 2025

Are you saying that port 8000 is somehow magic. That would seem very odd to me. I will see if I can verify that.

We should probably add @requires_sound_hardware to these tests.. which will have the result of causing them not to run in CI.

Or we should move them to test_interactive.

@cwoffenden
Copy link
Contributor Author

cwoffenden commented Feb 10, 2025

Are you saying that port 8000 is somehow magic.

Sounds odd, but yep. I'm going to repeat this on other machines too (I'm just worried my desktop has had its Chrome install tweaked to make my life easier).

Or we should move them to test_interactive.

Just compiling catches enough things, e.g., -sMINIMAL_RUNTIME and --closure.

Let me know your finding. Would it be feasible to run the tests on port 8000 (which changes things if it turns out to be magic)?

@sbc100
Copy link
Collaborator

sbc100 commented Feb 10, 2025

Just compiling catches enough things, e.g., -sMINIMAL_RUNTIME and --closure.

Compile-only tests don't need to live in the browser test suite. They can live in test_other.py, for example. See test_wasm_worker_closure in test_other.py for example.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 10, 2025

Are you saying that port 8000 is somehow magic.

Sounds odd, but yep. I'm going to repeat this on other machines too (I'm just worried my desktop has had its Chrome install tweaked to make my life easier).

Or we should move them to test_interactive.

Just compiling catches enough things, e.g., -sMINIMAL_RUNTIME and --closure.

Let me know your finding. Would it be feasible to run the tests on port 8000 (which changes things if it turns out to be magic)?

I don't see anything different about 8000 and, for example, 6931. The audio permissions in both cases is set to "Automatic (default)".

@cwoffenden
Copy link
Contributor Author

Are you saying that port 8000 is somehow magic.

Not on my home Mac laptop. I'll look into it.

@cwoffenden
Copy link
Contributor Author

cwoffenden commented Feb 11, 2025

It's the Media Engagement Index (see chrome://media-engagement) automatically playing the audio because my usual behaviour is to do so. It's stored in the Chrome user preferences file under media_engagement, and creating a new Chrome profile stops the audio play.

If Chrome is passed this flag on launch:

--autoplay-policy=no-user-gesture-required

E.g. on Mac:

/Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --autoplay-policy=no-user-gesture-required  

Then it's possible to run tests without the interaction, and this could be added to tests the same as here, for example:

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"

And for Firefox setting the media.autoplay.default to 0 will do the same if added as per the other prefs here:

user_pref("gfx.offscreencanvas.enabled", true);

@sbc100
Copy link
Collaborator

sbc100 commented Feb 11, 2025

Funnily enough when I run browser.test_sdl2_mixer_wav without interaction that test will passes with no errors, but I don't hear the audio. When I run it with interaction I do hear audio, so somehow SDL audio is able to run regardless of interaction status. I'm not sure how it does that.

Regarding enabling audio tests in CI, perhaps we could try that separately. I think it would involve removing EMTEST_LACKS_SOUND_HARDWARE from the circleci config file.

I don't think we could still fix the audio worklet tests regardless though, probably by making them fail correctly and then marking them as @requires_sound_hardware (which would effectively disable them in CI). Then we could separately look at removing EMTEST_LACKS_SOUND_HARDWARE (i.e. I think those things can happen separately and either order).

@cwoffenden
Copy link
Contributor Author

Funnily enough when I run browser.test_sdl2_mixer_wav without interaction that test will passes with no errors

The SDL implementation uses the traditional audio tag for playback, so doesn't have the same processing thread that never gets called.

I'll take a look tomorrow at options for getting these running. I'll start with splitting out the last of my audio tests into their own PR, then look at fixing up everything to make it fail as you suggest.

@cwoffenden
Copy link
Contributor Author

cwoffenden commented Feb 13, 2025

#23665 should now fail all of the parameterised test_audio_worklet tests in the CI (they'll wait forever). Once that's confirmed I'll add @requires_sound_hardware to get running again, then I'll look at adding browser flags to Circle CI.

Confirmed, they're all failing as expected with the test waiting forever:

Image

sbc100 pushed a commit that referenced this issue Feb 18, 2025
Step one of a fix for #23131 (the second part in #23695).

Enabled Chrome's `FakeAudioOutputStream` for the CI machines and
bypassed the need for user interaction. The Chrome tests are now enabled
with `@requires_sound_hardware`.

Many attempts at replicating the same functionally were tried with
Firefox (details below) but audio would never run so its
`EMTEST_LACKS_SOUND_HARDWARE` was left unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants