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"}