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

Issue with mocking value classes with coEvery #1073

Open
3 tasks done
Syex opened this issue Apr 1, 2023 · 34 comments
Open
3 tasks done

Issue with mocking value classes with coEvery #1073

Syex opened this issue Apr 1, 2023 · 34 comments

Comments

@Syex
Copy link

Syex commented Apr 1, 2023

Prerequisites

Please answer the following questions for yourself before submitting an issue.

  • I am running the latest version
  • I checked the documentation and found no answer
  • I checked to make sure that this issue has not already been filed

Failure Information (for bugs)

I'm running into an edge case issue with value classes being used as mock answers for suspending methods that implement an interface with generics.

Consider the following interface with an implementing class using Kotlin Result (or any other value class)

interface Action<Params, ReturnType> {

    suspend fun execute(params: Params): ReturnType
}

class ResultTest : Action<Unit, Result<String>> {

    override suspend fun execute(params: Unit): Result<String> {
        TODO("Not yet implemented")
    }
}

and a simple test like

private val resultTest = mockk<ResultTest>()

@Test
fun test() = runTest {
    coEvery { resultTest.execute(Unit) } returns Result.success("abc")

    val result = resultTest.execute(Unit)
    assertTrue(result.isSuccess)
}

it fails with

class java.lang.String cannot be cast to class kotlin.Result

when calling resultTest.execute(Unit).


With a modification of the return type of the interface method execute from ReturnType to Result<ReturnType>:

interface Action<Params, ReturnType> {

    suspend fun execute(params: Params): Result<ReturnType>
}

class ResultTest : Action<Unit, String> {

    override suspend fun execute(params: Unit): Result<String> {
        TODO("Not yet implemented")
    }
}

the same test will pass.

The test will also pass with the first version of the interface if I remove the suspend keyword and replace coEvery with every.

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • MockK version: 1.13.4
  • Kotlin version: 1.8.10
  • JDK version: 11
  • JUnit version: 5.9.2
  • Type of test: unit test OR android instrumented test: Unit

Stack trace

class java.lang.String cannot be cast to class kotlin.Result (java.lang.String is in module java.base of loader 'bootstrap'; kotlin.Result is in unnamed module of loader 'app')
java.lang.ClassCastException: class java.lang.String cannot be cast to class kotlin.Result (java.lang.String is in module java.base of loader 'bootstrap'; kotlin.Result is in unnamed module of loader 'app')
	at de.memorian.wearos.marsrover.domain.action.ResultTestB.getResult-IoAF18A(MockkResultTest.kt:60)
	at de.memorian.wearos.marsrover.domain.action.MockkResultTest$test$1.invokeSuspend(MockkResultTest.kt:21)
@boiler23
Copy link

boiler23 commented Apr 7, 2023

I'm facing the same issue with Kotlin 1.8.0+ (1.8.20 as well). Here is an even shorter example:

@JvmInline
value class Data(val v: Long)

interface Producer { suspend fun produce(): Data }

@Test
fun test() = runTest {
    val producer = mockk<Producer>()
    coEvery { producer.produce() } returns Data(123L)
    val result = producer.produce() // fails here
    result shouldBe Data(123L)
}

Removing the suspend modifier for produce() fixes the issue.

@Wrakor
Copy link

Wrakor commented Apr 27, 2023

