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

Coverage Check Re-Design #106

Merged
merged 5 commits into from May 26, 2013

Conversation

Projects
None yet
3 participants
@marchof
Member

marchof commented May 23, 2013

Goal of the re-design is to provide a common implementation for coverage checks and fulfill the following feature requests:

  • Ant support for coverage checks (#48)
  • Checks on any element level (#79)
New coverage check implementation.
New report APIs to check coverage, support for coverage checks in Ant,
rework Maven check goal implementation based on new APIs.
@muminc

This comment has been minimized.

Show comment
Hide comment
@muminc

muminc May 24, 2013

Contributor

I built this locally (had to do 2 minor renames to get maven tests to pass - see code comments).

I went to an old existing project (that was using cobertura for the check) and used jacoco instead and it worked really well!

Thanks.

One thing that wasn't obvious at first.

If I define rule like this:

 <rule>
  <element>CLASS</element>
  <limits>                                       
      <limit>
          <counter>BRANCH</counter>
          <value>COVEREDRATIO</value>
          <minimum>0.8</minimum>
      </limit>
  </limits>
</rule>

Warning are outputted to 1 d.p. (I wanted figures in 2 d.p.)

[WARNING] Rule violated for class xxx.xxxxx.xxxxxx: branches covered ratio is 0.7, but expected minimum is 0.8

So I had to do this to get the result I wanted

<rule>
  <element>CLASS</element>
  <limits>                                       
      <limit>
          <counter>BRANCH</counter>
          <value>COVEREDRATIO</value>
          <minimum>0.80</minimum>
      </limit>
  </limits>
</rule>

[WARNING] Rule violated for class xxx.xxxxx.xxxxxx: branches covered ratio is 0.78, but expected minimum is 0.80

Contributor

muminc commented May 24, 2013

I built this locally (had to do 2 minor renames to get maven tests to pass - see code comments).

I went to an old existing project (that was using cobertura for the check) and used jacoco instead and it worked really well!

Thanks.

One thing that wasn't obvious at first.

If I define rule like this:

 <rule>
  <element>CLASS</element>
  <limits>                                       
      <limit>
          <counter>BRANCH</counter>
          <value>COVEREDRATIO</value>
          <minimum>0.8</minimum>
      </limit>
  </limits>
</rule>

Warning are outputted to 1 d.p. (I wanted figures in 2 d.p.)

[WARNING] Rule violated for class xxx.xxxxx.xxxxxx: branches covered ratio is 0.7, but expected minimum is 0.8

So I had to do this to get the result I wanted

<rule>
  <element>CLASS</element>
  <limits>                                       
      <limit>
          <counter>BRANCH</counter>
          <value>COVEREDRATIO</value>
          <minimum>0.80</minimum>
      </limit>
  </limits>
</rule>

[WARNING] Rule violated for class xxx.xxxxx.xxxxxx: branches covered ratio is 0.78, but expected minimum is 0.80

@muminc

This comment has been minimized.

Show comment
Hide comment
@muminc

muminc May 24, 2013

Contributor

This, more about personal taste...

The upper-casing does feel a bit out of place in maven pom files, my preference is something like

<rule>
  <element>class</element>
  <limits>                                       
      <limit>
          <counter>branch</counter>
          <value>covered-ratio</value>
          <minimum>0.80</minimum>
      </limit>
  </limits>
</rule>
Contributor

muminc commented May 24, 2013

This, more about personal taste...

The upper-casing does feel a bit out of place in maven pom files, my preference is something like

<rule>
  <element>class</element>
  <limits>                                       
      <limit>
          <counter>branch</counter>
          <value>covered-ratio</value>
          <minimum>0.80</minimum>
      </limit>
  </limits>
</rule>
@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof May 24, 2013

Member

Uppercase attributes: These are the Java enum constant names. We could also wrap Limit objects and to provide a Maven specific syntax. Overloading the methods does not work as we've seen before.

I would actually prefer consistency here with the API.

Member

marchof commented May 24, 2013

Uppercase attributes: These are the Java enum constant names. We could also wrap Limit objects and to provide a Maven specific syntax. Overloading the methods does not work as we've seen before.

I would actually prefer consistency here with the API.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof May 24, 2013

Member

I can add documentation about decimals.

Member

marchof commented May 24, 2013

I can add documentation about decimals.

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive commented May 24, 2013

Java Code Coverage Tools » jacoco #66 SUCCESS
This pull request looks good
(what's this?)

@muminc

This comment has been minimized.

Show comment
Hide comment
@muminc

muminc May 24, 2013

Contributor

Updates look good, no build failures on my local build.

Thanks.

Contributor

muminc commented May 24, 2013

Updates look good, no build failures on my local build.

Thanks.

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive commented May 25, 2013

Java Code Coverage Tools » jacoco #67 SUCCESS
This pull request looks good
(what's this?)

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive commented May 26, 2013

Java Code Coverage Tools » jacoco #68 SUCCESS
This pull request looks good
(what's this?)

marchof added a commit that referenced this pull request May 26, 2013

Merge pull request #106 from jacoco/issue-106
Coverage check redesign incl. Ant support.

@marchof marchof merged commit 65d0700 into master May 26, 2013

1 check passed

default The Travis CI build passed
Details

@marchof marchof deleted the issue-106 branch May 26, 2013

@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.