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

runBlockingTest fails with "This job has not completed yet" #1204

Open
PaulWoitaschek opened this issue May 17, 2019 · 14 comments
Labels

Comments

@PaulWoitaschek
Copy link
Contributor

@PaulWoitaschek PaulWoitaschek commented May 17, 2019

This simple block:

  runBlockingTest {
   suspendCancellableCoroutine<Unit> { cont ->
     thread {
       Thread.sleep(1000)
       cont.resume(Unit)
     }
   }
 }

Fails with the exception:

Exception in thread "main" java.lang.IllegalStateException: This job has not completed yet
    at kotlinx.coroutines.JobSupport.getCompletionExceptionOrNull(JobSupport.kt:1114)
    at kotlinx.coroutines.test.TestBuildersKt.runBlockingTest(TestBuilders.kt:53)
    at kotlinx.coroutines.test.TestBuildersKt.runBlockingTest$default(TestBuilders.kt:45)
    at SamplKt.main(Sampl.kt:8)
    at SamplKt.main(Sampl.kt)

This looks like a bug to me as it should pass.

Kotlin 1.3.31
Coroutines 1.2.1

@objcode

This comment has been minimized.

Copy link
Contributor

@objcode objcode commented May 17, 2019

Thank you for this minimal repro bug report!

Adding some tests for this and taking a look at getting this patched.

@objcode

This comment has been minimized.

Copy link
Contributor

@objcode objcode commented May 17, 2019

as an aside if anyone else is seeing this until it gets fixed - it appears the failure is caused by the thread (the blocking call to sleep is not required)

@objcode

This comment has been minimized.

Copy link
Contributor

@objcode objcode commented May 18, 2019

Patch #1206 should fix it!

It turns out the behavior of completion in the presence of multiple threads had a non-deterministic outcome that could, in some cases, cause a random test result.

This patch fixes the issue reported here by making it pass (as expected) while providing a detailed error message and test failure if this race condition is detected.

@rajab57

This comment has been minimized.

Copy link

@rajab57 rajab57 commented Jul 30, 2019

Is there a workaround for this issue ?

@jimmymorales

This comment has been minimized.

Copy link

@jimmymorales jimmymorales commented Aug 4, 2019

I came with this exception for some Room database tests (androidTest) using the AndroidJUnit4 runner.
It stopped throwing the exception when I added the InstantTaskExecutorRule rule.

@get:Rule
val instantExecutorRule = InstantTaskExecutorRule()

I hope this helps!

@Mugurell

This comment has been minimized.

Copy link

@Mugurell Mugurell commented Oct 30, 2019

Is there a workaround for this issue ?

I'm using something like

@Test
fun test() = runBlocking {
    coroutineJob.join()
}
@Anton-Spaans-Intrepid

This comment has been minimized.

Copy link

@Anton-Spaans-Intrepid Anton-Spaans-Intrepid commented Oct 30, 2019

@objcode I'm not sure if this patch will fix this issue entirely.

E.g. this code, that uses no other threads, fails with the "This job has not completed" message"

    runBlockingTest {
        flow<Unit> {
            suspendCancellableCoroutine<Nothing> {  }
        }.collect {  }
    }

while this code just finishes without any problems:

    runBlockingTest {
        flow<Unit> {
            delay(Long.MAX_VALUE)
        }.collect {  }
    }
@objcode

This comment has been minimized.

Copy link
Contributor

@objcode objcode commented Oct 30, 2019

@Anton-Spaans-Intrepid I would expect that to fail as it the collect {} will never complete.

In the patched version, you should see a 30 second wait followed by an error.

What behavior are you expecting?

@streetsofboston

This comment has been minimized.

Copy link

@streetsofboston streetsofboston commented Oct 30, 2019

(I just discovered I used my other GitHub account ... ah well 😄)

I expect the test to never end. It will hang.

@objcode

This comment has been minimized.

Copy link
Contributor

@objcode objcode commented Oct 30, 2019

@streetsofboston Hm, interesting. Can you elaborate on why?

To me this seems like it "should" fail since you're suspending the test coroutine (the coroutine in rbt) with suspendCancellableCoroutine<Nothing> { } which means during cleanup there is a non-completed coroutine.

There's probably three options:

  1. Keep this error as is
  2. Generate a different error if the only uncompleted job at the end is the test lambda
  3. Actually let the test hang forever (this is probably not optimal imo since it's somewhat easy to trigger and hard to debug)
@streetsofboston

This comment has been minimized.

Copy link

@streetsofboston streetsofboston commented Oct 31, 2019

@objcode
If the code were regular non-suspendable code, just plain blocking code, writing this would hang the test forever as well:

fun test() {
    val flowable = Flowable.never<Unit>()
    flowable
        .firstOrError().blockingGet()
}

The above test would never return.

You could make the case that if a non-suspending piece of code never completes, a corresponding suspending piece of code should never resume (and when wrapping that in a runBlockingXXX { ... } block, the code would never return).

Also, a 30 seconds timeout may not be enough for some tests... should the timeout be configurable?

In the end, it is a matter of taste 😄
Hanging tests are not good, but when regular non-suspending code never completes, the test will hang as well. Should that be the same for suspending code inside a runBlockingTest { ... } block or should we use a timeout, keeping in mind that a plain runBlocking { ... } will hang forever?

@premnirmal

This comment has been minimized.

Copy link

@premnirmal premnirmal commented Oct 31, 2019

Any update on when this fix will be released?

@nschwermann

This comment has been minimized.

Copy link

@nschwermann nschwermann commented Nov 3, 2019

Having this issue as well, Seems it only happens if touching files in android framework or that contain livedata not quite sure...

@premnirmal

This comment has been minimized.

Copy link

@premnirmal premnirmal commented Nov 3, 2019

Happens to my tests (not using livedata). Here are the tests: https://github.com/premnirmal/StockTicker/blob/master/app/src/test/kotlin/com/github/premnirmal/ticker/network/StocksApiTest.kt
I have to use runBlocking right now instead of runBlockingTest until this is fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.