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

CHANGELOG.md

@@ -1,5 +1,20 @@
 # Changelog
 
+## 🤍 Unreleased
+
+- ### 🤕 Fixes
+
+  - **Pass through extra options when building tests - [azriel91], [issue/698] [pull/851]**
+
+    `wasm-pack test` accepts extra options to pass through to `cargo` when running tests.
+    Under the hood, this runs `cargo build` before `cargo test`, and the additional options were only passed through to the `test` command. This meant that crates that enabled native features by default could not be built using `wasm-pack`, as it would attempt to build tests for the `wasm32-unknown-unknown` target with the native features enabled.
+
+    This PR passes through the extra options to `cargo` when building the tests as well.
+
+    [azriel91]: https://github.com/azriel91
+    [pull/851]: https://github.com/rustwasm/wasm-pack/pull/851
+    [issue/698]: https://github.com/rustwasm/wasm-pack/issues/698
+
 ## ☁️  0.9.1
 
 - ### 🤕 Fixes

Cargo.toml

@@ -30,7 +30,7 @@ serde_ignored = "0.0.4"
 serde_json = "1.0.26"
 strsim = "0.8.0"
 siphasher = "0.2.3"
-structopt = "0.2"
+structopt = "0.3"
 toml = "0.4"
 which = "2.0.0"
 binary-install = "0.0.2"

src/build/mod.rs

@@ -112,11 +112,24 @@ pub fn cargo_build_wasm(
     Ok(())
 }
 
-/// Run `cargo build --tests` targetting `wasm32-unknown-unknown`.
+/// Runs `cargo build --tests` targeting `wasm32-unknown-unknown`.
 ///
 /// This generates the `Cargo.lock` file that we use in order to know which version of
 /// wasm-bindgen-cli to use when running tests.
