Skip to content
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

Incompatibility of JMockit, JaCoCo and Java 17 #729

Open
1 of 4 tasks
beatjost opened this issue Jul 6, 2022 · 10 comments
Open
1 of 4 tasks

Incompatibility of JMockit, JaCoCo and Java 17 #729

beatjost opened this issue Jul 6, 2022 · 10 comments

Comments

@beatjost
Copy link

beatjost commented Jul 6, 2022

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: conflict with jmockit on java 11 jacoco/jacoco#896

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

  • Check the following:
  • 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
Copy link
Author

beatjost commented Jul 6, 2022

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

@dlipofsky
Copy link

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 jacoco/jacoco#896 .
There would not appear to be any working combination for Java 17.

@dlipofsky
Copy link

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

@dlipofsky
Copy link

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
Copy link

Col-E commented Oct 6, 2022

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
Copy link

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

@khmarbaise
Copy link

@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
Copy link

@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
Copy link

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
Copy link

shivanisky commented Jul 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants