mockito / mockito-kotlin

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

Currently incompatible with inline class #309

Open matthewrkula opened 5 years ago

matthewrkula commented 5 years ago

We want to experiment with inline classes in our application, but we are running into a compatibility issue with mockito-kotlin. I know inline classes are currently experimental and expected to change, so if it's not worth the effort looking into at the moment I understand, I just want to start a conversation about it.

Issue

I can repro my issue with a unit test that looks like this:

interface Methods {
    ...
    fun inlineClass(): InlineClass
}

inline class InlineClass(val value: String)

class MockingTest : TestBase() {
    @Test
    fun testMockStubbing_inlineClass() {
        /* Given */
        val mock = mock<Methods>() {
            on { inlineClass() } doReturn InlineClass("A")
        }

        /* When */
        val result = mock.inlineClass()

        /* Then */
        expect(result).toBe(InlineClass("A"))
    } 
}

I get a stack trace that looks like this

java.lang.IllegalArgumentException: Parameter specified as non-null is null: method test.InlineClass.box-impl, parameter v

    at test.InlineClass.box-impl(Classes.kt)
    at test.MockingTest$testMockStubbing_inlineClass$mock$1$1.invoke(MockingTest.kt:19)
    at com.nhaarman.mockitokotlin2.KStubbing.on(KStubbing.kt:70)
    at test.MockingTest.testMockStubbing_inlineClass(MockingTest.kt:94)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)

which points to where we call on { inlineClass() } or expect(result).

I tried to dig down with the debugger but couldn't really get too far into what's going on. I assume reflection is being used to create an instance of InlineClass but that has some core logic that an instance can't be created without a value?

Has anyone seen this or have any ideas of how to go about fixing this? Or should we wait until inline classes mature a little bit?

bohsen commented 5 years ago

Inline classes are not yet supported.

The example you provided is somewhat a corner case because you are using a String property in your inline class and Strings have shown to be somewhat troublesome regarding Mockito-Kotlin. Try replacing it with an Int:

@Test
fun testMockStubbing_inlineClass() {
    /* Given */
    val mock = mock<Methods>() {
        on { inlineClass() } doReturn InlineClass(42)
    }

    /* When */
    val result = mock.inlineClass()

    /* Then */
    assertEquals(InlineClass(42), result)
}

and you will get the following output:

org.mockito.exceptions.misusing.WrongTypeOfReturnValue: 
InlineClass cannot be returned by inlineClass()
inlineClass() should return int
***
mkobit commented 5 years ago

Any known workarounds today?

bohsen commented 5 years ago

@mkobit Use a fake instead of a mock.

mkobit commented 5 years ago

It looks like another option may be using Mockito's Answer type.

The below is working for me with mockito-kotlin:2.1.0 and mockito-core:2.24.5

    @Test
    fun `Answer type implementation with IntInlineClass`() {
        /* Given */
        class IntInlineClassAnswer(private val value: Int) : Answer<Any> {
            override fun answer(invocation: InvocationOnMock?): Any = value
        }
        val mock = mock<Methods>() {
            on { intInlineClass() } doAnswer IntInlineClassAnswer(42)
        }

        assertEquals(IntInlineClass(42), mock.intInlineClass())
        // Can't use doAnswer to type constraints of mockito-kotlin `doAnswer {}`
        //whenever(mock.intInlineClass()).doAnswer { 50 }
        whenever(mock.intInlineClass()).thenAnswer { 50 }
        assertEquals(IntInlineClass(50), mock.intInlineClass())
    }

Answer itself seems like a bit of an odd feature, but it seems to work here.

One possible issue with the above is the doAnswer and thenAnswer being type constrained, when it doesn't seem like the Answer API in Mockito really requires it?

infix fun <T> OngoingStubbing<T>.doAnswer(answer: Answer<*>): OngoingStubbing<T> {
    return thenAnswer(answer)
}

/**
 * Sets a generic Answer for the method using a lambda.
 */
infix fun <T> OngoingStubbing<T>.doAnswer(answer: (InvocationOnMock) -> T?): OngoingStubbing<T> {
    return thenAnswer(answer)
}
mkobit commented 5 years ago

The other side of inline class issues is from the matcher side. I have ran into quite a few annoyances when trying to replace some of our primitive types with inline class and seeing our tests fail in some strange ways.

I haven't tried creating a specialized matcher implementation to see what happens. Here is an example for getting methods that take inline class as parameters to work with matchers.

import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.whenever
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.MethodOrderer
import org.junit.jupiter.api.Order
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestMethodOrder
import org.mockito.ArgumentMatchers

interface Methods {
    fun inlineClassAsParam(inlineClass: InlineClass): Long
}

inline class InlineClass(val value: Long)

@TestMethodOrder(MethodOrderer.OrderAnnotation::class)
internal class MockitoInlineClassTest {

    private lateinit var mockMethods: Methods

    @BeforeEach
    internal fun setUp() {
        mockMethods = mock()
    }

    @Disabled("for example")
    @Test
    @Order(1)
    internal fun `mocking with any fails, and can put mockito engine in bad state`() {
         whenever(mockMethods.inlineClassAsParam(any())).thenReturn(-1L)
        assertEquals(-1L, mockMethods.inlineClassAsParam(InlineClass(1L)))
    }

