jmockit / jmockit1

Advanced Java library for integration testing, mocking, faking, and code coverage
Other
461 stars 239 forks source link

condy arrayindexoutofboundsexception fix #665

Open vimil opened 4 years ago

vimil commented 4 years ago

this is to fix arrayindexoutofbounds issue when running jmockit in eclipse 2020.03 with code coverage

jbennett2091 commented 4 years ago

If a (real-world) test is required, please have a look at the example provided with #688

jbennett2091 commented 4 years ago

On a whim, I pulled this branch, built it locally, and used the resulting artifact in my project. It did not work for me, but that could very well be because I did something wrong.

Here's the build error (in my code) that I get when using JDK 11.0.8 + Jacoco 0.8.6 + JMockit 1.49-mod:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile (default-testCompile) on p
roject omni-core: Compilation failure: Compilation failure:
[ERROR] /C:/Users/jb14684/git/omni-project-alt4/omni/omni-core-project/omni-core/src/test/java/com/ibi/omni/util/CoreUtilsTest.java:[46,38] method newInstanceUsingCompatibleConstructor in class mockit.internal.reflection.ConstructorReflection cannot be applied to given types;
[ERROR]   required: java.lang.Class<?>,java.lang.String
[ERROR]   found: java.lang.Class<com.ibi.omni.util.CoreUtils>
[ERROR]   reason: actual and formal argument lists differ in length

Here's the test code being compiled:

    @Test
    public void testCtor()
    {
        ConstructorReflection.newInstanceUsingCompatibleConstructor(CoreUtils.class);
    }

Here's the class under test:

    private CoreUtils()
    {
        // static use only
    }

These combos work:

vimil commented 4 years ago

On a whim, I pulled this branch, built it locally, and used the resulting artifact in my project. It did not work for me, but that could very well be because I did something wrong.

Here's the build error (in my code) that I get when using JDK 11.0.8 + Jacoco 0.8.6 + JMockit 1.49-mod:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile (default-testCompile) on p
roject omni-core: Compilation failure: Compilation failure:
[ERROR] /C:/Users/jb14684/git/omni-project-alt4/omni/omni-core-project/omni-core/src/test/java/com/ibi/omni/util/CoreUtilsTest.java:[46,38] method newInstanceUsingCompatibleConstructor in class mockit.internal.reflection.ConstructorReflection cannot be applied to given types;
[ERROR]   required: java.lang.Class<?>,java.lang.String
[ERROR]   found: java.lang.Class<com.ibi.omni.util.CoreUtils>
[ERROR]   reason: actual and formal argument lists differ in length

Here's the test code being compiled:

  @Test
  public void testCtor()
  {
      ConstructorReflection.newInstanceUsingCompatibleConstructor(CoreUtils.class);
  }

Here's the class under test:

  private CoreUtils()
  {
      // static use only
  }

These combos work:

  • JDK8 + Jacoco 0.8.6 + JMockit 1.49 (i.e. use JDK8)
  • JDK11 + Jacoco 0.8.3 + JMockit 1.49 (i.e. use old-Jacoco)

ConstructorReflection.newInstanceUsingCompatibleConstructor(CoreUtils.class);

the method ConstructorReflection.newInstanceUsingCompatibleConstructor takes two parameters. A class and a String.

jbennett2091 commented 4 years ago

OK, mea culpa, and that's what I get for using a class in the 'internal' package. You are correct, and switching to the 'newInstanceUsingDefaultConstructor' method (rather than newInstanceUsingCompatibleConstructor) solved that unrelated issue. In my defense, "it used to work" 👍


My new answer, I forked this branch, and tried it in my own application (affected by the aforementioned issue). This code works for me. Take that for what it's worth.

I'd love to see this pull request get released. Even if it is just a 'milestone' or 'beta', it resolves blocking issues for a number of folks, including myself. JMockit, as a library, seems to have gone from releases every 3-months, to no release in the last 9-months, and this fix languishing (without real comment) for 6-months. I don't mean to sound critical, I am a big proponent of this framework. Just hoping to see it move forward.

