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

async wasm tests don't seem to actually do anything #368

Closed
carlsverre opened this issue Sep 3, 2023 · 2 comments · Fixed by #394
Closed

async wasm tests don't seem to actually do anything #368

carlsverre opened this issue Sep 3, 2023 · 2 comments · Fixed by #394
Labels
bug Something isn't working

Comments

@carlsverre
Copy link

Describe the Bug

Currently the async tests in net use spawn_local, but don't actually wait for the spawned tasks to do anything. In addition, since spawn_local should always be run on the next microtask - the wasm instance will exit before any of the spawn_locals fire.

You can "test" this by adding an assert!(false) somewhere in the spawn_local part of the test.

Fix

To fix this, wasm_bindgen supports async tests: https://rustwasm.github.io/wasm-bindgen/wasm-bindgen-test/asynchronous-tests.html

I fixed the websocket test in a quick way here: master...carlsverre:gloo:feat-arraybuf

I tried a similar quickfix to the eventsource tests but discovered that the echo server in place is not handling cors - so that test is already broken. Unfortunately I don't have time right now to change the echo server / do more extensive work. Sorry about that.

@carlsverre carlsverre added the bug Something isn't working label Sep 3, 2023
@ranile
Copy link
Collaborator

ranile commented Sep 5, 2023

Thanks for reporting! I would be happy to look at any PRs fixing the issue

This was referenced Oct 18, 2023
@richardpringle
Copy link
Contributor

@hamza1311, PTAL at #394

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants