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

Pass arbritrary arguments from wasm-pack test to wasm-bindgen-test #525

Closed
chinedufn opened this issue Jan 25, 2019 · 4 comments
Closed
Labels
Milestone

Comments

@chinedufn
Copy link
Contributor

I started using wasm-pack at 0.6.0 and it's been great - thanks!


My biggest point of friction so far is trying to TDD with wasm-pack test. Needing to run an entire test suite when I'm focused one one function is no fun.

In #524 I had an idea for how to fix that, but I'm realizing that it adds a one off flag that might not really need to exist.

Instead, what if you could just pass anything through to cargo test?

# Anything after `--` gets passed to the `cargo test`
wasm-pack test --firefox --headless -- --package my-workspace-crate my_test_name --color=always
[wasm_bindgen_test]
fn my_test_name() {
  assert_eq!(2, 2);
}

In this way wasm-pack wouldn't need to support every little request that someone might have. We just expose cargo test and call it a day.


Could this work in theory? Any holes in my thinking? How difficult would this be (I'm unfamiliar with the codebase)?

Cheers!

\cc @fitzgen since you know a good bit about wasm-pack test

@fitzgen
Copy link
Member

fitzgen commented Jan 25, 2019

This seems like a good plan.

I believe that wasm-bindgen-test doesn't support running only a single test, or only tests that match a given string yet. So we will need to implement that support first.

It shouldn't be hard to support, it is mostly just navigating the code base and shepherding some extra data through. If you look at wasm-bindgen/crates/cli/src/bin/wasm-bindgen-test-runner.rs, you can see it is currently parsing CLI args manually (since it takes such a limited set). A good first step would be to port this to structopt, and then it will be easier to add more arguments, such as test filtering.

I am not 100% sure exactly how cargo passes the test filter args down to the testing binary, but @alexcrichton certainly knows, and you can always just println!("{:?}, ::std::env::args().collect::<Vec<_>>()); to debug it.

If you want to start working on a PR that does this for wasm-bindgen, even if it is just WIP, that is a good start, and we can take discussion there 👍

How does that sound?

@chinedufn
Copy link
Contributor Author

chinedufn commented Jan 25, 2019

I've used wasm-bindgen-test to run individual tests / tests matching a given string many times - but maybe we're both talking about different things or I'm just misunderstanding?

An example of what I mean:

GECKODRIVER=geckodriver cargo test --manifest-path crates/web-sys/Cargo.toml \
 --target wasm32-unknown-unknown --all-features test_olist_element 

The above runs only the test_olist_element test in the web-sys crate.


So I don't think that wasm-bindgen-test needs to be touched here.

Seems like this is all on wasm-packs side.

I think we'd just need to pass everything after -- as arguments to cargo test wherever we're building that command.

How does that sound to you?


And yes, regardless I'd be happy to PR an attempt! Just want to get on the same page about implementation.

@fitzgen
Copy link
Member

fitzgen commented Jan 25, 2019

Ah, ok then I am misinformed!

I think we'd just need to pass everything after -- as arguments to cargo test wherever we're building that command.

👍

@alexcrichton
Copy link
Contributor

@chinedufn's spot on, I've got nothing else to add!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants