ci(unit-tests): bound the apt install steps so a mirror stall cannot burn the job (#974)

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix — corrects a defect
• feat — adds a feature
• perf — changes performance, not behaviour
• refactor — changes neither behaviour nor performance
• test — tests only
• docs — documentation only
• ci — workflows or build tooling
• chore — anything else

---

Description of your pull request and other information

Closes #974. An apt-get step with no bound ran to the job's own 30-minute budget when a mirror went silent, reporting cancelled — which Required checks passed correctly refuses to treat as a pass, so an unrelated mirror blocked two docs-only PRs.

Changed: timeout-minutes: 15 on the apt step of engine-tests and of pypcap-parity, plus a comment recording the failure mode and the measurements. No install line and no job logic touched.

Not changed, deliberately: the -o Acquire::Retries=3 -o Acquire::http::Timeout=30 that #974 proposed. Both are already the defaults in the apt noble ships — apt 2.7.14 has Retries(_config->FindI("Acquire::Retries", 3)) (apt-pkg/acquire-item.cc:783) and TimeOut(30) (methods/basehttp.cc:289) read back by ConfigFindI("Timeout", TimeOut) (methods/http.cc:413), so passing them changes no behaviour while reading as protection the step had gained. Their already being in force is also why the stall ran to 29m52s rather than failing after 30s and three retries.

Why 15 and not a tight "fail fast" number: measured over 12 runs (230 non-zero observations), the slowest successful run of these steps was 508s (PyShark), p95 338s; pypcap-parity reached 405s. A 5-minute cap would have failed ~4% of healthy legs — a worse defect than the one being fixed. Retry-with-timeout wrappers are unsafe for the same reason: a legitimate 508s run is indistinguishable from a stall inside any budget short enough to be worth retrying.

This PR's own CI confirms it: with the bound in place, pypcap-parity Python 3.10's apt step took 448s and Python 3.11's 288s, both green, no cancelled legs. A 5-minute cap would have failed the first outright and come within 12 seconds of failing the second.

YAML structure comparison: yaml.safe_load flattened to leaf paths, base vs head — 177 → 179 leaves, differing only by jobs.engine-tests.steps[2].timeout-minutes = 15 and jobs.pypcap-parity.steps[2].timeout-minutes = 15. Both run: bodies are byte-identical to before, and bash -n is clean on each.

Other apt-get sites: .github/workflows/ has exactly these two, both fixed. examples/benchmark/Dockerfile:76 also uses apt but no workflow builds it — it has no Actions job budget to burn and no required check to block, so it is out of scope. No brew/choco/snap/yum anywhere.

Tests: new tests/project/test_workflow_apt_timeouts.py asserts every apt-invoking step in every workflow declares a timeout-minutes below its job's budget — so a third unbounded step cannot land. 6 passed (pytest: 6 passed, 4 subtests; plain unittest: Ran 6 tests ... OK); against the pre-fix workflow it fails on both steps. tests/test_tier_guard.py 109 passed / 596 subtests, and _dependency_gates.pytest_jobs() returns byte-identical Job tuples before and after, so the text-based workflow guards are unaffected.

[JarryShaw]

NEEDS CHANGES at ab782cdba — sonnet cross-review, round 1. The substance holds;
the committed comment carried two stale facts. Fixed at 15212263f.

It verified the claim I could not. No apt on my host, so this was the open risk.
It pulled real Ubuntu noble source via docker run ubuntu:24.04 + apt-get source apt and matched all three citations verbatim — acquire-item.cc:783
Retries(…, 3), basehttp.cc:289 TimeOut(30), http.cc:413 reading it back. It
also independently re-derived the per-wait mechanism: Go() does one select() per
call with tv.tv_sec = ServerPending ? 0 : TimeOut, looped from http.cc:606, and no
cumulative deadline exists anywhere in acquire.cc/acquire-worker.cc. So a
connection making one byte of progress per 30s window can run forever — which is what
explains the 29m52s, and my #974 correction stands.

Two corrections to the committed comment:

• The 508s maximum was already false. On this PR's own run, Engines Python 3.10 (PyShark)'s apt step took 646s (17:29:39 → 17:40:25) — verified by me. 508 and
  338 now read as a floor on the tail rather than its ceiling, and "~1.8x the observed
  maximum" becomes ~1.4x. The 15-minute bound still holds against every number found.
• The apt version citation was unverifiable. 2.7.14's tarball is no longer in the
  pool; what is inspectable is 2.8.3, current noble-updates. The comment now says
  so rather than naming a version nobody can check.

One correction to the review, in the PR's favour. It disputed a docstring claim
that comment-stripping exists "so the new comment block does not read as an apt
invocation". That claim is not in the committed code — strip_comments' docstring
gives a different and defensible rationale about # inside script lines. Its
underlying measurement is right, though, and I reproduced it: patching
strip_comments to a no-op yields an identical step set, and the new comment never
contains the literal apt-get. So the stripping is defensive generality doing no work
here, not a bug, and nothing needed changing.

Re-derived by me: yaml.safe_load leaves 177 → 179, added exactly the two
timeout-minutes keys, nothing removed, nothing changed. Ran 6 tests ... OK.

[JarryShaw]

Round 2 GOOD TO GO at 15212263f — sonnet, both comment corrections confirmed. It
re-read the whole block and grepped every number in play: 338s/508s now appear only as
a floor on the tail, the retry sentence and the pypcap-parity paragraph both compare
against 646s, the math checks (900/646 ≈ 1.39), and the version citation matches what
it measured — apt 2.8.3 via container, 404 on 2.7.14's tarball. Diff confirmed
comment-only: one file, 14/12, every changed line #-prefixed.

I asked whether 646s should move the bound itself, and it said yes — so I have
raised 15 → 20 at 0b62fe021. Its argument, which I accept: one extra observation
moved the known maximum 27% (508 → 646), dropping the margin from the intended
~1.8x to ~1.4x, on a quantity the comment itself calls enormously variable. A bound
that close risks reproducing this very defect at smaller scale — a healthy-but-slow run
killed on a step timeout, still blocking the PR.

The asymmetry settles it rather than the ratio: too generous costs five extra minutes
on a hang; too tight turns a green PR red. 20 restores ~1.9x and stays under both job
budgets (30 here, 45 on pypcap-parity), so the headroom is free.

It offered this as a preference and explicitly did not gate on it. I took it anyway
because I had solicited it — asking "should this number move" and then filing the
answer as a nice-to-have would make the question ornamental.

Verified at the new head: yaml.safe_load leaves 179 → 179, the only difference
being those two values 15 → 20, nothing added, removed or otherwise changed. The new
test still passes (Ran 6 tests ... OK) and still binds — 20 < 30 and 20 < 45, which
is exactly what test_every_apt_step_timeout_actually_binds asserts.

The comment records the raise and its reason, so the next person does not read 20 as
arbitrary and trim it back.

[JarryShaw]

GOOD TO GO at 0b62fe021 — sonnet cross-review, round 3 clean, three rounds in
total. The 20-minute bound is agreed, not split.

It settled the number with its own reasoning rather than deferring: the two real jumps
in the measured maximum are p95 338s → max 508s (+50%) and 508s → 646s (+27%), so the
relative growth is shrinking, which points to convergence rather than an open-ended
climb. 1,200s gives ~1.86x over the current known max and would absorb another jump of
the size just seen (646 × 1.27 ≈ 820s) with room left. pypcap-parity's own measured
max of 405s sits comfortably inside it even granting the comment's expectation that its
heavier package set should tail longer. Its conclusion: no case for going higher, and
20 is what it would ship.

Verified independently at this head rather than taken from my report: yaml.safe_load
leaves 179 → 179, added: [], removed: [], and exactly two changed —
engine-tests.steps[2].timeout-minutes and pypcap-parity.steps[2].timeout-minutes,
both 15 → 20. No run: body, no step name, no job-level 30/45 touched. Test module
re-run at the head: Ran 6 tests ... OK.

One thing it checked that I would have missed: the surviving ~1.4x in the comment is
inside the clause explaining why 15 was raised, not a residual claim about the
current bound. I re-read unit-tests.yml:422-424 and agree — it reads as history, which
is the point of recording the raise at all. Every other bare 15 in the file is a
Python 3.15 reference or the unrelated "15 HAS_PYPCAPFILE-gated methods" note.

It also named a data point it declined to use: a fresh run for the force-push was still
pending, so it has no completed apt timings and contributed nothing either way. That is
the right call — reporting a pending run as evidence is how a number gets justified by
nothing.

Not merge-ready yet: CI is 12 ok / 0 fail / 49 inc on this head, restarted by the
force-push.