-pub fn cargo_build_wasm_tests(path: &Path, debug: bool) -> Result<(), Error> {
+///
+/// Note that the command to build tests and the command to run tests must use the same parameters, i.e. features to be
+/// disabled / enabled must be consistent for both `cargo build` and `cargo test`.
+///
+/// # Parameters
+///
+/// * `path`: Path to the crate directory to build tests.
+/// * `debug`: Whether to build tests in `debug` mode.
+/// * `extra_options`: Additional parameters to pass to `cargo` when building tests.
+pub fn cargo_build_wasm_tests(
+    path: &Path,
+    debug: bool,
+    extra_options: &[String],
+) -> Result<(), Error> {
     let mut cmd = Command::new("cargo");
 
     cmd.current_dir(path).arg("build").arg("--tests");
@@ -131,6 +144,8 @@ pub fn cargo_build_wasm_tests(path: &Path, debug: bool) -> Result<(), Error> {
 
     cmd.arg("--target").arg("wasm32-unknown-unknown");
 
+    cmd.args(extra_options);
+
     child::run(cmd, "cargo build").context("Compilation of your program failed")?;
     Ok(())
 }

src/command/test.rs

@@ -11,16 +11,21 @@ use lockfile::Lockfile;
 use log::info;
 use manifest;
 use std::path::PathBuf;
+use std::str::FromStr;
 use std::time::Instant;
+use structopt::clap::AppSettings;
 use test::{self, webdriver};
 
 #[derive(Debug, Default, StructOpt)]
+#[structopt(
+    // Allows unknown `--option`s to be parsed as positional arguments, so we can forward it to `cargo`.
+    setting = AppSettings::AllowLeadingHyphen,
+
+    // Allows `--` to be parsed as an argument, so we can forward it to `cargo`.
+    setting = AppSettings::TrailingVarArg,
+)]
 /// Everything required to configure the `wasm-pack test` command.
 pub struct TestOptions {
-    #[structopt(parse(from_os_str))]
-    /// The path to the Rust crate. If not set, searches up the path from the current dirctory.
-    pub path: Option<PathBuf>,
-
     #[structopt(long = "node")]
     /// Run the tests in Node.js.
     pub node: bool,
@@ -74,9 +79,14 @@ pub struct TestOptions {
     /// Build with the release profile.
     pub release: bool,
 
-    #[structopt(last = true)]
-    /// List of extra options to pass to `cargo test`
-    pub extra_options: Vec<String>,
+    /// Path to the Rust crate, and extra options to pass to `cargo test`.
+    ///
+    /// If the path is not provided, this command searches up the path from the current dirctory
+    ///
+    /// This is a workaround to allow wasm pack to provide the same command line interface as `cargo`.
+    /// See <https://github.com/rustwasm/wasm-pack/pull/851> for more information.
+    #[structopt(allow_hyphen_values = true)]
+    pub path_and_extra_options: Vec<String>,
 }
 
 /// A configured `wasm-pack test` command.
@@ -104,7 +114,6 @@ impl Test {
     /// Construct a test command from the given options.
     pub fn try_from_opts(test_opts: TestOptions) -> Result<Self, Error> {
         let TestOptions {
-            path,
             node,
             mode,
             headless,
@@ -115,9 +124,23 @@ impl Test {
             geckodriver,
             safari,
             safaridriver,
-            extra_options,
+            mut path_and_extra_options,
         } = test_opts;
 
+        let first_arg_is_path = path_and_extra_options
+            .get(0)
+            .map(|first_arg| !first_arg.starts_with("-"))
+            .unwrap_or(false);
+
+        let (path, extra_options) = if first_arg_is_path {
+            let path = PathBuf::from_str(&path_and_extra_options.remove(0))?;
+            let extra_options = path_and_extra_options;
+
+            (Some(path), extra_options)
+        } else {
+            (None, path_and_extra_options)
+        };
+
         let crate_path = get_crate_path(path)?;
         let crate_data = manifest::CrateData::new(&crate_path, None)?;
         let any_browser = chrome || firefox || safari;
@@ -243,7 +266,15 @@ impl Test {
     fn step_build_tests(&mut self) -> Result<(), Error> {
         info!("Compiling tests to wasm...");
 
-        build::cargo_build_wasm_tests(&self.crate_path, !self.release)?;
+        // If the user has run `wasm-pack test -- --features "f1" -- test_name`, then we want to only pass through
+        // `--features "f1"` to `cargo build`
+        let extra_options =
+            if let Some(index) = self.extra_options.iter().position(|arg| arg == "--") {
+                &self.extra_options[..index]
+            } else {
+                &self.extra_options
+            };
+        build::cargo_build_wasm_tests(&self.crate_path, !self.release, extra_options)?;
 
         info!("Finished compiling tests to wasm.");
         Ok(())

tests/all/build.rs

@@ -146,8 +146,8 @@ fn dash_dash_web_target_has_error_on_old_bindgen() {
     let output = String::from_utf8(cmd.get_output().stderr.clone()).unwrap();
 
     assert!(
-        output.contains("0.2.39"),
-        "Output did not contain '0.2.39', output was {}",
+        output.contains("Please update your project to wasm-bindgen version >= 0.2.39"),
+        "Output did not contain 'Please update your project to wasm-bindgen version >= 0.2.39', output was {}",
         output
     );
 }

tests/all/download.rs

@@ -12,7 +12,7 @@ fn can_download_prebuilt_wasm_bindgen() {
     let dir = tempfile::TempDir::new().unwrap();
     let cache = Cache::at(dir.path());
     if let install::Status::Found(dl) =
-        install::download_prebuilt(&Tool::WasmBindgen, &cache, "0.2.37", true).unwrap()
+        install::download_prebuilt(&Tool::WasmBindgen, &cache, "0.2.74", true).unwrap()
     {
         assert!(dl.binary("wasm-bindgen").unwrap().is_file());
         assert!(dl.binary("wasm-bindgen-test-runner").unwrap().is_file())
@@ -29,7 +29,7 @@ fn can_download_prebuilt_wasm_bindgen() {
 ))]
 fn downloading_prebuilt_wasm_bindgen_handles_http_errors() {
     let dir = tempfile::TempDir::new().unwrap();
-    let bad_version = "0.2.37-some-trailing-version-stuff-that-does-not-exist";
+    let bad_version = "0.2.74-some-trailing-version-stuff-that-does-not-exist";
     let cache = Cache::at(dir.path());
     let result = install::download_prebuilt(&Tool::WasmBindgen, &cache, bad_version, true);
     assert!(result.is_err());

tests/all/lockfile.rs

@@ -8,7 +8,7 @@ fn it_gets_wasm_bindgen_version() {
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path, None).unwrap();
     let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.37"),);
+    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.74"),);
 }
 
 #[test]
@@ -17,7 +17,7 @@ fn it_gets_wasm_bindgen_test_version() {
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path, None).unwrap();
     let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_test_version(), Some("0.2.37"),);
+    assert_eq!(lock.wasm_bindgen_test_version(), Some("0.3.24"),);
 }
 
 #[test]
@@ -46,7 +46,7 @@ fn it_gets_wasm_bindgen_version_in_crate_inside_workspace() {
                 crate-type = ["cdylib"]
 
                 [dependencies]
-                wasm-bindgen = "=0.2.37"
+                wasm-bindgen = "=0.2.74"
             "#,
         )
         .file(
@@ -62,7 +62,7 @@ fn it_gets_wasm_bindgen_version_in_crate_inside_workspace() {
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path.join("blah"), None).unwrap();
     let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.37"),);
+    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.74"),);
 }
 
 #[test]
@@ -91,7 +91,7 @@ fn it_gets_wasm_bindgen_version_from_dependencies() {
                 crate-type = ["cdylib"]
 
                 [dependencies]
-                wasm-bindgen = "=0.2.37"
+                wasm-bindgen = "=0.2.74"
             "#,
         )
         .file(
@@ -130,5 +130,5 @@ fn it_gets_wasm_bindgen_version_from_dependencies() {
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path.join("parent"), None).unwrap();
     let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.37"),);
+    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.74"),);
 }

tests/all/log_level.rs

@@ -9,7 +9,6 @@ fn matches_info() -> impl Predicate<str> + PredicateReflection {
     contains("[INFO]: Checking for the Wasm target...")
         .and(contains("[INFO]: Compiling to Wasm..."))
         .and(contains("[INFO]: License key is set in Cargo.toml but no LICENSE file(s) were found; Please add the LICENSE file(s) to your project directory"))
-        .and(contains("[INFO]: Installing wasm-bindgen..."))
         .and(contains("[INFO]: Optimizing wasm binaries with `wasm-opt`..."))
         .and(contains("[INFO]: :-) Done in "))
         .and(contains("[INFO]: :-) Your wasm pkg is ready to publish at "))

tests/all/test.rs

@@ -337,3 +337,61 @@ fn test_output_is_printed_once_in_both_stdout_and_failures() {
             out.matches("YABBA DABBA DOO").count() == log_cnt * 2
         }));
 }
+
+#[test]
+fn extra_options_is_passed_to_cargo_when_building_tests() {
+    let fixture = fixture::Fixture::new();
+    fixture
+        .readme()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+                authors = []
+
+                [dev-dependencies]
+                wasm-bindgen-test = "0.2"
+
+                [features]
+                default = ["native"]
+                native = []
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+                pub fn foo() -> u32 {
+                    #[cfg(feature = "native")]
+                    compile_error!("Test should pass through `--no-default-features` for this to pass.");
+
+                    1
+                }
+            "#,
+        )
+        .file(
+            "tests/foo.rs",
+            r#"
+                extern crate wasm_bindgen_test;
+                use wasm_bindgen_test::*;
+
+                #[wasm_bindgen_test]
+                fn smoke() {
+                    foo::foo();
+                }
+
+                #[wasm_bindgen_test]
+                fn fire() {
+                    panic!("This should be filtered from test execution.");
+                }
+            "#,
+        )
+        .install_local_wasm_bindgen();
+    let _lock = fixture.lock();
+    fixture
+        .wasm_pack()
+        .args(&["test", "--node", "--no-default-features", "--", "smoke"])
+        .assert()
+        .success();
+}