Incompatibility of JMockit, JaCoCo and Java 17

beatjost commented 2 years ago

Please provide the following information:

Version of JMockit that was used: -- JMockit 1.49 -- JaCoCo 0.8.3 (newer versions as 0.8.8 are not working with JMockit) -- Gradle 7.4.2 -- Java 17
Description of the problem or enhancement request: JaCoCo causes Stacktraces when runing the project with Java 17. Caused by: java.lang.IllegalArgumentException: Unsupported class file major version 61 This would be solved in newer JaCoCo versions, but those are not compatilbe with JMockit due to condy properly... See: https://github.com/jacoco/jacoco/issues/896

Run gradlew clean build for the attached sample project and see console output...

Check the following:
- [x] If a defect or unexpected result, JMockit project members should be able to reproduce it. For that, include an example test (perhaps accompanied by a Maven/Gradle build script) which can be executed without changes and reproduces the failure. jmockit_jacoco.zip
- [ ] If an enhancement or new feature request, it should be justified by an example test demonstrating the validity and usefulness of the desired enhancement or new feature.
- [ ] The issue does not fall outside the scope of the project (for example, attempting to use JMockit APIs from Groovy or Scala code, or with an Android runtime).
- [ ] The JDK where the problem occurs is a final release, not a development build.

beatjost commented 2 years ago

@rliesenfeld, @gliptak maybe you could quickly check if we're doning something wrong... (but think also others mentioned this problem). thank you 👍

dlipofsky commented 2 years ago

I've encountered the same issue with JMockit 1.44 - 1.49, JaCoCo, and Java 17. JaCoCo 0.8.3 doesn't work with Java 17 because "Unsupported class file major version 61" but JaCoCo 0.8.4 and beyond causes numerous ArrayIndexOutOfBoundsException as described in https://github.com/jacoco/jacoco/issues/896 . There would not appear to be any working combination for Java 17.

dlipofsky commented 2 years ago

@rliesenfeld is JMockit a dead project? Do we ever expect another release?

dlipofsky commented 2 years ago

I haven't tried it myself but it looks like com.github.hazendaz.jmockit:jmockit:1.49.2 might be a fork that supports this. Personally I am just moving forward by migrating to mockito but I figured I'd mention the fork in case it helps someone. As far as I can tell the main jmockit project is abandoned.

Col-E commented 2 years ago

Using com.github.hazendaz.jmockit:jmockit:1.49.2 as a base I was able to get things running on Java 17 for usage at work. Opened a PR in case anyone wants to build off of the additions (Better error handling, few more edge cases covered, etc) #736

Oddly enough at work after patching the immediate incompatibilities here in JMockit we also encountered issues with JaCoCo when combined with EE/Jakarta & aspect weaving. JaCoCo on Java 11+ uses the ldc instruction with a constant_dynamic to access $jacocoData but somewhere along the line the constant pool entry for that constant_dynamic becomes an invoke_dynamic which is illegal when used by a ldc instruction. I made a little hack that patches the constant pool item so if anyone has the same issue, just run this on your instrumented class byte[]:

    private void patchMalformedJacocoStub(byte[] bytes) {
        try {
            boolean dirty = false;
            byte[] copy = Arrays.copyOf(bytes, bytes.length);
            String target = "$jacocoData";
            // Patching the data field
            //  - Declared in the CP as a 'CONSTANT_InvokeDynamic_info'
            //    but should be 'CONSTANT_Dynamic_info'
            ClassReader cr = new ClassReader(copy);
            char[] stringBuffer = new char[cr.getMaxStringLength()];
            int cpMax = cr.getItemCount();
            for (int i = 1; i < cpMax; i++) {
                // offset = start of 'cp_info' structure, plus one.
                int cpOffset = cr.getItem(i);
                int cpTag = copy[cpOffset - 1];
                // CONSTANT_DYNAMIC_TAG = 17
                // CONSTANT_INVOKE_DYNAMIC_TAG = 18
                if (cpTag == 18) {
                    // CONSTANT_InvokeDynamic_info {
                    //    u1 tag;
                    //    u2 bootstrap_method_attr_index;
                    //    u2 name_and_type_index; <------- get this
                    //}
                    int nameTypeIndex = cr.readUnsignedShort(cpOffset + 2);
                    // CONSTANT_NameAndType_info {
                    //    u1 tag;
                    //    u2 name_index; <------ get value of utf8 at location of index
                    //    u2 descriptor_index;
                    //}
                    int nameTypeCpOffset = cr.getItem(nameTypeIndex);
                    String utf8 = cr.readUTF8(nameTypeCpOffset, stringBuffer);
                    // check if match
                    if (target.equals(utf8)) {
                        // Swap the original CP item we were looking from '18' to '17'
                        copy[cpOffset - 1] = 17;
                        dirty = true;
                    }
                } else if (cpTag == 5 || cpTag == 6) {
                    // CONSTANT_Long / CONSTANT_Double take up two slots in the constant pool for legacy reasons.
                    i++;
                }
            }
            // You can inline this without the byte[] copy, but if there's a failure half-way though then this approach won't contaminate the input array
            if (dirty)
                System.arraycopy(copy, 0, bytes, 0, copy.length);
        } catch (Throwable t)
        {
            // We're wrapping this and throwing an exception so that we don't get mysterious
            // errors thrown up the stack. This ideally should never be thrown but the above patch
            // may not consider some nebulous edge-case which would cause some failure that
            // materializes in your test results as a seemingly totally unrelated error.
            //
            // The common error I encountered in this case is:
            //   java.lang.NullPointerException: Cannot invoke "org.testng.IClass.getRealClass()"
            //   because the return value of "org.testng.ITestResult.getTestClass()" is null
            throw new IllegalStateException("Failed patching JaCoCo instrumented classes", t);
        }
    }

Without this you may see some really misleading exception messages depending on the test framework. Ideally they'd hard fail on invalid bytecode but TestNG in some cases powers through depending on how that invalid class is used. Debugging that was quite fun.

ajinkya-mulay commented 1 year ago

I am also looking for the same combination JMockit, JaCoCo and Java 17. Using all latest version it doesnt work. Any help here ?

khmarbaise commented 1 year ago

@ajinkya-mulay I would suggest to use most recent version of JaCoCo (0.8.10). The question is why you really need JMockit? Maybe you could go with Mockito?

dlipofsky commented 1 year ago

@ajinkya-mulay There are 2 solutions, but the best solution is to migrate from JMockit to Mockito - it can be a huge amount of work but eventually you are going to have to do it anyway. It's what I did, and I know many others have. The JMockit support community is rapidly shrinking to zero. You should consider it dead.

If you have an end-of-life project that you don't have the resources to maintain then maybe it's better to stay on Java 11 until it's decommissioned. That's what I'm doing for the projects I didn't migrate to Mockito.

Finally, there is a 3rd party fork of JMockit (com.github.hazendaz.jmockit:jmockit:1.49.2) for Java 17 that some have reported success with.

tinder-dthomson commented 7 months ago

For those of you facing this situation today, I have created an OpenRewrite recipe to help with the migration from JMockit to Mockito.

It is not complete (see this issue to follow along) but it certainly made a huge difference in our migration. Recipe contributions are welcome to improve the JMockit feature coverage (most notably static methods and MockUp)!

shivanisky commented 3 months ago

For those of you facing this situation today, I have created an OpenRewrite recipe to help with the migration from JMockit to Mockito.

It is not complete (see this issue to follow along) but it certainly made a huge difference in our migration. Recipe contributions are welcome to improve the JMockit feature coverage (most notably static methods and MockUp)!

Have also built on @tinder-dthomson 's great work with much help from @timtebeek and added automation for more statements and improved robustness, including Jmockit Expectations, JMockit Verifications (v 8.29.0) and Jmockit NonStrictExpectations (v 8.30.0) . It's now looking like it would cover the majority of cases and may be worth exploring for migration to mockito

https://docs.openrewrite.org/recipes/java/testing/jmockit/jmockittomockito

AnithaPerumalsamy commented 3 weeks ago

