From 20b3da1f22e9f62f6e3406a5d582ad4aa509122c Mon Sep 17 00:00:00 2001 From: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Date: Mon, 1 Jul 2024 20:55:12 +0200 Subject: [PATCH] feat(coverage): add option to ignore directories and files from coverage 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 Co-authored-by: Dima Zhornyk <55756184+dimazhornyk@users.noreply.github.com> --- crates/config/README.md | 1 + crates/config/src/lib.rs | 4 + crates/evm/coverage/src/lib.rs | 21 +++++- crates/forge/bin/cmd/coverage.rs | 21 ++++-- crates/forge/bin/cmd/test/filter.rs | 18 ++++- crates/forge/tests/cli/config.rs | 1 + crates/forge/tests/cli/coverage.rs | 112 ++++++++++++++++++++++++++++ crates/forge/tests/cli/test_cmd.rs | 2 + 8 files changed, 171 insertions(+), 9 deletions(-) diff --git a/crates/config/README.md b/crates/config/README.md index fae78bfab46c..337195276b72 100644 --- 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 index 58b1090ff037..3efdc44cf932 100644 --- 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, + /// Only show coverage for files that do not match the specified regex pattern. + #[serde(rename = "no_match_coverage")] + pub coverage_pattern_inverse: Option, /// 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 index 22f4b9808e79..90486b3cc515 100644 --- 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 index b6086b9739f7..cf50f7a971d3 100644 --- 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 index 7ececa7d4ae3..ec2e9b01b50e 100644 --- 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, + + /// 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, } 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 index 6cdd179b349c..63df640060c9 100644 --- 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 index 546a1103849d..42cd7f60d4c6 100644 --- 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::().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::().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 index 52584ab4402c..d0f5ad16b1d9 100644 --- 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