De-duplicate test filtering (#4373)

Author: daxpedda

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

@@ -4,9 +4,10 @@ use std::{fs, process};
 
 use anyhow::{Context, Error};
 
+use crate::Tests;
 use crate::{node::SHARED_SETUP, Cli};
 
-pub fn execute(module: &str, tmpdir: &Path, cli: Cli, tests: &[String]) -> Result<(), Error> {
+pub fn execute(module: &str, tmpdir: &Path, cli: Cli, tests: Tests) -> Result<(), Error> {
     let mut js_to_execute = format!(
         r#"import * as wasm from "./{module}.js";
 
@@ -21,11 +22,11 @@ pub fn execute(module: &str, tmpdir: &Path, cli: Cli, tests: &[String]) -> Resul
     "#,
         nocapture = cli.nocapture.clone(),
         console_override = SHARED_SETUP,
-        args = cli.into_args(),
+        args = cli.into_args(&tests),
     );
 
-    for test in tests {
-        js_to_execute.push_str(&format!("tests.push('{}')\n", test));
+    for test in tests.tests {
+        js_to_execute.push_str(&format!("tests.push('{}')\n", test.name));
     }
 
     js_to_execute.push_str(

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

@@ -71,30 +71,41 @@ struct Cli {
 }
 
 impl Cli {
-    fn into_args(self) -> String {
+    fn into_args(self, tests: &Tests) -> String {
         let include_ignored = self.include_ignored;
         let ignored = self.ignored;
-        let exact = self.exact;
-        let skip = self.skip;
-        let filter = if let Some(filter) = self.filter {
-            &format!("\"{filter}\"")
-        } else {
-            "undefined"
-        };
+        let filtered = tests.filtered;
 
         format!(
             r#"
             // Forward runtime arguments.
             cx.include_ignored({include_ignored:?});
             cx.ignored({ignored:?});
-            cx.exact({exact:?});
-            cx.skip({skip:?});
-            cx.filter({filter});
+            cx.filtered_count({filtered});
         "#
         )
     }
 }
 
+struct Tests {
+    tests: Vec<Test>,
+    filtered: usize,
+}
+
+impl Tests {
+    fn new() -> Self {
+        Self {
+            tests: Vec::new(),
+            filtered: 0,
+        }
+    }
+}
+
+struct Test {
+    name: String,
+    ignored: bool,
+}
+
 fn main() -> anyhow::Result<()> {
     env_logger::init();
 
@@ -114,36 +125,55 @@ fn main() -> anyhow::Result<()> {
     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();
+    let mut tests = Tests::new();
 
-    for export in wasm.exports.iter() {
-        if export.name.starts_with("__wbgt_") {
-            tests.push(export.name.to_string());
+    'outer: for export in wasm.exports.iter() {
+        let Some(name) = export.name.strip_prefix("__wbgt_") else {
+            continue;
+        };
+        let modifiers = name.split_once('_').expect("found invalid identifier").0;
+        let test = Test {
+            name: export.name.clone(),
+            ignored: modifiers.contains('$'),
+        };
+
+        if let Some(filter) = &cli.filter {
+            let matches = if cli.exact {
+                name == *filter
+            } else {
+                name.contains(filter)
+            };
+
+            if !matches {
+                tests.filtered += 1;
+                continue;
+            }
         }
-    }
 
-    if cli.list {
-        'outer: for test in tests {
-            if !cli.ignored || test.starts_with("__wbgt_$") {
-                if let Some(filter) = &cli.filter {
-                    let matches = if cli.exact {
-                        test == *filter
-                    } else {
-                        test.contains(filter)
-                    };
-
-                    if !matches {
-                        continue;
-                    }
-                }
+        for skip in &cli.skip {
+            let matches = if cli.exact {
+                name == *skip
+            } else {
+                name.contains(skip)
+            };
 
-                for skip in &cli.skip {
-                    if test.contains(skip) {
-                        continue 'outer;
-                    }
-                }
+            if matches {
+                tests.filtered += 1;
+                continue 'outer;
+            }
+        }
 
-                println!("{}: test", test.split_once("::").unwrap().1);
+        if !test.ignored && cli.ignored {
+            tests.filtered += 1;
+        } else {
+            tests.tests.push(test);
+        }
+    }
+
+    if cli.list {
+        for test in tests.tests {
+            if !cli.ignored || test.ignored {
+                println!("{}: test", test.name.split_once("::").unwrap().1);
             }
         }
 
@@ -159,7 +189,7 @@ fn main() -> anyhow::Result<()> {
     // Right now there's a bug where if no tests are present then the
     // `wasm-bindgen-test` runtime support isn't linked in, so just bail out
     // early saying everything is ok.
-    if tests.is_empty() {
+    if tests.tests.is_empty() {
         println!("no tests to run!");
         return Ok(());
     }
@@ -288,9 +318,9 @@ fn main() -> anyhow::Result<()> {
 
     match test_mode {
         TestMode::Node { no_modules } => {
-            node::execute(module, tmpdir.path(), cli, &tests, !no_modules, coverage)?
+            node::execute(module, tmpdir.path(), cli, tests, !no_modules, coverage)?
         }
-        TestMode::Deno => deno::execute(module, tmpdir.path(), cli, &tests)?,
+        TestMode::Deno => deno::execute(module, tmpdir.path(), cli, tests)?,
         TestMode::Browser { .. }
         | TestMode::DedicatedWorker { .. }
         | TestMode::SharedWorker { .. }
@@ -307,7 +337,7 @@ fn main() -> anyhow::Result<()> {
                 module,
                 tmpdir.path(),
                 cli,
-                &tests,
+                tests,
                 test_mode,
                 std::env::var("WASM_BINDGEN_TEST_NO_ORIGIN_ISOLATION").is_err(),
                 coverage,

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

@@ -7,6 +7,7 @@ use std::process::Command;
 use anyhow::{Context, Error};
 
 use crate::Cli;
+use crate::Tests;
 
 // depends on the variable 'wasm' and initializes te WasmBindgenTestContext cx
 pub const SHARED_SETUP: &str = r#"
@@ -48,7 +49,7 @@ pub fn execute(
     module: &str,
     tmpdir: &Path,
     cli: Cli,
-    tests: &[String],
+    tests: Tests,
     module_format: bool,
     coverage: PathBuf,
 ) -> Result<(), Error> {
@@ -96,14 +97,14 @@ pub fn execute(
         coverage = coverage.display(),
         nocapture = cli.nocapture.clone(),
         console_override = SHARED_SETUP,
-        args = cli.into_args(),
+        args = cli.into_args(&tests),
     );
 
     // Note that we're collecting *JS objects* that represent the functions to
     // execute, and then those objects are passed into Wasm for it to execute
     // when it sees fit.
-    for test in tests {
-        js_to_execute.push_str(&format!("tests.push('{}')\n", test));
+    for test in tests.tests {
+        js_to_execute.push_str(&format!("tests.push('{}')\n", test.name));
     }
     // And as a final addendum, exit with a nonzero code if any tests fail.
     js_to_execute.push_str(

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

@@ -7,15 +7,15 @@ use std::path::{Path, PathBuf};
 use anyhow::{anyhow, Context, Error};
 use rouille::{Request, Response, Server};
 
-use crate::{Cli, TestMode};
+use crate::{Cli, TestMode, Tests};
 
 pub(crate) fn spawn(
     addr: &SocketAddr,
     headless: bool,
     module: &'static str,
     tmpdir: &Path,
     cli: Cli,
-    tests: &[String],
+    tests: Tests,
     test_mode: TestMode,
     isolate_origin: bool,
     coverage: PathBuf,
@@ -71,7 +71,7 @@ pub(crate) fn spawn(
     };
 
     let nocapture = cli.nocapture;
-    let args = cli.into_args();
+    let args = cli.into_args(&tests);
 
     if test_mode.is_worker() {
         let mut worker_script = if test_mode.no_modules() {
@@ -281,8 +281,8 @@ pub(crate) fn spawn(
             "#,
         ));
     }
-    for test in tests {
-        js_to_execute.push_str(&format!("tests.push('{}');\n", test));
+    for test in tests.tests {
+        js_to_execute.push_str(&format!("tests.push('{}');\n", test.name));
     }
     js_to_execute.push_str("main(tests);\n");
 

crates/test-macro/src/lib.rs

@@ -97,7 +97,7 @@ pub fn wasm_bindgen_test(
         quote! {
             const _: () = {
                 #wasm_bindgen_path::__rt::wasm_bindgen::__wbindgen_coverage! {
-                #[export_name = ::core::concat!("__wbgt_", #ignore_name, ::core::module_path!(), "::", ::core::stringify!(#ident))]
+                #[export_name = ::core::concat!("__wbgt_", #ignore_name, "_", ::core::module_path!(), "::", ::core::stringify!(#ident))]
                 #[cfg(all(target_arch = "wasm32", any(target_os = "unknown", target_os = "none")))]
                 extern "C" fn __wbgt_test(cx: &#wasm_bindgen_path::__rt::Context) {
                     let test_name = ::core::concat!(::core::module_path!(), "::", ::core::stringify!(#ident));

crates/test/src/rt/mod.rs

@@ -127,31 +127,19 @@ pub struct Context {
 }
 
 struct State {
-    /// An optional filter used to restrict which tests are actually executed
-    /// and which are ignored. This is passed via the `args` function which
-    /// comes from the command line of `wasm-bindgen-test-runner`. Currently
-    /// this is the only "CLI option"
-    filter: RefCell<Option<String>>,
-
     /// Include ignored tests.
     include_ignored: Cell<bool>,
 
-    /// Include ignored tests.
+    /// Only run ignored tests.
     ignored: Cell<bool>,
 
-    /// Only execute with exactly matching name.
-    exact: Cell<bool>,
-
-    /// Tests to skip.
-    skip: RefCell<Vec<String>>,
-
     /// Counter of the number of tests that have succeeded.
     succeeded_count: Cell<usize>,
 
-    /// Counter of the number of tests that have been filtered
+    /// Number of tests that have been filtered.
     filtered_count: Cell<usize>,
 
-    /// Counter of the number of tests that have been ignored
+    /// Number of tests that have been ignored.
     ignored_count: Cell<usize>,
 
     /// A list of all tests which have failed.
@@ -353,17 +341,14 @@ impl Context {
 
         Context {
             state: Rc::new(State {
-                filter: Default::default(),
                 include_ignored: Default::default(),
                 ignored: Default::default(),
-                exact: Default::default(),
-                skip: Default::default(),
                 failures: Default::default(),
+                succeeded_count: Default::default(),
                 filtered_count: Default::default(),
                 ignored_count: Default::default(),
                 remaining: Default::default(),
                 running: Default::default(),
-                succeeded_count: Default::default(),
                 formatter,
                 timer,
             }),
@@ -380,19 +365,9 @@ impl Context {
         self.state.ignored.set(ignored);
     }
 
-    /// Handle `--exact` flag.
-    pub fn exact(&mut self, exact: bool) {
-        self.state.exact.set(exact);
-    }
-
-    /// Handle `--skip` arguments.
-    pub fn skip(&mut self, skip: Vec<String>) {
-        *self.state.skip.borrow_mut() = skip;
-    }
-
     /// Handle filter argument.
-    pub fn filter(&mut self, filter: Option<String>) {
-        *self.state.filter.borrow_mut() = filter;
+    pub fn filtered_count(&mut self, filtered: usize) {
+        self.state.filtered_count.set(filtered);
     }
 
     /// Executes a list of tests, returning a promise representing their
@@ -558,34 +533,9 @@ impl Context {
         should_panic: Option<Option<&'static str>>,
         ignore: Option<Option<&'static str>>,
     ) {
-        // Split away
+        // Remove the crate name to mimic libtest more closely.
+        // This also removes our `__wbgt_` prefix and the `ignored` and `should_panic` modifiers.
         let name = name.split_once("::").unwrap().1;
-        // If our test is filtered out, record that it was filtered and move
-        // on, nothing to do here.
-        let filter = self.state.filter.borrow();
-        if let Some(filter) = &*filter {
-            let exact = self.state.exact.get();
-
-            let matches = if exact {
-                name == filter
-            } else {
-                name.contains(filter)
-            };
-
-            if !matches {
-                let filtered = self.state.filtered_count.get();
-                self.state.filtered_count.set(filtered + 1);
-                return;
-            }
-        }
-
-        for skip in &*self.state.skip.borrow() {
-            if name.contains(skip) {
-                let filtered = self.state.filtered_count.get();
-                self.state.filtered_count.set(filtered + 1);
-                return;
-            }
-        }
 
         if self.state.ignored.get() && ignore.is_none() {
             let filtered = self.state.filtered_count.get();