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

Mockk cannot handle sealed classes in every block #934

Closed
ThanksForAllTheFish opened this issue Sep 29, 2022 · 4 comments · Fixed by #939
Closed

Mockk cannot handle sealed classes in every block #934

ThanksForAllTheFish opened this issue Sep 29, 2022 · 4 comments · Fixed by #939

Comments

@ThanksForAllTheFish
Copy link

Sample project: https://github.com/ThanksForAllTheFish/mockk832
Related: #832

Mockk 1.13.2 still throws

org.t4atf.mockk832.Node
java.lang.InstantiationError: org.t4atf.mockk832.Node
	at jdk.internal.reflect.GeneratedSerializationConstructorAccessor17.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)
	at io.mockk.proxy.jvm.ObjenesisInstantiator.instanceViaObjenesis(ObjenesisInstantiator.kt:75)
	at io.mockk.proxy.jvm.ObjenesisInstantiator.instance(ObjenesisInstantiator.kt:42)
	at io.mockk.impl.instantiation.JvmInstantiator$instantiate$2.invoke(JvmInstantiator.kt:16)
	at io.mockk.impl.instantiation.AbstractInstantiator.instantiateViaInstanceFactoryRegistry(AbstractInstantiator.kt:17)

when using sealed classes on kotlin 1.7 and java 17. The issue now is in every blocks, for instance:

class Root : Node()
class Leaf : Node()

interface Factory {
    fun create() : Node
    fun copy(node: Node) : Node
}

open class MyTest {
    @Test
    open fun aTest() {
        val mockk = mockk<Factory>()
        every { mockk.create() } returns Leaf() // this was fixed in mockk832

        println(mockk.create())

        every { mockk.copy(any()) } returns Root() //but this still breaks
    }
}
@aSemy
Copy link
Contributor

aSemy commented Sep 29, 2022

Looks like a good demo - can you (or someone else) update https://github.com/mockk/mockk/blob/6d5fe10cfd3662c7716f7381ecc6b06bde12a091/modules/mockk/src/commonTest/kotlin/io/mockk/it/SealedClassTest.kt and make a PR? I'd like to see a failing test, then it's easier to fix.

@diastremskii
Copy link
Contributor

diastremskii commented Sep 29, 2022

The test can be something like that:

Example test
class SealedInterfaceTest {

    @Test
    fun serviceTakesSealedInterfaceAsInput() {
        val mockNodeId = 1
        val factory = mockk<Factory> {
            every { getId(any()) } answers { mockNodeId }
        }

        val result = factory.getId(Root(0))

        assertEquals(mockNodeId, result)
    }

    companion object {

        sealed interface Node {
            val id: Int
        }

        data class Root(override val id: Int) : Node
        data class Leaf(override val id: Int) : Node

        interface Factory {
            fun create(): Node

            fun getId(node: Node): Int
        }

        class FactoryImpl : Factory {
            override fun create(): Node = Root(0)

            override fun getId(node: Node): Int = node.id
        }

    }
}
But it passes with current test setup since the issue is reproducible with JVM 17 bytecode. Should I still open a PR with the test if the test passes? 🤔

One way to make it fail is to change JVM target here to JavaVersion.VERSION_17 and run the tests with -PjavaToolchainTestVersion=17.

The exception itself seems to originate here, so the problem is that we are trying to create a subclass with Byte Buddy, but we can't since we are creating a subclass for interface that is sealed.

@aSemy
Copy link
Contributor

aSemy commented Sep 29, 2022

Should I still open a PR with the test if the test passes? 🤔

yes please!

One way to make it fail is to change JVM target here to JavaVersion.VERSION_17 and run the tests with -PjavaToolchainTestVersion=17.

The toolchain for tests should be overridden using the config in toolchain-jvm.gradle.kts, but I'm not confident that it works as expected

val javaToolchainMainVersion = javaLanguageVersion("javaToolchainMainVersion")
val javaToolchainTestVersion = javaLanguageVersion("javaToolchainTestVersion")

java-version: [ 11, 17, 19 ] # test LTS versions, and the newest

@cliffred
Copy link
Contributor

Created a PR with a fix: #939

cliffred pushed a commit to cliffred/mockk that referenced this issue Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants