Inconsistent Timer behavior for timeouts between MockTimer and DefaultTimer

[davidabrahams]

I ran into some slightly unintuitive behavior when using the MockTimer to test time-dependent code. I wanted to check if this is the intended behavior, or if perhaps there is a better way for me to be writing my tests.

Whether or not a Future is timed out is dependent on if the timeControl is advanced past when the slow computation completes when using MockTimer.

I wrote up a quick unit-test which demonstrates this behavior. There are three tests. I would expect all three to pass, but currently only the first two pass:

https://gist.github.com/davidabrahams/d69f5460a3a0fbf73985d5c3b6a0ae36

Expected behavior

I'd expect MockTimer and DefaultTimer to have similar semantics, and for the Future's result to not be dependent on how far I advance the timeControl

Actual behavior

If I advance the time control past both the timeout and the underlying sleep, the Future succeeds.

Steps to reproduce the behavior

The unit-test in the gist reproduces this behavior

[jyanJing]

Hi @davidabrahams ,

Thanks for all the details, they are really helpful!

I think what you have seen in the test is the expected behavior, because timeControl will manipulate Time.now, that affects mockTimer and Futures that timed by it. In test #3, when you call Future.sleep(100) with the mockTimer, it calculates the timestamp to satisfy this future, and put the Future.sleep job in a queue, after you advance the timer by 200, and call tick(), the mockTimer will executes all jobs in queue with time less than the new Time.now, by then, Future.sleep(100) is satisfied and won't time out.

[davidabrahams]

Hi @jyanJing

Thanks for the reply + explanation. Yeah it makes sense to me why this is the current behavior, I guess the question I'm asking is: "is this the right desired behavior?" I'd argue this behavior is a bit confusing, and makes it difficult to test code which has complex timeout logic, since it may be difficult to write a test which advances the timeControl in order to place it between the timeout and the slow Future's completion time. It is far easier to simply advance the timeControl to some time by which you know either the underlying Future is complete, or it has been timed out (which is what I did when I hit this behavior).

If there's agreement that the current behavior != ideal desired behavior, I'd be happy to dig into this and work on a PR once I have time :) Though I think this would be a non-backwards-compat change, so it'd probably have to wait until the next major-release version.

[jyanJing]

Hi @davidabrahams,

This behavior makes sense to me, because when creating a future via the mockTimer with val future = runTest(mockTimer), it makes sense to time that future so the Future.sleep should also be affected and timed by the mockTimer. What use cases do you have that you don't want the future to be affected?

I am not very strong about it, so also open to opinions from other folks.

[davidabrahams]

Hi @jyanJing

Sorry about the latency- been pretty busy recently :) Thanks for the discussion on this issue!

I'll close the issue. I may revisit this in the future and perhaps work on a PR, but I'm a bit too busy right now, so I think it's best I close the issue to avoid staleness.

Thanks again for the discussion