-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conflict with jmockit #35
Conversation
Can you please post the part of the stack trace after "Caused by"? |
I have managed to reproduce the error with a contrived example:
The output of Maven is (running
pom extract:
|
Ok, we've seen this before. What's happening here is that the JMockit agent loads classes and retransforms them before the JaCoCo agent sees them. Afterwards JaCoCo can not perform the required instrumentations any more. Not much we can do about this, except printing a less prominent error message. |
Thank you for your reply. I understand the reason for the issue - but since it was working up to 0.5.7 it might be "fixable". |
Ok, as soon as time permits I'll test your reproducer with 0.5.7. |
That's great thanks. I am using 0.5.7 for this reason for now, but since it does what I need I am a happy user anyway :-) |
Had same kind of issue, downgrading to 0.5.7 (actually eclemma 2.1.2) solved the issue |
I have a similar kind of issue with classes instrumented ("enhanced") by Solarmetric Kodo-JDO (now Oracle Kodo I think). My stacktrace with 0.6.1 is with 0.5.7 this is gone. When I use <sonar.dynamicAnalysis>true</sonar.dynamicAnalysis> and run "sonar:sonar" everything is fine even with 0.6.1. I would highly appreciate a fix. Further information can be supplied, if necessary. |
Ok, your're right that Kodo-JDO may be the root cause of all evil, but jacoco seems to tolerate this up to 0.6.0 according to my recent tests. Perhaps it's possible to tolerate these errors like before? |
@fjakop The reason is that JaCoCo has now full Java7 support (incl. stackmap frames). Probably it is possible to implement fallbacks, but this has now priority for me. Especially as Java 6 is end of life now and Oracle will therefore be forced to fix their own products. |
so, for my understanding: jacoco >0.6.1 cannot instrument classes, which are compiled with target 1.6? |
@barclay-reg you're wrong - JaCoCo > 0.6.1 is able to instrument Java 1.6 classes (as well as 1.5 and 1.7), but only if they have correct stackmap information. |
Some technical details: JaCoCo updates existing stackmap information incrementally. If cannot not re-calculate stackmaps, as this is a quite heavy operation and would require to know the type hierarchy of all referenced types. JaCoCo therefore has no chance to figure out whether the original stackmap information was correct. In some situations the incremental update fails on invalid stackmaps, in this case we now provide a better error message, see #73. Probably it is possible to implement some fallback strategies to remove stackmaps at all if the incremental update fails. But I don't feel that we should blur JaCoCo with such hacks because other tools create invalid class files. Expecially as Java 6 is end of live now and for Java 7 class files there is no fallback in the verifier any more, i.e. such broken tools will not work on Java 7 class anyways (with or without JaCoCo). |
going back to the original issue, has anything been done to see if there was a fix for Jmockit and anything past 5.7? We are having the same issue. |
UTs hang when I use JMockit and JaCoCo maven plugin. It hangs even if JMockit exists only in class path and no usage in unit tests. Here is my project: pom.xml: <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>test</groupId>
<artifactId>test</artifactId>
<version>0.0.1-SNAPSHOT</version>
<dependencies>
<dependency>
<groupId>com.googlecode.jmockit</groupId>
<artifactId>jmockit</artifactId>
<version>1.6</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.11</version>
</dependency>
</dependencies>
<properties>
<surefireArgLine>-javaagent:"${settings.localRepository}"/org/jacoco/org.jacoco.agent/0.6.4.201312101107/org.jacoco.agent-0.6.4.201312101107-runtime.jar=destfile=${project.baseUri}target/jacoco.exec</surefireArgLine>
</properties>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.6.4.201312101107</version>
<executions>
<execution>
<id>pre-unit-test</id>
<goals>
<goal>prepare-agent</goal>
</goals>
<configuration>
<destFile>${project.build.directory}/coverage-reports/jacoco-ut.exec</destFile>
<propertyName>surefireArgLine</propertyName>
</configuration>
</execution>
<execution>
<id>post-unit-test</id>
<phase>test</phase>
<goals>
<goal>report</goal>
</goals>
<configuration>
<dataFile>${project.build.directory}/coverage-reports/jacoco-ut.exec</dataFile>
<outputDirectory>${project.reporting.outputDirectory}/jacoco-ut</outputDirectory>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>1.5</source>
<target>1.5</target>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.16</version>
<configuration>
<argLine>${surefireArgLine}</argLine>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</project> Unit test: import mockit.Mock;
import mockit.MockUp;
import org.junit.Assert;
import org.junit.Test;
public class MyTest {
@Test
public void test() {
new MockUp<MyMock>() {
@Mock
public int getValue() {
return 4;
}
};
MyMock mock = new MyMock();
Assert.assertEquals(4, mock.getValue());
}
}
class MyMock {
public int getValue() {
return 6;
}
} |
Hello. I am the JMockit developer, and I am currently looking into these problems with the JaCoCo + JMockit combination. There is more than one. One of them is, I believe, in the JaCoCo CoverageTransformer class (latest version, 0.7.1-SNAPSHOT). The problem here is that, when JMockit calls Instrumentation#redefineClasses(<a mocked class>), the CoverageTransformer#transform(...) method is triggered with that same class passed in for the "classBeingRedefined" parameter. So far, so good as this is what java.lang.instrument does normally. Notice that the "classBeingRedefined" will necessarily already have passed through the CoverageTransformer when the class first got loaded by the JVM, as JMockit redefines classes sometime after they have been loaded. I don't know why JaCoCo is trying to re-transform classes in this kind of situation. Most likely, it should "return null;" whenever it gets called with "classBeingRedefined != null". So, if the JaCoCo developers agree, this would be a trivial fix at line 76 of CoverageTransformer.java. Alternatively, the problem can be solved by changing a) InstrSupport.java:81 to throw a more specific exception type, and b) CoverageTransformer.java:88 to catch this more specific exception and simply return null. I will continue investigating other problems (mainly the hang/deadlock that occurs when JMockit tries to load its agent through the Attach API) and report back here, but if the above can be solved it would help a lot. Thanks |
Ok, I have news regarding the hang reported in the comment by "baratali" above. I couldn't fully understand what causes the deadlock, since it occurs inside JVM native code (specifically, the sun.misc.Unsafe#ensureClassInitialized(Class) native method). I suppose this method ends up getting called again while a previous call is still executing. This seems to occur only for the org.junit.runners.BlockJUnit4ClassRunner class, when JMockit attempts to redefine it for integration purposes; JaCoCo then attemps to set the "$jacocoData" static field, previously introduced to the class when it went through the CoverageTransformer. A workaround which avoids the problem is to exclude the JUnit classes from JaCoCo's consideration. I used the following JVM initialization parameter in my testing, excluding both JUnit and TestNG classes: -javaagent:/jacoco-0.7.1/lib/jacocoagent.jar=excludes=junit.:org.junit.:org.testng.* The good news is that the fix I described in my previous comment also solves this deadlock problem, because then the JUnit classes will be ignored by JaCoCo as they get instrumented by JMockit. |
@rliesenfeld Many thanks for getting in touch and analyzing this issue! I will investigate why the classBeingRedefined parameter was is used this way and see whether we can adjust the method as you propose. |
Java Code Coverage Tools » jacoco #264 FAILURE |
@rliesenfeld I implemented the fixed as proposed by you and attached it to this issue as a pull request. It is also available as a snapshot build from here: http://download.eclipselab.org/jacoco/preview/jacoco-0.7.1.201403230515.zip Can you please confirm that the fix works? Thx |
@marchof Yes, I tested with the attached zip file above, and it works! Well, almost: once the CoverageTransformer stopped failing on class redefinitions triggered by JMockit, I discovered another problem (an "invalid class schema change" exception). However, I was able to make a few changes to JMockit to deal with this situation. I am still working on the final changes, but they will make it to the next JMockit release (1.8) which will be out next month (around April 20). So, with these two sets of fixes on both tools, I believe the JaCoCo + JMockit combination should finally work well. Thanks very much! |
@rliesenfeld Indeed, JaCoCo adds a static field and a static method to instrumented classes. This would be considered as an "invalid class schema change" if the class has already been loaded before. So I'm going to merge the JaCoCo change. |
I'm trying to use JMockit+EclEmma (JaCoCo). I have encountered the deadlock problems which has been solved by excluding JMockit & JUnit packages (mockit.:junit.:org.junit.*). In my test case, the warning is exaclty print once for each test methods and for each mocked (or injectable) class (none for interface) Is current development dealing with this warning ? |
When can we get a release of Emma with this fix in it? |
It's available now. See the following pages: http://www.eclemma.org/jacoco/trunk/doc/changes.html On Sun, Apr 27, 2014 at 1:24 PM, Stephen Cooper notifications@github.comwrote:
|
An incompatibility with jmockit seems to have been introduced in version 0.5.8 and still exists in 0.6.0 (no problems found with version 0.5.7). Full stacktrace with 0.5.8 below (some class names have been removed). Note: With 0.5.10, the exception is in
org.jacoco.agent.rt_kqcpih.CoverageTransformer.transform(CoverageTransformer.java:91)