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

java.lang.InstantiationError when upgrading to Kotlin 1.7 #832

Closed
hantsy opened this issue Jun 10, 2022 · 55 comments · Fixed by #916
Closed

java.lang.InstantiationError when upgrading to Kotlin 1.7 #832

hantsy opened this issue Jun 10, 2022 · 55 comments · Fixed by #916

Comments

@hantsy
Copy link

hantsy commented Jun 10, 2022

Java 17 + Kotlin 1.7 + Mockk 1.12.4/Spring Mockk 3.1.1

java.lang.InstantiationError: com.example.demo.UpdateAccountResult
    at jdk.internal.reflect.GeneratedSerializationConstructorAccessor18.newInstance(Unknown Source)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
    at org.objenesis.instantiator.sun.SunReflectionFactoryInstantiator.newInstance(SunReflectionFactoryInstantiator.java:48)

It occurred whens using coEvery{} to do some stub, which returns a UpdateAccountResult.Success().

Here UpdateAccountResult is a sealed class, it has some detailed result subclass/sub data classes.

And UpdateAccountResult extends from a base abstract class like this.

abstract class ApiBaseResult {
    @field:JsonProperty("SC")
    val statusCode: String? = null

    @field:JsonProperty("EC")
    val errorCode: String? = null

    @field:JsonProperty("EM")
    val errorMessage: String? = null
}

These codes are working well with Kotlin 1.6.21.

@yesmanmx
Copy link

Same behaviour for simple every {}

@ygaller
Copy link

ygaller commented Jun 14, 2022

Probably related: Getting the following error

io.mockk.MockKException: Can't instantiate proxy for class com.acme.FlowConfiguration
...
Caused by: java.lang.IncompatibleClassChangeError: class com.acme.FlowConfiguration$Subclass15 cannot inherit from sealed class com.acme.FlowConfiguration

Where code is:

sealed class FlowConfiguration constructor(val flowName: String)

and mock is:

flowConfigurationMock = mockk {
  every { flowName } returns "testFlow"
}

Worked well in 1.6.10 / Java 17 with Mockk 1.12.2

@cmdjulian
Copy link

I can confirm the problem as were having it too

@hantsy
Copy link
Author

hantsy commented Jun 16, 2022

@ygaller it seems related to sealed class.

@pkgonan
Copy link

pkgonan commented Jun 16, 2022

I have a same problem. (kotlin 1.7.0 & mockk 1.12.4)

  1. coEvery { any() } -> java.lang.InstantiationError !
  2. sealed interface -> mockk(relaxed = true) -> io.mockk.MockKException: Can't instantiate proxy for class !

@dhladik
Copy link

dhladik commented Jun 21, 2022

Meaningless bump. This is most definitely related to sealed class. Occurs with every every { ... } and with Kotlin 1.7.0 and mocck 1.12.4

@danielrehmann
Copy link

We encounter the same issue with sealed classes and interfaces with version 1.12.4 when we try to upgrade to Kotlin 1.7.0. Happens in the coEvery every, verify and coVerify blocks throwing a java.lang.InstantiationError.

@hantsy
Copy link
Author

hantsy commented Jun 27, 2022

Any update for this issue, is there any workaround?

gtcno added a commit to navikt/dp-mellomlagring that referenced this issue Jun 28, 2022
@ThanksForAllTheFish
Copy link

Thanks for the suggestion, here a reproduction project: https://github.com/ThanksForAllTheFish/mockk832

@QUEnDo
Copy link

QUEnDo commented Jul 8, 2022

Same to me...

kjetiljd added a commit to navikt/ep-personoppslag that referenced this issue Jul 8, 2022
mockk/mockk#832 - som igjen feiler tester i prefill.
@Raibaz
Copy link
Collaborator

Raibaz commented Jul 8, 2022

Apparently something changed with sealed classes in 1.7.

However, I couldn't find anything in the release notes: the only mention of sealed classes is that non-exhaustive sealed hierarchies now trigger an error rather than a warning.

@cliffred
Copy link
Contributor

Apparently something changed with sealed classes in 1.7.

However, I couldn't find anything in the release notes: the only mention of sealed classes is that non-exhaustive sealed hierarchies now trigger an error rather than a warning.

Apparently Kotlin 1.7 in combination with Java 17 uses Java sealed classes.

I compiled the following code with Kotlin 1.6 and 1.7 and then decompiled.

