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

refactor: only use channel to return test results #6550

Merged
merged 9 commits into from
Dec 8, 2023
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: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
38 changes: 21 additions & 17 deletions crates/common/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions crates/forge/bin/cmd/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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 {
Expand Down
38 changes: 19 additions & 19 deletions crates/forge/bin/cmd/test/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
}

Expand Down
Loading