fix(playwright): name the waiter that exhausts the suite budget, and stop retrying it

Two defects where a suite-budget kill is handled.

`SUITE_RESERVE_MS` exists so a waiter that runs out reports which predicate it
was waiting on, instead of letting the global timeout fire first and count the
in-flight test and everything behind it as "did not run". a490b28af6 set it to
60 s, but the cap it feeds is measured from the waiter's own start while the
global timeout runs from the suite's, so the reserve must also absorb every
test that ran before the waiter plus the post-failure trace and report write.
Measured prefixes in run 36651124458 were 3.5 s, 32.4 s and 86.2 s; at 86.2 s
the cap landed 26 s past the global timeout and OpenProject#1 kept only a
screenshot, no error-context. 180 s leaves roughly double the largest prefix
seen.

No waiter changes category: the newly cut band, between the old cap and the
new one, is empty on both budgets. The already-capped ones lose 120 s of about
5.28 M, and the only one that reached its cap was failing at the old value too.

The retry at orchestrate.yml exists for a Tor circuit flake. A run killed by
the global timeout fails the same way every time, so it burned a second full
budget for an identical result, up to 80% of some jobs' wall clock. Such a role
is now recorded and excluded from the retry loop.

The exclusion is only safe because the stash keeps it. That task reset
`_playwright_failures` to an empty list and only a retried role re-adds itself,
so dropping a role from the retry while leaving that reset would have removed
it from the final fail task and turned a red job green. The intersect keeps
exactly the budget timeouts in, and a test renders the task's own expressions to
hold that: the regression it guards against is a wrong set operation, not wrong
wording. Reverting the reset to `[]` makes it report `[] != ['web-app-docs']`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
kevinveenbirkenbachandClaude Opus 5 committed 2026-09-30 16:30:28 +02:00
1 parent 312d09e290
commit 7127c6b96b
5 files changed
+114 -5

No files matched your search