sealed class Foo
class Bar : Foo()

Kotlin 1.7 decompiled:

public abstract sealed class Foo permits Bar {
   private Foo() {
   }

   // $FF: synthetic method
   public Foo(DefaultConstructorMarker $constructor_marker) {
      this();
   }
}

Kotlin 1.6 decompiled:

public abstract class Foo {
   private Foo() {
   }

   // $FF: synthetic method
   public Foo(DefaultConstructorMarker $constructor_marker) {
      this();
   }
}

@eygraber
Copy link

eygraber commented Jul 14, 2022

Right, Java 17 added a sealed class feature, and I guess Kotlin is supporting that in the bytecode for Kotlin sealed classes when using Kotlin 1.7 with Java 17.

dskarpas added a commit to navikt/ep-personoppslag that referenced this issue Jul 19, 2022
dskarpas added a commit to navikt/ep-personoppslag that referenced this issue Jul 19, 2022
…bel for mockk: mockk/mockk#832 - som igjen feiler tester i prefill-appen"

This reverts commit 63c536c.
dskarpas added a commit to navikt/ep-personoppslag that referenced this issue Jul 20, 2022
…bel for mockk: mockk/mockk#832 - som igjen feiler tester i prefill-appen"

This reverts commit 63c536c.
@huginr
Copy link

huginr commented Jul 21, 2022

I can work around it in some cases by mocking a specific implementation class instead of the sealed interface itself.

However that does not work if the sealed class or interface is a "returns" value.

Is there any news if mocking sealed classes and interfaces will be possible again later?

@cliffred
Copy link
Contributor

I've locally tried a to solve it in a hacky way to see if it would work, by adding an extra if statement to

runCatching {
    if (clazz.isSealed) {
        return subclass(clazz.permittedSubclasses.first() as Class<T>, interfaces)
    }
}

This does work, but arbitrarily picking the first permitted sub class feels like an ugly hack. And this only works when building MockK with Java 17.

@hantsy
Copy link
Author

hantsy commented Jul 25, 2022

It seems a Kotlin-1.7 support merged into the master, not sure if it fixed this issue.

@pkgonan
Copy link

pkgonan commented Jul 25, 2022

@Raibaz
Hi.
Has this problem been fixed in Master?

aSemy added a commit to aSemy/mockk that referenced this issue Jul 25, 2022
@javiyt
Copy link

javiyt commented Jul 26, 2022

In my case is failing even with sealed classes returned by the mocked method, so the workaround to mock an implementation of the sealed class is not working, even when I'm returning the mock with that implementation.

Raibaz added a commit that referenced this issue Jul 26, 2022
@aSemy
Copy link
Contributor

aSemy commented Sep 4, 2022

Can you tell, why java.lang.Class<T> is not enought in this case?

From my understanding, java.lang.Class also has this data (.isSealed() method and .getPermittedSubclasses()). And starting from Kotlin 1.7, when using 17+ target bytecode level, kotlin generates classes that are also sealed in JVM terms. For older bytecode versions current issue is not applicable, so doesn't matter if JVM says that class is not sealed

Class.isSealed() is only available on JVM 17+. I tried using it #915, but then MockK won't compile on JVM 11: https://github.com/mockk/mockk/runs/8178643128?check_suite_focus=true#step:6:72

@stuebingerb
Copy link
Contributor

stuebingerb commented Sep 5, 2022

Two ideas, maybe one of them helps.

  1. We should be able to get the KClass from the existing Class using the .kotlin extension: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.jvm/java.lang.-class/
  2. We could call isSealed and getPermittedSubclasses using reflection, something along the lines of
    fun Class<*>.isSealedSafe() =
        javaClass.methods.firstOrNull { it.name == "isSealed" }?.invoke(this) ?: false
    fun Class<*>.getPermittedSubclassesSafe() =
        javaClass.methods.firstOrNull { it.name == "getPermittedSubclasses" }?.let { it.invoke(this) as Array<Class<*>> } ?: emptyArray()
    

Granted, both seem a bit dirty but it might be good enough to at least unblock Kotlin 1.7?

@ThanksForAllTheFish
Copy link

Class.isSealed() is only available on JVM 17+. I tried using it #915, but then MockK won't compile on JVM 11: https://github.com/mockk/mockk/runs/8178643128?check_suite_focus=true#step:6:72

Could you think of a switch specific to the version? something like https://github.com/apache/commons-lang/blob/master/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java#L1020:

if (current version > 17) {
  use native java 17 sealed check
} else {
  current code
}

it is not pretty, but should be a smaller change. Just "dumping" ideas if they might help

@NatsuOnFire
Copy link

Since Java 9, you can use Runtime.version()

println(Runtime.version()) // 17.0.1+12-LTS
println(Runtime.version().feature()) // 17

And before Java 9 :

println(System.getProperty("java.specification.version")) // 17

@aSemy
Copy link
Contributor

aSemy commented Sep 6, 2022

Great ideas, would you be able to give them a go?

  1. fork the MockK repo
  2. make some changes
  3. make a PR against the MockK repo your own fork master branch, which will automatically trigger the tests for all JVM versions
  4. check the test results
  5. Create a PR against the MockK repo
  6. gain eternal glory in the MockK patch notes :)

stuebingerb added a commit to stuebingerb/mockk that referenced this issue Sep 6, 2022
Attempt to fix mockk#832 by making `ObjenesisInstantiator` work with a `KClass`.
@stuebingerb
Copy link
Contributor

Tried but the test workflows are not so automated for first-time contributors 😉
#916

stuebingerb added a commit to stuebingerb/mockk that referenced this issue Sep 6, 2022
Attempt to fix mockk#832 by making `ObjenesisInstantiator` work with a `KClass`.
stuebingerb added a commit to stuebingerb/mockk that referenced this issue Sep 6, 2022
Attempt to fix mockk#832 by making `ObjenesisInstantiator` work with a `KClass`.
stuebingerb added a commit to stuebingerb/mockk that referenced this issue Sep 6, 2022
Attempt to fix mockk#832 by supporting sealed classes based on previous work done by @aSemy.
@aSemy
Copy link
Contributor

aSemy commented Sep 6, 2022

Tried but the test workflows are not so automated for first-time contributors 😉 #916

Good point! I forgot. As a workaround you can create a PR in your own fork, from the branch against master.

@sishbi
Copy link

sishbi commented Sep 13, 2022

Hey, I see from stuebingerb#1 that all of the tests now pass, is this good to merge and be included in a new MockK release? My projects are waiting to upgrade to Kotlin 1.7 due to this mockk failure.
Thanks!

@hantsy
Copy link
Author

hantsy commented Sep 14, 2022

I am also waiting a new release to upgrade my project to Kotlin 1.7.

@Raibaz
Copy link
Collaborator

Raibaz commented Sep 15, 2022

Hey sorry about the delay, releasing 1.12.8 now.

@raycoarana
Copy link

But the code has not been merged in the main repo, right? The new release do not include the fixing code.

@seljabali
Copy link

#916 attempts to fix the issue & hasn't been merged yet.

@sishbi
Copy link

sishbi commented Sep 15, 2022

#916 attempts to fix the issue & hasn't been merged yet.

What's holding up the merge? Can it be included in 1.12.8 also ?

@hantsy
Copy link
Author

hantsy commented Sep 16, 2022

Updated to 1.12.8, still failed https://github.com/hantsy/mockk-issue832/actions/runs/3065819014/jobs/4950308126#step:4:42
I used mockk-jvm instead.

@seljabali
Copy link

seljabali commented Sep 28, 2022

@Raibaz may we have a new release please now that #916 got merged? Thanks in advance!

@Raibaz
Copy link
Collaborator

Raibaz commented Sep 28, 2022

Done, v1.13.2 is out :)

@cliffred
Copy link
Contributor

cliffred commented Sep 28, 2022

1.13.2 only partly solves the issue:

// given the following sealed class
sealed class Foo
class FooSub : Foo()

// and the following class with a method that has function with a sealed class as parameter
class Bar {
    fun fooParam(foo: Foo): Int = 42
}

// then the following now works
val foo : Foo = mockk() // Fixed since 1.13.2

// but when using any() where a sealed class is expected the issue still persists
val bar: Bar = mockk()
every { bar.fooParam(any()) } returns 1 // throws InstantiationError

This can probably be fixed in JvmSignatureValueGenerator with the same trick: Passing the first subclass to instantiator.instantiate() in case cls is sealed.

@chrissearle
Copy link

@cliffred that matches what I am seeing locally with 1.13.2.

I wonder if this should be re-opened or a more specific issue raised?

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