Mock clock cannot jump into the past, but uses seconds_to_next_deadline which can !

[touilleMan]

When using hypothesis with the mock_clock fixture, I sometime end up with a raise ValueError("time can't go backwards")

The thing is we have MockClock._autojumper doing this:

trio/trio/testing/_mock_clock.py

Lines 149 to 152 in 4512ae3

	statistics = _core.current_statistics()
	jump = statistics.seconds_to_next_deadline
	if jump < inf:
	self.jump(jump)

However according to current_statisticts documentation:

trio/trio/_core/_run.py

Lines 1143 to 1146 in 4512ae3

	* ``seconds_to_next_deadline`` (float): The time until the next
	pending cancel scope deadline. May be negative if the deadline has
	expired but we haven't yet processed cancellations. May be
	:data:`~math.inf` if there are no pending deadlines.

I guess _autojumper should be corrected to do something like this:

statistics = _core.current_statistics() 
jump = statistics.seconds_to_next_deadline 
if 0 < jump < inf:
     self.jump(jump) 

What do you think ?

[njsmith]

Oh interesting!

We certainly don't want to try to do a negative jump, so your proposed fix seems safe in that regard.

But I am confused about how that deadline can be in the past at the same time that we're getting told that all tasks are blocked. If time were advancing normally then I get it, maybe the deadline expired in the fraction of a second between when we determined that all tests were blocked vs when the clock tries to advance. But I assume you're using MockClock in the mode where time only moves forward when someone calls jump, in which case... The run loop ought to have processed those deadlines before waking up the clock task. So that makes me nervous that there's another bug here we don't understand yet.

Do you have a reproducer?

[touilleMan]

But I assume you're using MockClock in the mode where time only moves forward when someone calls jump

Maybe my use of MockClock is a bit exotic:
I'm testing a background task that does some synchronization. To do that I have two kind of hypothesis rules:

1. Rules that create data that need to be synchronized. During those rules I don't want the background task to wake up randomly and do concurrent stuff so I keep MockClock.rate == 0
2. A single rule that wait for the synchronization to occur and then check the state of the tested system. This rule is the only time I switch MockClock.rate == 1 and wait for a long time (with autojump_threshold == 0, considering I've mocked all my real IOs with stapled streams) until I'm sure the sync should have occurred.

So in the end I'm freezing/unfreezing the MockClock really often

If time were advancing normally then I get it, maybe the deadline expired in the fraction of a second between when we determined that all tests were blocked vs when the clock tries to advance

I guess this is compatible with the time I have rate == 1

Do you have a reproducer?

Unfortunately not :'-(
I get this error randomly on a complex hypothesis stateful test

[njsmith]

Ahhh OK. I was just concerned that if you were hitting that error with rate=0 then there must be another issue we were missing. But if you're hitting it with rate > 0, then it's just that time is passing, and all we need to do is filter out those jumps, like you suggested. Want to make a PR?

I'm not thinking of any good way to test it though unfortunately...

BTW, regarding this:

A single rule that wait for the synchronization to occur and then check the state of the tested system. This rule is the only time I switch MockClock.rate == 1 and wait for a long time (with autojump_threshold == 0, considering I've mocked all my real IOs with stapled streams) until I'm sure the sync should have occurred.

If your whole system is running inside Trio, then I don't think you would ever need to set rate = 1? If your test code wants to wait for all the system-under-test tasks to have done their thing and settled down, you can call await trio.testing.wait_all_tasks_blocked(), or (since you're using the autojump clock) just await trio.sleep(1 hour) or some other value that you know is definitely longer than any timeouts inside the system under test.

[touilleMan]

Want to make a PR?

I've just created #1194 ;-)

I'm not thinking of any good way to test it though unfortunately...

Yeah, I fear we end up with a very had hoc test with intrusive mocks on _core.current_statistics(). So something complex with few added value...

If your whole system is running inside Trio, then I don't think you would ever need to set rate = 1?

The background task does some trio.sleep() as part of it handling (it keep track of modified files, and wait for the file not to be modified for a minimal duration before uploading it to a server), so I don't see how the trio.sleep could be passed if rate = 0... Am-I missing something there ?

Edit: Now I get what you mean, I didn't realize autojump_threshold worked independently of rate (which itself only describe how real time affect the trio time, so it's pretty logical from this point of view ^^). It's pretty cool to get rid of the rate tweaking in my code ;-)