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.

Sign up for GitHub

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

Jump to bottom

feat(forge): disable coverage for assertions #4294

Closed
Tracked by #4442
PaulRBerg opened this issue Feb 7, 2023 · 4 comments · Fixed by #8413
Closed
Tracked by #4442

feat(forge): disable coverage for assertions #4294

PaulRBerg opened this issue Feb 7, 2023 · 4 comments · Fixed by #8413
Labels
C-forge Command: forge Cmd-forge-coverage Command: forge coverage T-feature Type: feature

Comments

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Feb 7, 2023 •
edited
Loading

Component

Forge

Describe the feature you would like

Assertions are never meant to fail unless there is a bug, but Forge currently reports partial coverage for assert statements:

Partial coverage for assertions

Related issue in the solidity-coverage repo: sc-forks/solidity-coverage#269.
@PaulRBerg PaulRBerg added the T-feature Type: feature label Feb 7, 2023
This was referenced Feb 9, 2023
Closed
Closed
@KholdStare
Copy link
Contributor

@mattsse @gakonst Any plans to fix this? At the moment whenever auditors run coverage on our code we always have to give the caveat that coverage is under-reported because of assert statements.

Thank you.

@PaulRBerg
Copy link
Contributor Author

Also if FREI-PI picks up steam, assertions' coverage will become an even bigger problem.

@zerosnacks zerosnacks mentioned this issue Jun 26, 2024
Open
11 tasks
@zerosnacks zerosnacks added Cmd-forge-coverage Command: forge coverage C-forge Command: forge labels Jun 28, 2024
@grandizzy grandizzy mentioned this issue Jul 11, 2024
Merged
@KholdStare
Copy link
Contributor

KholdStare commented Jul 11, 2024 •
edited
Loading

I think this is not fixed in spirit yet. require should still be a branch. Linking to my comment from the PR: #8413 (comment)

@grandizzy
Copy link
Collaborator

I think this is not fixed in spirit yet. require should still be a branch. Linking to my comment from the PR: #8413 (comment)

Thank you, will check!

@grandizzy grandizzy mentioned this issue Jul 12, 2024
Closed
2 tasks
@grandizzy grandizzy mentioned this issue Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-forge Command: forge Cmd-forge-coverage Command: forge coverage T-feature Type: feature
Projects
Foundry
Status: Completed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(coverage): treat assert/require as lines instead branch grandizzy/foundry
4 participants
@KholdStare @PaulRBerg @grandizzy @zerosnacks