refactor: only use channel to return test results (#6550)

* refactor: only use channel to return test results

* error

* rm auto_impl

* rm unused

* inline useless function

* rename to test_collect

* doc

* fmt

* fmt

crates/common/Cargo.toml

@@ -24,7 +24,6 @@ alloy-primitives = { workspace = true, features = ["serde", "getrandom", "arbitr
 alloy-sol-types.workspace = true
 
 async-trait = "0.1"
-auto_impl = "1.1.0"
 clap = { version = "4", features = ["derive", "env", "unicode", "wrap_help"] }
 comfy-table = "7"
 dunce = "1"

crates/common/src/traits.rs

@@ -3,31 +3,35 @@
 use alloy_json_abi::Function;
 use alloy_primitives::Bytes;
 use alloy_sol_types::SolError;
-use auto_impl::auto_impl;
+use std::path::Path;
 
-/// Extension trait for matching tests
-#[auto_impl(&)]
+/// Test filter.
 pub trait TestFilter: Send + Sync {
-    /// Returns whether the test should be included
-    fn matches_test(&self, test_name: impl AsRef<str>) -> bool;
-    /// Returns whether the contract should be included
-    fn matches_contract(&self, contract_name: impl AsRef<str>) -> bool;
-    /// Returns a contract with the given path should be included
-    fn matches_path(&self, path: impl AsRef<str>) -> bool;
+    /// Returns whether the test should be included.
+    fn matches_test(&self, test_name: &str) -> bool;
+
+    /// Returns whether the contract should be included.
+    fn matches_contract(&self, contract_name: &str) -> bool;
+
+    /// Returns a contract with the given path should be included.
+    fn matches_path(&self, path: &Path) -> bool;
 }
 
-/// Extension trait for `Function`
-#[auto_impl(&)]
+/// Extension trait for `Function`.
 pub trait TestFunctionExt {
-    /// Whether this function should be executed as invariant test
+    /// Returns whether this function should be executed as invariant test.
     fn is_invariant_test(&self) -> bool;
-    /// Whether this function should be executed as fuzz test
+
+    /// Returns whether this function should be executed as fuzz test.
     fn is_fuzz_test(&self) -> bool;
-    /// Whether this function is a test
+
+    /// Returns whether this function is a test.
     fn is_test(&self) -> bool;
-    /// Whether this function is a test that should fail
+
+    /// Returns whether this function is a test that should fail.
     fn is_test_fail(&self) -> bool;
-    /// Whether this function is a `setUp` function
+
+    /// Returns whether this function is a `setUp` function.
     fn is_setup(&self) -> bool;
 }
 
@@ -82,7 +86,7 @@ impl TestFunctionExt for str {
     }
 
     fn is_fuzz_test(&self) -> bool {
-        unimplemented!("no naming convention for fuzz tests.")
+        unimplemented!("no naming convention for fuzz tests")
     }
 
     fn is_test(&self) -> bool {

crates/forge/bin/cmd/coverage.rs

@@ -310,21 +310,19 @@ impl CoverageArgs {
         let filter = self.filter;
         let (tx, rx) = channel::<(String, SuiteResult)>();
         let handle =
-            tokio::task::spawn(
-                async move { runner.test(filter, Some(tx), Default::default()).await },
-            );
+            tokio::task::spawn(async move { runner.test(&filter, tx, Default::default()).await });
 
         // Add hit data to the coverage report
-        for (artifact_id, hits) in rx
+        let data = rx
             .into_iter()
             .flat_map(|(_, suite)| suite.test_results.into_values())
             .filter_map(|mut result| result.coverage.take())
             .flat_map(|hit_maps| {
                 hit_maps.0.into_values().filter_map(|map| {
                     Some((known_contracts.find_by_code(map.bytecode.as_ref())?.0, map))
                 })
-            })
-        {
+            });
+        for (artifact_id, hits) in data {
             // TODO: Note down failing tests
             if let Some(source_id) = report.get_source_id(
                 artifact_id.version.clone(),
@@ -344,7 +342,12 @@ impl CoverageArgs {
         }
 
         // Reattach the thread
-        let _ = handle.await;
+        if let Err(e) = handle.await {
+            match e.try_into_panic() {
+                Ok(payload) => std::panic::resume_unwind(payload),
+                Err(e) => return Err(e.into()),
+            }
+        }
 
         // Output final report
         for report_kind in self.report {

crates/forge/bin/cmd/test/filter.rs

@@ -101,38 +101,39 @@ impl FileFilter for FilterArgs {
 }
 
 impl TestFilter for FilterArgs {
-    fn matches_test(&self, test_name: impl AsRef<str>) -> bool {
+    fn matches_test(&self, test_name: &str) -> bool {
         let mut ok = true;
-        let test_name = test_name.as_ref();
         if let Some(re) = &self.test_pattern {
-            ok &= re.is_match(test_name);
+            ok = ok && re.is_match(test_name);
         }
         if let Some(re) = &self.test_pattern_inverse {
-            ok &= !re.is_match(test_name);
+            ok = ok && !re.is_match(test_name);
         }
         ok
     }
 
-    fn matches_contract(&self, contract_name: impl AsRef<str>) -> bool {
+    fn matches_contract(&self, contract_name: &str) -> bool {
         let mut ok = true;
-        let contract_name = contract_name.as_ref();
         if let Some(re) = &self.contract_pattern {
-            ok &= re.is_match(contract_name);
+            ok = ok && re.is_match(contract_name);
         }
         if let Some(re) = &self.contract_pattern_inverse {
-            ok &= !re.is_match(contract_name);
+            ok = ok && !re.is_match(contract_name);
         }
         ok
     }
 
-    fn matches_path(&self, path: impl AsRef<str>) -> bool {
+    fn matches_path(&self, path: &Path) -> bool {
+        let Some(path) = path.to_str() else {
+            return false;
+        };
+
         let mut ok = true;
-        let path = path.as_ref();
-        if let Some(ref glob) = self.path_pattern {
-            ok &= glob.is_match(path);
+        if let Some(re) = &self.path_pattern {
+            ok = ok && re.is_match(path);
         }
-        if let Some(ref glob) = self.path_pattern_inverse {
-            ok &= !glob.is_match(path);
+        if let Some(re) = &self.path_pattern_inverse {
+            ok = ok && !re.is_match(path);
         }
         ok
     }
@@ -195,18 +196,17 @@ impl FileFilter for ProjectPathsAwareFilter {
 }
 
 impl TestFilter for ProjectPathsAwareFilter {
-    fn matches_test(&self, test_name: impl AsRef<str>) -> bool {
+    fn matches_test(&self, test_name: &str) -> bool {
         self.args_filter.matches_test(test_name)
     }
 
-    fn matches_contract(&self, contract_name: impl AsRef<str>) -> bool {
+    fn matches_contract(&self, contract_name: &str) -> bool {
         self.args_filter.matches_contract(contract_name)
     }
 
-    fn matches_path(&self, path: impl AsRef<str>) -> bool {
-        let path = path.as_ref();
+    fn matches_path(&self, path: &Path) -> bool {
         // we don't want to test files that belong to a library
-        self.args_filter.matches_path(path) && !self.paths.has_library_ancestor(Path::new(path))
+        self.args_filter.matches_path(path) && !self.paths.has_library_ancestor(path)
     }
 }