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

ignore the assert statement's conditional branch. (#1378) #1379

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

morganga
Copy link

@morganga morganga commented Oct 21, 2022

assert statement coverage should be considered complete if the assert statement was visited. There's no need to track multiple branches at the assert statement.

This is in agreement with the previously reported issue #324, which suggested "They should not be treated as code instructions."

I've extended the ignored instructions to include the assert statement check. This will make coverage complete with or without assertions enabled.

@morganga
Copy link
Author

before fix:
image
after fix:
image

@morganga
Copy link
Author

morganga commented Oct 21, 2022

An alternative solution to AssertFilter: 62

output.ignore(start, ((JumpInsnNode) cursor).label);

results in this report, where they're not even considered coverage lines:
image

@ice1000
Copy link
Contributor

ice1000 commented Jan 19, 2023

I love that

@Cybermite
Copy link

Going to bump this and see if we can get it looked at and merged?

This has been the main reason why our team hasn't migrated off of clover.

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.

None yet

4 participants