From 8d4988462e08abba27c2258070dcbdf1a244a6fb Mon Sep 17 00:00:00 2001 From: "turbolytics.io" Date: Sat, 10 Oct 2026 07:16:23 -0400 Subject: [PATCH] ci: growth runs on main only; a pull request's coverage keeps its growth status Every pull request push ran the Growth job, about 1.5 minutes of billed time, before any change could merge. A leak check proves memory does not grow with the work. Main catches a leak before any release, because a release waits for main's CI run on the tagged commit. Growth now runs only on push, which is main, and always at full length. Skipping the job alone fails Coverage. With #452's own reports and no growth report, the check found 6 gaps ("cli.serve: growth is missing"), and --write rewrote those 6 cells from covered to missing, which fails the stale check. The generator takes --not-run LEVEL: - A missing cell at that level is no gap. - The status files keep the committed status at that level. CI passes --not-run growth on pull requests and nothing on main. Evidence, from #452's reports through make coverage-check: - PR shape, no growth report, with --not-run growth: exit 0, docs unchanged. - Main shape, no growth report: fails on "growth is missing". - Main shape, with the growth report: exit 0. Two tooling tests: the gap filter and the kept status. If this is wrong, a pull request that adds a leak merges, and main's Growth job fails on it before a release. A change that breaks coverage at another level still fails the pull request. --- .github/workflows/ci.yml | 20 ++++++++++++-------- Makefile | 9 +++++++-- scripts/coverage_matrix/cli.py | 16 +++++++++++++--- scripts/coverage_matrix/statusfiles.py | 15 +++++++++++++++ tests/tooling/test_cli.py | 17 +++++++++++++++++ tests/tooling/test_statusfiles.py | 19 +++++++++++++++++-- 6 files changed, 81 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51ddf403..c8b332a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -133,22 +133,24 @@ jobs: # Image does not wait for it. Image is the long pole, and chaining it after # Growth would roughly double a push's wall time. # - # The length follows the ref. CI runs on push, so a pull request arrives as - # pushes to its branch, and a branch runs bounded: about ten seconds a test. - # Main runs full, where a slow leak has the time to show. A release waits - # for main's CI run on the tagged commit, so full growth gates every - # release. That puts this job on the release path: at full length it must - # finish within about three minutes. + # Main only, at full length, where a slow leak has the time to show. A pull + # request skips it: a leak check spends minutes on every push, and main + # catches a leak before any release. A release waits for main's CI run on + # the tagged commit, so full growth gates every release. That puts this + # job on the release path: it must finish within about three minutes. + # `make test-growth GROWTH=bounded` runs it locally in about ten seconds a + # test. # # No -race; the Makefile's test-growth says why. growth: name: Growth needs: [go, integration] + if: github.event_name == 'push' runs-on: ubuntu-latest env: SQLFLOW_DUCKDB_LIB: ${{ github.workspace }}/bin/libduckdb.so GOTOOLCHAIN: local - GROWTH: ${{ github.ref == 'refs/heads/main' && 'full' || 'bounded' }} + GROWTH: full steps: - uses: actions/checkout@v4 @@ -277,8 +279,10 @@ jobs: pattern: report-* merge-multiple: true + # A pull request runs no growth pass, so its growth cells keep their + # committed status. Main runs every level and checks all of them. - name: Check the coverage matrix - run: make coverage-check + run: make coverage-check COVERAGE_NOT_RUN="${{ github.event_name != 'push' && '--not-run growth' || '' }}" # The report carries every test name and count, which left the committed # page so that a PR adding a test changes nothing under docs/coverage. diff --git a/Makefile b/Makefile index 8b3a4e98..338e9ad5 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,11 @@ schema: @echo "regenerated turbostats/wire/schema/bundle.schema.json and response.schema.json" @echo "regenerated internal/cli/testdata/config_example.golden and serve_example.golden" +# The levels this run skipped, as --not-run flags. A pull request runs no +# growth pass, so CI passes --not-run growth there: a growth cell keeps its +# committed status rather than reading missing. Empty runs every level. +COVERAGE_NOT_RUN ?= + # Renders everything from reports that already exist. Runs no tests. # # Under docs/coverage: the status directory and the page, which are committed. @@ -116,7 +121,7 @@ coverage-write: --go .coverage/go.json \ --go-integration .coverage/go-integration.json \ --go-growth .coverage/go-growth.json \ - --pytest .coverage/pytest.json --write + --pytest .coverage/pytest.json $(COVERAGE_NOT_RUN) --write # Renders the page from the committed status files and registries. Reads no # report, so it runs anywhere, and it is what resolves a merge conflict on @@ -150,7 +155,7 @@ coverage-check: coverage-write --go .coverage/go.json \ --go-integration .coverage/go-integration.json \ --go-growth .coverage/go-growth.json \ - --pytest .coverage/pytest.json --check + --pytest .coverage/pytest.json $(COVERAGE_NOT_RUN) --check @# The Package field sits between Action and Test in go test -json, so @# the pattern spans it. Without it the list below printed nothing. @! grep -sq '"Action":"fail"' $(GO_REPORTS) || { \ diff --git a/scripts/coverage_matrix/cli.py b/scripts/coverage_matrix/cli.py index b1fa2de7..f7d801ec 100644 --- a/scripts/coverage_matrix/cli.py +++ b/scripts/coverage_matrix/cli.py @@ -8,14 +8,15 @@ from .features import build, snapshot from .invariants import build_invariants, snapshot_invariants from .page import write_page -from .registries import (COVERAGE_DIR, MATRIX_JSON, MATRIX_MD, REPO, +from .registries import (COVERAGE_DIR, LEVELS, MATRIX_JSON, MATRIX_MD, REPO, REPORT_MD, STATUS_DIR, TYPES_PAGE, load_features, load_integrations, load_invariants, load_lattice, validate_registries) from .report import render_report from .suites import parse_go, parse_pytest -from .statusfiles import read_status, status_from_snapshot, write_status +from .statusfiles import (keep_levels, read_status, status_from_snapshot, + write_status) from .types import validate_types @@ -27,6 +28,9 @@ def main(): ap.add_argument("--go-growth", help="go test -json output from the growth pass") ap.add_argument("--pytest", help="pytest result json from the conftest hook") + ap.add_argument("--not-run", action="append", default=[], choices=LEVELS, + help="a level this run did not run; its missing cells " + "are no gap. A pull request runs no growth pass") ap.add_argument("--write", action="store_true", help="write the status directory, the page, and the report") ap.add_argument("--check", action="store_true", @@ -83,8 +87,14 @@ def main(): {"unit": go_invariants, "integration": it_invariants, "growth": gr_invariants, "release": py_invariants}) snap.update(snapshot_invariants(invariants, integrations, built)) + # A level the run skipped has no report, so every cell at it reads + # missing. That says nothing about the change; main runs the level. + for key in ("gaps", "invariant_gaps"): + snap[key] = [g for g in snap[key] + if not (g["level"] in args.not_run and g["status"] == "missing")] - status = status_from_snapshot(snap) + status = keep_levels(status_from_snapshot(snap), read_status(STATUS_DIR), + args.not_run) report = render_report(snap) if args.write: diff --git a/scripts/coverage_matrix/statusfiles.py b/scripts/coverage_matrix/statusfiles.py index f60a1121..7e3118c2 100644 --- a/scripts/coverage_matrix/statusfiles.py +++ b/scripts/coverage_matrix/statusfiles.py @@ -38,6 +38,21 @@ def status_from_snapshot(snap): return {"features": features, "integrations": integrations} +def keep_levels(status, committed, levels): + """status with each cell at levels taken from committed, where it has + one. A run that skipped a level has no report for it, so its cells + read missing; the committed status is the last run that had one.""" + def merge(rows, old): + for rid, cells in rows.items(): + for lvl in levels: + if lvl in old.get(rid, {}): + cells[lvl] = old[rid][lvl] + merge(status["features"], committed.get("features", {})) + for iid, rows in status["integrations"].items(): + merge(rows, committed.get("integrations", {}).get(iid, {})) + return status + + def render_status_file(rows, about): """One row per line, sorted by id, levels in a flow mapping. diff --git a/tests/tooling/test_cli.py b/tests/tooling/test_cli.py index 3ab12b3a..e88f7c83 100644 --- a/tests/tooling/test_cli.py +++ b/tests/tooling/test_cli.py @@ -63,6 +63,23 @@ def test_the_check_reads_the_growth_report(tmp_path): assert "handler.inferred_mem: unit is skipped" in both.stderr +def test_a_level_not_run_is_no_gap(tmp_path): + """A pull request does not run the growth pass; main does. With + --not-run growth, a growth level with no report is no gap, and a gap + at a level that did run still fails the check.""" + name = "TestGrowthInferredInvoke_DoesNotLeakNativeMemory" + marker = json.dumps({"Action": "output", "Test": name, + "Output": " x.go:1: COVERS handler.inferred_mem\n"}) + unit = write(tmp_path, "go.json", "\n".join([ + marker, json.dumps({"Action": "skip", "Test": name})])) + + proc = run_check(unit, "--not-run", "growth") + + assert "growth is missing" not in proc.stderr + assert "handler.inferred_mem: unit is skipped" in proc.stderr + assert proc.returncode == 1 + + def test_the_check_exits_non_zero_on_an_unknown_marker(tmp_path): """Once the unknown-marker list left the reviewed page, failing is the only way it stays visible.""" diff --git a/tests/tooling/test_statusfiles.py b/tests/tooling/test_statusfiles.py index 450fb683..df5962fa 100644 --- a/tests/tooling/test_statusfiles.py +++ b/tests/tooling/test_statusfiles.py @@ -6,7 +6,8 @@ LEVELS, PASS, STATUS_DIR, load_features, load_integrations, load_invariants) from coverage_matrix.statusfiles import ( - read_status, render_status_file, status_from_snapshot, write_status) + keep_levels, read_status, render_status_file, status_from_snapshot, + write_status) from samples import * # noqa: F401,F403 @@ -148,4 +149,18 @@ def test_the_page_snapshot_reaches_the_same_gaps_as_the_full_snapshot(): ps = page_of(status=status_from_snapshot(full), invariants=required) assert ps["gaps"] == full["gaps"] - assert ps["invariant_gaps"] == full["invariant_gaps"] \ No newline at end of file + assert ps["invariant_gaps"] == full["invariant_gaps"] + +def test_a_level_not_run_keeps_its_committed_status(): + """A pull request runs no growth pass, so every growth cell it + generates reads missing. The committed status stands for that level, + and every other level is the run's own.""" + run = {"features": {"a": {"unit": "covered", "growth": "missing"}}, + "integrations": {"kafka": {"inv": {"unit": "failing", "growth": "missing"}}}} + committed = {"features": {"a": {"unit": "missing", "growth": "covered"}}, + "integrations": {"kafka": {"inv": {"unit": "covered", "growth": "covered"}}}} + + kept = keep_levels(run, committed, ["growth"]) + + assert kept["features"]["a"] == {"unit": "covered", "growth": "covered"} + assert kept["integrations"]["kafka"]["inv"] == {"unit": "failing", "growth": "covered"}