feat(coverage): add option to ignore directories and files from cover…

…age report (#8321) * feat: add option to ignore directories from coverage report * add docs, rename no-coverage-path to ignore-coverage-path * cargo fmt * small refactor * revert formatting changes * revert formatting * path_pattern_ignore_coverage -> coverage_pattern_inverse * use regex instead of glob * re-enable ignoring of sources after report * fix formatting * add basic filter test * remove redundant Path cast * use HashMap::retain * greatly simplify, remove CoverageFilter * move constants out of filter map --------- Co-authored-by: dimazhornyk <[email protected]> Co-authored-by: Dima Zhornyk <[email protected]>
foundry-rs · Jul 1, 2024 · 20b3da1 · 20b3da1
1 parent afcf5b1
commit 20b3da1
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 9 deletions.
diff --git a/crates/config/README.md b/crates/config/README.md
@@ -114,6 +114,7 @@ match_contract = "Foo"
 no_match_contract = "Bar"
 match_path = "*/Foo*"
 no_match_path = "*/Bar*"
+no_match_coverage = "Baz"
 ffi = false
 always_use_create_2_factory = false
 prompt_timeout = 120

diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs
@@ -252,6 +252,9 @@ pub struct Config {
     /// Only run tests in source files that do not match the specified glob pattern.
     #[serde(rename = "no_match_path", with = "from_opt_glob")]
     pub path_pattern_inverse: Option<globset::Glob>,
+    /// Only show coverage for files that do not match the specified regex pattern.
+    #[serde(rename = "no_match_coverage")]
+    pub coverage_pattern_inverse: Option<RegexWrapper>,
     /// Configuration for fuzz testing
     pub fuzz: FuzzConfig,
     /// Configuration for invariant testing
@@ -2073,6 +2076,7 @@ impl Default for Config {
             contract_pattern_inverse: None,
             path_pattern: None,
             path_pattern_inverse: None,
+            coverage_pattern_inverse: None,
             fuzz: FuzzConfig::new("cache/fuzz".into()),
             invariant: InvariantConfig::new("cache/invariant".into()),
             always_use_create_2_factory: false,

diff --git a/crates/evm/coverage/src/lib.rs b/crates/evm/coverage/src/lib.rs
@@ -9,18 +9,17 @@
 extern crate tracing;
 
 use alloy_primitives::{Bytes, B256};
+use eyre::{Context, Result};
 use foundry_compilers::artifacts::sourcemap::SourceMap;
 use semver::Version;
 use std::{
     collections::{BTreeMap, HashMap},
     fmt::Display,
     ops::{AddAssign, Deref, DerefMut},
-    path::PathBuf,
+    path::{Path, PathBuf},
     sync::Arc,
 };
 
-use eyre::{Context, Result};
-
 pub mod analysis;
 pub mod anchors;
 
@@ -150,6 +149,22 @@ impl CoverageReport {
         }
         Ok(())
     }
+
+    /// Removes all the coverage items that should be ignored by the filter.
+    ///
+    /// This function should only be called after all the sources were used, otherwise, the output
+    /// will be missing the ones that are dependent on them.
+    pub fn filter_out_ignored_sources(&mut self, filter: impl Fn(&Path) -> bool) {
+        self.items.retain(|version, items| {
+            items.retain(|item| {
+                self.source_paths
+                    .get(&(version.clone(), item.loc.source_id))
+                    .map(|path| filter(path))
+                    .unwrap_or(false)
+            });
+            !items.is_empty()
+        });
+    }
 }
 
 /// A collection of [`HitMap`]s.

diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs
@@ -26,7 +26,11 @@ use foundry_config::{Config, SolcReq};
 use rayon::prelude::*;
 use rustc_hash::FxHashMap;
 use semver::Version;
-use std::{collections::HashMap, path::PathBuf, sync::Arc};
+use std::{
+    collections::HashMap,
+    path::{Path, PathBuf},
+    sync::Arc,
+};
 use yansi::Paint;
 
 // Loads project's figment and merges the build cli arguments into it
@@ -247,10 +251,8 @@ impl CoverageArgs {
 
         let known_contracts = runner.known_contracts.clone();
 
-        let outcome = self
-            .test
-            .run_tests(runner, config.clone(), verbosity, &self.test.filter(&config))
-            .await?;
+        let filter = self.test.filter(&config);
+        let outcome = self.test.run_tests(runner, config.clone(), verbosity, &filter).await?;
 
         outcome.ensure_ok()?;
 
@@ -288,6 +290,15 @@ impl CoverageArgs {
             }
         }
 
+        // Filter out ignored sources from the report
+        let file_pattern = filter.args().coverage_pattern_inverse.as_ref();
+        let file_root = &filter.paths().root;
+        report.filter_out_ignored_sources(|path: &Path| {
+            file_pattern.map_or(true, |re| {
+                !re.is_match(&path.strip_prefix(file_root).unwrap_or(path).to_string_lossy())
+            })
+        });
+
         // Output final report
         for report_kind in self.report {
             match report_kind {

diff --git a/crates/forge/bin/cmd/test/filter.rs b/crates/forge/bin/cmd/test/filter.rs
@@ -1,5 +1,5 @@
 use clap::Parser;
-use forge::TestFilter;
+use foundry_common::TestFilter;
 use foundry_compilers::{FileFilter, ProjectPathsConfig};
 use foundry_config::{filter::GlobMatcher, Config};
 use std::{fmt, path::Path};
@@ -38,6 +38,10 @@ pub struct FilterArgs {
         value_name = "GLOB"
     )]
     pub path_pattern_inverse: Option<GlobMatcher>,
+
+    /// Only show coverage for files that do not match the specified regex pattern.
+    #[arg(long = "no-match-coverage", visible_alias = "nmco", value_name = "REGEX")]
+    pub coverage_pattern_inverse: Option<regex::Regex>,
 }
 
 impl FilterArgs {
@@ -71,6 +75,9 @@ impl FilterArgs {
         if self.path_pattern_inverse.is_none() {
             self.path_pattern_inverse = config.path_pattern_inverse.clone().map(Into::into);
         }
+        if self.coverage_pattern_inverse.is_none() {
+            self.coverage_pattern_inverse = config.coverage_pattern_inverse.clone().map(Into::into);
+        }
         ProjectPathsAwareFilter { args_filter: self, paths: config.project_paths() }
     }
 }
@@ -84,6 +91,7 @@ impl fmt::Debug for FilterArgs {
             .field("no-match-contract", &self.contract_pattern_inverse.as_ref().map(|r| r.as_str()))
             .field("match-path", &self.path_pattern.as_ref().map(|g| g.as_str()))
             .field("no-match-path", &self.path_pattern_inverse.as_ref().map(|g| g.as_str()))
+            .field("no-match-coverage", &self.coverage_pattern_inverse.as_ref().map(|g| g.as_str()))
             .finish_non_exhaustive()
     }
 }
