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

>= 0.7.3 breaks Gradle + JUnit + Robolectric coverage (Android) #288

Merged
merged 1 commit into from Jan 13, 2016

Conversation

Projects
None yet
@marchof
Member

marchof commented Jan 12, 2016

Version info:
Gradle 2.2.1
Android Gradle plugin 1.0.1
Robolectric Gradle plugin 0.14.1
JUnit 4.10
Robolectric 2.4

Setting up a simple test using a configuration like:

apply plugin: 'jacoco'
jacoco.toolVersion = project.property('jacocoVersion')

and running my unit tests with:

$ ./gradlew -PjacocoVersion=0.7.1.+ clean :testDebug :jacocoTestDebugReport
...
$ ls -l build/jacoco/testDebug.exec
-rw-r--r--+ 1 jhansche  staff  278218 Feb 26 16:45 build/jacoco/testDebug.exec
$ strings build/jacoco/testDebug.exec | wc -l
    4325

This all works correctly, and I get a proper JaCoCo report generated. Changing nothing else but the toolVersion, however, breaks the execution data that is gathered:

$ ./gradlew -PjacocoVersion=0.7.4-SNAPSHOT clean :testDebug :jacocoTestDebugReport
...
$ ls -l build/jacoco/testDebug.exec
-rw-r--r--+ 1 jhansche  staff  36910 Feb 26 16:56 build/jacoco/testDebug.exec
$ strings build/jacoco/testDebug.exec | wc -l
     639

You can see that the execution data file is much smaller, and contains far fewer strings. Looking at the "Sessions" link in the top-right of the HTML report lists only one class from my application's package (the Android app's R$styleable class). The majority of the classes listed are in one of the org.gradle, org.junit, or org.robolectric packages. Using 0.7.1, it included all the android.** classes, and more importantly, the classes from my application.

However, when I set the test.jacoco.classDumpFile = file("${project.buildDir}/jacoco/dump") property, I do see most of my application's (tested) classes listed in that directory -- they just don't show up in the execution data file.

One other difference that I notice in the dump/ directory (for a single example test class):

0.7.1:
build/jacoco/dump//com/example/MyClass.class
build/jacoco/dump//com/example/MyClassTest.class

0.7.4:
build/jacoco/dump//com/example/MyClass.9b700a23ba643d32.class
build/jacoco/dump//com/example/MyClassTest.22115a9aa9d0d3ee.class

I'm in a difficult position as now I am forced to use two different versions of JaCoCo: 0.7.1 with Robolectric+JUnit to get coverage of unit tests on the local JVM; and 0.7.3+ with Calabash to get coverage of instrumentation tests running on the device's runtime -- I am forced to update to 0.7.3 in order to allow support for the Android ART runtime, as otherwise the instrumented classes cannot be installed on a Lollipop device.

What other information can I provide to help track down what is going wrong here? If it's not a bug in JaCoCo, then who is to blame? (I realize there is a very long list of potential candidates for where it should be fixed).

@Godin Godin added this to the 0.7.4 milestone Feb 27, 2015

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Feb 27, 2015

Member

Probably related with two issues here:

  • #286 is a regression in exec file format which will be fixed in 0.7.4 soon
  • #225 was a feature request to use unique file names, but this shouldn't make a difference here

Can you please re-run your tests with 0.7.2 and 0.7.4 (should be available this weekend)?

Member

marchof commented Feb 27, 2015

Probably related with two issues here:

  • #286 is a regression in exec file format which will be fixed in 0.7.4 soon
  • #225 was a feature request to use unique file names, but this shouldn't make a difference here

Can you please re-run your tests with 0.7.2 and 0.7.4 (should be available this weekend)?

@jhansche

This comment has been minimized.

Show comment
Hide comment
@jhansche

jhansche Feb 27, 2015

By 0.7.4 you mean the non-SNAPSHOT release? Or it will be a new SNAPSHOT later this weekend?

Checking on 0.7.2, it works correctly; similarly to 0.7.1.

Re #286, I saw that but didn't think it was related, since I'm not getting an IllegalStateException. But the reports on that ticket mention that it's already fixed; and my example above was using the 0.7.4-SNAPSHOT, so wouldn't that mean that the fix should have already been in what I was testing?

jhansche commented Feb 27, 2015

By 0.7.4 you mean the non-SNAPSHOT release? Or it will be a new SNAPSHOT later this weekend?

Checking on 0.7.2, it works correctly; similarly to 0.7.1.

Re #286, I saw that but didn't think it was related, since I'm not getting an IllegalStateException. But the reports on that ticket mention that it's already fixed; and my example above was using the 0.7.4-SNAPSHOT, so wouldn't that mean that the fix should have already been in what I was testing?

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Feb 27, 2015

Member

0.7.4 has also just been staged to the Maven repository. Can you please re-test on the release version?

Member

marchof commented Feb 27, 2015

0.7.4 has also just been staged to the Maven repository. Can you please re-test on the release version?

@jhansche

This comment has been minimized.

Show comment
Hide comment
@jhansche

jhansche Feb 27, 2015

Using 0.7.4.+, I got "0.7.4.201502262128" and it reports this size:

-rw-r--r--+ 1 jhansche  staff  58138 Feb 27 14:19 testDebug.exec

and the coverage report shows 0% coverage. So seems to be the same problem. Did I get the wrong version? Is there a specific revision I should try?

jhansche commented Feb 27, 2015

Using 0.7.4.+, I got "0.7.4.201502262128" and it reports this size:

-rw-r--r--+ 1 jhansche  staff  58138 Feb 27 14:19 testDebug.exec

and the coverage report shows 0% coverage. So seems to be the same problem. Did I get the wrong version? Is there a specific revision I should try?

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Feb 27, 2015

Member

@jhansche @marchof 0.7.4 has been already promoted to Maven Central and available for downloading, will be announced in few minutes.

Member

Godin commented Feb 27, 2015

@jhansche @marchof 0.7.4 has been already promoted to Maven Central and available for downloading, will be announced in few minutes.

@jhansche

This comment has been minimized.

Show comment
Hide comment
@jhansche

jhansche Feb 27, 2015

@Godin see above -- is that the correct version? 201502262128?

jhansche commented Feb 27, 2015

@Godin see above -- is that the correct version? 201502262128?

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Feb 27, 2015

Member

@jhansche yep, so can we get a reduced reproducer of your problem?

Member

Godin commented Feb 27, 2015

@jhansche yep, so can we get a reduced reproducer of your problem?

@jhansche

This comment has been minimized.

Show comment
Hide comment
@jhansche

jhansche Feb 27, 2015

Sure, I'll work on getting a simple test project set up to show it.

jhansche commented Feb 27, 2015

Sure, I'll work on getting a simple test project set up to show it.

jhansche added a commit to MeetMe/jacoco-288 that referenced this pull request Feb 27, 2015

jhansche added a commit to MeetMe/jacoco-288 that referenced this pull request Feb 27, 2015

@jhansche

This comment has been minimized.

Show comment
Hide comment
@jhansche

jhansche Feb 27, 2015

@Godin @marchof Please see https://github.com/MeetMe/jacoco-288 for a reproducible test project. The README explains how to run the tests, check the .exec file, and open the jacoco report, for each version.

jhansche commented Feb 27, 2015

@Godin @marchof Please see https://github.com/MeetMe/jacoco-288 for a reproducible test project. The README explains how to run the tests, check the .exec file, and open the jacoco report, for each version.

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Feb 27, 2015

Member

@jhansche Cool! I'll check in few minutes, hoping that this might be something related to the build, or at least to provide rough overview of problem if this comes from bytecode, and then we'll wait our bytecode master @marchof to dig deeper

Member

Godin commented Feb 27, 2015

@jhansche Cool! I'll check in few minutes, hoping that this might be something related to the build, or at least to provide rough overview of problem if this comes from bytecode, and then we'll wait our bytecode master @marchof to dig deeper

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Feb 27, 2015

Member

@jhansche It took a while to setup Android SKD and correct version of it for this project. But I finally managed to do bisection of changes. @marchof The issue indeed comes from version 0.7.3 and relates to #272 - revert of bc47c02 solves this issue.

Member

Godin commented Feb 27, 2015

@jhansche It took a while to setup Android SKD and correct version of it for this project. But I finally managed to do bisection of changes. @marchof The issue indeed comes from version 0.7.3 and relates to #272 - revert of bc47c02 solves this issue.

@shaobos

This comment has been minimized.

Show comment
Hide comment
@shaobos

shaobos Apr 10, 2015

not sure if this is related, i have also observed test coverage drop when using powermock 1.6.0

shaobos commented Apr 10, 2015

not sure if this is related, i have also observed test coverage drop when using powermock 1.6.0

@marchof marchof modified the milestone: 0.7.5 May 20, 2015

@sneuberger-amazon

This comment has been minimized.

Show comment
Hide comment
@sneuberger-amazon

sneuberger-amazon Oct 1, 2015

