What is the purpose of this PR
- This PR fixes a flaky test to avoid an assertion on an asynchronous operation
retrofit2.adapter.rxjava.CancelDisposeTest#cancelDoesNotDispose
Reproduce the test failure
- The test fails non-deterministically when execution delays occur. To observe a test failure, the unmodified test can be re-executed many times. the likelihood of observing a test failure increases when the test executes on a slow machine. To deterministically reproduce the test failure, introduce an execution delay:
@Test
public void cancelDoesNotDispose() throws Exception{
Subscription subscription = service.go().subscribe();
List<Callcalls = client.dispatcher().runningCalls();
assertEquals(1, calls.size());
calls.get(0).cancel();
Thread.sleep(30);
assertFalse(subscription.isUnsubscribed()); //fail
}
Expected result:
- The test should run successfully in all runs.
Actual result:
- In 47 out of 100 runs, we get the failure:
java.lang.AssertionError
Why the test fails
- Our understanding is that the last assertion of this test is inappropriate. (We suggest alternatives below.) This test case subscribes to receive notifications (line
Subscription subscription = service.go().subscribe();). Later, the test requests a cancellation of that subscription (line calls.get(0).cancel();). The problem is that this cancellation is an asynchronous operation; the cancellation does not take effect immediately. Developers certainly did not realize the problem because, as of now, there is no pause between the cancellation statement and the final assertion (line assertFalse(subscription.isUnsubscribed());) passes. But note that the assertion states that the subscription is valid (false that it is _un_subscribed). That only holds because the main thread that runs the test is faster than the thread that runs the cancellation process.
Fix
- Add a timeout for the test;
- keep checking whether the subscription is unsubscribed.