diff --git a/tests/render_harness.py b/tests/render_harness.py index 5f4be87d3..4752a0a57 100644 --- a/tests/render_harness.py +++ b/tests/render_harness.py @@ -599,9 +599,11 @@ def __init__(self, page): set() ) # event posts in the air, each counted once whichever way it ends self._answered = set() # requests whose one response has entered the counters + self._complete = set() # requests the browser has finished delivering self._responses = [] # bodies are read outside Playwright's response callback page.on("request", self._out) page.on("response", self._responded) + page.on("requestfinished", self._delivered) page.on("requestfailed", self._back) # A navigation is the third way a trip ends, and the one the browser reports # for neither kind: a post the reload kills mid-flight gets no `response` and @@ -642,15 +644,22 @@ def _responded(self, response): self._back(request) self._responses.append(response) - def settle_response(self, response): - """Account for the exact response a causal wait just consumed. + def _delivered(self, request): + """The browser has the whole body, so reading it cannot block.""" + self._complete.add(request) - Playwright resolves `wait_for_event("response")` independently of ordinary - response listeners. Under load the waiter can resume first; explicitly entering - that response here closes the ordering without polling or sleeping. `_responded` - deduplicates the later listener whichever one wins. + def settle_finished(self, request): + """Account for the exact trip a causal wait just consumed. + + Playwright resolves `wait_for_event("requestfinished")` independently of + ordinary listeners. Under load the waiter can resume first; explicitly entering + that trip here closes the ordering without polling or sleeping. `_responded` + and `_delivered` deduplicate the later listeners whichever one wins. """ - self._responded(response) + self._delivered(request) + response = request.response() + if response is not None: + self._responded(response) self.settle() def settle(self): @@ -658,10 +667,27 @@ def settle(self): Playwright may yield while `response.json()` asks its driver for the body. Doing that inside `_responded` let the test re-enter between `acked` and `pending`, - after the response event that could wake it had already fired.""" - while self._responses: - responses, self._responses = self._responses, [] - for response in responses: + after the response event that could wake it had already fired. + + A body is read only once `requestfinished` says the browser holds all of it. The + `response` event fires on the headers, and `Response.json` then waits on the + finished fact with no deadline of its own — the one wait in this harness that + cannot run out. A page that abandons an answer it no longer wants leaves a + request the browser never finishes, and reading that body stopped a worker + dead: a locally reproduced wedge was traced with both workers inside + `Response.json`, and the runs on main that spend their whole 45-minute bound + name no test and leave no traceback. A run reaches that bound only with both + workers stopped, and on the CI runs parsed for it the second one was past this + site, in teardown. Queued and unread, such a response instead lets `_until` + reach its own deadline and print the counters.""" + while True: + ready = [r for r in self._responses if r.request in self._complete] + if not ready: + return + self._responses = [ + r for r in self._responses if r.request not in self._complete + ] + for response in ready: self._settle(response) def _settle(self, response): @@ -732,22 +758,24 @@ def _until(page, fact, wanted): """Block until `fact` holds of the page's traffic. The events the counters are built from arrive while the client is blocked inside a - Playwright call, so this blocks on each next response and asks again — no polling - interval to pick, and nothing added to the page. The response returned by that wait - is entered into Traffic directly because Playwright does not order the waiter after - ordinary response listeners; the delivery fact therefore changes before this caller - asks again. - - It wakes on responses alone, where the counters answer to failures too, so a fact that - came true through a failed request waits for the next poll that is answered to be - noticed. A page with every poll routed to `abort` has no such next, and a wait on one - runs its timeout out and says so rather than passing. + Playwright call, so this blocks on each next finished trip and asks again — no + polling interval to pick, and nothing added to the page. The request returned by that + wait is entered into Traffic directly because Playwright does not order the waiter + after ordinary listeners; the delivery fact therefore changes before this caller asks + again. + + It wakes on finished trips rather than on arriving headers, because a body is what + the counters are read from and `requestfinished` is the browser saying it has one. + The counters answer to failures too, so a fact that came true through a failed + request waits for the next trip that finishes to be noticed. A page with every poll + routed to `abort` has no such next, and a wait on one runs its timeout out and says + so rather than passing. A wait that runs out names the caller's wanted fact and prints its starting and final - counters. The final reading comes after the timeout because the response listener may - settle the fact as the timeout is delivered. No response preserves Playwright's - timeout as the cause; a busy response stream reaches the same explicit deadline - instead of waking this loop forever.""" + counters. The final reading comes after the timeout because the page's own + `requestfinished` listener may settle the fact as the timeout is delivered. No + finished trip preserves Playwright's timeout as the cause; a busy stream reaches the + same explicit deadline instead of waking this loop forever.""" if fact(_traffic(page)): return began = str(_traffic(page)) @@ -757,8 +785,8 @@ def _until(page, fact, wanted): remaining = int((deadline - time.monotonic()) * 1000) if remaining <= 0: raise PlaywrightTimeout("responses outlived the wait deadline") - response = page.wait_for_event("response", timeout=remaining) - page.lf_traffic.settle_response(response) + request = page.wait_for_event("requestfinished", timeout=remaining) + page.lf_traffic.settle_finished(request) except PlaywrightTimeout as ran_out: ended = _traffic(page) if fact(ended): diff --git a/tests/test_render_gate.py b/tests/test_render_gate.py index 6e0376241..7101ed013 100644 --- a/tests/test_render_gate.py +++ b/tests/test_render_gate.py @@ -48,6 +48,7 @@ UNANSWERED_CODE_PAGE, UNMARKABLE_PAGE, WIDE_TABLE_PAGE, + Traffic, _traffic, _until, arrange_return, @@ -1717,8 +1718,8 @@ def test_the_render_gate_reports_code_the_reader_cannot_tell_from_its_block( ) -def test_a_traffic_wait_accounts_for_the_response_it_consumes(): - """The waiter may resume before Traffic's ordinary response listener under load.""" +def test_a_traffic_wait_accounts_for_the_trip_it_consumes(): + """The waiter may resume before Traffic's ordinary listeners under load.""" class LateTraffic: done = False @@ -1726,8 +1727,8 @@ class LateTraffic: def settle(self): pass - def settle_response(self, response): - assert response == "answer" + def settle_finished(self, request): + assert request == "trip" self.done = True def __str__(self): @@ -1737,14 +1738,56 @@ class EarlyPage: lf_traffic = LateTraffic() def wait_for_event(self, event, **_kwargs): - assert event == "response" - return "answer" + assert event == "requestfinished" + return "trip" - _until(EarlyPage(), lambda traffic: traffic.done, "accounted for the response") + _until(EarlyPage(), lambda traffic: traffic.done, "accounted for the trip") + + +def test_traffic_leaves_a_body_the_browser_has_not_finished_handing_over(): + """`Response.json` waits on the finished fact with no deadline of its own, so a + body read before the browser has one is the single wait here that cannot run out — + the one that spent a whole CI run's bound and named no test. A response settles + when its trip finishes and waits in the queue until then.""" + + class Request: + url = "http://page/api/state" + + class Unfinished: + ok = True + read = False + request = Request() + + def json(self): + self.read = True + return {"events": []} + + class Page: + """The page's own event surface, which is all Traffic asks of one.""" + + def __init__(self): + self.listeners = {} + + def on(self, event, handler): + self.listeners[event] = handler + + page = Page() + traffic = Traffic(page) + response = Unfinished() + + page.listeners["response"](response) + traffic.settle() + assert not response.read, "a body was read before the browser had all of it" + assert traffic.heard == 1, "the headers stopped counting as a state answer" + + page.listeners["requestfinished"](response.request) + traffic.settle() + assert response.read, "the finished body never settled, so the queue only grows" def test_a_traffic_wait_accepts_completion_delivered_with_its_timeout(): - """The ordinary response listener can settle the trip as the waiter times out.""" + """Traffic's own requestfinished listener can settle the trip as the waiter times + out, so the final reading is taken after the deadline rather than before it.""" class EdgeTraffic: done = False @@ -1759,11 +1802,11 @@ class EdgePage: lf_traffic = EdgeTraffic() def wait_for_event(self, event, **_kwargs): - assert event == "response" + assert event == "requestfinished" self.lf_traffic.done = True - raise PlaywrightTimeout("response met its deadline") + raise PlaywrightTimeout("the trip met its deadline") - _until(EdgePage(), lambda traffic: traffic.done, "accounted for the response") + _until(EdgePage(), lambda traffic: traffic.done, "accounted for the trip") def test_an_authored_project_widget_loads_through_the_real_layer(