Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(coverage): add option to ignore directories and files from coverage report #8321

Merged
merged 18 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 18 additions & 3 deletions crates/evm/coverage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
21 changes: 16 additions & 5 deletions crates/forge/bin/cmd/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()?;

Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 17 additions & 1 deletion crates/forge/bin/cmd/test/filter.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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>,
Comment on lines +42 to +44
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opted for no-match-coverage to stay in line with the no-match-* pattern established (no-match-test, no-match-contract, no-match-path)

See: #7301 (comment)

}

impl FilterArgs {
Expand Down Expand Up @@ -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() }
}
}
Expand All @@ -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()
}
}
Expand Down Expand Up @@ -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(())
}
}
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
112 changes: 112 additions & 0 deletions crates/forge/tests/cli/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
});
2 changes: 2 additions & 0 deletions crates/forge/tests/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
Loading