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.

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

Add wasm-pack tests for net to CI #486

Merged
merged 7 commits into from
Jul 21, 2024
+13 −2
Merged

Add wasm-pack tests for net to CI #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
377ec67
add wasm-pack test for net to ci
littlebenlittle Jul 18, 2024
df289af
remove commented line
littlebenlittle Jul 18, 2024
272323c
fix path to net crate
littlebenlittle Jul 18, 2024
66fb932
enable query tests
littlebenlittle Jul 18, 2024
8294631
cfg gate for browser tests
littlebenlittle Jul 18, 2024
65dd70e
revert cargo.lock
littlebenlittle Jul 18, 2024
1e9e002
add browser-test feature
littlebenlittle Jul 19, 2024
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
11 changes: 9 additions & 2 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ jobs:
SSE_ECHO_SERVER_URL: 'http://localhost:8081/.sse'
run: |
cd crates/net
wasm-pack test --chrome --firefox --headless --all-features
wasm-pack test --chrome --firefox --headless --features=default,io-util,browser-test

- uses: dtolnay/rust-toolchain@master
with:
Expand All @@ -262,4 +262,11 @@ jobs:
HTTPBIN_URL: 'http://localhost:8080'
WS_ECHO_SERVER_URL: 'ws://localhost:8081'
SSE_ECHO_SERVER_URL: 'http://localhost:8081/.sse'
run: cargo test -p gloo-net --all-features
run: cargo test -p gloo-net --features=default,io-util

- name: Run node tests
env:
HTTPBIN_URL: 'http://localhost:8080'
WS_ECHO_SERVER_URL: 'ws://localhost:8081'
SSE_ECHO_SERVER_URL: 'http://localhost:8081/.sse'
run: wasm-pack test --node crates/net --features=default,io-util
2 changes: 2 additions & 0 deletions crates/net/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,5 @@ eventsource = [
]
# As of now, only implements `AsyncRead` and `AsyncWrite` on `WebSocket`
io-util = ["futures-io"]
# For test runner only. Enables browser tests.
browser-test = []
1 change: 1 addition & 0 deletions crates/net/tests/http.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
use wasm_bindgen_test::*;

#[cfg(feature = "browser_test")]
wasm_bindgen_test_configure!(run_in_browser);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is needed for browser tests, please keep this in (and gate it behind wasm32-unknown-unknown target)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clippy doesn't like #[cfg(target="wasm32-unkonwn-unknown")] but it seems to work. Let me know if I misunderstood.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think target_arch refers to wasm32 in this case (see https://doc.rust-lang.org/reference/conditional-compilation.html#target_arch). That might be why clippy complains. You want to update CI so clippy is also run on wasm32 target.

You can look at how wasi is cfg-gated in other parts of the code base and do the opposite of it here. If it's target_os = "wasi" then you may be able to do not(target_os = "wasi") to target browser wasm targets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this seems like a quirk of wasm_bindgen_test. As far as I can tell there's no way straightforward way to run in both node and browser.

Here's one suggestion: rustwasm/wasm-bindgen#2571 (comment)

Added to latest commit.


static HTTPBIN_URL: Lazy<&'static str> =
Expand Down
1 change: 1 addition & 0 deletions crates/net/tests/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use gloo_net::http::QueryParams;
use wasm_bindgen_test::*;

#[cfg(feature = "browser_test")]
wasm_bindgen_test_configure!(run_in_browser);
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as previous comment


#[wasm_bindgen_test]
Expand Down
Loading