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

condy arrayindexoutofboundsexception fix #665

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vimil
Copy link

@vimil vimil commented Mar 22, 2020

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

@jbennett2091
Copy link

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

@jmockit jmockit deleted a comment from philzq Sep 6, 2020
@jbennett2091
Copy link

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)

@vimil
Copy link
Author

vimil commented Sep 20, 2020

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

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

Saljack commented Oct 12, 2020

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

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

@reinhapa
Copy link

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

@nissane
Copy link

nissane commented Mar 9, 2021

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

pblew pushed a commit to pblew/jmockit1 that referenced this pull request Aug 24, 2021
@legacycode
Copy link

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

@dellgreen
Copy link

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

loetscher pushed a commit to BisonSchweizAG/bisonschweizag.github.io that referenced this pull request Nov 12, 2021
@DhavalUpadhyayaTR
Copy link

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

@sumit1sen
Copy link

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

rliesenfeld commented Mar 1, 2022

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

Work for me too after a JDK bump

JDK: 16
Gradle : 7.4.1

@abtabhi
Copy link

abtabhi commented Jun 29, 2022

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

@karnick
Copy link

karnick commented Sep 30, 2022

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

@peterkempy
Copy link

@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 #736 if required too ?)

@weifang6601
Copy link

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

imay pushed a commit to StarRocks/starrocks that referenced this pull request May 16, 2023
there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
mergify bot pushed a commit to StarRocks/starrocks that referenced this pull request May 16, 2023
there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
(cherry picked from commit 2f3ed5a)
wanpengfei-git pushed a commit to StarRocks/starrocks that referenced this pull request May 16, 2023
there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
(cherry picked from commit 2f3ed5a)
Moonm3n pushed a commit to Moonm3n/starrocks that referenced this pull request May 23, 2023
…Rocks#23445)

there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
Signed-off-by: Moonm3n <[email protected]>
numbernumberone pushed a commit to numbernumberone/starrocks that referenced this pull request May 31, 2023
…Rocks#23445)

there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
numbernumberone pushed a commit to numbernumberone/starrocks that referenced this pull request May 31, 2023
…Rocks#23445)

there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
abc982627271 pushed a commit to abc982627271/starrocks that referenced this pull request Jun 5, 2023
…Rocks#23445)

there is conflicting with jdk11 + jmockit + jcoco0.85, and more details in
jmockit/jmockit1#665
jmockit/jmockit1#615

Signed-off-by: zombee0 <[email protected]>
@zbflcy
Copy link

zbflcy commented Jul 4, 2023

really hope this can get fixed,

@raytapan
Copy link

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

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

Successfully merging this pull request may close these issues.