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

fix: do not remove contract from coverage report if a function is missing statements or branches #1178

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

danhper
Copy link
Contributor

@danhper danhper commented Aug 2, 2021

What I did

The previous behavior expects all functions to have a report for both
statements and branches. If not, it removes the whole contract from the
coverage report.
While the assumption seems reasonable, some functions seem to only have
either one of statements or branches, ending up in removing contracts
that would otherwise contain useful coverage information.

This is related to #1087 and should at least be a partial fix to it

How I did it

Avoid excluding the contract when one of the functions in the contract was missing coverage information

How to verify it

Run the coverage report

Checklist

  • I have confirmed that my PR passes all linting checks
  • I have included test cases - slightly tedious to recreate this in test but could be doable if needed
  • I have updated the documentation - nothing to document here
  • I have added an entry to the changelog

@danhper danhper force-pushed the master branch 3 times, most recently from c6a4e35 to d20cc52 Compare August 16, 2021 11:39
…sing statements or branches

The previous behavior expects all functions to have a report for both
statements and branches. If not, it removes the whole contract from the
coverage report.
While the assumption seems reasonable, some functions seem to only have
either one of statements or branches, ending up in removing contracts
that would otherwise contain useful coverage information.

This is related to eth-brownie#1087 and should at least be a partial fix to it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants