diff --git a/changelog.d/19602.misc.1 b/changelog.d/19602.misc.1 new file mode 100644 index 0000000000..222f26980d --- /dev/null +++ b/changelog.d/19602.misc.1 @@ -0,0 +1 @@ +Update `HomeserverTestCase.pump()` docstring to demystify behavior (Twisted reactor/clock). diff --git a/changelog.d/19602.misc.2 b/changelog.d/19602.misc.2 new file mode 100644 index 0000000000..4cadad653c --- /dev/null +++ b/changelog.d/19602.misc.2 @@ -0,0 +1 @@ +Deprecate `HomeserverTestCase.pump()` in favor of more direct `HomeserverTestCase.reactor.advance(...)` usage. diff --git a/tests/unittest.py b/tests/unittest.py index 6022c750d0..03db9a4282 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -697,8 +697,43 @@ class HomeserverTestCase(TestCase): def pump(self, by: float = 0.0) -> None: """ - Pump the reactor enough that Deferreds will fire. + XXX: Deprecated: This method is deprecated. Use `self.reactor.advance(...)` + directly instead. + + Pump the reactor enough that `clock.call_later` scheduled callbacks will fire. + + To demystify this function, it simply advances time by the number of seconds + specified (defaults to `0`, we also multiply by 100, so `pump(1)` is 100 seconds + in 1 second steps/increments) whilst calling any pending callbacks, allowing any + queued/pending tasks to run because enough time has passed. + + So for example, if you have some Synapse code that does + `clock.call_later(Duration(seconds=2), callback)`, then calling + `self.pump(by=0.02)` will advance time by 2 seconds, which is enough for that + callback to be ready to run now. Same for `clock.sleep(...)` , + `clock.looping_call(...)`, and whatever other clock utilities that use + `clock.call_later` under the hood for scheduling tasks. Trying to use + `pump(by=...)` with exact math to meet a specific deadline feels pretty dirty + though which is why we recommend using `self.reactor.advance(...)` directly + nowadays. + + We don't have any exact historical context for why `pump()` was introduced into + the codebase beyond the code itself. We assume that we multiply by 100 so that + when you use the clock to schedule something that schedules more things, it + tries to run the whole chain to completion. + + XXX: If you're having to call this function, please call out in comments, which + scheduled thing you're aiming to trigger. Please also check whether the + `pump(...)` is even necessary as it was often misused. + + Args: + by: The time increment in seconds to advance time by. We will advance time + in 100 steps, each step by this value. """ + # We multiply by 100, so `pump(1)` actually advances time by 100 seconds in 1 + # second steps/increments. We assume this was done so that when you use the + # clock to schedule something that schedules more things, it tries to run the + # whole chain to completion. self.reactor.pump([by] * 100) def get_success(self, d: Awaitable[TV], by: float = 0.0) -> TV: