wasm-pack test <extra_options> does not work as expected

[alexlapa]

🐛 Bug description

wasm-pack test does not pass <extra_options> to cargo build --tests --target wasm32-unknown-unknown.

🤔 Expected Behavior

wasm-pack test --firefox -- --features some_feature

should invoke

cargo build --tests --target wasm32-unknown-unknown --features some_feature

👟 Steps to reproduce

Running

wasm-pack test --firefox -- --features some_feature

Causes error during tested crate compilation, since some_feature is not enabled.

Last log entry:

INFO 2019-08-08T19:58:57Z: wasm_pack::child: Running "cargo" "build" "--tests" "--target" "wasm32-unknown-unknown"

However, running:

cargo test --target wasm32-unknown-unknown --features some_feature

works.

Code example: https://github.com/alexlapa/wasm-pack-698-reproduction

Possible solution

It seems to me that there is no reason to run cargo build --tests --target wasm32-unknown-unknown when running wasm-pack test.

wasm-pack test will run wasm-bindgen-test, which will run cargo test, which will run cargo build --tests correctly propagating all extra options.

i.e. you can just remove step_build_tests

🌍 Your environment

wasm-pack version: 0.8.1
rustc version: 1.36.0 (a53f9df32 2019-07-03)

[alexlapa]

@ashleygwilliams ,

Added reproduction https://github.com/alexlapa/wasm-pack-698-reproduction

[najamelan]

yes, this is an annoying bug.

[lukidoescode]

@ashleygwilliams is the label needs reproduction still valid?

[najamelan]

I am using it with -- --all-features and it works, but I had to remove the .cargo/config with:

[target.wasm32-unknown-unknown]
runner = 'wasm-bindgen-test-runner'

It seems wasm-pack doesn't like this one, so it's not possible to switch between cargo test and wasm-pack test. Maybe @alexlapa could check if that is the issue they have? It would be nice if both were compatible.

Update: sorry for the confusion. I just tested it again and it seems to be working with both --all-features and --features some_feature. Even with the .cargo/config in place. I haven't yet tried with several features.

Update 2: I know what happened now. It doesn't combine with .cargo/config on travis, but it does work at home. Travis log: https://travis-ci.org/github/najamelan/ws_stream_wasm/jobs/663705905

[alexlapa]

Retested reproduction with latest wasm-pack (0.9.1), it is still reproducible.

wasm-pack test --chrome --headless -- --features some_feature

[INFO]: Checking for the Wasm target...
   Compiling wasm-pack-698-reproduction v0.1.0 (/home/alexlapa/CLionProjects/wasm-pack-698-reproduction)
error[E0432]: unresolved import `wasm_pack_698_reproduction::foo`
 --> tests/integration_tests.rs:9:9
  |
9 |     use wasm_pack_698_reproduction::foo;
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `foo` in the root

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
error: could not compile `wasm-pack-698-reproduction`.

To learn more, run the command again with --verbose.
Error: Compilation of your program failed
Caused by: failed to execute `cargo build`: exited with exit code: 101
  full command: "cargo" "build" "--tests" "--target" "wasm32-unknown-unknown"

It also prints full command:

"cargo" "build" "--tests" "--target" "wasm32-unknown-unknown"

So features are not propagated properly.

[najamelan]

strange:

$ wasm-pack test  --chrome --headless -- --features tokio_io --verbose

    Running `rustc --crate-name ws_stream_wasm --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --emit=dep-info,link -C debuginfo=2 --test --cfg 'feature="tokio_io"' ...

This is version: wasm-pack 0.9.1. On travis for me this only works if there isn't the .cargo/config with the conf above for me. Don't know what's different on my system.
rustc 1.42.0 (b8cedc004 2020-03-09)

Nightly works too for me, actually looks like wasm-pack runs nightly even if stable is set as default...

[najamelan]

@alexlapa I had a look at your reproduction case. It will work as expected if you add this line to the top of your integration test:

#![cfg(feature = "some_feature")]

This will indicate that this test file requires the feature. It will be excluded when running wasm-pack test without the features, and will be included and working if you set the feature.

I also checked wasm-pack build --target web --dev -- --features some_feature and it works fine, including the feature.

[lukidoescode]

Isn't that even the behaviour as declared in the command help?

It says:

ARGS:
    <path>                The path to the Rust crate. If not set, searches up the path from the current dirctory.
    <extra_options>...    List of extra options to pass to `cargo test`

So it explicity tells the user that these extra arguments get passed to cargo test. There is no mention of cargo build or cargo in general. Which is strange. I would assume there would be the possibility of passing arguments to cargo build. Maybe this should be more of a kind of feature request. But I really need this stuff for testing. Take a look at https://github.com/lukidoescode/css-in-rust/blob/master/ci/run_tests.sh if you need an example as to why this feature is important.

[alexlapa]

So it explicity tells the user that these extra arguments get passed to cargo test. There is no mention of cargo build or cargo in general.

cargo test invokes cargo build --tests before running any tests. Problem is, that wasm-pack test invokes cargo build --tests before doing cargo test.

[najamelan]

I had a look into this a bit deeper. The code responsible comes from here: #525

Right now everything after -- get's passed to cargo test. We could pass that to cargo build, except cargo build does not accept the same parameters as cargo test. Eg. you can just include test names in params to cargo test to filter your tests, but cargo build will choke on these.

Trying to solve the issue raises the question as to why we run cargo build manually as cargo test should compile everything when called. The explanation in the source code says this:

/// This generates the `Cargo.lock` file that we use in order to know which version of
/// wasm-bindgen-cli to use when running tests.

I don't quite understand why this is necessary and why this requirement is not met by just running cargo test? From the git history it looks like @chinedufn added the comment, but the function cargo_build_wasm_tests already existed before and seems to come from @fitzgen.

So I can imagine 3 solutions here:

• Don't manually run build, needs to have the question above answered.
• wasm-pack knows which arguments to pass to which command -> seems a hassle to implement correctly.
• cargo test only arguments go before --. Or some other re-organization of the command line parsing.

[najamelan]

I checked the integration tests of wasm-pack. Removing the extra build pass does not make any tests fail.

[azriel91]

wasm-pack knows which arguments to pass to which command -> seems a hassle to implement correctly.

Turns out it wasn't a hassle -- it's only called in one place in the repository.

[Saruniks]

It would be cool to have this functionality