Skip to content

Replace empty table in HTML report by messages #833

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
Merged
merged 7 commits into from
Jan 21, 2019
Merged

Replace empty table in HTML report by messages #833

merged 7 commits into from
Jan 21, 2019

Conversation

Godin
Copy link
Member

@Godin Godin commented Jan 19, 2019 •
edited
Loading

Using Ant Task and therefore Gradle Plugin, that is based on it, possible to generate report without classes.

For example in case of following build.xml

<project xmlns:jacoco="antlib:org.jacoco.ant" default="build">
  <taskdef uri="antlib:org.jacoco.ant" resource="org/jacoco/ant/antlib.xml">
    <classpath path="jacoco/lib/jacocoant.jar"/>
  </taskdef>

  <target name="build">
    <jacoco:report>
      <structure name="example"/>
      <html destdir="report"/>
    </jacoco:report>
  </target>
</project>

and build.gradle

apply plugin: 'base'
apply plugin: 'jacoco'

repositories {
  mavenCentral()
  mavenLocal()
}

task report(type:JacocoReport) {
  executionData.from(files(file('jacoco.exec')))
}

execution of

$ ant
Buildfile: /private/tmp/example/build.xml

build:
[jacoco:report] Writing bundle 'example' with 0 classes

BUILD SUCCESSFUL
Total time: 0 seconds

$ ant -silent
Buildfile: /private/tmp/example/build.xml

$ gradle-5.0 clean report

BUILD SUCCESSFUL in 1s
2 actionable tasks: 1 executed, 1 up-to-date

will produce

empty

Note the absence of messages in the second and third cases.

Also unfortunately many users don't provide screenshots and the way they describe such report is ambiguous: "empty report" and "zero(s)" - in both cases some indeed refer to such report, some other refer to report with classes where covered counters are zero.

Could also be noted that produced index.html claims to be XHTML 1.0 Strict whereas this is not the case because of <tbody/>.

So I propose to replace empty table by messages for two cases thanks to #817

if (bundle.getPackages().isEmpty()) {
  ... // No class files specified.
} else if (bundle.isEmpty()) {
  ... // None of the analyzed classes contain code relevant for code coverage.
} else {
  ...
}

@Godin Godin added this to the 0.8.3 milestone Jan 19, 2019
@Godin Godin self-assigned this Jan 19, 2019
@Godin Godin requested a review from marchof January 19, 2019 04:52
@marchof
Copy link
Member

marchof commented Jan 20, 2019

@Godin I'm not so sure about this message:

No classes to display - all analyzed classes are abstract or generated code.

Strictly speaking abstract classes will at least have a constructor (which might only be filtered if it is empty and private) also we have other filters also for non generated code. So I'm not sure whether this is correct right now or might become wrong when adding more filters in future. Therefore I would provide a less specific message, like

None of the analyzed classes contain code relevant for code coverage.

@Godin
Copy link
Member Author

Godin commented Jan 21, 2019

@marchof I was looking for neutral message and your proposal looks great!

@marchof
Copy link
Member

marchof commented Jan 21, 2019

@Godin Just to make sure: same applies for change log entry.

@Godin
Copy link
Member Author

Godin commented Jan 21, 2019

@marchof sure. And done.

@marchof marchof merged commit b7441f2 into master Jan 21, 2019
@marchof marchof deleted the issue-833 branch January 21, 2019 18:21
@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.
Reviewers

@marchof marchof marchof approved these changes

Assignees

@Godin Godin

Labels
component: report type: enhancement
Projects
Current work items
Archived in project
Milestone
0.8.3
Development

Successfully merging this pull request may close these issues.
2 participants
@Godin @marchof