Use monotonic clock.

[wapiflapi]

This fixes #149 and #125.

This pull request proposes the use of time.monotonic by default instead of datatime.now. A monotonic clock has the garantee of never going back in time, which is what we want when computing elapsed time.

This also has the benefit of cleaning up tests, because we can easily patch both time.sleep and time.monotonic so that the latter moves forward by exactly the amount of seconds that we pretend-slept for.

This found a "bug" in one of the test which I changed, it requires special attention when reviewing this code. The amount of expect calls to sleep didn't seem logical to me and I believe tthey where the result of time imprecisions.

[bgreen-litl]

Thanks for this! It's going to take me a little bit to review it carefully, but I will do so as soon as I get a chance.

[PLPeeters]

Random dude passing through here, but IMO it would be cleaner to just declare monotonic_time=time.monotonic, sleep=asyncio.sleep and then just use monotonic_time() and sleep(n) everywhere instead of the (monotonic_time or time.monotonic)() and (sleep or asyncio.sleep)(seconds) calls that lack legibility and clarity.

[wapiflapi]

I definitely agree with you that it would be cleaner. If no one has issues with having the interface be explicit about the default functions that are being used I'm happy with changing that.

If we want to keep having None to indicate "use whatever default the implementation thinks is best" we could have something like sleep_function = time.sleep if sleep is None else sleep somewhere and use that.

[PLPeeters]

I'd go with the Zen of Python on this: Explicit is better than implicit. Users shouldn't have to guess what the default is when it can be right there in the definition.

The only reason to use None as a default argument and override it with the actual default in the body of the function is if your default is mutable and could lead to unexpected behaviour, such as a dict or list. That's the only case I know of where it's standard to go arg=None and then set the actual default in the body:

if arg is None:
    arg = {}

Unless I'm missing some specifics here of course.

[wapiflapi]

Looking at changing this I remember why i did it like this. The unit tests monkeypatch time.sleep in a lot of places, and if you pass it as a default value to the functions then it makes monkey patching much harder because it needs to be done before the module loads instead of simply before calling the function that is going to use time.sleep.

I am going to have to refactor most unit tests to pass the sleep argument instead of monkey patching.