fix(coverage): proper instruction for 1st branch anchor (foundry-rs#8512

)
CompassLabs · Sep 11, 2024 · 880399b · 880399b
1 parent 967d0ca
commit 880399b
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 23 deletions.
diff --git a/crates/evm/coverage/src/analysis.rs b/crates/evm/coverage/src/analysis.rs
@@ -417,7 +417,7 @@ impl<'a> ContractVisitor<'a> {
  if let Some(NodeType::Identifier) = expr.as_ref().map(|expr| &expr.node_type) {
  // Might be a require call, add branch coverage.
  let name: Option<String> = expr.and_then(|expr| expr.attribute("name"));
- if let Some("require") = name.as_deref() {
+ if let Some("require" | "assert") = name.as_deref() {
  let branch_id = self.branch_id;
  self.branch_id += 1;
  self.push_item(CoverageItem {

diff --git a/crates/evm/coverage/src/anchors.rs b/crates/evm/coverage/src/anchors.rs
@@ -149,7 +149,7 @@ pub fn find_anchor_branch(
  ItemAnchor {
  item_id,
  // The first branch is the opcode directly after JUMPI
- instruction: pc + 1,
+ instruction: pc + 2,
  },
  ItemAnchor { item_id, instruction: pc_jump },
  ));

diff --git a/crates/forge/tests/cli/coverage.rs b/crates/forge/tests/cli/coverage.rs
@@ -173,8 +173,8 @@ forgetest!(test_assert_coverage, |prj, cmd| {
  "AContract.sol",
  r#"
 contract AContract {
- function checkA() external pure returns (bool) {
- assert(10 > 2);
+ function checkA(uint256 a) external pure returns (bool) {
+ assert(a > 2);
  return true;
  }
 }
@@ -188,26 +188,66 @@ contract AContract {
 import "./test.sol";
 import {AContract} from "./AContract.sol";
 
+interface Vm {
+ function expectRevert() external;
+}
+
 contract AContractTest is DSTest {
- function testA() external {
+ Vm constant vm = Vm(HEVM_ADDRESS);
+ function testAssertBranch() external {
  AContract a = new AContract();
- bool result = a.checkA();
+ bool result = a.checkA(10);
  assertTrue(result);
  }
+
+ function testAssertRevertBranch() external {
+ AContract a = new AContract();
+ vm.expectRevert();
+ a.checkA(1);
+ }
 }
  "#,
  )
  .unwrap();
 
+ // Assert 50% branch coverage for assert failure.
+ cmd.arg("coverage")
+ .args(["--mt".to_string(), "testAssertRevertBranch".to_string()])
+ .assert_success()
+ .stdout_eq(str![[r#"
+...
+| File | % Lines | % Statements | % Branches | % Funcs |
+|-------------------|--------------|--------------|--------------|---------------|
+| src/AContract.sol | 50.00% (1/2) | 50.00% (1/2) | 50.00% (1/2) | 100.00% (1/1) |
+| Total | 50.00% (1/2) | 50.00% (1/2) | 50.00% (1/2) | 100.00% (1/1) |
+
+"#]]);
+
+ // Assert 50% branch coverage for proper assert.
+ cmd.forge_fuse()
+ .arg("coverage")
+ .args(["--mt".to_string(), "testAssertBranch".to_string()])
+ .assert_success()
+ .stdout_eq(str![[r#"
+...
+| File | % Lines | % Statements | % Branches | % Funcs |
+|-------------------|---------------|---------------|--------------|---------------|
+| src/AContract.sol | 100.00% (2/2) | 100.00% (2/2) | 50.00% (1/2) | 100.00% (1/1) |
+| Total | 100.00% (2/2) | 100.00% (2/2) | 50.00% (1/2) | 100.00% (1/1) |
+
+"#]]);
+
  // Assert 100% coverage (assert properly covered).
- cmd.arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(str![[r#"
+ cmd.forge_fuse().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% (1/1) |
-| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (0/0) | 100.00% (1/1) |
+| src/AContract.sol | 100.00% (2/2) | 100.00% (2/2) | 100.00% (2/2) | 100.00% (1/1) |
+| Total | 100.00% (2/2) | 100.00% (2/2) | 100.00% (2/2) | 100.00% (1/1) |
 
-"#]]);
+"#]],
+ );
 });
 
 forgetest!(test_require_coverage, |prj, cmd| {
@@ -262,6 +302,20 @@ contract AContractTest is DSTest {
 | src/AContract.sol | 100.00% (1/1) | 100.00% (1/1) | 50.00% (1/2) | 100.00% (1/1) |
 | Total | 100.00% (1/1) | 100.00% (1/1) | 50.00% (1/2) | 100.00% (1/1) |
 
+"#]]);
+
+ // Assert 50% branch coverage if only happy path tested.
+ cmd.forge_fuse()
+ .arg("coverage")
+ .args(["--mt".to_string(), "testRequireNoRevert".to_string()])
+ .assert_success()
+ .stdout_eq(str![[r#"
+...
+| File | % Lines | % Statements | % Branches | % Funcs |
+|-------------------|---------------|---------------|--------------|---------------|
+| src/AContract.sol | 100.00% (1/1) | 100.00% (1/1) | 50.00% (1/2) | 100.00% (1/1) |
+| Total | 100.00% (1/1) | 100.00% (1/1) | 50.00% (1/2) | 100.00% (1/1) |
+
 "#]]);
 
  // Assert 100% branch coverage.
@@ -384,7 +438,7 @@ contract Foo {
 
  function foo(uint256 a) external pure {
  if (a < 10) {
- if (a == 1) {
+ if (a < 3) {
  assert(a == 1);
  } else {
  assert(a == 5);
@@ -423,16 +477,23 @@ import {Foo} from "./Foo.sol";
 
 interface Vm {
  function expectRevert(bytes calldata revertData) external;
+ function expectRevert() external;
 }
 
 contract FooTest is DSTest {
  Vm constant vm = Vm(HEVM_ADDRESS);
  Foo internal foo = new Foo();
 
- function test_issue_7784() external view {
+ function test_issue_7784() external {
  foo.foo(1);
+ vm.expectRevert();
+ foo.foo(2);
+ vm.expectRevert();
+ foo.foo(4);
  foo.foo(5);
  foo.foo(60);
+ vm.expectRevert();
+ foo.foo(70);
  }
 
  function test_issue_4310() external {
@@ -506,13 +567,16 @@ contract FooTest is DSTest {
  )
  .unwrap();
 
- // Assert 100% coverage.
+ // TODO: fix following issues for 100% coverage
+ // https://github.com/foundry-rs/foundry/issues/4309
+ // https://github.com/foundry-rs/foundry/issues/4310
+ // https://github.com/foundry-rs/foundry/issues/4315
  cmd.arg("coverage").args(["--summary".to_string()]).assert_success().stdout_eq(str![[r#"
 ...
-| File | % Lines | % Statements | % Branches  | % Funcs |
-|-------------|-----------------|-----------------|-----------------|---------------|
-| src/Foo.sol | 100.00% (20/20) | 100.00% (23/23) | 100.00% (12/12) | 100.00% (7/7) |
-| Total | 100.00% (20/20) | 100.00% (23/23) | 100.00% (12/12) | 100.00% (7/7) |
+| File | % Lines | % Statements | % Branches | % Funcs |
+|-------------|-----------------|-----------------|----------------|---------------|
+| src/Foo.sol | 100.00% (20/20) | 100.00% (23/23) | 83.33% (15/18) | 100.00% (7/7) |
+| Total | 100.00% (20/20) | 100.00% (23/23) | 83.33% (15/18) | 100.00% (7/7) |
 
 "#]]);
 });
@@ -681,10 +745,10 @@ contract FooTest is DSTest {
  cmd.arg("coverage").args(["--mt".to_string(), "happy".to_string()]).assert_success().stdout_eq(
  str![[r#"
 ...
-| File | % Lines | % Statements | % Branches  | % Funcs |
-|-------------|----------------|----------------|---------------|---------------|
-| src/Foo.sol | 66.67% (10/15) | 66.67% (14/21) | 100.00% (4/4) | 100.00% (5/5) |
-| Total | 66.67% (10/15) | 66.67% (14/21) | 100.00% (4/4) | 100.00% (5/5) |
+| File | % Lines | % Statements | % Branches | % Funcs |
+|-------------|----------------|----------------|--------------|---------------|
+| src/Foo.sol | 66.67% (10/15) | 66.67% (14/21) | 83.33% (5/6) | 100.00% (5/5) |
+| Total | 66.67% (10/15) | 66.67% (14/21) | 83.33% (5/6) | 100.00% (5/5) |
 
 "#]],
  );
@@ -695,8 +759,8 @@ contract FooTest is DSTest {
 ...
 | File | % Lines | % Statements | % Branches | % Funcs |
 |-------------|-----------------|-----------------|---------------|---------------|
-| src/Foo.sol | 100.00% (15/15) | 100.00% (21/21) | 100.00% (4/4) | 100.00% (5/5) |
-| Total | 100.00% (15/15) | 100.00% (21/21) | 100.00% (4/4) | 100.00% (5/5) |
+| src/Foo.sol | 100.00% (15/15) | 100.00% (21/21) | 100.00% (6/6) | 100.00% (5/5) |
+| Total | 100.00% (15/15) | 100.00% (21/21) | 100.00% (6/6) | 100.00% (5/5) |
 
 "#]],
  );