Prepare for use from tornado's release process - #7
Merged
Merged
Conversation
The testbed is about to be called from tornado's release build, so a green run
has to mean "the requested tornado passed" rather than "some tornado passed".
Several paths did not meet that bar:
- A failed `uv pip install --upgrade ${TORNADO_SPEC}` was only a warning, so a
typo'd spec or a dead git ref left the downstream resolver's own tornado in
place and the package still reported PASS. It is now TORNADO_INSTALL_FAIL.
- Nothing verified which tornado ended up in the venv. The version and
__file__ are now recorded in results/<name>.txt, and TORNADO_EXPECT_VERSION
(set by the caller when the spec names a version, e.g. a wheel filename)
makes the check an assertion.
- Several test commands install more packages mid-run, so a transitive
`tornado<X` pin could quietly downgrade the thing under test after it was
verified. The version is re-probed after the run and drift fails the package.
- `uv venv` was unchecked, so on failure the following `source activate` failed
too and the suite ran against the system python. Both are checked now.
- ci.sh always exited 0. It now exits non-zero unless every package passed,
gated by FAIL_ON_REGRESSION=1 so report-only runs are still available. The
summary, step summary and failing-log tails are all written first, so a
failing gate still leaves a full report behind.
- ci.sh now clears results/ and logs/ first. Otherwise a stale result file from
an earlier run counts toward the gate, which bites hardest on ONLY= runs.
Also collapses the three duplicated result-writing blocks into one writer so
every exit path emits the same schema, and retries once on TIMEOUT only -- the
documented ZeroMQ deadlock class. Genuine failures are not retried; re-running
red tests to see if they go green is how a gate loses its meaning.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
common.sh enables `set -e`, which the previous commit's `run_tests; rc=$?` refactor did not account for (the code it replaced had an explicit `set +e` / `set -e` pair around the same call). The effect was worse than the bug it was part of fixing: the first failing suite aborted run_one.sh before any result file was written, so run_all.sh reported NORESULT and ci.sh's new gate -- which iterates the result files that exist -- never saw the package at all. A red package was invisible. Verified against all of run_one.sh's exit paths: PASS, FAIL (a suite exiting non-zero now writes status=FAIL and returns its own code), TORNADO_INSTALL_FAIL, and TORNADO_MISMATCH in both the expected-version and mid-run-drift forms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
The manifest was 14 KB, of which only about six short fields per package were actually data (repo, ref, subdir, install_method, pypi_spec, name). The rest was in the wrong container: ~8.3 KB of prose in `notes` and `reason`, and ~2.5 KB of shell in `test_cmd` and `setup_extra`, escaped into JSON one-liners. jupyter_server alone carried a 370-character command and 1.8 KB of notes describing that command from a distance. Each package is now packages/<name>/ holding requirements.txt (the pin), package.env (metadata as strict key=value), test.sh, an optional setup.sh, and a README.md. The downstream sources move to checkouts/, freeing packages/ for the definitions. What this buys beyond tidiness: - The reason a test is deselected now sits on the line above the deselect. In a harness whose whole premise is "red means a Tornado regression", that rationale is the thing needing the most upkeep, and it was living in a separate field. - shellcheck covers the test commands, which were invisible to it inside JSON. - The git ref is derived from the pin via tag_template instead of stored beside it, so `ref` and `pypi_spec` can no longer disagree about which version is under test. - pkg_field is gone, along with its ~6 python3 spawns per package and its interpolation of the lookup key into python source. scripts/pkglib.py is now the single parser, shared by bash and by validate_packages.py. - Adding a package is a mkdir and four files, with no conflicts in a shared manifest. Behaviour-preserving, checked before any pin was touched: the derived refs match the old manifest exactly for all ten packages, and a full run at tornado 6.5.5 reproduces the committed snapshot for eight of them (statuses and comparable timings; flower's "162 passed, 2 skipped" and the rest unchanged). The two that differ are pre-existing rot, not regressions from this change: jupyter_server 2.14.2 and notebook 7.2.2 no longer build under current hatchling, which a bare `uv pip install -e .` in their checkouts reproduces with no harness code involved. Refreshing those pins is the next commit. (panel also failed once on a PyPI metadata timeout and passes on retry.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
ci.sh carried the summary renderer, the failing-log tails and the exit-code decision inline. The job matrix added in the next commit needs all three too, and two implementations of "did this run pass?" would eventually disagree -- which for a release gate is the expensive kind of bug. scripts/report.sh now owns rendering the markdown summary, echoing each failing package's log tail, publishing total/passed/failed, and choosing the exit code. Both paths call it: ci.sh execs it at the end, and the matrix's report job runs it over the downloaded per-package artifacts. To make that second case work, common.sh lets RESULTS_DIR, LOGS_DIR and CHECKOUTS_DIR be overridden, so the report can be pointed at a directory of collected artifacts rather than a local run's output. (It previously assigned them unconditionally, which silently ignored any override.) ci.sh also now validates the package definitions before doing anything, so a malformed definition fails in the first second rather than twenty minutes in, and skips the HTML coverage build when COVERAGE=0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
The workflow was workflow_dispatch only, so using it meant a human remembering to. It now also accepts workflow_call, which is how tornadoweb/tornado's build.yml will gate on it: that workflow already runs on exactly the right events (pushes to release branches and v* tags) and already builds the artifacts we want to test. The artifact path is the reason to prefer a reusable workflow over a cross-repo dispatch. A called workflow runs as jobs inside the *caller's* run, so actions/download-artifact resolves against that run and the testbed can install the exact wheel or sdist the release build just produced, rather than a re-resolved git ref. resolve_tornado_artifact.sh picks the artifact, prefers a wheel over an sdist, and reads the version out of the filename so run_one.sh can assert that version is what ends up installed. It fails loudly on an artifact with no tornado build in it, because falling back to PyPI's tornado would make the entire run meaningless. Packages now run as a job matrix rather than serially: - The old single job had a 60-minute limit against a worst case of 10 x 900s. A slow run was killed by GitHub with no summary at all; now each package has its own budget. - fail-fast is off, so one red package does not hide the others, and a single flaky package can be re-run from the Actions UI. - It also isolates the orphaned-pytest-process contention in REPORT.md. A weekly schedule runs it against master. Without that, a release run would be where we discover downstream drift has broken the baseline -- which is how gates get ignored. This is not hypothetical: two pins had already rotted past installing. Coverage in the matrix path uploads the raw .coverage files only. Per-package HTML needs each package's venv, which does not survive a matrix job, so HTML reports stay with the serial ci.sh path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
Every one of the ten pins had drifted, several by years: distributed 2024.10.0 against 2026.8.0, ipykernel 6.29.5 against 7.3.0, streamlit 1.40.0 against 1.64.0. Two had drifted far enough to stop installing at all. Testing two-year-old downstream code against a new Tornado loses most of the signal, since downstream may already have fixed whatever would have broken. Rather than write a pin checker, this uses the tool that already does the job. The obstacle was that the harness installs from a git checkout at a tag -- because the test suites are not in the wheels -- and dependabot only understands package-manager files. The tag_template indirection added with the per-package directories is the bridge: dependabot manages the version in requirements.txt, and the harness derives the tag from it. One directory per package is deliberate. A single shared requirements file would ask dependabot to resolve streamlit, distributed and jupyterhub into one environment, which cannot succeed. The pins workflow then runs the package a bump actually changed, against the latest stable Tornado, so a bump needing its deselects re-curated says so on its own pull request instead of surfacing during a release build. It also runs validate_packages.py --check-refs, which catches the case where dependabot raises a version whose git tag does not exist -- again on the bump's PR rather than during a release. What dependabot cannot do is re-curate deselects or follow a downstream major that moved its test layout. That stays human work, by design. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
Prep for the repo becoming org infrastructure rather than a personal one-off. LICENSE is Apache 2.0, byte-identical to tornado's. The repo was unlicensed, which is a blocker for adopting it into the release process. tests/harness_test.sh covers the harness's own behaviour -- 37 checks, each building a throwaway repo root out of the real scripts plus synthetic packages, so nothing touches the real definitions and paths resolve as they do in a real checkout. It covers every status run_one.sh can produce, including the ones that exist because a run had silently tested the wrong Tornado, and the definition shapes validate_packages.py must reject. One case is there specifically as a regression test: a failing suite must still write its result file, because when it did not, a red package was invisible to the gate. The run_one.sh cases need to install tornado, so they skip with a message when that is not possible rather than failing ten times over. lint.yml runs validate_packages.py, shellcheck, zizmor and the self-tests. shellcheck is clean, which the previous layout could not have been: the test commands were strings inside JSON and invisible to it. zizmor is clean too, under a policy file mirroring tornado's, with matrix values passed through the environment rather than expanded into run blocks. CONTRIBUTING.md states the rule everything else follows from -- a red package must mean a Tornado regression -- and CLAUDE.md collects the traps, including that common.sh enables `set -e` and that editing a script mid-run corrupts the running shell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
jupyter_server and notebook both failed `uv pip install -e .` outright, which looked like their 2024-era pins having rotted past usability. It was not: a build-backend release had broken them, and bumping the pins did not help. hatchling 1.32.1 gave BuildHookInterface a second type parameter (Generic[BuilderConfigBound, PluginManagerBound]). hatch_jupyter_builder 0.9.1, the latest release, still subscribes it with one, so every source build in the Jupyter stack dies with "TypeError: Too few arguments for BuildHookInterface; actual 1, expected 2". hatchling 1.32.0 is the last release with one parameter. Packages do not pin their own build backends, so nothing downstream protects against this, and the harness had no opinion about its build environment at all. It does now: UV_BUILD_CONSTRAINT points every `uv pip install` in a run -- including those inside a package's test.sh -- at build-constraints.txt. This constrains build-time (PEP 518) dependencies only; it does not touch what a package installs at runtime, and it cannot touch the Tornado under test. With the constraint in place, jupyter_server 2.14.2 installs and passes again at its original pin, so this was never about pin age. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
The change that makes this testbed part of tornado's release build lives in tornadoweb/tornado, which needs someone with push access there, so it ships here as an applyable patch plus the reasoning behind its shape. It adds a `downstream` job to build.yml calling this repo's testbed.yml as a reusable workflow, and adds `tornadoweb/*: ref-pin` to tornado's zizmor policy, without which its workflow lint rejects a `uses:` that is not hash-pinned. Verified by applying both to a tornado checkout: the workflows parse and `uvx zizmor .github/workflows` reports no findings. The job is deliberately not in upload_pypi's `needs` -- a flaky downstream suite must not be able to wedge a release upload -- and is skipped on forks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
Every pin had drifted, several by years. Bumping them turned up one finding that changed the set and one that changed what a green run means. streamlit is removed. It migrated to Starlette/uvicorn in 1.57.0: 1.64.0 declares no dependency on Tornado and contains no tornado imports at all, so it failed the testbed's first selection criterion. It was rank 1 and the single highest-coverage package (45%), but a pin frozen at 1.56.0 could never move again and would have gone on reporting coverage for code the ecosystem is leaving behind. Merged coverage only fell from 61% to 59% -- the other web-layer packages already covered most of the same ground. Ranks are renumbered 1-9 so the rank selector stays contiguous. jupyter_server moves to 2.21.1 (1136 tests, up from 970) and with it the testbed loses its tornado#3724 detector: all four affected tests now pass on 6.5.9, because 2.21.1 changed FileFindHandler enough not to trip the bug. Tracking current downstream code is the point, so this is the right trade, but it means a green run is no longer self-evidently proof the gate works. The replay is now a documented manual procedure that pins the package back to 2.14.2, in that package's README, with the caveat repeated in CLAUDE.md so nobody runs it against the current pin and concludes from green that all is well. The three bumps that did not come up green: - bokeh 3.10 requires Python >=3.12. PYTHON_VERSION now defaults to 3.13, matching tornado's own default_python_minor and the workflow input, rather than 3.11. - flower's broker tests failed with "NameError: name 'Retry' is not defined": broker.py imports it inside a try/except ImportError and redis was not installed. A setup.sh hook installs it, which beats deselecting 22 tests for a reason unrelated to Tornado -- 251 pass now, up from 162. - panel's suite uses pytest.mark.xdist_group without pytest-xdist installed, and its filterwarnings=error turns the unknown-mark warning into a collection error. pytest-xdist is now installed to register the mark, not to parallelize. Also fixes a bug the self-tests caught in the build-constraints change: uv errors on a constraints file that is not present, so setting UV_BUILD_CONSTRAINT unconditionally turned every install in a checkout without that file into an INSTALL_FAIL. It is now set only when the file exists, with a test covering it. gen_reports.sh additionally prunes report directories for packages that no longer exist, which is how a stale streamlit coverage page survived its removal. 9/9 green against tornado 6.5.10 on Python 3.13. 38/38 harness self-tests, shellcheck and zizmor clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
All nine matrix jobs computed an identical setup-uv cache key, so they raced to reserve one entry and eight logged "Unable to reserve cache". setup-uv builds the key as setup-uv-<v>-<arch>-<platform>-<os>-<python><pruned><glob-hash><suffix>, and with no cache-suffix nothing in it varied per package. Each job now carries cache-suffix: <package>-py<python_version>. Per-job caches rather than one shared cache is the right shape here: the nine packages install completely disjoint dependency trees, so a shared entry would be the union of all of them and mostly irrelevant to any single job, and sharing one "properly" would need a warm-up job that serialises the matrix behind it. The default cache-dependency-glob was a second problem: it matches **/*requirements*.txt and **/*constraints*.txt, so every job hashed all nine packages' pins plus build-constraints.txt, and one dependabot bump invalidated all nine caches at once. Each job now keys on its own pin and the shared build constraints. The interpreter is in the suffix rather than passed as setup-uv's python-version input, so UV_PYTHON stays unset and run_one.sh remains the only thing choosing an interpreter, while a python_version change still busts the caches. lint.yml gets cache-suffix: lint so it cannot collide with a matrix job running at the same time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
Coverage resolved to 0 on every automatic path. `schedule` runs have no inputs so IN_COVERAGE was empty; neither pins.yml nor tornado's release gate passed it. Only a hand-run dispatch turned it on, and even then the matrix uploaded nine opaque coverage-<pkg> artifacts that the report job never downloaded, merged or rendered -- gen_reports.sh was not invoked in the matrix path at all. It now runs on the weekly schedule and on pin-update pull requests, and stays off for the release gate, whose job is pass/fail. gen_reports.sh no longer needs the per-package venvs. That was a limitation of the script rather than a real one: the [paths] remap already collapses every package's own site-packages tornado onto one canonical path, so all the script really needed was a venv with coverage and a tornado source tree to annotate. It now builds a report venv holding coverage plus the tornado under test when no package venv is around, and renders each package through combine-then-report so the remap applies. Verified by deleting every checkout and rebuilding all ten reports from nothing but the .coverage files: identical numbers. The floor is the part that matters. A passing run is not evidence that anything was tested: when a new downstream release moves or renames the files a test.sh names, the command keeps exiting 0 while exercising almost none of Tornado, and no other signal here would notice. Each package now declares min_coverage in its package.env, and a run that measures coverage fails as COVERAGE_LOW when it comes in under that floor. Arming it on pin-update PRs is the point -- that is exactly when this happens, and dependabot never touches those values, so a bump that guts a test command fails on its own pull request. Implemented as a status rather than a bespoke workflow step so report.sh's existing "anything that is not PASS fails the run" applies unchanged, and the serial and matrix paths cannot disagree. It is checked only when coverage was requested and only when the suite passed: with COVERAGE=0 every package that declares a floor would otherwise fail the release gate, and on a failing run the low coverage is explained by the failure, so reporting COVERAGE_LOW there would bury the real one. Both are covered by tests. Floors start ~20% under measured, so drift does not trip them and a collapse does. The report prints a reminder when a package runs well clear of its own, because a floor that has stopped tracking reality has stopped being a check. Verified: 9/9 green at tornado 6.5.10 with coverage on, every package clearing its floor, and every measured value matching the table in REPORT.md. 45/45 harness self-tests, shellcheck and zizmor clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TDiwCvRXgWijEDDsbuiT3g
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.