lang.IllegalArgumentException: No enum constant org.openrewrite.java.testing.jmo ckit.JMockitBlockType.VerificationsInOrder [ERROR] java.base/java.lang.Enum.valueOf(Enum.java:273) [ERROR] org.openrewrite.java.testing.jmockit.JMockitBlockType.valueOf(JMockitB lockType.java:20) [ERROR] org.openrewrite.java.testing.jmockit.JMockitUtils.getJMockitBlock(JMoc kitUtils.java:46) [ERROR] org.openrewrite.java.testing.jmockit.SetupStatementsRewriter.rewriteMe thodBody(SetupStatementsRewriter.java:41) [ERROR] org.openrewrite.java.testing.jmockit.JMockitBlockToMockito$RewriteJMoc kitBlockVisitor.visitMethodDeclaration(JMockitBlockToMockito.java:66) [ERROR] org.openrewrite.java.testing.jmockit.JMockitBlockToMockito$RewriteJMoc kitBlockVisitor.visitMethodDeclaration(JMockitBlockToMockito.java:56) [ERROR] org.openrewrite.java.tree.J$MethodDeclaration.acceptJava(J.java:3651) [ERROR] org.openrewrite.java.tree.J.accept(J.java:59) [ERROR] org.openrewrite.TreeVisitor.visit(TreeVisitor.java:248) [ERROR] org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:318)
[ERROR] org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:136 5) [ERROR] org.openrewrite.java.JavaVisitor.lambda$visitBlock$4(JavaVisitor.java: 397) [ERROR] org.openrewrite.internal.ListUtils.map(ListUtils.java:176) [ERROR] org.openrewrite.java.JavaVisitor.visitBlock(JavaVisitor.java:396)
[ERROR] org.openrewrite.java.JavaIsoVisitor.visitBlock(JavaIsoVisitor.java:88) [ERROR] org.openrewrite.java.JavaIsoVisitor.visitBlock(JavaIsoVisitor.java:30) [ERROR] ... which version of jmockito to Mocikto is supported? i am currently using 1.49 but getting above error @tinder-dthomson

shivanisky commented 3 weeks ago

lang.IllegalArgumentException: No enum constant org.openrewrite.java.testing.jmo

ckit.JMockitBlockType.VerificationsInOrder

[ERROR] java.base/java.lang.Enum.valueOf(Enum.java:273)

[ERROR] org.openrewrite.java.testing.jmockit.JMockitBlockType.valueOf(JMockitB

lockType.java:20)

[ERROR] org.openrewrite.java.testing.jmockit.JMockitUtils.getJMockitBlock(JMoc

kitUtils.java:46)

[ERROR] org.openrewrite.java.testing.jmockit.SetupStatementsRewriter.rewriteMe

thodBody(SetupStatementsRewriter.java:41)

[ERROR] org.openrewrite.java.testing.jmockit.JMockitBlockToMockito$RewriteJMoc

kitBlockVisitor.visitMethodDeclaration(JMockitBlockToMockito.java:66)

[ERROR] org.openrewrite.java.testing.jmockit.JMockitBlockToMockito$RewriteJMoc

kitBlockVisitor.visitMethodDeclaration(JMockitBlockToMockito.java:56)

[ERROR] org.openrewrite.java.tree.J$MethodDeclaration.acceptJava(J.java:3651)

[ERROR] org.openrewrite.java.tree.J.accept(J.java:59)

[ERROR] org.openrewrite.TreeVisitor.visit(TreeVisitor.java:248)

[ERROR] org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:318)

[ERROR] org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:136

5)

[ERROR] org.openrewrite.java.JavaVisitor.lambda$visitBlock$4(JavaVisitor.java:

397)

[ERROR] org.openrewrite.internal.ListUtils.map(ListUtils.java:176)

[ERROR] org.openrewrite.java.JavaVisitor.visitBlock(JavaVisitor.java:396)

[ERROR] org.openrewrite.java.JavaIsoVisitor.visitBlock(JavaIsoVisitor.java:88)

[ERROR] org.openrewrite.java.JavaIsoVisitor.visitBlock(JavaIsoVisitor.java:30)

[ERROR] ...

which version of jmockito to Mocikto is supported? i am currently using 1.49 but getting above error @tinder-dthomson

Thanks for sharing this. This JMockit type is not supported currently - VerificationsInOrder, however we should not be generating this exception.

The following JMockit types are supported: Expectations NonStrictExpectaions Verifications

and hopefully soon, MockUp.

This stack trace looks like a bug, and it would be an easy fix to skip migrating this.

We can a) fix this bug b) raise issue to migrate VerificationsInOrder

In the past, when I have seen these types of exceptions when running recipes, I've temporarily deleted the files generating it (in this case it would be files containing JMockitVerificarionsInOrder), rerun the recipe and then reverted them.

I think we can finish part a) above pretty quickly.

@timtebeek as fyi

AnithaPerumalsamy commented 3 weeks ago

Thanks for the quick respone @shivanisky , will raise a git issue for the fix.

shivanisky commented 3 weeks ago

Thanks for the quick respone @shivanisky , will raise a git issue for the fix.

And it's fixed and merged to not generate the exception and run smoothly! Again, thanks for commenting on this thread about this issue, feel free to raise the issue to migrate VerificationsInOrder, and if you like, you could fix it! In Mockito, need to create InOrder mock https://www.javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/InOrder.html

jmockit / jmockit1

Incompatibility of JMockit, JaCoCo and Java 17 #729