Support disabling reporting of uncaught exceptions in tests

[dkhalanskyjb]

The solution for #1205 may be undesirable for those who already have their own solution, like setting the default exception handlers. In this case, there's a usability issue without the corresponding benefit: instead of all tests being ran and the exceptions from them being reported, unrelated tests may fail, making looking for problems more difficult.

This is probably a very rare issue, so we don't provide public API for that, instead introducing a need-to-know internal variable.

[dkhalanskyjb]

This isn't tested for two reasons:

• The variable has to be set before the very first use of runTest, or the exception-eating monster wakes up, and it's difficult to ensure the test order;
• Any tests that show not processing uncaught exceptions would crash on Native and pollute the logs on JVM and JS.

[qwwdfsad]

Could you please file an issue to get rid of that eventually?

[dkhalanskyjb]

I could, but I don't see a way to fix the issue after thinking about it. Even when the CoroutineExceptionHandler semantics is fixed, we still won't be able to guarantee that this particular exception handler doesn't go first. We don't want to introduce explicit priorities to CoroutineExceptionHandler implementations the way we do with the Dispatchers.Main factories, right?

[dkhalanskyjb]

(For the record, the test I used to check that everything works; not sure if I should include it in the code, it seems like a deadweight):

    /**
     * Tests that the uncaught exceptions that happen during the test are not reported if the feature is disabled.
     */
    @Test
    @Ignore // this test can only be run manually, because it will only work if it's the first test to run.
    fun testDisablingStrayUncaughtExceptionReporting() {
        var exception: Throwable? = null
        Thread.setDefaultUncaughtExceptionHandler { t, e -> exception = e }
        catchNonTestRelatedExceptions = false
        runTest {
            val job = GlobalScope.launch {
                throw RuntimeException("x")
            }
            job.join()
        }
        assertEquals("x", exception?.message)
    }

[qwwdfsad]

I don't see a way to fix the issue after thinking about it.

We could start with a deprecation, so either folks will move from their workarounds (e.g. fix leaking tests) or report a descriptive use-case for us

[dkhalanskyjb]

Something like this #3738?

[soygabimoreno]

Thanks for the solution, @dkhalanskyjb. 🙂

How should we disable it?

I am trying with this, but it's not accessing the variable catchNonTestRelatedExceptions.

public inline fun disableCatchNonTestRelatedExceptions() {
   TestScope.catchNonTestRelatedExceptions = false
}

Neither this way:

[dkhalanskyjb]

Could you clarify here #3738 why you want to disable the functionality? The overwhelming majority of projects should never ever need this.

To answer your question: one way would be to use reflection, like

Class.forName("kotlinx.coroutines.test.TestScopeKt")
  .getDeclaredMethod("setCatchNonTestRelatedExceptions", Boolean::class.java)
  .invoke(null, true)

This only works on the JVM, though. If you need this for other platforms as well, some other specific trickery is needed.

[ankitrapido]

@dkhalanskyjb the above solution you proposed, where to put the code I tried to add it in before block of test doesn't seems to work