Facing the same issue, using functional interfaces (#1089).
Removing the suspend modifier does not fix the issue. Not sure if it's the same underlying cause.

@krzdabrowski
Copy link

krzdabrowski commented May 25, 2023

Basically facing the same issue as @Wrakor for a quite long time and it is very annoying.

More example showing the issue can be found in my project's test class.

From my experience, the combination below fails when mocking:

  • fun interface
  • suspend
  • kotlin.Result

However, in commercial projects where I use fun interface, suspend and this kotlin-result external library, it does not happen at all.

@Wrakor
Copy link

Wrakor commented Sep 21, 2023

Any updates?

There is something really lacking here.

I've noticed that If I have the following UseCase:

fun interface GetNearbyPostalCodesUseCase {
    suspend operator fun invoke(address: Address?): Result<List<PostalCode>>
}

This mock works without any issue:
coEvery { getNearbyPostalCodesUseCase(null) } returns Result.success(mockPostalCodes)

But if I rewrite the same interface as
fun interface GetNearbyPostalCodesUseCase : suspend (Address?) -> Result<List<PostalCode>>

It will have the error mentioned at the start of this thread.

java.lang.ClassCastException: class java.util.Collections$SingletonList cannot be cast to class kotlin.Result (java.util.Collections$SingletonList is in module java.base of loader 'bootstrap'; kotlin.Result is in unnamed module of loader 'app')

@boiler23
Copy link

Are there any updates?
The issue prevents us from designing clean APIs, as we have to adjust them to avoid value classes in return types. This isn't very pleasant :(

@abcarrell
Copy link

I struggled with this as well, until I realized that, since a fun interface is at its heart still an interface, it's not something that needs to be mocked; a fake object can be used more easily.

For example, in the example above:

fun interface GetNearbyPostalCodesUseCase : suspend (Address?) -> Result<List<PostalCode>>

we can simply define the following in our test:

val getNearbyPostalCodesUseCase = GetNearbyPostalCodesUseCase {
    Result.success(mockPostalCodes)
}

and call getNearbyPostalCodesUseCase as a function wherever needed.

@Wrakor
Copy link

Wrakor commented Jan 23, 2024

I struggled with this as well, until I realized that, since a fun interface is at its heart still an interface, it's not something that needs to be mocked; a fake object can be used more easily.

For example, in the example above:

fun interface GetNearbyPostalCodesUseCase : suspend (Address?) -> Result<List<PostalCode>>

we can simply define the following in our test:

val getNearbyPostalCodesUseCase = GetNearbyPostalCodesUseCase {
    Result.success(mockPostalCodes)
}

and call getNearbyPostalCodesUseCase as a function wherever needed.

Yes, you can manually define the use case, but that would be a workaround of the underlying issue, which is not being able to mock with coEvery as you do for every other case.

@aaaleem
Copy link

aaaleem commented Mar 13, 2024

Any updates on this?

@nick-titov-sw
Copy link

@Raibaz will you be able to take a look on this issue or at least point me into the direction where to fix it? It's a real blocker for my team in order to finish the migration from the Mockito and I would love to help to solve it. Thank you and appreciate your help! 🙏

@Raibaz
Copy link
Collaborator

Raibaz commented Aug 13, 2024

While I think @abcarrell's reasoning is correct and mocking interfaces is generally not a good idea, I understand the need to fix this issue: perhaps looking into the changes that were introduced in #1253 and around that part of the codebase could help.

@mkotsbakws
Copy link

mkotsbakws commented Oct 10, 2024

Here is a reproduction of the issue: https://github.com/mkotsbakws/mockk-reproducer
I ran "git bisect" and found the commit that broke it is actually:

commit a6e12ec5f41d6790827e0b901e00ff06e6810814
Author: Felipe <[email protected]>
Date:   Thu Jan 11 19:15:08 2024 +0100

    Add support to nested value classes

 .../src/jvmMain/kotlin/io/mockk/core/ValueClassSupport.kt     |  2 +-
 .../mockk/src/commonTest/kotlin/io/mockk/it/ValueClassTest.kt | 11 -----------
 2 files changed, 1 insertion(+), 12 deletions(-)

which is from PR #1202 @Raibaz
Version 1.13.9 works fine.

@Raibaz
Copy link
Collaborator

Raibaz commented Oct 10, 2024

Makes sense, #1202 adds an automatic unboxing to support nested value class, which breaks this case.

We should add logic to determine whether a value class has another nested value class inside and do the autounboxing only in that case; I'd gladly review a PR that adds this.

@dadadom
Copy link

dadadom commented Oct 10, 2024

Just to add to this, I think it's related.

The following works wit 1.13.12 but fails with 1.13.13:

    @JvmInline
    private value class MyId(private val id: String)

    private interface Y {
        suspend fun getId(): MyId?
    }

    private class X(private val y: Y) {
        suspend fun doStuff(): MyId? = y.getId()
    }

    @Test
    fun `test stuff`() {
        val mocked = mockk<Y>()
        coEvery { mocked.getId() } returns MyId("987")

        runBlocking { X(mocked).doStuff() } shouldBe MyId("987")
    }

Error message:

12:26:43.893 [Test worker] DEBUG io.mockk.impl.instantiation.AbstractMockFactory -- Creating mockk for Y name=#1
12:26:44.895 [Test worker @coroutine#4] DEBUG io.mockk.impl.recording.states.AnsweringState -- Answering MyId(id=987) on Y(#1).getId-oOl4dpc(continuation {})

class MyTest$MyId cannot be cast to class java.lang.String (MyTest$MyId is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap')
java.lang.ClassCastException: class MyTest$MyId cannot be cast to class java.lang.String (MyTest$MyId is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap')
	at MyTest$X.doStuff-oOl4dpc(MyTest.kt:17)
	at MyTest$test stuff$2.invokeSuspend(MyTest.kt:25)
...

@mkotsbakws
Copy link

I tried adding a test to reproduce the issue to the test suite: #1309 but check runs requires maintainer approval @Raibaz.

@mkotsbakws
Copy link

@dadadom Might be the same, or a different: #1308?

@dadadom
Copy link

dadadom commented Oct 10, 2024

@dadadom Might be the same, or a different: #1308?

I am also not sure if it is this one, #1308 or the issue tackled in PR #1295 .
As these issues, mainly with value classes appear(ed) in different places at different times it is a bit hard to keep track.

@mkotsbakws
Copy link

@dadadom Might be the same, or a different: #1308?

I am also not sure if it is this one, #1308 or the issue tackled in PR #1295 . As these issues, mainly with value classes appear(ed) in different places at different times it is a bit hard to keep track.

Yes, I tried to reproduce with wrapped value class in value class (including generic parameter), but did not, also with async, but only succeeded with Result. Not sure what is special with Result. Maybe "@kotlin.internal.InlineOnly" or that wrapped value is only available through getter.

@mkotsbakws
Copy link

I noticed there are a lot of tests related to value classes that are disabled. Errors expected then...

kpadhiamex pushed a commit to kpadhiamex/mockk that referenced this issue Jan 3, 2025
…ses with coEvery.

Enable few disabled tests for support nested value classes (issue: 859)
Raibaz added a commit that referenced this issue Jan 4, 2025
…value-classes-with-coEvery

Fix( Issue #1073): Bug fix for the issue with mocking value classes with coEvery
@simeonApproppo
Copy link

Is there a plan for a next release?
We would love to see a release with this fix! 👀 ❤️

@Raibaz
Copy link
Collaborator

Raibaz commented Jan 10, 2025

Just published v1.13.16 :)

@swa-chacor
Copy link

swa-chacor commented Jan 10, 2025

still failing using 1.13.16

	interface InterfaceWithResult {
		suspend fun getResult(): Result<Boolean>
	}

	@Test
	fun `test failing`() = runTest {
		val mockedInterface = mockk<InterfaceWithResult>()

		coEvery {
			mockedInterface.getResult()
		} returns Result.success(false)

		val result = mockedInterface.getResult().getOrNull() // failing here
	}

getting the error

class kotlin.Result cannot be cast to class java.lang.Boolean (kotlin.Result is in unnamed module of loader 'app'; java.lang.Boolean is in module java.base of loader 'bootstrap')

@sschuberth
Copy link

class kotlin.Result cannot be cast to class java.lang.Boolean

Yeah, looks like this only works for Strings.

At the same time, the PR seems to break some regular cases.

@kpadhiamex
Copy link

@swa-chacor
Could you please see if you can use mockedInterface.getResult().isSuccess instead of mockedInterface.getResult().getOrNull()?

If you want to use mockedInterface.getResult().getOrNull(), can you define your interface like below which should work.

```
interface InterfaceWithResult<T> {
    suspend fun getResult(): T
}

@Test
fun `test failing`() = runTest {
    val mockedInterface = mockk<InterfaceWithResult<Result<Boolean>>>()

    coEvery {
        mockedInterface.getResult()
    } returns Result.success(false)

    val result = mockedInterface.getResult().getOrNull() // failing here
    assertEquals(result, false)
    assertTrue(mockedInterface.getResult().isSuccess)
}

@kpadhiamex
Copy link

@sschuberth
I will try to see your issue and the use case.

@swa-chacor
Copy link

@kpadhiamex it could technically work but I don't want to change my interface to generics.
@sschuberth this is the same issue that I had, I will follow the issue you created thanks

@nico-gonzalez
Copy link

We started experiencing issues with many of our tests after the bump to 1.13.16

class kotlin.Result cannot be cast to class com.core.domain.model.Onboarding (kotlin.Result and com.domain.model.Onboarding are in unnamed module of loader 'app')

these fails when calling a suspend operator fun invoke(): Result<Onboarding> function, like this one

internal class GetOnboarding  {

    suspend operator fun invoke(): Result<Onboarding>  {
        return enrollRepository.getOnboarding()
    }
}

marcelstoer pushed a commit to marcelstoer/mockk that referenced this issue Jan 15, 2025
…ses with coEvery.

Enable few disabled tests for support nested value classes (issue: 859)
@kpadhiamex
Copy link

@Raibaz
It looks like, there is no straight forward fix for issue# 1073, I am planning to revert partially the changes done as part of #1332 as this is causing more issues than fixing one use case. I need your input before raising a new PR to revert the fix for issue# 1073. Please let me know.

@Raibaz
Copy link
Collaborator

Raibaz commented Jan 15, 2025

@kpadhiamex makes sense, please go ahead with the revert.

Thanks!

@DBroli
Copy link

DBroli commented Jan 15, 2025

Hello, this bug still occurs on 1.13.16
When can we expect a fix? Thank you in advance.

@simeonApproppo
Copy link

v1.13.16 fixed our issue 👍

@iamironz
Copy link

The issue started reproducing after the update to 1.13.16, it was not faced in 1.13.14 though.

@luispollo
Copy link

luispollo commented Jan 30, 2025

Hi there! I'm seeing this with 1.13.12. What's the earliest version that does not have the issue?
If it helps, we only see ClassCastException when running parameterized tests with a lot of iterations. The mock works fine for the first several hundred iterations, then one iteration has the error, and all subsequent ones fail with the same exception.

@mkotsbakws
Copy link

You probably mean the latest? I did the bisect and concluded the latest working for us is 1.13.9.

@luispollo
Copy link

Yes, sorry, you were right. I meant the "earliest version going backwards". 🙂

FWIW, I was able to work around my problem as described here: #1098 (comment)

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

No branches or pull requests