Passes extra_options to cargo build --tests when running wasm-pack test.

[azriel91]

Fixes #698.

Before this change, when running wasm-pack test -- --features "feature" -- test_filter, wasm-pack runs

cargo build --tests --target wasm32-unknown-unknown
cargo test --target wasm32-unknown-unknown --features "feature" -- test_filter

Now it does:

cargo build --tests --target wasm32-unknown-unknown --features "feature"
cargo test --target wasm32-unknown-unknown --features "feature" -- test_filter

The command to build tests is only used once throughout the whole code base, that's why I passed the extra_options through without wrapping in Option<_>.

Comes with a compile_error! test too ✌️.

---

Make sure these boxes are checked! 📦✅

• You have the latest version of rustfmt installed

$ rustup component add rustfmt

• You ran cargo fmt on the code base before submitting
• You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

✌️ welcome 🙇‍♂️!

[najamelan]

So if I get it, users must correctly use -- to pass arguments to both build and test and then -- again to pass options to cargo test only. If this is considered the correct UI design, then it should at least be documented in the help message of wasm-pack help test and in the rust wasm book. A helpful reminder if the user passes extra arguments causing the build to fail might be good as well. Not sure if cargo build emits a special exit status when it doesn't recognize input arguments.

[azriel91]

no (or at least, that's not intentionally the UI, but a result of how cargo works)

the remainder after the first -- is the same as what you'd do for cargo test, i.e. if you normally would run:

cargo test --features "feature" -- test_name

under wasm-pack you would run:

wasm-pack test -- --features "feature" -- test_name

The special handling for build is, we don't pass through the test_name part because when running cargo build, the test_name filter isn't accepted / doesn't make sense.

Not sure if cargo build emits a special exit status when it doesn't recognize input arguments.

It does, which is how I discovered the test_name filter being passed through causing the command to fail.

---

If we could remove the need for the first -- then it would be a consistent UI with cargo

[Pauan]

Thanks, though I think this needs to wait for #805 to land, since it changes the way that -- works.

[azriel91]

Heya, I've rebased over master. Given #805 is closed, this one should be ready to review.

[najamelan]

Just noticed I didn't get back to this before. Just know that with cargo test you can do:

cargo test test_name or even a partial test name, without --.

That's why I think this isn't very intuitive and should be well documented in wasm-pack help test and the wasm guide.

[azriel91]

@najamelan Heya, I had one more go, and managed to convince clap to not require the extra -- before forwarding all arguments through (clap-rs/clap#1847 investigation says it's impossible, but I had another go).
To enable this, I had to:

• Change the Cargo.toml from being a positional argument, to be under the --manifest-path option. ⚠️ see edit below
• Use some not-so-recommended AppSettings.

If that doesn't sit well for this project, I could undo it and just add -- to the docs, but the user experience is much better without the extra --. Also, the --manifest-path is consistent with cargo's interface, so I thought that's alright.

edit: ah, path was for the crate path, not for Cargo.toml. Should I:

• change the option to --path
• change the argument to take in Cargo.toml
• revert and go back to the -- method
• something else

[azriel91]

Heya @najamelan / @Pauan, I managed to get wasm-pack to now have the same command line interface as cargo.

So now users can run:

wasm-pack test --features "feature" -- test_name  # which corresponds to:
#   cargo test --features "feature" -- test_name

The path positional parameter is retained as the first (optional) argument

In the implementation, I tell clap to take in path and all --hyphen-options in a single Vec field, and if the first String does not begin with -, then we take that to be the crate path. The remainder is passed through to cargo.

I'm really keen on seeing this through, as it would enable the governor crate to be published with WASM support.

[Pauan]

@azriel91 Unfortunately it's impossible, because only @ashleygwilliams has the rights to publish new changes, and she has disappeared.

[kigawas]

Any ETA on merging this?

[Saruniks]

It would be good to have this functionality.

[drager]

Thanks a lot @azriel91!