    @Test
    @Order(0)
    internal fun `using ArgumentMatchers anyLong`() {
        whenever(mockMethods.inlineClassAsParam(InlineClass(ArgumentMatchers.anyLong())))
            .thenReturn(100L)
        assertEquals(100L, mockMethods.inlineClassAsParam(InlineClass(2L)))
    }

    @Test
    @Order(2)
    internal fun `using ArgumentMatchers longThat`() {
        whenever(mockMethods.inlineClassAsParam(InlineClass(ArgumentMatchers.longThat { it > 3L })))
            .thenReturn(100L)
        assertEquals(100L, mockMethods.inlineClassAsParam(InlineClass(10L)))
    }

    @Test
    @Order(3)
    internal fun `using ArgumentMatchers eq`() {
        whenever(mockMethods.inlineClassAsParam(InlineClass(ArgumentMatchers.eq(3L))))
            .thenReturn(100L)
        assertEquals(100L, mockMethods.inlineClassAsParam(InlineClass(3L)))
    }
}

When the @Disabled is left in, all the tests work for me on Mockito 2.26.0.

Wrapping the ArgumentMatchers return values in the InlineClass seems to be an OK workaround.

Using the above method ordering and commenting out @Disabled show org.mockito.exceptions.misusing.InvalidUseOfMatchersException and 2 tests fail, so it seems to be putting the Mockito engine in a bad state. The exception and stacktrace is non-obvious if you aren't using MockitoExtension or some form that invokes Mockito.validateMockitoUsage() (or validateMockitoUsage() from this library) method (which I would recommend using).

mkobit commented 5 years ago

@nhaarman I noticed going through some of this exploration that the many primitive matchers from ArgumentMatchers (like public static long eq(long value)) are not included. This makes it a little bit annoying using eq from this library and having to alias import the eq like import org.mockito.ArgumentMatchers.eq as argMatcherEq/don't use import com.nhaarman.mockitokotlin2.eq.

Were those overloads excluded as a design choice, or would it make sense to also add those overloaded methods, too?

wem commented 4 years ago

For me the following is working:

inline class InlineClass(val value: String)

fun matchesInlineClass(inlineClass: InlineClass) : InlineClass {
    return Mockito.argThat { arg: Any ->
        if (arg is String) {
            arg == inlineClass.value
        } else arg == inlineClass
    } as InlineClass? ?: inlineClass
}
jckoenen commented 4 years ago

Created this generic definition based on @wem's (Thank you!) workaround. The test parameter can of course be replaced with argWhere - but in my usecase I don't have access to mockito where this function is defined.

inline fun <Outer, reified Inner> eqInline(
    expected: Outer,
    crossinline access: (Outer) -> Inner,
    test: ((actual: Any) -> Boolean) -> Any?
): Outer {
    val assertion: (Any) -> Boolean = { actual ->
        if (actual is Inner) {
            access(expected) == actual
        } else {
            expected == actual
        }
    }

    @Suppress("UNCHECKED_CAST")
    return test(assertion) as Outer? ?: expected
}

As a drawback, you cannot reuse instances of Outer produced by this:

inline class SomeInlineClass(val value: Int)
class Mocked {
  fun toBeMatched(inlineInstance: SomeInlineClass): Any = TODO()
}

whenever(mocked.toBeMatched(eqInline(expected, SomeInlineClass::value, ::argWhere))) doAnswer ... // works

inline fun reusableAsFunction() = eqInline(expected, SomeInlineClass::value, ::argWhere)
whenever(mocked.toBeMatched(reusableAsFunction())) doAnswer ... // works too

val notReusableAsValue = eqInline(expected, SomeInlineClass::value, ::argWhere)
whenever(mocked.toBeMatched(notReusableAsValue)) doAnswer ... // does not work

Tested on mockito 3.4.0, mockito-kotlin 2.2.0

guitcastro commented 2 years ago

@mkobit Thanks for the workaround, it worked.

Noia commented 2 months ago

This is already partially solved in Mockito, at least for returning inline values: https://github.com/mockito/mockito/commit/8f95dcaa68e7bb76b597353739046274929da418

In org.mockito.internal.stubbing.answers.Returns, we see the use of org.mockito.internal.util.KotlinInlineClassUtil.unboxUnderlyingValueIfNeeded to handle inline values classes. This is why doReturn will work, but doAnswer won't.

Ideally this should be fixed in Mockito proper, by expanding the use of KotlinInlineClassUtil.unboxUnderlyingValueIfNeeded but an interim solution may be to do this in mockito-kotlin instead, since thats where the value classes crop up the most.

Supposedly this works as a work around: https://github.com/mockito/mockito/pull/2280#issuecomment-1023413543

So a change doAnswer could simply be to include this workaround class as so:

/**
 * Sets a generic Answer for the method.
 *
 * Alias for [OngoingStubbing.thenAnswer].
 */
infix fun <T> OngoingStubbing<T>.doAnswer(answer: Answer<*>): OngoingStubbing<T> {
    return thenAnswer(InlineClassesAnswer(answer))
}

I've tested similar solutions myself, and they work. It might be possible to also use this for matchers & arguments but I haven't looked at that.

chrisjenx commented 1 month ago

@jckoenen thanks!