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
Merged

Add wasm-pack tests for net to CI #486

Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,10 @@ jobs:
WS_ECHO_SERVER_URL: 'ws://localhost:8081'
SSE_ECHO_SERVER_URL: 'http://localhost:8081/.sse'
run: cargo test -p gloo-net --all-features

- 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 --all-features
31 changes: 21 additions & 10 deletions Cargo.lock
View file
Open in desktop
littlebenlittle marked this conversation as resolved.
Show resolved Hide resolved

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions crates/net/tests/http.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
use wasm_bindgen_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> =
Lazy::new(|| option_env!("HTTPBIN_URL").expect("Did you set HTTPBIN_URL?"));

Expand Down
2 changes: 0 additions & 2 deletions crates/net/tests/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use gloo_net::http::QueryParams;
use wasm_bindgen_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]
fn query_params_iter() {
let params = QueryParams::new();
Expand Down
Loading