test: use runTest of coroutines.test instead of runBlocking

[mervyn-mccreight]

Using it has the following benefits over runBlocking:

• concept of virtual time when using e.g. delay, which is useful to e.g. specify something runs in parallel instead of in sequence
• less manual calls to runBlocking which are IMO clutter in tests
• built-in timeout functionality (configurable)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.96%. Comparing base (882e89b) to head (79fc736).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   81.90%   81.96%   +0.06%     
==========================================
  Files         152      152              
  Lines        4879     4879              
  Branches      846      846              
==========================================
+ Hits         3996     3999       +3     
+ Misses        563      560       -3     
  Partials      320      320              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stuebingerb]

One downside at the moment is that IntelliJ apparently has not caught up yet and shows the usage of the suspend modifier on @Test-annotated functions as an error.

Which is why I decided to not use suspending test functions when I updated to JUnit 6 😀 Is there anything in particular that would benefit from a migration now already?

[mervyn-mccreight]

One downside at the moment is that IntelliJ apparently has not caught up yet and shows the usage of the suspend modifier on @Test-annotated functions as an error.

Which is why I decided to not use suspending test functions when I updated to JUnit 6 😀 Is there anything in particular that would benefit from a migration now already?

We could anyway use runTest of kotlinx.coroutines.test: https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-test/kotlinx.coroutines.test/run-test.html

This would have some benefits:

• concept of virtual time when using e.g. delay, which is useful to e.g. specify something runs in parallel instead of in sequence
• less manual calls to runBlocking which are IMO clutter in tests
• built-in timeout functionalty (configurable)

[stuebingerb]

Unfortunately, there seems to be an issue with (at least) one of the refactored tests that makes me wonder a bit how reliable runTest really is for certain test cases.
Can you please have a look?

[stuebingerb]

This test now fails to catch the error case it was created for.

In ParallelRequestExecutor, when the extra coroutineScope within plan.toMapAsync is removed, execution in fact no longer runs parallel (as in: the previous test failed because execution time is above 3 seconds):

            val resultMap = plan.toMapAsync(dispatcher) {
//                coroutineScope {
                    val ctx = ExecutionContext(this, Variables(variables, it.variables), context, loaders)
                    if (shouldInclude(ctx, it)) {
                        writeOperation(
                            isSubscription = plan.isSubscription,
                            ctx = ctx,
                            node = it,
                            operation = it.field as Field.Function<*, *>
                        )
                    } else {
                        null
                    }
//                }
            }

However, the new test still succeeds.

[mervyn-mccreight]

I'm a bit confused right now, because in my current understanding opening a CoroutineScope itself does not change whether something runs async or sync.

So it would have been my expectation that the test still succeeds. Why it changed behaviour with the old one is what I currently do not understand 🤔