Migrate wasm-bindgen-cli to clap (rustwasm#4354)

crates/cli/Cargo.toml

@@ -22,7 +22,7 @@ pkg-url = "https://github.com/rustwasm/wasm-bindgen/releases/download/{ version
 
 [dependencies]
 anyhow = "1.0"
-docopt = "1.0"
+clap = { version = "4", features = ["derive"] }
 env_logger = "0.11.5"
 log = "0.4"
 native-tls = { version = "0.2", default-features = false, optional = true }

crates/cli/src/bin/wasm-bindgen-test-runner/deno.rs

@@ -1,35 +1,28 @@
-use std::ffi::OsString;
 use std::fs;
 use std::path::Path;
 use std::process::Command;
 
 use anyhow::{Context, Error};
 
-use crate::node::{exec, SHARED_SETUP};
+use crate::{
+    node::{exec, SHARED_SETUP},
+    Cli,
+};
 
-pub fn execute(
-    module: &str,
-    tmpdir: &Path,
-    args: &[OsString],
-    tests: &[String],
-) -> Result<(), Error> {
+pub fn execute(module: &str, tmpdir: &Path, cli: Cli, tests: &[String]) -> Result<(), Error> {
     let mut js_to_execute = format!(
-        r#"import * as wasm from "./{0}.js";
+        r#"import * as wasm from "./{module}.js";
 
         {console_override}
 
         window.__wbg_test_invoke = f => f();
 
-        // Forward runtime arguments. These arguments are also arguments to the
-        // `wasm-bindgen-test-runner` which forwards them to deno which we
-        // forward to the test harness. this is basically only used for test
-        // filters for now.
-        cx.args(Deno.args);
+        {args}
 
         const tests = [];
     "#,
-        module,
         console_override = SHARED_SETUP,
+        args = cli.into_args(),
     );
 
     for test in tests {
@@ -69,7 +62,6 @@ if (!ok) Deno.exit(1);"#,
         Command::new("deno")
             .arg("run")
             .arg("--allow-read")
-            .arg(&js_path)
-            .args(args),
+            .arg(&js_path),
     )
 }

crates/cli/src/bin/wasm-bindgen-test-runner/main.rs

@@ -12,6 +12,7 @@
 //! and source code.
 
 use anyhow::{anyhow, bail, Context};
+use clap::Parser;
 use log::error;
 use std::env;
 use std::fs;
@@ -26,70 +27,61 @@ mod node;
 mod server;
 mod shell;
 
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-enum TestMode {
-    Node { no_modules: bool },
-    Deno,
-    Browser { no_modules: bool },
-    DedicatedWorker { no_modules: bool },
-    SharedWorker { no_modules: bool },
-    ServiceWorker { no_modules: bool },
+#[derive(Parser)]
+#[command(version, about, long_about = None)]
+struct Cli {
+    #[arg(
+        index = 1,
+        help = "The file to test. `cargo test` passes this argument for you."
+    )]
+    file: PathBuf,
+    #[arg(long = "include-ignored", help = "Run ignored tests")]
+    include_ignored: bool,
+    #[arg(
+        long = "skip",
+        value_name = "FILTER",
+        help = "Skip tests whose names contain FILTER (this flag can be used multiple times)"
+    )]
+    skip: Vec<String>,
+    #[arg(
+        index = 2,
+        value_name = "FILTER",
+        help = "The FILTER string is tested against the name of all tests, and only those tests \
+                whose names contain the filter are run."
+    )]
+    filter: Option<String>,
 }
 