Any workaround for this? The latest version of android's gradle plugin no longer works with jacoco 0.7.1, so I'm trying to find a workaround for version 0.7.3+.

sneuberger-amazon commented Oct 1, 2015

Any workaround for this? The latest version of android's gradle plugin no longer works with jacoco 0.7.1, so I'm trying to find a workaround for version 0.7.3+.

@brianguertin

This comment has been minimized.

Show comment
Hide comment
@brianguertin

brianguertin Oct 1, 2015

@sneuberger-amazon What version of the gradle plugin? I'm using jacoco 0.7.2 successfully with android gradle plugin 1.3.0

brianguertin commented Oct 1, 2015

@sneuberger-amazon What version of the gradle plugin? I'm using jacoco 0.7.2 successfully with android gradle plugin 1.3.0

@TommyDevAll

This comment has been minimized.

Show comment
Hide comment
@TommyDevAll

TommyDevAll Jan 13, 2016

sorry i mean 2.9.... with android plugin for gradle @1.5.0
i'm able to update to 0.7.5...

everybody here knows that jacoco is for coverage.... we are trying to help each other.... do you remember?

TommyDevAll commented Jan 13, 2016

sorry i mean 2.9.... with android plugin for gradle @1.5.0
i'm able to update to 0.7.5...

everybody here knows that jacoco is for coverage.... we are trying to help each other.... do you remember?

@jaredsburrows

This comment has been minimized.

Show comment
Hide comment
@jaredsburrows

jaredsburrows Jan 13, 2016

I am using gradle 2.10, Android plugin 1.5.0. So now you are saying 0.7.5
works? Did you do a gradle clean testDebug?
On Jan 13, 2016 2:44 AM, "Tommaso Resti" notifications@github.com wrote:

sorry i mean 2.9.... with android plugin for gradle @1.5.0
i'm able to update to 0.7.5...

everybody here know that jacoco is for coverage.... we are trying to help
each other.... do you remember?


Reply to this email directly or view it on GitHub
#288 (comment).

jaredsburrows commented Jan 13, 2016

I am using gradle 2.10, Android plugin 1.5.0. So now you are saying 0.7.5
works? Did you do a gradle clean testDebug?
On Jan 13, 2016 2:44 AM, "Tommaso Resti" notifications@github.com wrote:

sorry i mean 2.9.... with android plugin for gradle @1.5.0
i'm able to update to 0.7.5...

everybody here know that jacoco is for coverage.... we are trying to help
each other.... do you remember?


Reply to this email directly or view it on GitHub
#288 (comment).

@TommyDevAll

This comment has been minimized.

Show comment
Hide comment
@TommyDevAll

TommyDevAll Jan 13, 2016

i'm trying to figure out why you can't upgrade to 0.7.5. Did you faced some problem?
i never told you that jacoco 0.7.5 works for me... i told you that i'm able to upgrade to that version.

Now i'm trying to use the 0.7.6-SNAPSHOT with the new attribute inclNoLocationClasses=true but can't find the right place to add this new attribute.... is it clear?

TommyDevAll commented Jan 13, 2016

i'm trying to figure out why you can't upgrade to 0.7.5. Did you faced some problem?
i never told you that jacoco 0.7.5 works for me... i told you that i'm able to upgrade to that version.

Now i'm trying to use the 0.7.6-SNAPSHOT with the new attribute inclNoLocationClasses=true but can't find the right place to add this new attribute.... is it clear?

@jaredsburrows

This comment has been minimized.

Show comment
Hide comment
@jaredsburrows

jaredsburrows Jan 13, 2016

By saying that you "can upgrade", I would assume you mean that that version
works. But then you said, the coverage doesn't? To be clear, 0.7.5 coverage
does not work.
On Jan 13, 2016 2:51 AM, "Tommaso Resti" notifications@github.com wrote:

i'm trying to figure out why you can't upgrade to 0.7.5. Did you faced
some problem?
i never told you that jacoco 0.7.5 works for me... i told you that i'm
able to upgrade to that version.

Now i'm trying to use the 0.7.6-SNAPSHOT with the new attribute
inclNoLocationClasses=true but can't find the right place to add this new
attribute.... is it clear?


Reply to this email directly or view it on GitHub
#288 (comment).

jaredsburrows commented Jan 13, 2016

By saying that you "can upgrade", I would assume you mean that that version
works. But then you said, the coverage doesn't? To be clear, 0.7.5 coverage
does not work.
On Jan 13, 2016 2:51 AM, "Tommaso Resti" notifications@github.com wrote:

