foundry icon indicating copy to clipboard operation
foundry copied to clipboard

No coverage report inside `do...while` loops

Open beeb opened this issue 1 year ago • 4 comments

Component

Forge

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

  • [X] Foundry
  • [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (caef136 2024-01-29T00:20:25.410303708Z)

What command(s) is the bug in?

forge coverage --report lcov

Operating System

Linux

Describe the bug

The coverage report generated by foundry does not include any coverage information inside do...while loops.

Repro: https://github.com/beeb/repro_foundry_coverage

Report below was generated with lcov's genhtml using the report generated by forge.

image

beeb avatar Jan 31 '24 07:01 beeb

I did a little bit of digging for this one, my first guess was an error with the visitor implementation but it was not the case, it looks like statements are omitted while generating lcov reports. I am not sure about the reason but there is comment that says statement's are not compatible with lcov. I am not very familiar with lcov so not sure about the next statement but, I feel like adding DA to lcov report for statements should work. https://github.com/foundry-rs/foundry/blob/e897bd6874312d2c2d9acc7d24081ef721845b29/crates/forge/src/coverage.rs#L119-L120

By changing it to emit DA values I obtained following coverage report for the project reported:

image

I have the change, produced this coverage report here on my fork: https://github.com/kayagokalp/foundry/tree/kayagokalp/add-statements-to-coverage

I can open a PR but if we have some context on why the comment said statements are not compatible, that would be great. I don't want to introduce a regression.

kayagokalp avatar Feb 10 '24 20:02 kayagokalp

Thanks a lot for looking into this! The fixed example you showed looks great, hopefully someone can confirm it wouldn't cause problems elsewhere.

beeb avatar Feb 11 '24 09:02 beeb

@onbjerg since git blame points at you maybe you can pitch in? Thanks ^^

beeb avatar Feb 13 '24 15:02 beeb

we can add DA to lcov but note that expressions and statements are two different constructs

please open a PR - coverage does not cover everything because we had to reconstruct the AST data structure ourselves since it is undocumented in solidity, and even worse, changes over different versions.

onbjerg avatar Feb 13 '24 16:02 onbjerg