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

Preserve empty class and sourcefile nodes in XML report #817

Merged
merged 24 commits into from
Jan 18, 2019
Merged

Conversation

Godin
Copy link
Member

@Godin Godin commented Jan 6, 2019

Fixes #806

Our own XML report before this change is 915700 bytes, after is 920811 - increase is just 0.6%.

@Godin Godin self-assigned this Jan 6, 2019
@Godin Godin added this to Implementation in Current work items via automation Jan 6, 2019
@marchof
Copy link
Member

marchof commented Jan 9, 2019

As we have this snippet now all over the place:

coverage.getInstructionCounter().getTotalCount() == 0

I would like to propose to add a new method to ICoverageNode:

/**
 * Checks whether this is an empty node.
 * 
 * @return <code>true</code> is this node does not contain any instructions
 */
boolean isEmpty();

@Godin
Copy link
Member Author

Godin commented Jan 18, 2019

@marchof added

@Godin Godin requested a review from marchof January 18, 2019 15:10
@Godin Godin moved this from Implementation to Review in Current work items Jan 18, 2019
@marchof
Copy link
Member

marchof commented Jan 18, 2019

@Godin Thx for adding isEmpty(). I found three more candidates for using it:

  • org.jacoco.core.internal.analysis.ClassAnalyzer.addMethodCoverage(String, String, String, InstructionsBuilder, MethodNode)
  • org.jacoco.ant.ReportTask.checkForMissingDebugInformation(ICoverageNode)
  • org.jacoco.maven.ReportSupport.logBundleInfo(IBundleCoverage, Collection<IClassCoverage>)

Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will add more isEmpty() use cases in a separate PR.

@Godin
Copy link
Member Author

Godin commented Jan 18, 2019

@marchof thanks for spotting this - made changes, hopefully correctly 😉

@marchof marchof merged commit 13f29eb into master Jan 18, 2019
Current work items automation moved this from Review to Done Jan 18, 2019
@marchof marchof deleted the issue-806 branch January 18, 2019 17:26
@jacoco jacoco locked as resolved and limited conversation to collaborators May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

Preserve empty IClassCoverage nodes in CoverageBuilder and in XML report
2 participants