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.

Sign up for GitHub

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

fix: display test results for tests with same name #1097

Merged
merged 4 commits into from
Mar 28, 2022
Merged

fix: display test results for tests with same name #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
510f67d
fix: display test results for tests with same name
onbjerg Mar 28, 2022
a3a3900
fix: strip file prefix from cli output
onbjerg Mar 28, 2022
3c15c17
build: bump ethers
onbjerg Mar 28, 2022
6b8d512
test: adjust tests to new keys
onbjerg Mar 28, 2022
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
26 changes: 13 additions & 13 deletions Cargo.lock
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion cli/src/cmd/forge/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl Cmd for TestArgs {
.evm_spec(evm_spec)
.sender(evm_opts.sender)
.with_fork(utils::get_fork(&evm_opts, &config.rpc_storage_caching))
.build(output, evm_opts)?;
.build(project.paths.root, output, evm_opts)?;

if self.debug.is_some() {
self.filter.test_pattern = self.debug;
Expand Down
7 changes: 4 additions & 3 deletions forge/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,17 @@ pub mod test_helpers {
};
use std::str::FromStr;

pub static COMPILED: Lazy<ProjectCompileOutput> = Lazy::new(|| {
pub static PROJECT: Lazy<Project> = Lazy::new(|| {
let paths = ProjectPathsConfig::builder()
.root("../testdata")
.sources("../testdata")
.build()
.unwrap();
let project = Project::builder().paths(paths).ephemeral().no_artifacts().build().unwrap();
project.compile().unwrap()
Project::builder().paths(paths).ephemeral().no_artifacts().build().unwrap()
});

pub static COMPILED: Lazy<ProjectCompileOutput> = Lazy::new(|| (*PROJECT).compile().unwrap());

pub static EVM_OPTS: Lazy<EvmOpts> = Lazy::new(|| EvmOpts {
env: Env {
gas_limit: 18446744073709551615,
Expand Down
80 changes: 46 additions & 34 deletions forge/src/multi_runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use foundry_evm::executor::{
use foundry_utils::PostLinkInput;
use proptest::test_runner::TestRunner;
use rayon::prelude::*;
use std::{collections::BTreeMap, marker::Sync, sync::mpsc::Sender};
use std::{collections::BTreeMap, marker::Sync, path::Path, sync::mpsc::Sender};

/// Builder used for instantiating the multi-contract runner
#[derive(Debug, Default)]
Expand All @@ -30,13 +30,14 @@ pub struct MultiContractRunnerBuilder {
pub fork: Option<Fork>,
}

pub type DeployableContracts = BTreeMap<String, (Abi, Bytes, Vec<Bytes>)>;
pub type DeployableContracts = BTreeMap<ArtifactId, (Abi, Bytes, Vec<Bytes>)>;

impl MultiContractRunnerBuilder {
/// Given an EVM, proceeds to return a runner which is able to execute all tests
/// against that evm
pub fn build<A>(
self,
root: impl AsRef<Path>,
output: ProjectCompileOutput<A>,
evm_opts: EvmOpts,
) -> Result<MultiContractRunner>
Expand All @@ -46,17 +47,17 @@ impl MultiContractRunnerBuilder {
// This is just the contracts compiled, but we need to merge this with the read cached
// artifacts
let contracts = output
.with_stripped_file_prefixes(root)
.into_artifacts()
.map(|(i, c)| (i, c.into_contract_bytecode()))
.collect::<Vec<(ArtifactId, CompactContractBytecode)>>();

let mut known_contracts: BTreeMap<ArtifactId, (Abi, Vec<u8>)> = Default::default();
let source_paths = contracts
.iter()
.map(|(i, _)| (i.slug(), i.source.to_string_lossy().into()))
.collect::<BTreeMap<String, String>>();

let mut known_contracts: BTreeMap<ArtifactId, (Abi, Vec<u8>)> = Default::default();

// create a mapping of name => (abi, deployment code, Vec<library deployment code>)
let mut deployable_contracts = DeployableContracts::default();

Expand Down Expand Up @@ -89,14 +90,14 @@ impl MultiContractRunnerBuilder {
abi.functions().any(|func| func.name.starts_with("test"))
{
deployable_contracts
.insert(id.slug(), (abi.clone(), bytecode, dependencies.to_vec()));
.insert(id.clone(), (abi.clone(), bytecode, dependencies.to_vec()));
}

contract
.deployed_bytecode
.and_then(|d_bcode| d_bcode.bytecode)
.and_then(|bcode| bcode.object.into_bytes())
.and_then(|bytes| known_contracts.insert(id, (abi, bytes.to_vec())));
.and_then(|bytes| known_contracts.insert(id.clone(), (abi, bytes.to_vec())));
Ok(())
},
)?;
Expand Down Expand Up @@ -151,7 +152,7 @@ impl MultiContractRunnerBuilder {
pub struct MultiContractRunner {
/// Mapping of contract name to Abi, creation bytecode and library bytecode which
/// needs to be deployed & linked against
pub contracts: BTreeMap<String, (Abi, ethers::prelude::Bytes, Vec<ethers::prelude::Bytes>)>,
pub contracts: DeployableContracts,
/// Compiled contracts by name that have an Abi and runtime bytecode
pub known_contracts: BTreeMap<ArtifactId, (Abi, Vec<u8>)>,
/// The EVM instance used in the test runner
Expand All @@ -174,9 +175,9 @@ impl MultiContractRunner {
pub fn count_filtered_tests(&self, filter: &(impl TestFilter + Send + Sync)) -> usize {
self.contracts
.iter()
.filter(|(name, _)| {
filter.matches_path(&self.source_paths.get(*name).unwrap()) &&
filter.matches_contract(name)
.filter(|(id, _)| {
filter.matches_path(id.source.to_string_lossy()) &&
filter.matches_contract(&id.name)
})
.flat_map(|(_, (abi, _, _))| {
abi.functions().filter(|func| filter.matches_test(func.signature()))
Expand All @@ -197,14 +198,12 @@ impl MultiContractRunner {
let results = self
.contracts
.par_iter()
.filter(|(name, _)| {
filter.matches_path(&self.source_paths.get(*name).unwrap()) &&
filter.matches_contract(name)
})
.filter(|(_, (abi, _, _))| {
abi.functions().any(|func| filter.matches_test(func.signature()))
.filter(|(id, _)| {
filter.matches_path(id.source.to_string_lossy()) &&
filter.matches_contract(&id.name)
})
.map(|(name, (abi, deploy_code, libs))| {
.filter(|(_, (abi, _, _))| abi.functions().any(|func| filter.matches_test(&func.name)))
.map(|(id, (abi, deploy_code, libs))| {
let mut builder = ExecutorBuilder::new()
.with_cheatcodes(self.evm_opts.ffi)
.with_config(env.clone())
Expand All @@ -216,9 +215,15 @@ impl MultiContractRunner {
}

let executor = builder.build(db.clone());
let result =
self.run_tests(name, abi, executor, deploy_code.clone(), libs, filter)?;
Ok((name.clone(), result))
let result = self.run_tests(
&id.identifier(),
abi,
executor,
deploy_code.clone(),
libs,
filter,
)?;
Ok((id.identifier(), result))
})
.filter_map(Result::<_>::ok)
.filter(|(_, results)| !results.is_empty())
Expand Down Expand Up @@ -266,7 +271,7 @@ mod tests {
use super::*;
use crate::{
decode::decode_console_logs,
test_helpers::{filter::Filter, COMPILED, EVM_OPTS},
test_helpers::{filter::Filter, COMPILED, EVM_OPTS, PROJECT},
};
use foundry_evm::trace::TraceKind;

Expand All @@ -277,14 +282,14 @@ mod tests {

/// Builds a non-tracing runner
fn runner() -> MultiContractRunner {
base_runner().build((*COMPILED).clone(), EVM_OPTS.clone()).unwrap()
base_runner().build(&(*PROJECT).paths.root, (*COMPILED).clone(), EVM_OPTS.clone()).unwrap()
}

/// Builds a tracing runner
fn tracing_runner() -> MultiContractRunner {
let mut opts = EVM_OPTS.clone();
opts.verbosity = 5;
base_runner().build((*COMPILED).clone(), opts).unwrap()
base_runner().build(&(*PROJECT).paths.root, (*COMPILED).clone(), opts).unwrap()
}

/// A helper to assert the outcome of multiple tests with helpful assert messages
Expand Down Expand Up @@ -352,35 +357,42 @@ mod tests {
&results,
BTreeMap::from([
(
"FailingSetupTest.json:FailingSetupTest",
"core/FailingSetup.t.sol:FailingSetupTest",
vec![(
"setUp()",
false,
Some("Setup failed: setup failed predictably".to_string()),
None,
)],
),
("RevertingTest.json:RevertingTest", vec![("testFailRevert()", true, None, None)]),
(
"SetupConsistencyCheck.json:SetupConsistencyCheck",
"core/Reverting.t.sol:RevertingTest",
vec![("testFailRevert()", true, None, None)],
),
(
"core/SetupConsistency.t.sol:SetupConsistencyCheck",
vec![("testAdd()", true, None, None), ("testMultiply()", true, None, None)],
),
(
"DSStyleTest.json:DSStyleTest",
"core/DSStyle.t.sol:DSStyleTest",
vec![("testFailingAssertions()", true, None, None)],
),
(
"core/DappToolsParity.t.sol:DappToolsParityTest",
vec![
("testAddresses()", true, None, None),
("testEnvironment()", true, None, None),
],
),
(
"PaymentFailureTest.json:PaymentFailureTest",
"core/PaymentFailure.t.sol:PaymentFailureTest",
vec![("testCantPay()", false, Some("Revert".to_string()), None)],
),
(
"LibraryLinkingTest.json:LibraryLinkingTest",
"core/LibraryLinking.t.sol:LibraryLinkingTest",
vec![("testDirect()", true, None, None), ("testNested()", true, None, None)],
),
("AbstractTest.json:AbstractTest", vec![("testSomething()", true, None, None)]),
("core/Abstract.t.sol:AbstractTest", vec![("testSomething()", true, None, None)]),
]),
);
}
Expand All @@ -394,7 +406,7 @@ mod tests {
&results,
BTreeMap::from([
(
"DebugLogsTest.json:DebugLogsTest",
"logs/DebugLogs.t.sol:DebugLogsTest",
vec![
("test1()", true, None, Some(vec!["0".into(), "1".into(), "2".into()])),
("test2()", true, None, Some(vec!["0".into(), "1".into(), "3".into()])),
Expand All @@ -413,7 +425,7 @@ mod tests {
],
),
(
"HardhatLogsTest.json:HardhatLogsTest",
"logs/HardhatLogs.t.sol:HardhatLogsTest",
vec![
(
"testInts()",
Expand Down Expand Up @@ -529,7 +541,7 @@ mod tests {
fn test_doesnt_run_abstract_contract() {
let mut runner = runner();
let results = runner.test(&Filter::new(".*", ".*", ".*core/Abstract.t.sol"), None).unwrap();
assert!(results.get("AbstractTestBase.json:AbstractTestBase").is_none());
assert!(results.get("AbstractTest.json:AbstractTest").is_some());
assert!(results.get("core/Abstract.t.sol:AbstractTestBase").is_none());
assert!(results.get("core/Abstract.t.sol:AbstractTest").is_some());
}
}
2 changes: 1 addition & 1 deletion testdata/core/DappToolsParity.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity >=0.8.0;

import "ds-test/test.sol";

contract DSStyleTest is DSTest {
contract DappToolsParityTest is DSTest {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was improperly named!

function chainId() internal view returns (uint256 id) {
assembly {
id := chainid()
Expand Down