Saljack commented 3 years ago

I can confirm that this MR fix our problem with JMockit and Jacoco too. So with this MR I can use a newer version of Jacoco (I tested it with 0.8.6). I hope that it will be merged soon. I start to be little nervous because I love this project.

rliesenfeld commented 3 years ago

Next year, once I am no longer working at home; no time for JMockit now.

reinhapa commented 3 years ago

@rliesenfeld looking forward for this fix to land in the next release

nissane commented 3 years ago

@rliesenfeld Can this get merged in? Love this project, but don't see much work on it recently. Is JMockit going to be maintained? I really hope so!

legacycode commented 2 years ago

@rliesenfeld This fix works for me. Can you please merge it and create a new release for mvnrepository.com?

dellgreen commented 2 years ago

Works for my projects also. Recently did an investigation at work for our internal projects. The graph below may or may not be useful to others:

gradle java jmockit jacoco result notes recommend
7.2 11 1.49 0.8.3 pass
7.3 11 1.49 0.8.3 pass *
7.2 11 1.49a 0.8.3 pass
7.2 11 1.49a 0.8.6 pass
7.2 11 1.49a 0.8.7 pass
7.3 11 1.49a 0.8.7 pass *
7.2 14 1.49a 0.8.3 fail jacoco agent exceptions
7.2 14 1.49a 0.8.6 pass
7.2 14 1.49a 0.8.7 pass
7.3 14 1.49a 0.8.7 pass *
7.2 16 1.49a 0.8.3 fail jacoco agent exceptions
7.2 16 1.49a 0.8.6 pass
7.2 16 1.49a 0.8.7 pass
7.3 16 1.49a 0.8.7 pass *
7.2 17 1.49a 0.8.3 fail jacoco agent exceptions
7.2 17 1.49a 0.8.6 fail jacoco agent exceptions
7.2 17 1.49a 0.8.7 pass
7.3 17 1.49a 0.8.7 pass *
7.2 18ea 1.49a 0.8.7 fail gradle/groovy exceptions
7.3 18ea 1.49a 0.8.7 fail jacoco agent exceptions (will be fixed in 0.8.8)
7.3 18ea 1.49a 0.8.8 TBC
DhavalUpadhyayaTR commented 2 years ago

Any idea when it is going to be merged and a new release be available?

sumit1sen commented 2 years ago

Does anyone know when this will be merged and released? I'd prefer to use an official release instead of creating my own private jmockit jar.

rliesenfeld commented 2 years ago

Hopefully I will have some time for this, in 2022. That said, I see it as really the fault of JaCoCo. The change they made that generates the Condy bytecode, was supposedly a performance optimization. However, no benchmarking was ever done (that I know of). IMO, the JaCoCo team should reconsider if generating such unusual bytecode is a good idea.

altair2010 commented 2 years ago

Work for me too after a JDK bump

JDK: 16 Gradle : 7.4.1

abtabhi commented 2 years ago

works for me too, because of Jacoco to SonarQube integration its becoming hard to skip any of these components :(

karnick commented 2 years ago

It is surprising, why no more releases after Dec 2019 (v1.49)?

peterkempy commented 1 year ago

@rliesenfeld We all understand that you have competing priorities, and we appreciate the time you have devoted to JMockit.

As regards this fix from @vimil , if this PR were merged then it would unblock a lot of devs - many more than are listed above.

(Plus maybe https://github.com/jmockit/jmockit1/pull/736 if required too ?)

weifang6601 commented 1 year ago

Hope this can get fixed, I use jmockit for many years and I think it is the best testing tool for java.

zbflcy commented 1 year ago

really hope this can get fixed,

raytapan commented 1 year ago

Is it possible to merge this PR and make a new release ? I have been blocked for a while now.