i'm trying to figure out why you can't upgrade to 0.7.5. Did you faced
some problem?
i never told you that jacoco 0.7.5 works for me... i told you that i'm
able to upgrade to that version.

Now i'm trying to use the 0.7.6-SNAPSHOT with the new attribute
inclNoLocationClasses=true but can't find the right place to add this new
attribute.... is it clear?


Reply to this email directly or view it on GitHub
#288 (comment).

@TommyDevAll

This comment has been minimized.

Show comment
Hide comment
@TommyDevAll

TommyDevAll Jan 13, 2016

jacoco 0.7.5 works but not for classes that use a RobolectricGradleTestRunner.
that's the problem.

"0.7.5 coverage does not work." it's not true

TommyDevAll commented Jan 13, 2016

jacoco 0.7.5 works but not for classes that use a RobolectricGradleTestRunner.
that's the problem.

"0.7.5 coverage does not work." it's not true

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jan 14, 2016

Member

@marchof FYI my colleague pointed out that there is an issue in combination JaCoCo(>= 0.7.2) + Gradle, even without Android and Robolectric, so I'll do end-to-end testing today.

Member

Godin commented Jan 14, 2016

@marchof FYI my colleague pointed out that there is an issue in combination JaCoCo(>= 0.7.2) + Gradle, even without Android and Robolectric, so I'll do end-to-end testing today.

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jan 14, 2016

Member

@marchof after investigations JaCoCo works just fine with Gradle, when something else not involved into process of execution of tests:

Hence I'm wondering what is better on JaCoCo side - enabled by default or disabled with PR for Gradle?

Member

Godin commented Jan 14, 2016

@marchof after investigations JaCoCo works just fine with Gradle, when something else not involved into process of execution of tests:

Hence I'm wondering what is better on JaCoCo side - enabled by default or disabled with PR for Gradle?

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Jan 14, 2016

Member

@Godin Many thanks for retesting!
I also think Gradle plugin needs to be updated. If we change the default we cause a regression for users of mocking libraries and lock them out from Gradle usage -- assuming Gradle is not only used for Android projects.

Member

marchof commented Jan 14, 2016

@Godin Many thanks for retesting!
I also think Gradle plugin needs to be updated. If we change the default we cause a regression for users of mocking libraries and lock them out from Gradle usage -- assuming Gradle is not only used for Android projects.

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jan 14, 2016

Member

@marchof ok, btw quick and dirty patch for Gradle, which allows to specify arbitrary options can be found in Godin/gradle@7e4f886

Member

Godin commented Jan 14, 2016

@marchof ok, btw quick and dirty patch for Gradle, which allows to specify arbitrary options can be found in Godin/gradle@7e4f886

@marchof marchof added this to the 0.7.6 milestone Jan 14, 2016

@emartynov

This comment has been minimized.

Show comment
Hide comment
@emartynov

emartynov Jan 21, 2016

Guys, when to expect 0.7.6 release? Any workaround with snapshot usage and some gradle snipet?

Because of jenkins plugin update I lost coverage to 0, after update jacoco plugin my coverage grew to 47%. But it was 92% before

emartynov commented Jan 21, 2016

Guys, when to expect 0.7.6 release? Any workaround with snapshot usage and some gradle snipet?

Because of jenkins plugin update I lost coverage to 0, after update jacoco plugin my coverage grew to 47%. But it was 92% before

@TommyDevAll

This comment has been minimized.

Show comment
Hide comment
@TommyDevAll

TommyDevAll Feb 8, 2016

@emartynov +1
In my case all the "non-robolectred" test classes was correctly analysed and included in the report but not the others that use a Robolectric test runner". Is it the same for you?

TommyDevAll commented Feb 8, 2016

@emartynov +1
In my case all the "non-robolectred" test classes was correctly analysed and included in the report but not the others that use a Robolectric test runner". Is it the same for you?

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Feb 19, 2016

Member

Note for Gradle users

JaCoCo 0.7.6 has been released, however

  • you need to wait for gradle/gradle#575
  • either without it find a way to pass new option (inclnolocationclasses) to agent
Member

Godin commented Feb 19, 2016

Note for Gradle users

JaCoCo 0.7.6 has been released, however

  • you need to wait for gradle/gradle#575
  • either without it find a way to pass new option (inclnolocationclasses) to agent
@jaredsburrows

This comment has been minimized.

Show comment
Hide comment
@jaredsburrows

jaredsburrows Feb 19, 2016

👍
@Godin Thanks for the update!

jaredsburrows commented Feb 19, 2016

