Adding a test-helper class TestCoroutineContext.

[streetsofboston]

The purpose of this PR is to add a component that allows unit-tests that exercise code with Coroutines with delays and timeouts to run as fast as possible in near zero time.

Running a unit-test for code that contains one or more Coroutine with delays or timeouts requires the unit-test to wait and delay until the delays and timeouts of the various Coroutines come to pass.

This can cause unit-tests to take too much time to finish.

The addition of a new Coroutine context, called TestCoroutineContext alleviates this problem much like a TestScheduler does for RxJava2. When Coroutines use an instance of TestCoroutineContext, delays and timeouts can be fast-forwarded by calling its methods such as advanceTimeBy and advanceTimeTo and this will speed up the execution of these Coroutines.

[elizarov]

I've reviewed implementation, rebased it onto the latest develop brach, made a number of optimization and pushed into a separate test-context branch here. Please, pull it from that branch for further work.

I will not go into optimizations (you can take a look) since they are not really important and more a matter of taste.

The reason that I have not yet merged it into develop is that there are few design questions left:

1. This implementation uses atomics to manipulate current time value, but it is not clear why it would ever be needed. It seems that current "virtual time" is only even updated explicitly by the testing code (running in some single-threaded test harness) or implicitly from inside of runBlocking which would also (supposedly?) be running in the same test thread. I wonder if current virtual time can be plain (non-atomic) variable. That is a minor question.

2. The more pressing question is the desing for public val exceptions. What is the user of the TestCoroutineContext is supposed to do with them? It seems to be that test-writing with TestCoroutineContext would be quite error-prone unless unhandled exceptions are somehow handled automatically. It is easy to forget about and they just become "eaten".

I suggest to have some kind of "testing DSL" around this context implementation, so that tests can be written like this:

fun testSomething() = withTestContext { // this: TestCoroutineContext
    // test code here, TestCoroutineContext is implicitly in scope for advanceTimeXXX
}

If has any unhandled exception, then the test should fail. For expected exception something like this looks reasonable:

fun testSomething() = withTestContext( 
    expected = { it is MyExpectedException } // predicate on expected exception
) { 
    // test code
}

I would suggest that any any exception recorded by CoroutineExceptionHandler (unhandled exceptions from launch) lead to test failure.

[streetsofboston]

Good points.

Ad 1) I think you are right. The 'virtual time' can be a plain variable. I will change that.

Ad 2) I based handling exceptions on the way the RxJava2's TestObserver handles exceptions. It observes emissions and errors and allows explicit examinations of emitted values, errors (and completions, etc). But I have to admit, I like the way you suggest it, using the 'testing DSL'-like approach you showed. Let me know if you'd like to stick with the RxJava2's TestObserver style approach or the one you suggested.

[elizarov]

I think that Rx approach is error-prone. However, in my own test I sometimes write test where I expect to have some unhandled exception and want to examine them. We can support this use-case. I propose something like:

fun testSomething() = withTestContext { // this: TestCoroutineContext
    // test code here
    expectUnhandledException { it is SomeExceptionClass } // predicate on exception
}

The way expectUnhandledException works is by popping exception from a queue of unhandled exceptions and checking it. If there are some unhandled exceptions that were left unexpected at the end of withTestContext block, then the test should fail.

[elizarov]

Also, note that normally all descendants of CancellationException are ignored (are not printed to the log, do not crash app). We should have it here, too + properly test those cases. That is, if I launch { delay(1000) } and cancel the resulting job (without advancing time to the point where it should stop waiting), then it should not produce any "unhandled exceptions".

[streetsofboston]

Thank you @elizarov
I'll make the suggested changes on the test-context branch.

[streetsofboston]

@elizarov I made the changes on the test-context branch. Should I open a new PR that is based on that branch?

[elizarov]

@streetsofboston I've fetched the code your from your test-context branch, rebased it onto the latest work in develop and pushed into test-context branch here. I'd prefer that you update this PR (so that we don't loose conversation history). You can do it by force-pushing the corresponding revision into your testcoroutinecontext branch that this PR points too.

Now, I see you've added launch, async, and runBlocking extensions for TestCoroutineContext this is convenient that it is actually similar to something we are looking to do in other places of kotlinx.coroutines library. However, I'm not 100% sure this is a right design to pursue. It looks quite error-prone to have some builders defined as extensions, while other do not defined as such (if I write async it works in the test context, while if I write produce it does not...). Give us a few days to think this through.

[streetsofboston]

@elizarov Thank you for the review. I'll push the changes to this branch and PR.

About the three extension functions: I'll remove them from the TestCoroutineContext.kt. file and move them to the TestCoroutineContextTest.kt instead.

[streetsofboston]

@elizarov I added the suggested changes to this branch and PR.

[elizarov]

Thanks. Merged.