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: add option to ignore directories from coverage report #7301

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/common/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ pub trait TestFilter: Send + Sync {
fn matches_path(&self, path: &Path) -> bool;
}

/// Coverage filter.
pub trait CoverageFilter: Send + Sync {
/// Returns whether the file path should be included in coverage report.
fn matches_file_path(&self, path: &Path) -> bool;
}

/// Extension trait for `Function`.
pub trait TestFunctionExt {
/// Returns whether this function should be executed as invariant test.
Expand Down
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*"
ignore_coverage_path = "*/Baz*"
ffi = false
always_use_create_2_factory = false
# These are the default callers, generated using `address(uint160(uint256(keccak256("foundry default caller"))))`
Expand Down
50 changes: 27 additions & 23 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,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 glob pattern.
#[serde(rename = "ignore_coverage_path", with = "from_opt_glob")]
pub path_pattern_ignore_coverage: Option<globset::Glob>,
/// Configuration for fuzz testing
pub fuzz: FuzzConfig,
/// Configuration for invariant testing
Expand Down Expand Up @@ -710,7 +713,7 @@ impl Config {
if self.offline {
return Err(SolcError::msg(format!(
"can't install missing solc {version} in offline mode"
)))
)));
}
Solc::blocking_install(version)?;
solc = Solc::find_svm_installed_version(&v)?;
Expand All @@ -722,12 +725,12 @@ impl Config {
return Err(SolcError::msg(format!(
"`solc` {} does not exist",
solc.display()
)))
)));
}
Some(Solc::new(solc))
}
};
return Ok(solc)
return Ok(solc);
}

Ok(None)
Expand All @@ -737,7 +740,7 @@ impl Config {
#[inline]
pub fn evm_spec_id(&self) -> SpecId {
if self.cancun {
return SpecId::CANCUN
return SpecId::CANCUN;
}
evm_spec_id(&self.evm_version)
}
Expand All @@ -748,7 +751,7 @@ impl Config {
/// `auto_detect_solc`
pub fn is_auto_detect(&self) -> bool {
if self.solc.is_some() {
return false
return false;
}
self.auto_detect_solc
}
Expand Down Expand Up @@ -950,7 +953,7 @@ impl Config {
) -> Result<Option<ResolvedEtherscanConfig>, EtherscanConfigError> {
if let Some(maybe_alias) = self.etherscan_api_key.as_ref().or(self.eth_rpc_url.as_ref()) {
if self.etherscan.contains_key(maybe_alias) {
return self.etherscan.clone().resolved().remove(maybe_alias).transpose()
return self.etherscan.clone().resolved().remove(maybe_alias).transpose();
}
}

Expand All @@ -977,7 +980,7 @@ impl Config {
// etherscan fallback via API key
if let Some(key) = self.etherscan_api_key.as_ref() {
let chain = chain.or(self.chain).unwrap_or_default();
return Ok(ResolvedEtherscanConfig::create(key, chain))
return Ok(ResolvedEtherscanConfig::create(key, chain));
}

Ok(None)
Expand Down Expand Up @@ -1231,7 +1234,7 @@ impl Config {
{
let file_path = self.get_config_path();
if !file_path.exists() {
return Ok(())
return Ok(());
}
let contents = fs::read_to_string(&file_path)?;
let mut doc = contents.parse::<toml_edit::Document>()?;
Expand Down Expand Up @@ -1393,14 +1396,14 @@ impl Config {
return match path.is_file() {
true => Some(path.to_path_buf()),
false => None,
}
};
}
let cwd = std::env::current_dir().ok()?;
let mut cwd = cwd.as_path();
loop {
let file_path = cwd.join(path);
if file_path.is_file() {
return Some(file_path)
return Some(file_path);
}
cwd = cwd.parent()?;
}
Expand Down Expand Up @@ -1474,7 +1477,7 @@ impl Config {
if let Some(cache_dir) = Config::foundry_rpc_cache_dir() {
let mut cache = Cache { chains: vec![] };
if !cache_dir.exists() {
return Ok(cache)
return Ok(cache);
}
if let Ok(entries) = cache_dir.as_path().read_dir() {
for entry in entries.flatten().filter(|x| x.path().is_dir()) {
Expand Down Expand Up @@ -1518,7 +1521,7 @@ impl Config {
fn get_cached_blocks(chain_path: &Path) -> eyre::Result<Vec<(String, u64)>> {
let mut blocks = vec![];
if !chain_path.exists() {
return Ok(blocks)
return Ok(blocks);
}
for block in chain_path.read_dir()?.flatten() {
let file_type = block.file_type()?;
Expand All @@ -1530,7 +1533,7 @@ impl Config {
{
block.path()
} else {
continue
continue;
};
blocks.push((file_name.to_string_lossy().into_owned(), fs::metadata(filepath)?.len()));
}
Expand All @@ -1540,7 +1543,7 @@ impl Config {
//The path provided to this function should point to the etherscan cache for a chain
fn get_cached_block_explorer_data(chain_path: &Path) -> eyre::Result<u64> {
if !chain_path.exists() {
return Ok(0)
return Ok(0);
}

fn dir_size_recursive(mut dir: fs::ReadDir) -> eyre::Result<u64> {
Expand Down Expand Up @@ -1757,7 +1760,7 @@ pub(crate) mod from_opt_glob {
{
let s: Option<String> = Option::deserialize(deserializer)?;
if let Some(s) = s {
return Ok(Some(globset::Glob::new(&s).map_err(serde::de::Error::custom)?))
return Ok(Some(globset::Glob::new(&s).map_err(serde::de::Error::custom)?));
}
Ok(None)
}
Expand Down Expand Up @@ -1869,6 +1872,7 @@ impl Default for Config {
contract_pattern_inverse: None,
path_pattern: None,
path_pattern_inverse: None,
path_pattern_ignore_coverage: None,
fuzz: FuzzConfig::new("cache/fuzz".into()),
invariant: Default::default(),
always_use_create_2_factory: false,
Expand Down Expand Up @@ -2064,7 +2068,7 @@ impl TomlFileProvider {
if let Some(file) = self.env_val() {
let path = Path::new(&file);
if !path.exists() {
return true
return true;
}
}
false
Expand All @@ -2084,7 +2088,7 @@ impl TomlFileProvider {
"Config file `{}` set in env var `{}` does not exist",
file,
self.env_var.unwrap()
)))
)));
}
Toml::file(file)
} else {
Expand Down Expand Up @@ -2128,7 +2132,7 @@ impl<P: Provider> Provider for ForcedSnakeCaseData<P> {
if Config::STANDALONE_SECTIONS.contains(&profile.as_ref()) {
// don't force snake case for keys in standalone sections
map.insert(profile, dict);
continue
continue;
}
map.insert(profile, dict.into_iter().map(|(k, v)| (k.to_snake_case(), v)).collect());
}
Expand Down Expand Up @@ -2268,7 +2272,7 @@ impl Provider for DappEnvCompatProvider {
if val > 1 {
return Err(
format!("Invalid $DAPP_BUILD_OPTIMIZE value `{val}`, expected 0 or 1").into()
)
);
}
dict.insert("optimizer".to_string(), (val == 1).into());
}
Expand Down Expand Up @@ -2334,7 +2338,7 @@ impl<P: Provider> Provider for RenameProfileProvider<P> {
fn data(&self) -> Result<Map<Profile, Dict>, Error> {
let mut data = self.provider.data()?;
if let Some(data) = data.remove(&self.from) {
return Ok(Map::from([(self.to.clone(), data)]))
return Ok(Map::from([(self.to.clone(), data)]));
}
Ok(Default::default())
}
Expand Down Expand Up @@ -2380,7 +2384,7 @@ impl<P: Provider> Provider for UnwrapProfileProvider<P> {
for (profile_str, profile_val) in profiles {
let profile = Profile::new(&profile_str);
if profile != self.profile {
continue
continue;
}
match profile_val {
Value::Dict(_, dict) => return Ok(profile.collect(dict)),
Expand All @@ -2391,7 +2395,7 @@ impl<P: Provider> Provider for UnwrapProfileProvider<P> {
));
err.metadata = Some(self.provider.metadata());
err.profile = Some(self.profile.clone());
return Err(err)
return Err(err);
}
}
}
Expand Down Expand Up @@ -2503,7 +2507,7 @@ impl<P: Provider> Provider for OptionalStrictProfileProvider<P> {
// provider and can't map the metadata to the error. Therefor we return the root error
// if this error originated in the provider's data.
if let Err(root_err) = self.provider.data() {
return root_err
return root_err;
}
err
})
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/coverage/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<'a> ContractVisitor<'a> {
let kind: String =
node.attribute("kind").ok_or_else(|| eyre::eyre!("Function has no kind"))?;
if kind == "constructor" || kind == "receive" {
return Ok(())
return Ok(());
}

match node.body.take() {
Expand Down Expand Up @@ -473,7 +473,7 @@ impl SourceAnalyzer {
for (source_id, ast) in asts.into_iter() {
for child in ast.nodes {
if !matches!(child.node_type, NodeType::ContractDefinition) {
continue
continue;
}

let node_id =
Expand Down
32 changes: 32 additions & 0 deletions crates/evm/coverage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@ use std::{
collections::{BTreeMap, HashMap},
fmt::Display,
ops::{AddAssign, Deref, DerefMut},
path::Path,
};

use eyre::{Context, Result};
use foundry_common::CoverageFilter;

pub mod analysis;
pub mod anchors;

mod inspector;

pub use inspector::CoverageCollector;

/// A coverage report.
Expand Down Expand Up @@ -151,6 +154,35 @@ 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 CoverageFilter) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: docs

Copy link
Author

Choose a reason for hiding this comment

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

added

let mut new_items = HashMap::new();
for (version, items) in self.items.iter() {
let new_items_for_version = items
.iter()
.filter(|item| {
filter.matches_file_path(Path::new(
&self.get_source_path(version, item.loc.source_id),
))
})
.cloned()
.collect();
new_items.insert(version.clone(), new_items_for_version);
}
self.items = new_items;
}

/// Get the source path for a specific source file ID and version.
fn get_source_path(&self, version: &Version, source_id: usize) -> String {
Copy link
Member

Choose a reason for hiding this comment

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

docs here too

Copy link
Author

Choose a reason for hiding this comment

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

added

self.source_paths
.get(&(version.clone(), source_id))
.cloned()
.unwrap_or_else(|| format!("Unknown (ID: {}, solc: {version})", source_id))
}
}

/// A collection of [HitMap]s
Expand Down