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

Support test methods with Kotlin suspend modifier #1914

Open
IRus opened this issue Jun 4, 2019 · 20 comments
Open

Support test methods with Kotlin suspend modifier #1914

IRus opened this issue Jun 4, 2019 · 20 comments

Comments

@IRus
Copy link

IRus commented Jun 4, 2019

Goals

Support running suspend test in JUnit Jupiter:

class Kit {
    @Test 
    suspend fun foo() {
        delay(1000) // suspend call
        assertEquals(1, 1)
    } 
}

Currently, such test can be written this way:

class Kit {
    @Test 
    fun foo() = runBlocking {
        delay(1000) // suspend call
        assertEquals(1, 1)
        assertThrows { /* ... */ } 
        Unit // test should return void, but `assertThrows` returns `Throwable`, so `foo` returns `Throwable` too 
    } 
}

Also, will be nice to provide CoroutineScope through params, or as receiver in extension:

class Kit {
    suspend fun foo(scope: CoroutinesScope) {  /* ... */  } // (1)
    suspend fun CoroutinesScope.foo() {  /* ... */  } // (2)
}

1 and 2 actually the same on bytecode level. suspend is optional.

And finally, support for runBlockingTest:

class Kit {
    suspend fun foo(scope: TestCoroutinesScope) {  /* ... */  }
    suspend fun TestCoroutinesScope.foo() {  /* ... */  } 
}

What can be done currently

ParameterResolver can be used to provide stubs for Continuation, CoroutineScope and TestCoroutineScope. These stub arguments can be replaced with real arguments in invocation.

Problems

Current extensions points not enough to implement this feature as extensions, since:

  1. Discovery. Jupiter discovers tests that returns void, but suspend fun returns Object;
  2. Invocation. InvocationInterceptor in 5.5-M1(SNAPSHOT) don't providing mechanism to override actual invocation, only to decoration of existing invocation. Conversion of method to kotlinFunction, and then executing using callSuspend is necessary to execute suspend fun.

Also, my slides about this topic.

@sormuras
Copy link
Member

sormuras commented Jun 4, 2019

Related to #1851

@marcphilipp
Copy link
Member

@IRus This is not resolved by #2042, is it?

@IRus
Copy link
Author

IRus commented Mar 20, 2020

@marcphilipp Yes, not resolved, this is different issues

@xeruf
Copy link

xeruf commented Feb 4, 2021

Please resolve this! My tests looked all fine until I discovered that they were not run at all anymore after adding suspending functions :/

@marcphilipp marcphilipp removed this from the 5.8 Backlog milestone Jun 19, 2021
@checketts
Copy link

An alternate work around is to create a wrapper around runBlocking() that returns Unit

    fun suspendingTest(context: CoroutineContext = EmptyCoroutineContext, block: suspend CoroutineScope.() -> Any): Unit {
        runBlocking(context, block)
        Unit
    }

That will allow tests to use suspending methods:

    @Test
    fun `a suspending test should compile and be found`() = suspendingTest {
        assertThat(thing()).isEqualTo("a string")
    }

    suspend fun thing() = "a string"

Would adding that suspendingTest method be useful for JUnit or would supporting tests with suspend be preferred?

@IRus
Copy link
Author

IRus commented Oct 8, 2021 via email

@checketts
Copy link

@IRus Thanks for pointing that out! At least now this issue documents it as an option.

@EanLombardo
Copy link

I would be careful with using runBlockingTest. It does a lot of things, one of them is overriding the behavior of delay(), to allow the test to control the passage of time across coroutines. By default it makes all calls to delay() a no-op.

Anyone who needs to wait in a test will get unexpected behavior if they use delay (which would be the recommended way of doing it).

@jlink
Copy link
Contributor

jlink commented Oct 24, 2021

Might be interesting to know that I implemented @IRus' suggestion in jqwik.net. With the extension point (aka lifecycle hook) for invokeMethods present, the implementation is rather simple: https://github.com/jlink/jqwik/blob/main/kotlin/src/main/kotlin/net/jqwik/kotlin/internal/SuspendedPropertyMethodsHook.kt

@IRus I did not implement parameter resolution for CoroutineScope and TestCoroutineScope. In which cases would these be necessary?

@IRus
Copy link
Author

IRus commented Oct 24, 2021

@jlink I wonder how you work around discovery issue: suspend methods returns Object

Scope is nice to have to set test dispatcher by default to TestCoroutineDispatcher (which supports "time" manipulations) for example.

@jlink
Copy link
Contributor

jlink commented Oct 25, 2021

I wonder how you work around discovery issue: suspend methods returns Object

jqwik doesn't have that problem since it allows any return type in test/property methods.

@IRus
Copy link
Author

IRus commented Oct 25, 2021

I wonder how you work around discovery issue: suspend methods returns Object

jqwik doesn't have that problem since it allows any return type in test/property methods.

So it's an engine on its own, ok

@jmfayard
Copy link

This is a recurrent source of bugs for us.
A correct test that test a buggy piece of production code get ignored because of this.
Would be nice to see junit supports this.
In the meantime, I opeend a ticket to prevent IntelliJ users against that mistake

https://youtrack.jetbrains.com/issue/KT-52818/Provide-a-quick-fix-against-using-suspending-functions-in-Unit-Test

@sbrannen sbrannen changed the title Support for tests with suspend modifier Support test methods with Kotlin suspend modifier Jun 16, 2022
@ephemient
Copy link

I have discovered a way to make suspend work with JUnit Jupiter tests by bridging them through a @TestTemplate.
https://gist.github.com/ephemient/01d6e5766e6f8ea02839b4d7c3f94e55
However, this is not ideal as it doesn't play well with other extensions (e.g. #378).
There really needs to be either better extension points or built-in support for this.

@allertonm
Copy link

allertonm commented Jun 27, 2023

I also ran into this because I had the idea of trying to write a TestMethodInvoker that worked with the Quarkus test extensions that would handle setting up Vertx & Panache session context for suspend functions similar to the existing @RunOnVertxContext annotation that works with Mutiny.Uni. This would appear to be perfectly possible if it were not for JUnit ignoring non-void methods - because of the logic in IsTestableMethod - and there being no way for an extension to override this.

(I suspect this limitation also explains the cumbersome UniAsserter mechanism that @RunOnVertxContext uses, rather than having test methods that return Uni.)

@ianthetechie
Copy link

For anyone coming from a search engine, this is still not supported, but it looks possible using runTest, which replaces the deprecated runBlockingTest.

@marcphilipp
Copy link
Member

Has anyone tried using the new runTest?

@JavierSegoviaCordoba
Copy link

I am using it.

Adding suspend support is not straightforward as runTest accepts parameters and add a receiver to the scope, which would be missing by only adding suspend to the test.

@ianthetechie
Copy link

Yes runTest works fine. It’s just rather confusing that it’s not supported out of the box at this point.

@viluon
Copy link

viluon commented Jan 4, 2024

runTest is great...

... until you need to integrate with Lincheck and would like to run suspending set up code in @BeforeAll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests