A group of tests with mixed usage of runTest can cause silent failures

[gqmulligan]

Describe the bug

When using runTest it is possible for a callback to remain in the ExceptionCollector after the test completes. This can then cause future test runs to pass with a swallowed exception in certain cases.

Provide a Reproducer

This behavior was found when migrating a large code base to use runTest for all unit tests involving coroutines with version 1.7.0+ of the coroutines test library. Currently there is a mix of coroutine tests that use runTest, runBlocking, or neither. To help find silent failures in tests that are not yet using runTest an UncaughtExceptionHandler is used so the tests fail. When running all tests we were unexpectedly finding a different number of test failures each time. Below is a simplified sequence of tests that reproduces the issue with explanation. The final test is expected to fail but it currently passes:

1. The first test uses runTest to cause the ExceptionCollector to become enabled. This test is expected to pass and it does.
2. The second test uses runBlocking and throws an uncaught exception in a coroutine. An unprocessed exception is added to the ExceptionCollector and the test also fails as expected due to the UncaughtExceptionHandler.
3. This runs the same test from step 1 again. This time though the test fails as expected due to an UncaughtExceptionsBeforeTest exception. However, what is not expected is after this test fails an exception callback remains in the ExceptionCollector.
4. This runs the same test from step 2 again except now it passes when it is expected to fail. Because of the callback still in the ExceptionCollector it seems the UncaughtExceptionHandler is never called since the exception is considered handled.

@TestMethodOrder(value = MethodOrderer.MethodName::class)
class CoroutineTests {
    private var defaultUncaughtExceptionHandler: Thread.UncaughtExceptionHandler? = null
    private val testDispatcher: TestDispatcher = UnconfinedTestDispatcher()

    @BeforeEach
    fun beforeEach() {
        defaultUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler()
        Thread.setDefaultUncaughtExceptionHandler { _, e -> throw e }
        Dispatchers.setMain(testDispatcher)
    }

    @AfterEach
    fun afterEach() {
        Thread.setDefaultUncaughtExceptionHandler(defaultUncaughtExceptionHandler)
        Dispatchers.resetMain()
    }

    @Test
    fun `1 use runTest so ExceptionCollector becomes enabled`() = runTest(testDispatcher) {
        assertTrue(true)
    }

    @Test
    fun `2 use runBlocking and throw uncaught exception that is added to unprocessed exceptions`() = runBlocking {
        CoroutineScope(testDispatcher).launch { throw Exception("Fail") }.join()
    }

    @Test
    fun `3 use runTest again and UncaughtExceptionsBeforeTest is thrown`() = runTest(testDispatcher) {
        assertTrue(true)
    }

    @Test
    fun `4 use runBlocking and throw uncaught exception that is now ignored when it should not be`() = runBlocking {
        CoroutineScope(testDispatcher).launch { throw Exception("Fail") }.join()
    }
}

[dkhalanskyjb]

Thank you, yes, this is a bug. I managed to reproduce it and find the cause. When runTest throws on initialization because there were exceptions caught before it arrived, it then forgets to unsubscribe from receiving them, so the system thinks it has a runTest to pass exceptions to.