Prevent MockClock from mutating the current runner #3205
Conversation
|
I thought we were over dtls's smoke test timing out the macos pypy runner! For now I'll just rerun CI. |
| @@ -119,7 +119,8 @@ def _try_resync_autojump_threshold(self) -> None: | |||
| except AttributeError: | |||
| pass | |||
| else: | |||
| runner.clock_autojump_threshold = self._autojump_threshold | |||
| if runner.clock is self: | |||
| runner.clock_autojump_threshold = self._autojump_threshold | |||
There was a problem hiding this comment.
I'm wondering why we even need this attribute on the runner. I only see it referenced in two spots in src/trio/_core/_run.py, which could as easily be changed to runner.clock.autojump_threshold, no? I suspect I'm missing something related to an edge case when the threshold value is changed.
There was a problem hiding this comment.
Because runner.clock isn't necessarily a MockClock which is the only kind with autojump_threshold. Of course it may be possible to getattr(runner.clock, "autojump_threshold", math.inf). Maybe that's a better solution?
On the other hand that means a user-provided clock can't have an attribute named autojump_threshold. So it's limiting but probably not in a way that matters to anyone.
There was a problem hiding this comment.
I think it's better continue with an attribute on runner. It doesn't have any disadvantages in practice (now that this is solved) and allows user clocks not to worry about accidentally overriding some undocumented attribute.
Based on the issue @jc211 had on Gitter. You probably shouldn't be creating a
MockClockand not using it, but this is a footgun.