Better handle the exceptions from child coroutines in runTest

[dkhalanskyjb]

Fixes #1910

[dkhalanskyjb]

Should it be fixed that way

I actually did spend quite some time attempting to eliminate `SupervisorJob`, but in the end, decided against it. Consider this code: ```kotlin fun testFoo() = runTest { launch { throw TestException() } delay(10) } ``` If we use a plain `Job`, `runTest` will fail with `CancellationException`. Normally, it would be expected, but here, the whole test will fail with that exception, which works against `runTest` not getting in the way. We could special-case this by treating `CancellationException` differently, deprioritizing it in the test output. However, also consider this: ```kotlin fun testBar() = runTest { var f = false launch { delay(200) if (f != true) throw TestException() } withTimeout(100) { delay(1000) f = true } } ``` Here, we would be deprioritizing `TimeoutCancellationException`, which, in my opinion, is incorrect. We could special-case *that*… but then a question arises: do we want to consider cancellation exceptions other than `TimeoutCancellationException` safe? Is `TimeoutCancellationException` a very special thing, the only cancellation exception that's not just a support mechanism for structured concurrency? If so, it's a big conceptual shift that affects the project as a whole. If not, then why the special-casing? Instead, we can avoid the whole question by using the following intuition: failures in the test body take priority over failures in the spawned coroutines. This agrees with using `SupervisorJob`.

[qwwdfsad]

Is it a leftover or deliberate code?

[dkhalanskyjb]

A tricky question! I deliberately left it as is so we could discuss it.

cleanupTestCoroutines, too, prints a bunch of exceptions to the console if there's more than one, only throwing a single one. It would be consistent here to print the only thrown exception, too, if it turns out that there's a more important exception (the one with which the testScope completed).

If I understood correctly, you advise to, instead, get the exception with which testScope completed, add a suppressed exception from the cleanup, and then just throw it, right? But then, for consistency, we would also need the other exceptions from cleanupTestCoroutines to be suppressed and not printed, right? This would somewhat complicate the API, and I'm not sure what would the benefits be, other than the ability to better test this test module (validating printed exceptions is a pain).

[qwwdfsad]

The main benefits here are programmatic access for any test frameworks -- exception aggregation, logging on test failures on CI and so on.

Printing the error straight to the console seems neither controllable nor usable.

[qwwdfsad]

[nit] RunningInRunTest probably can be object with prettified toString

[qwwdfsad]

The main benefits here are programmatic access for any test frameworks -- exception aggregation, logging on test failures on CI and so on.

Printing the error straight to the console seems neither controllable nor usable.