-impl TestMode {
-    fn is_worker(self) -> bool {
-        matches!(
-            self,
-            Self::DedicatedWorker { .. } | Self::SharedWorker { .. } | Self::ServiceWorker { .. }
+impl Cli {
+    fn into_args(self) -> String {
+        let include_ignored = self.include_ignored;
+        let skip = self.skip;
+        let filter = if let Some(filter) = self.filter {
+            &format!("\"{filter}\"")
+        } else {
+            "undefined"
+        };
+
+        format!(
+            r#"
+            // Forward runtime arguments.
+            cx.include_ignored({include_ignored:?});
+            cx.skip({skip:?});
+            cx.filter({filter});
+        "#
         )
     }
-
-    fn no_modules(self) -> bool {
-        match self {
-            Self::Deno => true,
-            Self::Browser { no_modules }
-            | Self::Node { no_modules }
-            | Self::DedicatedWorker { no_modules }
-            | Self::SharedWorker { no_modules }
-            | Self::ServiceWorker { no_modules } => no_modules,
-        }
-    }
-
-    fn env(self) -> &'static str {
-        match self {
-            TestMode::Node { .. } => "WASM_BINDGEN_USE_NODE_EXPERIMENTAL",
-            TestMode::Deno => "WASM_BINDGEN_USE_DENO",
-            TestMode::Browser { .. } => "WASM_BINDGEN_USE_BROWSER",
-            TestMode::DedicatedWorker { .. } => "WASM_BINDGEN_USE_DEDICATED_WORKER",
-            TestMode::SharedWorker { .. } => "WASM_BINDGEN_USE_SHARED_WORKER",
-            TestMode::ServiceWorker { .. } => "WASM_BINDGEN_USE_SERVICE_WORKER",
-        }
-    }
-}
-
-struct TmpDirDeleteGuard(PathBuf);
-
-impl Drop for TmpDirDeleteGuard {
-    fn drop(&mut self) {
-        if let Err(e) = fs::remove_dir_all(&self.0) {
-            error!("failed to remove temporary directory: {}", e);
-        }
-    }
 }
 
 fn main() -> anyhow::Result<()> {
     env_logger::init();
-    let mut args = env::args_os().skip(1);
-    let shell = shell::Shell::new();
 
-    // Currently no flags are supported, and assume there's only one argument
-    // which is the Wasm file to test. This'll want to improve over time!
-    let wasm_file_to_test = match args.next() {
-        Some(file) => PathBuf::from(file),
-        None => bail!("must have a file to test as first argument"),
-    };
+    let cli = Cli::parse();
 
-    let file_name = wasm_file_to_test
+    let shell = shell::Shell::new();
+
+    let file_name = cli
+        .file
         .file_name()
         .and_then(|s| s.to_str())
         .context("file to test is not a valid file, can't extract file name")?;
@@ -100,12 +92,12 @@ fn main() -> anyhow::Result<()> {
     // - a tmp directory, generated by rustdoc
     // we would like a directory we have write access to. if we assume cargo-like directories,
     // we end up with the path `/wbg-out`
-    let wasm_file_str = wasm_file_to_test.to_string_lossy();
+    let wasm_file_str = cli.file.to_string_lossy();
     let tmpdir =
         if wasm_file_str.starts_with("/tmp/rustdoc") || wasm_file_str.starts_with("/var/folders") {
-            wasm_file_to_test.parent() // chop off the file name and give us the /tmp/rustdoc<hash> directory
+            cli.file.parent() // chop off the file name and give us the /tmp/rustdoc<hash> directory
         } else {
-            wasm_file_to_test
+            cli.file
                 .parent() // chop off file name
                 .and_then(|p| p.parent()) // chop off `deps`
                 .and_then(|p| p.parent()) // chop off `debug`
@@ -123,7 +115,7 @@ fn main() -> anyhow::Result<()> {
     // Collect all tests that the test harness is supposed to run. We assume
     // that any exported function with the prefix `__wbg_test` is a test we need
     // to execute.
-    let wasm = fs::read(&wasm_file_to_test).context("failed to read Wasm file")?;
+    let wasm = fs::read(&cli.file).context("failed to read Wasm file")?;
     let mut wasm =
         walrus::Module::from_buffer(&wasm).context("failed to deserialize Wasm module")?;
     let mut tests = Vec::new();
@@ -267,13 +259,11 @@ fn main() -> anyhow::Result<()> {
         .context("executing `wasm-bindgen` over the Wasm file")?;
     shell.clear();
 
-    let args: Vec<_> = args.collect();
-
     match test_mode {
         TestMode::Node { no_modules } => {
-            node::execute(module, &tmpdir, &args, &tests, !no_modules, coverage)?
+            node::execute(module, &tmpdir, cli, &tests, !no_modules, coverage)?
         }
-        TestMode::Deno => deno::execute(module, &tmpdir, &args, &tests)?,
+        TestMode::Deno => deno::execute(module, &tmpdir, cli, &tests)?,
         TestMode::Browser { .. }
         | TestMode::DedicatedWorker { .. }
         | TestMode::SharedWorker { .. }
@@ -289,7 +279,7 @@ fn main() -> anyhow::Result<()> {
                 headless,
                 module,
                 &tmpdir,
-                &args,
+                cli,
                 &tests,
                 test_mode,
                 std::env::var("WASM_BINDGEN_TEST_NO_ORIGIN_ISOLATION").is_err(),
@@ -321,6 +311,57 @@ fn main() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+enum TestMode {
+    Node { no_modules: bool },
+    Deno,
+    Browser { no_modules: bool },
+    DedicatedWorker { no_modules: bool },
+    SharedWorker { no_modules: bool },
+    ServiceWorker { no_modules: bool },
+}
+
+impl TestMode {
+    fn is_worker(self) -> bool {
+        matches!(
+            self,
+            Self::DedicatedWorker { .. } | Self::SharedWorker { .. } | Self::ServiceWorker { .. }
+        )
+    }
+
+    fn no_modules(self) -> bool {
+        match self {
+            Self::Deno => true,
+            Self::Browser { no_modules }
+            | Self::Node { no_modules }
+            | Self::DedicatedWorker { no_modules }
+            | Self::SharedWorker { no_modules }
+            | Self::ServiceWorker { no_modules } => no_modules,
+        }
+    }
+
+    fn env(self) -> &'static str {
+        match self {
+            TestMode::Node { .. } => "WASM_BINDGEN_USE_NODE_EXPERIMENTAL",
+            TestMode::Deno => "WASM_BINDGEN_USE_DENO",
+            TestMode::Browser { .. } => "WASM_BINDGEN_USE_BROWSER",
+            TestMode::DedicatedWorker { .. } => "WASM_BINDGEN_USE_DEDICATED_WORKER",
+            TestMode::SharedWorker { .. } => "WASM_BINDGEN_USE_SHARED_WORKER",
+            TestMode::ServiceWorker { .. } => "WASM_BINDGEN_USE_SERVICE_WORKER",
+        }
+    }
+}
+
+struct TmpDirDeleteGuard(PathBuf);
+
+impl Drop for TmpDirDeleteGuard {
+    fn drop(&mut self) {
+        if let Err(e) = fs::remove_dir_all(&self.0) {
+            error!("failed to remove temporary directory: {}", e);
+        }
+    }
+}
+
 fn coverage_args(tmpdir: &Path) -> PathBuf {
     fn generated(tmpdir: &Path, prefix: &str) -> String {
         let res = format!(

crates/cli/src/bin/wasm-bindgen-test-runner/node.rs

@@ -1,11 +1,12 @@
 use std::env;
-use std::ffi::OsString;
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
 use anyhow::{Context, Error};
 
+use crate::Cli;
+
 // depends on the variable 'wasm' and initializes te WasmBindgenTestContext cx
 pub const SHARED_SETUP: &str = r#"
 const handlers = {};
@@ -41,7 +42,7 @@ handlers.on_console_error = wasm.__wbgtest_console_error;
 pub fn execute(
     module: &str,
     tmpdir: &Path,
-    args: &[OsString],
+    cli: Cli,
     tests: &[String],
     module_format: bool,
     coverage: PathBuf,
@@ -57,11 +58,7 @@ pub fn execute(
         global.__wbg_test_invoke = f => f();
 
         async function main(tests) {{
-            // Forward runtime arguments. These arguments are also arguments to the
-            // `wasm-bindgen-test-runner` which forwards them to node which we
-            // forward to the test harness. this is basically only used for test
-            // filters for now.
-            cx.args(process.argv.slice(2));
+            {args}
 
             const ok = await cx.run(tests.map(n => wasm.__wasm[n]));
 
@@ -92,6 +89,7 @@ pub fn execute(
         },
         coverage = coverage.display(),
         console_override = SHARED_SETUP,
+        args = cli.into_args(),
     );
 
     // Note that we're collecting *JS objects* that represent the functions to
@@ -139,8 +137,7 @@ pub fn execute(
             .env("NODE_PATH", env::join_paths(&path).unwrap())
             .arg("--expose-gc")
             .args(&extra_node_args)
-            .arg(&js_path)
-            .args(args),
+            .arg(&js_path),
     )
 }