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.

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

No coverage report inside do...while loops #6968

Closed
2 tasks done
beeb opened this issue Jan 31, 2024 · 4 comments · Fixed by #7146
Closed
2 tasks done

No coverage report inside do...while loops #6968

beeb opened this issue Jan 31, 2024 · 4 comments · Fixed by #7146
Labels
T-bug Type: bug

Comments

@beeb
Copy link
Contributor

beeb commented Jan 31, 2024

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 (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 beeb added the T-bug Type: bug label Jan 31, 2024
@beeb beeb changed the title No coverage report is do...while loops No coverage report inside do...while loops Jan 31, 2024
@gakonst gakonst added this to Foundry Jan 31, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Jan 31, 2024
@kayagokalp
Copy link
Contributor

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.

// Statements are not in the LCOV format
CoverageItemKind::Statement => (),

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.

@beeb
Copy link
Contributor Author

beeb commented Feb 11, 2024

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
Copy link
Contributor Author

beeb commented Feb 13, 2024

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

@onbjerg
Copy link
Member

onbjerg commented Feb 13, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants