mockito / mockito-kotlin

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

Proposal: improve support for Strictness.STRICT_STUBS #315

Closed MyDogTom closed 5 years ago

MyDogTom commented 5 years ago

Strictness.STRICT_STUBS

Ensures clean tests, reduces test code duplication, improves debuggability. Offers best combination of flexibility and productivity. Highly recommended. Planned as default for Mockito v3. Enable it via our JUnit support (MockitoJUnit) or MockitoSession.

from JavaDoc

Example without Strictness.STRICT_STUBS

I have the following interface and class.

interface Foo {
    fun doFoo(value: Int): String
}

class Boo(private val foo: Foo) {
    fun doBoo(value: Int): String {
        return foo.doFoo(value) + " times"
    }
}

Test, might look like that. But it has several problems (see comments):

class ExampleTest {
    val mockedFoo: Foo = mock()
    lateinit var booUnderTest: Boo

    @Before
    fun setUp() {
        booUnderTest = Boo(mockedFoo)
        whenever(mockedFoo.doFoo(5)).thenReturn("five")
        // Problem: leftover after modification. should be removed.
        whenever(mockedFoo.doFoo(3)).thenReturn("three")
    }

    @Test
    fun withEmptyString() {
        assertThat(booUnderTest.doBoo(5)).isEqualTo("five times")

        // Problem: not really needed since we verified value
        verify(mockedFoo).doFoo(5)
        // Problem: we have to write it explicitly
        verifyNoMoreInteractions(mockedFoo)
    }
}

Another problem. When I change booUnderTest.doBoo(5) to booUnderTest.doBoo(6), I receive an error

org.junit.ComparisonFailure: 
Expected :"five times"
Actual   :"null times"

Which doesn't point me directly to the problem.

Example with Strictness.STRICT_STUBS

class ExampleTest {
    @get:Rule
    val rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS)
    lateinit var mockedFoo: Foo
    lateinit var booUnderTest: Boo

    @Before
    fun setUp() {
        mockedFoo = mock()
        booUnderTest = Boo(mockedFoo)
        whenever(mockedFoo.doFoo(5)).thenReturn("five")
        // Problem: left over after modification. could be removed.
        whenever(mockedFoo.doFoo(3)).thenReturn("three")
    }

    @Test
    fun withEmptyString() {
        assertThat(booUnderTest.doBoo(5)).isEqualTo("five times")
    }
}

Mockito tells me, that I have unused configuration :

org.mockito.exceptions.misusing.PotentialStubbingProblem: 
Strict stubbing argument mismatch. Please check:
 - this invocation of 'doFoo' method:
    foo.doFoo(3);
    -> at ExampleTest.setUp(ExperimentsTest.kt:61)
 - has following stubbing(s) with different arguments:
    1. foo.doFoo(5);
      -> at ExampleTest.setUp(ExperimentsTest.kt:59)

This allows me to keep my tests clean. And If I change booUnderTest.doBoo(5) to booUnderTest.doBoo(6), Mockito tells me:

org.mockito.exceptions.misusing.PotentialStubbingProblem: 
Strict stubbing argument mismatch. Please check:
 - this invocation of 'doFoo' method:
    foo.doFoo(6);
    -> at Boo.doBoo(ExperimentsTest.kt:21)
 - has following stubbing(s) with different arguments:
    1. foo.doFoo(5);
      -> at ExampleTest.setUp(ExperimentsTest.kt:59)

This points me to exact problem.

Another reason to use Strictness.STRICT_STUBS

In Mockito 3.0 all stubbings will be "strict" and validated.

Source. I'd like to already write tests, that are ready bot Mockit 3.0.

What's missing?

In second example, I had to remove val mockedFoo: Foo = mock() and call mock inside setUp method. This is because mock should be created after Rule is executed.

Proposal

Create a lazy variant of Mocking.mock. Something like lazyMock:

inline fun <reified T : Any> lazyMock(
        extraInterfaces: Array<KClass<out Any>>? = null,
        name: String? = null,
        spiedInstance: Any? = null,
        defaultAnswer: Answer<Any>? = null,
        serializable: Boolean = false,
        serializableMode: SerializableMode? = null,
        verboseLogging: Boolean = false,
        invocationListeners: Array<InvocationListener>? = null,
        stubOnly: Boolean = false,
        @Incubating useConstructor: UseConstructor? = null,
        @Incubating outerInstance: Any? = null
): Lazy<T> {
    return lazy {
        mock<T>(
                extraInterfaces,
                name,
                spiedInstance,
                defaultAnswer,
                serializable,
                serializableMode,
                verboseLogging,
                invocationListeners,
                stubOnly,
                useConstructor,
                outerInstance
        )
    }
}

This allows test like that:

class ExampleTest {
    @get:Rule
    val rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS)
    val mockedFoo: Foo by lazyMock()
    val booUnderTest: Boo by lazy { Boo(mockedFoo) }

    @Test
    fun withEmptyString() {
        whenever(mockedFoo.doFoo(5)).thenReturn("five")
        booUnderTest.doBoo(5)
    }
}

Questions

  1. Perhaps, I'm missing other ways how to achieve the same? (enable STRICT_STUBS and create mock when field is defined)
  2. Are you open for such contribution? I'd happy to open a PR.
nhaarman commented 5 years ago

First of all, thanks for your detailed proposal! The STRICT_STUBS feature definitely seems like the way to go.

I'm not really sure however about the lazyMock solution. It would be just as easy to write val mock by lazy { mock<Foo>() }, right?

I feel like adding lazyMock introduces another 'layer of complexity' to the library possibly leading to confusion, whereas using by lazy { mock() } seems like a explicit decision of the test author.

Do you know if the folks over at Mockito have run into this problem? In plain Java you should be able to write private Foo foo = Mockito.mock(Foo.class); as well, leading to the same problem?

MyDogTom commented 5 years ago

Yes, you are right val mock by lazy { mock<Foo>() } is enough. It's a bit more verbose, but I agree it's more explicit.

I asked also Mockito people, but no answer so far. My question on StackOverflow: https://stackoverflow.com/q/52345881/2711056 My question in mailing list (asked today): https://groups.google.com/forum/#!topic/mockito/X7jkA4BdzSk