forge coverage does not work for if statements within for loops

[KittyFu307]

Component

Forge

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

• Foundry
• Foundryup

What version of Foundry are you on?

forge 0.2.0 (70f4fb5 2022-11-03T00:06:25.653195Z)

What command(s) is the bug in?

forge coverage

Operating System

macOS (Apple Silicon)

Describe the bug

When I run forge coverage to test the coverage for my code, I found that I have written the code to match the all branches but the result seems not. If I call the external function, it will not cover this branch.

The coverage result is

But I think my test function have covered all situations, and the result is not

[chrismin]

I'm unfortunately seeing the same issue. It seems to be related to coverage of "if" statements. Is there any update on this bug?

[KittyFu307]

I'm unfortunately seeing the same issue. It seems to be related to coverage of "if" statements. Is there any update on this bug?

unfortunately not, maybe you can @their-developers in this issue to see if there any updates

[PaulRBerg]

Just bumped into this, as well:

It looks like the common denominator between my case and @KittyFu307's is the presence of an if statement in a for loop.

Therefore, the title of this issue should be changed to something like "forge coverage does not work for if statements within for loops".

[KittyFu307]

Hi @rkrasiuk any update for this issue? Thank you for your time

[PaulRBerg]

@KittyFu307 it may be useful to rename the title of this issue to include a reference to for loops so that other users who bump into the same problem can reach us here.

[KittyFu307]

@KittyFu307 it may be useful to rename the title of this issue to include a reference to for loops so that other users who bump into the same problem can reach us here.

Sure~ have already done

[PaulRBerg]

Actually, as per the code shared by @KittyFu307 in the OP, and per @chrismin's comment, this issue may be about if statements more broadly, not just some cases within for loops.

I've recently bumped into another nasty bug in the coverage command, which may be related (though this is based on a gut feeling):

#4309

[PaulRBerg]

I can't manage to pin down what this bug is about, exactly.

#3497 may also be related.

[PaulRBerg]

Update: I have managed to get to the bottom of this, opened an issue here:

#4310

@KittyFu307 this is definitely the same issue you encountered in your loopChoose function, though the issue you had in branchChoose may be something completely unrelated.

[PaulRBerg]

I think that I have figured it out - @KittyFu307, your original issue was actually reporting on two bugs in Foundry. One I documented myself in #4310; the other has been documented and discussed in #3497.

If someone else could confirm my guess, then I suggest we close this issue in favor of #4310 and #3497, which are more targeted.

[KittyFu307]

I think that I have figured it out - @KittyFu307, your original issue was actually reporting on two bugs in Foundry. One I documented myself in #4310; the other has been documented and discussed in #3497.

If someone else could confirm my guess, then I suggest we close this issue in favor of #4310 and #3497, which are more targeted.

Oh @PaulRBerg Thank you for taking the time to debug this error. I am pretty sure that the problem of the loopChoose function is the same as that of #4310. This has troubled me for a long time. I hope foundry team can solve it.

[PaulRBerg]

No worries, @KittyFu307. Pinging @onbjerg in case he could take a quick look? Seems like an easy fix.

[onbjerg]

I am completely out of the loop on Foundry so I do not have the context necessary to figure out if this is an easy fix or not. My best guess is that the common denominator for all of these complexly linked issues is that we detect branches by detecting sequences of bytecode, and it is possible that detection is not strict enough for overlapping cases.

For the other isssues (array access, calls) there are hidden reverts, so essentially all of the issues boil down to this one, as they can be thought of as an if-statement with a revert in the else clause

[PaulRBerg]

Thank you for taking the time to respond, @onbjerg.

For the other isssues (array access, calls) there are hidden reverts

There are cases when there are clearly no hidden reverts, e.g. the one I documented in #4310:

function isOdd(uint256 number) external view returns (bool odd) {
    uint256[] memory arr = new uint256[](1);
    arr[0] = block.timestamp;

    if (number % 2 == 1) {
        odd = true;
        arr[0];
    } else {
        odd = false;
    }
}

The arr array is initialized with a size of 1, so arr[0] cannot revert in this function.

Regarding calls - it may be the case that a hidden revert is what generates partial coverage for certain calls. However, as I suggested here, there should definitely be a way to opt-out of the default revert-covering requirements with a comment (e.g. // forgecov: ignore-next-line-revert).

[mds1]

That isOdd function is not payable, so there is a revert in the case where msg.value > 0. Not sure if that's the issue, but just mentioning since there is in fact a hidden revert there. One tip is using forge inspect MyContract ir-optimized and checking out what solidity generates to see what it's doing under the hood (you may already know this)

[PaulRBerg]

Interesting point, @mds1, but unfortunately, no, this wasn't the issue. After adding the payable modifier, the 50% branch coverage stayed put:

| File        | % Lines       | % Statements  | % Branches   | % Funcs       |
|-------------|---------------|---------------|--------------|---------------|
| src/Foo.sol | 100.00% (5/5) | 100.00% (6/6) | 50.00% (1/2) | 100.00% (1/1) |
| Total       | 100.00% (5/5) | 100.00% (6/6) | 50.00% (1/2) | 100.00% (1/1) |

However, if Forge did actually report partial coverage for non-payable functions, that would be a bad user experience IMO.

1. This is not something obvious, not at all.
2. Developers are interested in testing the business logic of their protocol - not that a standard compiler feature works.

[deboguer-jng]

I've same issues on our project. @PaulRBerg any update on this issue?

[PaulRBerg]

No, @deboguer-jng. Also, I have never been involved in the development of the forge coverage command - I am just a user like you.

[grandizzy]

confirmed fixed with #8414 please reopen if you hit any similar issue