@@ -152,6 +160,9 @@ impl fmt::Display for FilterArgs {
         if let Some(p) = &self.path_pattern_inverse {
             writeln!(f, "\tno-match-path: `{}`", p.as_str())?;
         }
+        if let Some(p) = &self.coverage_pattern_inverse {
+            writeln!(f, "\tno-match-coverage: `{}`", p.as_str())?;
+        }
         Ok(())
     }
 }
@@ -178,6 +189,11 @@ impl ProjectPathsAwareFilter {
     pub fn args_mut(&mut self) -> &mut FilterArgs {
         &mut self.args_filter
     }
+
+    /// Returns the project paths.
+    pub fn paths(&self) -> &ProjectPathsConfig {
+        &self.paths
+    }
 }
 
 impl FileFilter for ProjectPathsAwareFilter {

diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs
@@ -64,6 +64,7 @@ forgetest!(can_extract_config_values, |prj, cmd| {
         contract_pattern_inverse: None,
         path_pattern: None,
         path_pattern_inverse: None,
+        coverage_pattern_inverse: None,
         fuzz: FuzzConfig {
             runs: 1000,
             max_test_rejects: 100203,

diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs
@@ -76,3 +76,115 @@ contract AContractTest is DSTest {
     };
     assert!(lcov_data.lines().any(valid_line), "{lcov_data}");
 });
+
+forgetest!(test_no_match_coverage, |prj, cmd| {
+    prj.insert_ds_test();
+    prj.add_source(
+        "AContract.sol",
+        r#"
+contract AContract {
+    int public i;
+
+    function init() public {
+        i = 0;
+    }
+
+    function foo() public {
+        i = 1;
+    }
+}
+    "#,
+    )
+    .unwrap();
+
+    prj.add_source(
+        "AContractTest.sol",
+        r#"
+import "./test.sol";
+import {AContract} from "./AContract.sol";
+
+contract AContractTest is DSTest {
+    AContract a;
+
+    function setUp() public {
+        a = new AContract();
+        a.init();
+    }
+
+    function testFoo() public {
+        a.foo();
+    }
+}
+    "#,
+    )
+    .unwrap();
+
+    prj.add_source(
+        "BContract.sol",
+        r#"
+contract BContract {
+    int public i;
+
+    function init() public {
+        i = 0;
+    }
+
+    function foo() public {
+        i = 1;
+    }
+}
+    "#,
+    )
+    .unwrap();
+
+    prj.add_source(
+        "BContractTest.sol",
+        r#"
+import "./test.sol";
+import {BContract} from "./BContract.sol";
+
+contract BContractTest is DSTest {
+    BContract a;
+
+    function setUp() public {
+        a = new BContract();
+        a.init();
+    }
+
+    function testFoo() public {
+        a.foo();
+    }
+}
+    "#,
+    )
+    .unwrap();
+
+    let lcov_info = prj.root().join("lcov.info");
+    cmd.arg("coverage").args([
+        "--no-match-coverage".to_string(),
+        "AContract".to_string(), // Filter out `AContract`
+        "--report".to_string(),
+        "lcov".to_string(),
+        "--report-file".to_string(),
+        lcov_info.to_str().unwrap().to_string(),
+    ]);
+    cmd.assert_success();
+    assert!(lcov_info.exists());
+
+    let lcov_data = std::fs::read_to_string(lcov_info).unwrap();
+    // BContract.init must be hit at least once
+    let re = Regex::new(r"FNDA:(\d+),BContract\.init").unwrap();
+    let valid_line = |line| {
+        re.captures(line)
+            .map_or(false, |caps| caps.get(1).unwrap().as_str().parse::<i32>().unwrap() > 0)
+    };
+    assert!(lcov_data.lines().any(valid_line), "{lcov_data}");
+
+    // AContract.init must not be hit
+    let re = Regex::new(r"FNDA:(\d+),AContract\.init").unwrap();
+    let valid_line = |line| {
+        re.captures(line)
+            .map_or(false, |caps| caps.get(1).unwrap().as_str().parse::<i32>().unwrap() > 0)
+    };
+    assert!(!lcov_data.lines().any(valid_line), "{lcov_data}");
+});
diff --git a/crates/forge/tests/cli/test_cmd.rs b/crates/forge/tests/cli/test_cmd.rs
@@ -21,6 +21,7 @@ forgetest!(can_set_filter_values, |prj, cmd| {
         contract_pattern_inverse: None,
         path_pattern: Some(glob.clone()),
         path_pattern_inverse: None,
+        coverage_pattern_inverse: None,
         ..Default::default()
     };
     prj.write_config(config);
@@ -33,6 +34,7 @@ forgetest!(can_set_filter_values, |prj, cmd| {
     assert_eq!(config.contract_pattern_inverse, None);
     assert_eq!(config.path_pattern.unwrap(), glob);
     assert_eq!(config.path_pattern_inverse, None);
+    assert_eq!(config.coverage_pattern_inverse, None);
 });
 
 // tests that warning is displayed when there are no tests in project