Skip to content

Commit 1c57854

Browse files
authored
fix(forge): preprocess mocks declared in test file (#12516)
* fix(forge): preprocess mockes declared in same test file * Bump compilers version
1 parent faff6a3 commit 1c57854

File tree

4 files changed

+124
-30
lines changed

4 files changed

+124
-30
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ foundry-linking = { path = "crates/linking" }
223223

224224
# solc & compilation utilities
225225
foundry-block-explorers = { version = "0.22.0", default-features = false }
226-
foundry-compilers = { version = "0.19.5", default-features = false, features = [
226+
foundry-compilers = { version = "0.19.6", default-features = false, features = [
227227
"rustls",
228228
"svm-solc",
229229
] }

crates/common/src/preprocessor/deps.rs

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -33,41 +33,54 @@ impl PreprocessorDependencies {
3333
) -> Self {
3434
let mut preprocessed_contracts = BTreeMap::new();
3535
let mut referenced_contracts = HashSet::new();
36-
for contract_id in gcx.hir.contract_ids() {
37-
let contract = gcx.hir.contract(contract_id);
38-
let source = gcx.hir.source(contract.source);
36+
let mut current_mocks = HashSet::new();
37+
38+
// Helper closure for iterating candidate contracts to preprocess (tests and scripts).
39+
let candidate_contracts = || {
40+
gcx.hir.contract_ids().filter_map(|id| {
41+
let contract = gcx.hir.contract(id);
42+
let source = gcx.hir.source(contract.source);
43+
let FileName::Real(path) = &source.file.name else {
44+
return None;
45+
};
3946

40-
let FileName::Real(path) = &source.file.name else {
41-
continue;
42-
};
47+
if !paths.contains(path) {
48+
trace!("{} is not test or script", path.display());
49+
return None;
50+
}
4351

44-
// Collect dependencies only for tests and scripts.
45-
if !paths.contains(path) {
46-
let path = path.display();
47-
trace!("{path} is not test or script");
48-
continue;
49-
}
52+
Some((id, contract, source, path))
53+
})
54+
};
5055

51-
// Do not collect dependencies for mock contracts. Walk through base contracts and
52-
// check if they're from src dir.
53-
if contract.linearized_bases.iter().any(|base_contract_id| {
54-
let base_contract = gcx.hir.contract(*base_contract_id);
55-
let FileName::Real(path) = &gcx.hir.source(base_contract.source).file.name else {
56-
return false;
57-
};
58-
path.starts_with(src_dir)
56+
// Collect current mocks.
57+
for (_, contract, _, path) in candidate_contracts() {
58+
if contract.linearized_bases.iter().any(|base_id| {
59+
let base = gcx.hir.contract(*base_id);
60+
matches!(
61+
&gcx.hir.source(base.source).file.name,
62+
FileName::Real(base_path) if base_path.starts_with(src_dir)
63+
)
5964
}) {
60-
// Record mock contracts to be evicted from preprocessed cache.
61-
mocks.insert(root_dir.join(path));
62-
let path = path.display();
63-
trace!("found mock contract {path}");
65+
let mock_path = root_dir.join(path);
66+
trace!("found mock contract {}", mock_path.display());
67+
current_mocks.insert(mock_path);
68+
}
69+
}
70+
71+
// Collect dependencies for non-mock test/script contracts.
72+
for (contract_id, contract, source, path) in candidate_contracts() {
73+
let full_path = root_dir.join(path);
74+
75+
if current_mocks.contains(&full_path) {
76+
trace!("{} is a mock, skipping", path.display());
6477
continue;
65-
} else {
66-
// Make sure current contract is not in list of mocks (could happen when a contract
67-
// which used to be a mock is refactored to a non-mock implementation).
68-
mocks.remove(&root_dir.join(path));
6978
}
7079

80+
// Make sure current contract is not in list of mocks (could happen when a contract
81+
// which used to be a mock is refactored to a non-mock implementation).
82+
mocks.remove(&full_path);
83+
7184
let mut deps_collector =
7285
BytecodeDependencyCollector::new(gcx, source.file.src.as_str(), src_dir);
7386
// Analyze current contract.
@@ -76,9 +89,14 @@ impl PreprocessorDependencies {
7689
if !deps_collector.dependencies.is_empty() {
7790
preprocessed_contracts.insert(contract_id, deps_collector.dependencies);
7891
}
92+
7993
// Record collected referenced contract ids.
8094
referenced_contracts.extend(deps_collector.referenced_contracts);
8195
}
96+
97+
// Add current mocks.
98+
mocks.extend(current_mocks);
99+
82100
Self { preprocessed_contracts, referenced_contracts }
83101
}
84102
}

crates/forge/tests/cli/test_optimizer.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,82 @@ Compiling 2 files with [..]
716716
"#]]);
717717
});
718718

719+
// <https://github.com/foundry-rs/foundry/issues/12452>
720+
// - CounterMock contract is Counter contract
721+
// - CounterMock declared in CounterTest
722+
//
723+
// ├── src
724+
// │ └── Counter.sol
725+
// └── test
726+
// ├── Counter.t.sol
727+
forgetest_init!(preprocess_mock_declared_in_test_contract, |prj, cmd| {
728+
prj.update_config(|config| {
729+
config.dynamic_test_linking = true;
730+
});
731+
732+
prj.add_source(
733+
"Counter.sol",
734+
r#"
735+
contract Counter {
736+
function add(uint256 x, uint256 y) public pure returns (uint256) {
737+
return x + y;
738+
}
739+
}
740+
"#,
741+
);
742+
743+
prj.add_test(
744+
"Counter.t.sol",
745+
r#"
746+
import {Test} from "forge-std/Test.sol";
747+
import {Counter} from "src/Counter.sol";
748+
749+
contract CounterMock is Counter {}
750+
751+
contract CounterTest is Test {
752+
function test_add() public {
753+
CounterMock impl = new CounterMock();
754+
assertEq(impl.add(2, 2), 4);
755+
}
756+
}
757+
"#,
758+
);
759+
// 20 files plus one mock file are compiled on first run.
760+
cmd.args(["test"]).with_no_redact().assert_success().stdout_eq(str![[r#"
761+
...
762+
Compiling 21 files with [..]
763+
...
764+
765+
"#]]);
766+
cmd.with_no_redact().assert_success().stdout_eq(str![[r#"
767+
...
768+
No files changed, compilation skipped
769+
...
770+
771+
"#]]);
772+
773+
// Change Counter implementation to fail tests.
774+
prj.add_source(
775+
"Counter.sol",
776+
r#"
777+
contract Counter {
778+
function add(uint256 x, uint256 y) public pure returns (uint256) {
779+
return x + y + 1;
780+
}
781+
}
782+
"#,
783+
);
784+
// Assert that Counter and CounterTest files are compiled and tests fail.
785+
cmd.with_no_redact().assert_failure().stdout_eq(str![[r#"
786+
...
787+
Compiling 2 files with [..]
788+
...
789+
[FAIL: assertion failed: 5 != 4] test_add() (gas: [..])
790+
...
791+
792+
"#]]);
793+
});
794+
719795
// ├── src
720796
// │ ├── CounterA.sol
721797
// │ ├── CounterB.sol

0 commit comments

Comments
 (0)