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

Jump to bottom

fix(coverage): treat assert/require as lines instead branch #8413

Merged
merged 6 commits into from
Jul 11, 2024
Merged

fix(coverage): treat assert/require as lines instead branch #8413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
21f41ea
fix(coverage): treat assert/require as lines instead branch
grandizzy Jul 11, 2024
7eb9847
Changes after review: use snapbox assertion
grandizzy Jul 11, 2024
8b1e2ad
More cov tests ported
grandizzy Jul 11, 2024
00207b8
Merge remote-tracking branch 'origin' into issue-4294
grandizzy Jul 11, 2024
736e481
Nicer pattern match
grandizzy Jul 11, 2024
84aa392
Merge branch 'master' into issue-4294
grandizzy Jul 11, 2024
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
32 changes: 4 additions & 28 deletions crates/evm/coverage/src/analysis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,10 @@ impl<'a> ContractVisitor<'a> {
// tupleexpression
// yulfunctioncall
match node.node_type {
NodeType::Assignment | NodeType::UnaryOperation => {
NodeType::Assignment |
NodeType::UnaryOperation |
NodeType::FunctionCall |
NodeType::Conditional => {
self.push_item(CoverageItem {
kind: CoverageItemKind::Statement,
loc: self.source_location_for(&node.src),
Expand Down Expand Up @@ -322,33 +325,6 @@ impl<'a> ContractVisitor<'a> {

Ok(())
}
NodeType::FunctionCall => {
self.push_item(CoverageItem {
kind: CoverageItemKind::Statement,
loc: self.source_location_for(&node.src),
hits: 0,
});

let expr: Option<Node> = node.attribute("expression");
if let Some(NodeType::Identifier) = expr.as_ref().map(|expr| &expr.node_type) {
// Might be a require/assert call
let name: Option<String> = expr.and_then(|expr| expr.attribute("name"));
if let Some("assert" | "require") = name.as_deref() {
self.push_branches(&node.src, self.branch_id);
self.branch_id += 1;
}
}
Comment on lines -332 to -340
Copy link
Member

Choose a reason for hiding this comment

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

ah okay, this is now no longer required because all this did was check for assert | require


Ok(())
}
NodeType::Conditional => {
self.push_item(CoverageItem {
kind: CoverageItemKind::Statement,
loc: self.source_location_for(&node.src),
hits: 0,
});
Ok(())
}
// Does not count towards coverage
NodeType::FunctionCallOptions |
NodeType::Identifier |
Expand Down
116 changes: 69 additions & 47 deletions crates/forge/tests/cli/coverage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use regex::Regex;
use foundry_test_utils::str;

forgetest!(basic_coverage, |_prj, cmd| {
cmd.args(["coverage"]);
Expand Down Expand Up @@ -57,24 +57,15 @@ contract AContractTest is DSTest {
)
.unwrap();

let lcov_info = prj.root().join("lcov.info");
cmd.arg("coverage").args([
"--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();
// AContract.init must be hit at least once
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}");
// Assert 100% coverage (init function coverage called in setUp is accounted).
cmd.arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(str![[r#"
...
| File | % Lines | % Statements | % Branches | % Funcs |
|-------------------|---------------|---------------|---------------|---------------|
| src/AContract.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) |
| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) |

"#]]);
});

forgetest!(test_no_match_coverage, |prj, cmd| {
Expand Down Expand Up @@ -159,32 +150,63 @@ contract BContractTest is DSTest {
)
.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}");
// Assert AContract is not included in report.
cmd.arg("coverage")
.args([
"--no-match-coverage".to_string(),
"AContract".to_string(), // Filter out `AContract`
])
.assert_success()
.stdout_eq(str![[r#"
...
| File | % Lines | % Statements | % Branches | % Funcs |
|-------------------|---------------|---------------|---------------|---------------|
| src/BContract.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) |
| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (2/2) |

"#]]);
});

forgetest!(test_assert_require_coverage, |prj, cmd| {
prj.insert_ds_test();
prj.add_source(
"AContract.sol",
r#"
contract AContract {
function checkA() external pure returns (bool) {
assert(10 > 2);
require(10 > 2, "true");
return true;
}
}
"#,
)
.unwrap();

prj.add_source(
"AContractTest.sol",
r#"
import "./test.sol";
import {AContract} from "./AContract.sol";

contract AContractTest is DSTest {
function testA() external {
AContract a = new AContract();
bool result = a.checkA();
assertTrue(result);
}
}
"#,
)
.unwrap();

// Assert 100% coverage (assert and require properly covered).
cmd.arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(str![[r#"
...
| File | % Lines | % Statements | % Branches | % Funcs |
|-------------------|---------------|---------------|---------------|---------------|
| src/AContract.sol | 100.00% (3/3) | 100.00% (3/3) | 100.00% (0/0) | 100.00% (1/1) |
| Total | 100.00% (3/3) | 100.00% (3/3) | 100.00% (0/0) | 100.00% (1/1) |

"#]]);
});
Loading