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

[Kotlin] thenThrow(Throwable) should be able to stub the method with any checked exceptions #1166

Open
tmurakami opened this issue Aug 15, 2017 · 11 comments

Comments

@tmurakami
Copy link
Contributor

@tmurakami tmurakami commented Aug 15, 2017

Hi,

Kotlin does not have checked exceptions.
https://kotlinlang.org/docs/reference/exceptions.html#checked-exceptions

However, ThrowsException#validateFor(InvocationOnMock) does not allow the checked exception that does not match the method signature.
Therefore, the following test fails.

class MyTest {

    @Test(expected = MyException::class)
    fun test() {
        val mock = Mockito.mock(I::class.java)
        Mockito.`when`(mock.doIt()).thenThrow(MyException()) // This line throws a MockitoException
        mock.doIt()
    }

    interface I {
        fun doIt(): String
    }

    class MyException : Exception()

}

I'm going to file the PR to fix this.

@tmurakami

This comment has been minimized.

Copy link
Contributor Author

@tmurakami tmurakami commented Aug 26, 2017

This issue was more difficult to solve than I thought, so I closed my PR.
Sorry...

@ChristianSchwarz

This comment has been minimized.

Copy link
Contributor

@ChristianSchwarz ChristianSchwarz commented Oct 11, 2017

I looked into your PR, what was the problem you encountered?

@tmurakami

This comment has been minimized.

Copy link
Contributor Author

@tmurakami tmurakami commented Oct 11, 2017

I think that PR(#1167) has two problems related to Kotlin-Java interoperability.

Problem 1

The following code is correct in Kotlin.

// Kotlin
val runnable = object: java.lang.Runnable {
    override fun run() {
        throw java.io.IOException()
    }
}

The above code is equivalent to the following code using Mockito.

// Kotlin
val runnable = Mockito.mock(java.lang.Runnable::class.java)
Mockito.doThrow(java.io.IOException()).`when`(runnable).run()

However, this code will cause MockitoException: Checked exception is invalid ... because java.lang.Runnable is not marked with the kotlin.Metadata annotation.

Problem 2

Consider the following a Kotlin interface and a Java test.

// Kotlin
interface KotlinInterface {
    fun doIt()
}
// Java
KotlinInterface mock = Mockito.mock(KotlinInterface.class);
Mockito.doThrow(new java.io.IOException()).when(mock).doIt();

Since the signature of KotlinInterface#doIt() does not have checked exceptions, doThrow should throw MockitoException: Checked exception is invalid ....
However, actually, it will not be thrown because KotlinInterface is marked with the kotlin.Metadata annotation.

@ChristianSchwarz

This comment has been minimized.

Copy link
Contributor

@ChristianSchwarz ChristianSchwarz commented Oct 12, 2017

Good explanation! Your PR solved already Problem 2 right? Problem 1 is tricky, as mockito doesn't know that it runs a kotlin test. But this can be solved as follows:

  • MockitoRule and TestRunner know the JUnit-Test class it runs on.
  • They set a flag (possibly in MockitoConfiguration) when a kotlin test is detected.
  • When the validation of ThrowsException is called, it skip any checks if the kotlin flag is set.
@tmurakami

This comment has been minimized.

Copy link
Contributor Author

@tmurakami tmurakami commented Oct 13, 2017

Your PR solved already Problem 2 right?

No, that PR did not solve both problems.

But this can be solved as follows:

Thank you.
However, I think that it is necessary to consider users who use testing framework other than JUnit 4.
For those users, Mockito offers MockitoSession, but it cannot know the test class unless MockitoSessionBuilder#initMocks(Object) is called.

@ChristianSchwarz

This comment has been minimized.

Copy link
Contributor

@ChristianSchwarz ChristianSchwarz commented Oct 13, 2017

However, I think that it is necessary to consider users who use testing framework other than JUnit 4.
For those users, Mockito offers MockitoSession, but it cannot know the test class unless MockitoSessionBuilder#initMocks(Object) is called.

Right now I don't see a problem here. The MockitoSessionBuilder implementation can set the flag too. WDYT?


An other option is to disable the exception type check for a mock explicit. E.g. like this:

@Mock(disableThrowsValidation=true)
private Dependency mock

or

mock = mock(Dependency.class, withSettings().disableThrowsValidation());
@tmurakami

This comment has been minimized.

Copy link
Contributor Author

@tmurakami tmurakami commented Oct 14, 2017

Right now I don't see a problem here. The MockitoSessionBuilder implementation can set the flag too. WDYT?

I am not good at English 😓, so I'm sorry if I misunderstood your intention.

I assume that the general usage of the Mockito Session API is as follows:

// Kotlin
private lateinit var mockitoSession: MockitoSession

mockitoSession = Mockito.mockitoSession().initMocks(testInstance).startMocking()

As Mockito users call MockitoSessionBuilder#initMocks(Object), Mockito get to know the test instance, so it will also be able to detect Kotlin tests, just like using MockitoRule/MockitoJUnitRunner.
However, I think there are two problems with this way.

  1. Calling initMocks() is not mandatory

MockitoSession can be generated without calling initMocks().

// Kotlin
mockitoSession = Mockito.mockitoSession().startMocking()

Actually, my Kotlin project using JUnit 5 does not use Mockito annotations, so I am using that API as above.

  1. Method name mismatch

It seems odd to me to call a method named initMocks just to let Mockito know the test instance.

If MockitoSession API were like the following code, I might have filed a new PR to detect Kotlin tests without worrying about it.

// Kotlin
mockitoSession = Mockito.mockitoSession(testInstance).startMocking()

An other option is to disable the exception type check for a mock explicit.

Ah I see, it seems better than the way to detect Kotlin tests.
Your option is available for not only Kotlin but also Alt Java which does not have checked exceptions.
However, it seems a bit troublesome to set that flag each time making a mock object...

@holubec-petr

This comment has been minimized.

Copy link

@holubec-petr holubec-petr commented Jan 5, 2018

Hi guys,
I hope that this issue is not stucked :-) We have quite big project in Kotlin and because of this issue we cannot use actual version of Mockito (last usable is 2.10.0). We don't want to use @throws annotation everywhere, it's against Kotlin ideology.