+8 -1
View File
@@ -35,8 +35,14 @@ function onionTimeoutMultiplier() {
* timeout reports which predicate it was waiting on. Without it the global
* timeout fires first and Playwright counts the in-flight test, and every
* test behind it, as "did not run" — naming nothing.
*
* The cap below is measured from the waiter's own start while the global
* timeout runs from the suite's, so this reserve must also absorb every test
* that ran before the waiter plus the post-failure trace and report write. At
* 60_000 a 86 s prefix already pushed the cap past the global timeout and the
* failure went unnamed.
*/
const SUITE_RESERVE_MS = 60_000;
const SUITE_RESERVE_MS = 180_000;
function suiteBudgetMs() {
const budget = Number(process.env.INFINITO_PLAYWRIGHT_GLOBAL_TIMEOUT_MS);
@@ -57,6 +63,7 @@ function resolveTimeout(baseMs) {
}
module.exports = {
SUITE_RESERVE_MS,
isOnionTarget,
isSplitRealmOidc,
timeoutFactor,
@@ -23,8 +23,10 @@
- name: 🔁 Stash first-pass failures for one flake retry
ansible.builtin.set_fact:
_playwright_retry_roles: "{{ _playwright_failures }}"
_playwright_failures: []
_playwright_retry_roles: "{{ _playwright_failures | difference(_budget_timeouts) }}"
_playwright_failures: "{{ _playwright_failures | intersect(_budget_timeouts) }}"
vars:
_budget_timeouts: "{{ _playwright_budget_timeouts | default([]) }}"
when: (_playwright_failures | default([])) | length > 0
- name: 🔁 Retry failed roles once on rotated Tor circuits
@@ -278,6 +278,13 @@
_playwright_failures: "{{ (_playwright_failures | default([])) + [application_id] }}"
when: (_playwright_run.rc | default(1)) != 0
- name: 📝 Record a suite-budget timeout, which no retry can change
ansible.builtin.set_fact:
_playwright_budget_timeouts: "{{ (_playwright_budget_timeouts | default([])) + [application_id] }}"
when:
- (_playwright_run.rc | default(1)) != 0
- "'for the test suite to run' in (_playwright_run.stdout | default(''))"
- name: 🗄️ Archive Playwright reports for this variant + pass
ansible.builtin.script: >-
{{ role_path }}/files/archive_playwright_reports.sh
@@ -0,0 +1,92 @@
"""A role killed by the suite budget is reported, and not retried.
The retry exists for a Tor circuit flake. A run that hit the global timeout
fails the same way every time, so retrying it burns a second full budget: in
run 36651124458 that was 80% of some jobs' wall clock.
Excluding one is only safe while it still reaches the final fail task. The
stash resets ``_playwright_failures`` and only a retried role re-adds itself,
so an exclusion that emptied that list would turn a red job green. This test
renders the task's own expressions rather than matching their text, because the
failure it guards against is a wrong set operation, not wrong wording.
"""
from __future__ import annotations
import unittest
from typing import Any
import ast
import jinja2
from ansible.plugins.filter.mathstuff import FilterModule
from utils.cache.files import read_text
from utils.cache.yaml import load_yaml_str
from . import PROJECT_ROOT
ORCHESTRATE = (
PROJECT_ROOT
/ "roles"
/ "test-e2e-playwright"
/ "tasks"
/ "01_setup"
/ "orchestrate.yml"
)
STASH = "Stash first-pass failures"
def stash_facts(failures: list[str], budget_timeouts: list[str]) -> dict[str, Any]:
"""Return the facts the stash task sets for the given first pass.
Args:
failures: every role the first pass recorded as failed.
budget_timeouts: those of them killed by the suite budget.
"""
env = jinja2.Environment(autoescape=False) # noqa: S701 - renders a task's own list expression, not markup
env.filters.update(FilterModule().filters())
tasks = load_yaml_str(read_text(str(ORCHESTRATE)))
task = next(t for t in tasks if STASH in t.get("name", ""))
def resolve(expression: Any, context: dict[str, Any]) -> Any:
if not isinstance(expression, str):
return expression
return ast.literal_eval(env.from_string(expression).render(**context))
context: dict[str, Any] = {
"_playwright_failures": failures,
"_playwright_budget_timeouts": budget_timeouts,
}
for key, expression in (task.get("vars") or {}).items():
context[key] = resolve(expression, context)
return {
key: resolve(expression, context)
for key, expression in task["ansible.builtin.set_fact"].items()
}
class TestBudgetTimeoutNotRetried(unittest.TestCase):
def test_a_budget_timeout_is_kept_but_not_retried(self) -> None:
facts = stash_facts(["web-app-docs", "web-app-foo"], ["web-app-docs"])
self.assertEqual(facts["_playwright_retry_roles"], ["web-app-foo"])
self.assertEqual(facts["_playwright_failures"], ["web-app-docs"])
def test_an_ordinary_failure_is_retried_and_cleared(self) -> None:
facts = stash_facts(["web-app-foo"], [])
self.assertEqual(facts["_playwright_retry_roles"], ["web-app-foo"])
self.assertEqual(facts["_playwright_failures"], [])
def test_every_budget_timeout_still_reaches_the_fail_task(self) -> None:
facts = stash_facts(["web-app-docs"], ["web-app-docs"])
self.assertEqual(facts["_playwright_retry_roles"], [])
self.assertEqual(facts["_playwright_failures"], ["web-app-docs"])
if __name__ == "__main__":
unittest.main()
@@ -108,7 +108,7 @@ class TestTimeoutsHelper(unittest.TestCase):
the global timeout fired first and every remaining spec was reported
"did not run" without naming the waiter."""
out = self._run(
"t.resolveTimeout(2700000)",
"t.resolveTimeout(2700000) + ':' + t.SUITE_RESERVE_MS",
{
"CANONICAL_DOMAIN": '"docs.abc123.onion"',
"PLAYWRIGHT_TIMEOUT_FACTOR": "1",
@@ -116,7 +116,8 @@ class TestTimeoutsHelper(unittest.TestCase):
"INFINITO_PLAYWRIGHT_GLOBAL_TIMEOUT_MS": "5340000", # nocheck: the playwright role passes this into the sidecar container, it is not a .env key
},
)
self.assertIn("RESULT:5280000", out)
capped, reserve = out.split("RESULT:")[1].strip().strip('"').split(":")
self.assertEqual(int(capped), 5340000 - int(reserve))
def test_a_timeout_inside_the_budget_is_left_alone(self):
out = self._run(