mockito / mockito-kotlin

Using Mockito with Kotlin
MIT License
3.11k stars 202 forks source link

Mockito 5.0 breaks vararg argument capture in Kotlin #474

Closed emartynov closed 1 year ago

emartynov commented 1 year ago

Hence issue was wrongly put on the mockito project.

Have code:

class PreferencesRepository {
    suspend fun getPreferences(
        vararg preferences: String
    ): Map<String, Any> 
{}

Write tests:

        val preferencesRepository: PreferencesRepository = mock()
        val preferencesCapture = argumentCaptor<String>()
        whenever(preferencesRepository.getPreferences(anyVararg())) doReturn emptyMap()

        verify(preferencesRepository).getPreferences(preferencesCapture.capture())

Run test and observe:

at PreferencesRepository.getPreferences(PreferecesRepository.kt:11)
Actual invocations have different arguments:
preferencesRepository.getPreferences(
    [String, String]
);
emartynov commented 1 year ago

Moved from https://github.com/mockito/mockito/issues/2878

TimvdLippe commented 1 year ago

If anybody would like to stab at this, I am happy to review a PR with a regression test + fix.

kschults commented 1 year ago

I see that the PR 2807 got merged, which looks like it's trying to fix this. Any estimate on when this will be available?

TimvdLippe commented 1 year ago

@kschults I think 2807 is the PR that introduced the problems for Kotlin (while it works fine for Java).

kschults commented 1 year ago

Oh, I see, I misread the timeline. Thanks.

samabcde commented 1 year ago

Come up with this workaround. Honestly speaking, I don't really understand why it works.

inline fun <reified T : Any, reified E : Any> anyVararg(clazz: KClass<E>): T {
    return ArgumentMatchers.any(T::class.java)
          ?: arrayOf(createInstance(E::class)) as T
}

// below methods are from org/mockito/kotlin/internal/CreateInstance.kt
fun <T : Any> createInstance(kClass: KClass<T>): T {
    return castNull()
}
@Suppress("UNCHECKED_CAST")
private fun <T> castNull(): T = null as T
    @Test
    fun testVararg() {
        val preferencesRepository: PreferencesRepository = mock()
        val preferencesCapture = argumentCaptor<String>()
        // not the * before anyVararg
        whenever(preferencesRepository.getPreferences(*anyVararg(String::class))) doReturn emptyMap()
        preferencesRepository.getPreferences("works!")
        verify(preferencesRepository).getPreferences(preferencesCapture.capture())
        assert(preferencesCapture.lastValue == "works!")
    }
lukas-krecan commented 1 year ago

This might help https://github.com/mockito/mockito-kotlin/pull/482/files#diff-1354c4c5b4595d316a42f5172d59654a6d90a1516f7b8ee0fd116ef45c3d3eb3R62

TimvdLippe commented 1 year ago

Fixed in #482

emartynov commented 1 year ago

@TimvdLippe @lukas-krecan What about the vararg argument capturing for Kotlin?

emartynov commented 1 year ago

The original code still needs to be fixed - even it fails with a similar error. Please re-open.

lukas-krecan commented 1 year ago

Yes, it's indeed still broken

emartynov commented 1 year ago

@TimvdLippe please reopen

TimvdLippe commented 1 year ago

Instead of anyVararg I think you need to use any now according to the Mockito 5 release notes. Can you try that out?

emartynov commented 1 year ago

@TimvdLippe The problem is not in the mocking but in the line with argument capturing. Or I understood your comment wrongly.

snyman commented 1 year ago

To perhaps clarify the issue, you can capture varargs if you know how many are being passed in, e.g. I'm pretty sure the example in the OP could be fixed with

verify(preferencesRepository).getPreferences(preferencesCapture.capture(), preferencesCapture.capture())

Which will capture both arguments passed in for that instance, in the same way the previous behavior seemed to work.

The problem is that there is now no way to capture all varargs regardless of how many are actually passed in, which is an annoyance if you're trying to write e.g. a helper method that will get used in multiple situations that result in differing numbers of arguments that need to be captured.

kschults commented 1 year ago

Yeah. We've ended up with places where we have to write:

        whenever(myMock.doThing(any())).thenAnswer { it.arguments[0] }
        whenever(myMock.doThing(any(), any())).thenAnswer { it.arguments[0] }
        whenever(myMock.doThing(any(), any(), any())).thenAnswer { it.arguments[0] }
        whenever(myMock.doThing(any(), any(), any(), any())).thenAnswer { it.arguments[0] }

It works, but it's a little silly.

lukas-krecan commented 1 year ago

This seems to be working. Please check the tests if they reflect your use-case.

emartynov commented 1 year ago

@lukas-krecan, they do reflect our case!

kschults commented 1 year ago

This is awesome to see. When do you expect a release containing this fix to be available?

TimvdLippe commented 1 year ago

5.1.0 should be published later today