👍
@Godin Thanks for the update!

@rliesenfeld

This comment has been minimized.

Show comment
Hide comment
@rliesenfeld

rliesenfeld Feb 26, 2016

If I am not mistaken, this problem has the same root cause as when using PowerMock, which is that the custom JUnit test runner creates copies of classes without preserving the original protection domain. I opened an issue in the PowerMock tracker for it: powermock/powermock#564

I believe the same applies to the Robolectric custom test runner, which does very much the same thing as PowerMock.

rliesenfeld commented Feb 26, 2016

If I am not mistaken, this problem has the same root cause as when using PowerMock, which is that the custom JUnit test runner creates copies of classes without preserving the original protection domain. I opened an issue in the PowerMock tracker for it: powermock/powermock#564

I believe the same applies to the Robolectric custom test runner, which does very much the same thing as PowerMock.

@dampcake

This comment has been minimized.

Show comment
Hide comment
@dampcake

dampcake Feb 28, 2016

Small Android sample with the changes here:
https://github.com/dampcake/Robolectric-JaCoCo-Sample
dampcake/Robolectric-JaCoCo-Sample@f9884b9 shows the change to make it work with JaCoCo 0.7.6

dampcake commented Feb 28, 2016

Small Android sample with the changes here:
https://github.com/dampcake/Robolectric-JaCoCo-Sample
dampcake/Robolectric-JaCoCo-Sample@f9884b9 shows the change to make it work with JaCoCo 0.7.6

@RomainPiel

This comment has been minimized.

Show comment
Hide comment
@RomainPiel

RomainPiel Mar 4, 2016

Just catching up with this, so to sum up:

  • there is no potential fix for jacoco <2.7.6
  • jacoco 2.7.6 requires gradle 2.13

Correct?

That's a bit annoying. Any chance there could be a hacky fix for previous versions (on robolectric or jacoco)?

RomainPiel commented Mar 4, 2016

Just catching up with this, so to sum up:

  • there is no potential fix for jacoco <2.7.6
  • jacoco 2.7.6 requires gradle 2.13

Correct?

That's a bit annoying. Any chance there could be a hacky fix for previous versions (on robolectric or jacoco)?

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Mar 4, 2016

Member

@RomainPiel JaCoCo doesn't require Gradle. Options:

  1. upgrade Gradle
  2. do not use Gradle Plugin for JaCoCo and attach JaCoCo 0.7.6 agent to your tests by yourself - it is just "-javaagent:..." option for JVM
  3. use JaCoCo <= 0.7.2
  4. ask Gradle to backport changes ( gradle/gradle#575 ) to older version or create you own version with such backport
Member

Godin commented Mar 4, 2016

@RomainPiel JaCoCo doesn't require Gradle. Options:

  1. upgrade Gradle
  2. do not use Gradle Plugin for JaCoCo and attach JaCoCo 0.7.6 agent to your tests by yourself - it is just "-javaagent:..." option for JVM
  3. use JaCoCo <= 0.7.2
  4. ask Gradle to backport changes ( gradle/gradle#575 ) to older version or create you own version with such backport
@RomainPiel

This comment has been minimized.

Show comment
Hide comment
@RomainPiel

RomainPiel commented Mar 4, 2016

@Godin cool thanks!

@mkjensen

This comment has been minimized.

Show comment
Hide comment
@mkjensen

mkjensen Mar 9, 2016

Thank you all for pointing me in the right direction. I had no luck using JaCoCo 0.7.6 and Gradle 2.13 but I was not really comfortable using that anyway since Android Studio 2 uses Gradle 2.10 per default. However, I also had problems getting JaCoCo 0.7.2 to work since toolVersion didn't seem to get respected. I ended up with the following hack that I'm looking forward to deleting as soon as possible: mkjensen/danish-media-license@5d9eb52

mkjensen commented Mar 9, 2016

Thank you all for pointing me in the right direction. I had no luck using JaCoCo 0.7.6 and Gradle 2.13 but I was not really comfortable using that anyway since Android Studio 2 uses Gradle 2.10 per default. However, I also had problems getting JaCoCo 0.7.2 to work since toolVersion didn't seem to get respected. I ended up with the following hack that I'm looking forward to deleting as soon as possible: mkjensen/danish-media-license@5d9eb52

raatiniemi added a commit to raatiniemi/worker that referenced this pull request Jul 3, 2016

include tests with Robolectric in code coverage
when using `RobolectricTestRunner` with jacoco 7.6 the code coverage is
not included in the report.

ref: jacoco/jacoco#288

@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.