@jibidus

This comment has been minimized.

Copy link

@jibidus jibidus commented Jun 21, 2018

Here is a workaround :

doAnswer { throw MyException() }
            .`when`(mock)
            .doIt()
@mockitoguy

This comment has been minimized.

Copy link
Member

@mockitoguy mockitoguy commented Jun 24, 2018

@jibidus, thank you for providing a workaround!

Can someone summarize the state of this problem? Is it a big issue, how people deal with it at the moment? What would be the preferred solution from the standpoint of a Kotlin user?

@ChristianSchwarz

This comment has been minimized.

Copy link
Contributor

@ChristianSchwarz ChristianSchwarz commented Jun 25, 2018

@mockitoguy

Is it a big issue, how people deal with it at the moment?

  • Some users reject Mockito versions with "throw-type-checking" because the workarounds are not known or inaccepable for there project. The expectation is that the same API as for java can be used.
  • I guess Mockito-Kotlin doesn't solve it too, cause its OngoingStubbing implementation delegates to Mockito.

What would be the preferred solution from the standpoint of a Kotlin user?

Mockito have more issues that don't fit into the Kotlin world:

  1. BLOCKER: Methods with Non-Nullable-Args are the default in Kotlin, therefore Argument-Matchers must return non-null values in the majority of cases, otherwise you get an exception at call time even when null is passed to a mocked instance.
  2. Methods can throw checked exceptions without declaring them (this ticket)
  3. when is a keyword in Kotlin, so it must be surrounded by `` (looks ugly)

Mockito-Kotlin fixes 2. & 3. and has the ability to provide a solution for 3. too.

When Mockito fixed this issue, it should fix 1. also make Mockito Kotlin "compatible" / usable. IMHO this is a can of worms cause Kotlin evolves fast and other languages may have there own requirements or compaility issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.