Demystify and deprecate HomeserverTestCase.pump() (Twisted reactor/clock)

changelog.d/19602.misc.1

@@ -0,0 +1 @@
+Update `HomeserverTestCase.pump()` docstring to demystify behavior (Twisted reactor/clock).

changelog.d/19602.misc.2

@@ -0,0 +1 @@
+Deprecate `HomeserverTestCase.pump()` in favor of more direct `HomeserverTestCase.reactor.advance(...)` usage.

tests/unittest.py

@@ -697,8 +697,43 @@ async def run_bg_updates() -> None:
 
     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: