foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
7.89k stars 1.58k forks source link

forge coverage report does not show modifiers that revert #8347

Closed joaobrunoah closed 12 hours ago

joaobrunoah commented 5 days ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.2.0 (7226aa06b 2024-07-02T23:13:43.699201000Z)

What command(s) is the bug in?

forge coverage

Operating System

macOS (Apple Silicon)

Describe the bug

I have the following contract and test:

Contract A:

pragma solidity ^0.8.24;

contract ContractA {
    error CustomError();

    modifier withValidNumber(uint256 number) {
        if (number > 1e18) {
             revert CustomError(); // this line is never hit by `forge coverage`
        }
        _;
    }

    function mockWithValidNumber(uint256 number) external view withValidNumber(number) {
        // Do Nothing
    }
}

Test of Contract A

pragma solidity ^0.8.24;

import {ContractA} from "ContractA.sol";

contract ContractATest is Test{
    function testInvalidNumber() public {
        ContractA contract = new ContractA();
        vm.expectRevert(ContractA.CustomError.selector);
        contract.mockWithValidNumber(1.5e18);
    }
}

The test testInvalidNumber passes, but in the forge coverage the line with revert CustomError(); is never hit.

grandizzy commented 5 days ago

hm, I cannot reproduce the issue, see below

image

I used

forge coverage --mt testInvalidNumber --report lcov
genhtml lcov.info -o report --branch-coverage && firefox report/index.html

with a slightly changed test (imported ds-test and renamed contract var)

pragma solidity ^0.8.24;

import {Test} from "forge-std/Test.sol";

import {ContractA} from "src/ContractA.sol";

contract ContractATest is Test {
    function testInvalidNumber() public {
        ContractA a = new ContractA();
        vm.expectRevert(ContractA.CustomError.selector);
        a.mockWithValidNumber(1.5e18);
    }
}

Are you sure forge coverage --mt testInvalidNumber --report lcov does not yield any test failure?

grandizzy commented 12 hours ago

ping @joaobrunoah can you pls check comment above? thanks!

joaobrunoah commented 12 hours ago

@grandizzy, you're correct. I tested it here, and it works with ContractA. I have a scenario in which it does not work, but I couldn't isolate the reason. It may be related to my code somehow.

For now, I think the best alternative is to close this issue. If I find the reason and it's a forge bug, I can reopen this or open another ticket.