diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c5cb11da..250e61f0a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -270,6 +270,17 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" exit "$status" + # The step keeps the report that coverage xml writes; a run with no report warns. + - name: Upload Coverage XML + if: always() && github.event_name == 'pull_request' && steps.changes.outputs.outside_plan != 'false' + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: coverage-xml + path: .data/coverage.xml + include-hidden-files: true + if-no-files-found: warn + retention-days: 7 + # AGENTS.md rule 5: a check that averages more than 5 minutes is a P0 bug. The run of one pull # request is one sample, so this job reads the last twenty completed pushes to main from the # Actions API and fails when the mean of the `Tests & Coverage` check is over 300 s (#2699). The diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b6f953aba..1988dcead 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,7 +25,7 @@ These rules apply to every contributor. [`AGENTS.md`](AGENTS.md) sends agents he - Each exception class declares its `error_code` and its `exit_code` as class attributes, and declares only the one that differs from its base. Its `__init__` adds detail keys or builds the message, or it has no `__init__`. Pass `exit_code=` only to give one error a status that its class does not have, such as the exit status of a failed subprocess. - Raising bare Python built-in exceptions (`ValueError`, `RuntimeError`, `TypeError`) is strictly prohibited. - A command raises the error and does not report it. The wrapper of `new_typer()` reports each `DevOpsCLIError` that a command or a callback raises, and exits with the error's `exit_code` ([ARCHITECTURE.md](ARCHITECTURE.md), section 7). Catch a `DevOpsCLIError` in a command only to recover from it. -- **Minimum 85% Code Coverage, and 90% of the Changed Lines**: Continuous quality gate. `devops ci` enforces 85% of the lines of `src/` (`fail_under` in `pyproject.toml`). A pull request must also cover 90% of its changed lines: the `Changed-Line Coverage` step of CI fails below that. +- **Minimum 85% Code Coverage, and 90% of the Changed Lines**: Continuous quality gate. `devops ci` enforces 85% of the lines and branches of `src/` (`fail_under` in `pyproject.toml`). The aggregate CI job keeps `.data/coverage.xml` as an artifact for 7 days. A pull request must also cover 90% of its changed lines: the `Changed-Line Coverage` step of CI fails below that. - **Pre-1.0 Alpha Status & Zero Backwards Compatibility**: DevOps CLI is pre-1.0 alpha software. Until at least release `1.0.0`, there is no intention of maintaining backwards compatibility. The codebase must remain clean of legacy references and obsolete shims at all times so that it can reach maturity at a reasonable rate. Any version after `1.0.0` will follow strict semantic versioning conventions, and employ all change management best practices including feature flags, deprecations, and migration functionality. - **Zero Zombie Code & Legacy Remnants**: Ruthlessly remove obsolete shims, legacy workarounds, and deprecated aliases. Never introduce compatibility shims during the pre-1.0 development cycle. diff --git a/docs/SDLC.md b/docs/SDLC.md index c341fd852..43a83f143 100644 --- a/docs/SDLC.md +++ b/docs/SDLC.md @@ -216,7 +216,7 @@ CI is the authoritative gate ([ADR 0004](adr/0004-ci-is-the-authoritative-gate.m #### The Test Shards (`ci.yml`, `test-shard` and `test`) The suite has about 18,000 tests, and on one runner it took 355 s to 528 s, depending on the speed of the runner that GitHub gave. AGENTS.md rule 5 sets the bound at 300 s. The test row therefore runs in four shards on four runners, the jobs `Tests & Coverage (shard of 4)`. Each runs `uv run devops ci --check --no-cache --only test --shard --shards 4`, which passes the paths of about one quarter of the test files to pytest, so a shard collects only its own files. A shard checks no coverage floor, since it measures a part of the tree. It uploads its coverage data as an artifact. -The required check `Tests & Coverage` is the job that waits for the shards. It fails when a shard did not succeed, because a skipped job would pass a required check. It downloads the data, runs `coverage combine` and `coverage report`, which applies `fail_under` of `pyproject.toml` to the whole tree, and then runs the step `Changed-Line Coverage`. The ruleset binds to this job, never to a shard ([ADR 0004](adr/0004-ci-is-the-authoritative-gate.md)). To add a shard, add a number to the matrix `shard` of `test-shard`: the name, `--shards` and the split follow the count. +The required check `Tests & Coverage` is the job that waits for the shards. It fails when a shard did not succeed, because a skipped job would pass a required check. It downloads the data, runs `coverage combine` and `coverage report`, which applies `fail_under` of `pyproject.toml` to the lines and branches of the whole tree (`[tool.coverage.run]` sets `branch = true`), and then runs the step `Changed-Line Coverage`. Beside the step that writes it, the job keeps `.data/coverage.xml` as an artifact with a 7-day retention. The ruleset binds to this job, never to a shard ([ADR 0004](adr/0004-ci-is-the-authoritative-gate.md)). To add a shard, add a number to the matrix `shard` of `test-shard`: the name, `--shards` and the split follow the count. #### The Plan-Only Test Run (`ci.yml`, `test-shard` and `test`) A pull request that only adds a plan under `openspec/changes/-/` changes no line that the suite tests. The full suite takes minutes for it. The jobs thus find such a pull request with `dorny/paths-filter`, and shard 1 runs `uv run devops ci test tests/invariants` and nothing more of the suite. No shard runs the gate or uploads coverage data, and the job `Tests & Coverage` skips its download, its combine and its changed-line coverage. The command expands the directory to its test modules, and exits non-zero when the directory gives none, so a green step means that tests ran. Those tests read the change folders: their names and forms, private addresses in committed files, the pull request template and the architectural rules. The step `Changed-Line Coverage` does not run, because the reduced run writes no coverage data and a plan holds no Python line. diff --git a/docs/TESTING.md b/docs/TESTING.md index 95717ffe8..9d307ee45 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -177,7 +177,7 @@ Every new test follows these rules. Some of them name a path, fixture or check t ### Coverage -- The coverage floor is 85% of the lines of `src/` (`fail_under` in `pyproject.toml`). A pull request must also cover 90% of its changed lines: the `Changed-Line Coverage` step of `ci.yml` fails below that. +- The coverage floor is 85% of the lines and branches of `src/` (`fail_under` in `pyproject.toml`). The aggregate CI job keeps `.data/coverage.xml` as an artifact for 7 days. A pull request must also cover 90% of its changed lines: the `Changed-Line Coverage` step of `ci.yml` fails below that. - A line that no public behaviour reaches is dead code. It is deleted, not covered by a test that reaches past the public interface. ### Floor and ratchets diff --git a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/.openspec.yaml b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/.openspec.yaml similarity index 100% rename from openspec/changes/2163-enable-branch-coverage-and-upload-coverage/.openspec.yaml rename to openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/.openspec.yaml diff --git a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/design.md b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/design.md similarity index 100% rename from openspec/changes/2163-enable-branch-coverage-and-upload-coverage/design.md rename to openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/design.md diff --git a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/proposal.md b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/proposal.md similarity index 100% rename from openspec/changes/2163-enable-branch-coverage-and-upload-coverage/proposal.md rename to openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/proposal.md diff --git a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/specs/ci-coverage-reports/spec.md b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/specs/ci-coverage-reports/spec.md similarity index 100% rename from openspec/changes/2163-enable-branch-coverage-and-upload-coverage/specs/ci-coverage-reports/spec.md rename to openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/specs/ci-coverage-reports/spec.md diff --git a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/specs/coverage-branch-measurement/spec.md b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/specs/coverage-branch-measurement/spec.md similarity index 100% rename from openspec/changes/2163-enable-branch-coverage-and-upload-coverage/specs/coverage-branch-measurement/spec.md rename to openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/specs/coverage-branch-measurement/spec.md diff --git a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/tasks.md b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/tasks.md similarity index 92% rename from openspec/changes/2163-enable-branch-coverage-and-upload-coverage/tasks.md rename to openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/tasks.md index db7633b08..8df3db059 100644 --- a/openspec/changes/2163-enable-branch-coverage-and-upload-coverage/tasks.md +++ b/openspec/changes/archive/2026-11-07-2163-enable-branch-coverage-and-upload-coverage/tasks.md @@ -4,30 +4,30 @@ Reads: `pyproject.toml`, `tests/invariants/test_agents_md_claims.py`. Changes: `pyproject.toml`, `tests/invariants/test_agents_md_claims.py`. -- [ ] 1.1 Add `branch = true` to `[tool.coverage.run]` after `parallel = true` in `pyproject.toml`, and leave `fail_under = 85` and `data_file = ".data/.coverage"` as they are. In `tests/invariants/test_agents_md_claims.py`, next to the test that reads the coverage floor, add a test that loads `pyproject.toml` with `tomllib` and asserts the `[tool.coverage.run]` table equals `{"data_file": ".data/.coverage", "parallel": True, "branch": True}`. The test fails on the base because the table has no `branch` key +- [x] 1.1 Add `branch = true` to `[tool.coverage.run]` after `parallel = true` in `pyproject.toml`, and leave `fail_under = 85` and `data_file = ".data/.coverage"` as they are. In `tests/invariants/test_agents_md_claims.py`, next to the test that reads the coverage floor, add a test that loads `pyproject.toml` with `tomllib` and asserts the `[tool.coverage.run]` table equals `{"data_file": ".data/.coverage", "parallel": True, "branch": True}`. The test fails on the base because the table has no `branch` key - Verification: `$ uv run devops ci test tests/invariants/test_agents_md_claims.py` passes ## 2. Upload the coverage XML from the aggregate job Reads: `.github/workflows/ci.yml`. Changes: `.github/workflows/ci.yml`. -- [ ] 2.1 In `.github/workflows/ci.yml`, add the step `Upload Coverage XML` to the job `test`, after `Changed-Line Coverage`. Set `if: always() && github.event_name == 'pull_request' && steps.changes.outputs.outside_plan != 'false'`, `uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2`, and `with: name: coverage-xml`, `path: .data/coverage.xml`, `include-hidden-files: true`, `if-no-files-found: warn`, `retention-days: 7`. Add a comment that says the step keeps the report that `coverage xml` writes and that a run with no report warns +- [x] 2.1 In `.github/workflows/ci.yml`, add the step `Upload Coverage XML` to the job `test`, after `Changed-Line Coverage`. Set `if: always() && github.event_name == 'pull_request' && steps.changes.outputs.outside_plan != 'false'`, `uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2`, and `with: name: coverage-xml`, `path: .data/coverage.xml`, `include-hidden-files: true`, `if-no-files-found: warn`, `retention-days: 7`. Add a comment that says the step keeps the report that `coverage xml` writes and that a run with no report warns - Verification: `$ uv run devops ci --check --only actionlint,zizmor` passes ## 3. Hold the upload step in the workflow contract test Reads: `.github/workflows/ci.yml`, `tests/invariants/test_workflow_contracts.py`. Changes: `tests/invariants/test_workflow_contracts.py`. -- [ ] 3.1 Add a test to `tests/invariants/test_workflow_contracts.py` that loads `.github/workflows/ci.yml`, takes the steps of the job `test`, and asserts that a step named `Upload Coverage XML` comes after the step named `Changed-Line Coverage`. Assert that its `uses` is `actions/upload-artifact` with a 40-character commit SHA, that its `if` is `always() && github.event_name == 'pull_request' && steps.changes.outputs.outside_plan != 'false'`, and that its `with` equals `{"name": "coverage-xml", "path": ".data/coverage.xml", "include-hidden-files": True, "if-no-files-found": "warn", "retention-days": 7}`. Assert that the `Changed-Line Coverage` step still runs `uv run coverage xml` before `diff-cover .data/coverage.xml`, so the step that reads the report is unchanged +- [x] 3.1 Add a test to `tests/invariants/test_workflow_contracts.py` that loads `.github/workflows/ci.yml`, takes the steps of the job `test`, and asserts that a step named `Upload Coverage XML` comes after the step named `Changed-Line Coverage`. Assert that its `uses` is `actions/upload-artifact` with a 40-character commit SHA, that its `if` is `always() && github.event_name == 'pull_request' && steps.changes.outputs.outside_plan != 'false'`, and that its `with` equals `{"name": "coverage-xml", "path": ".data/coverage.xml", "include-hidden-files": True, "if-no-files-found": "warn", "retention-days": 7}`. Assert that the `Changed-Line Coverage` step still runs `uv run coverage xml` before `diff-cover .data/coverage.xml`, so the step that reads the report is unchanged - Verification: `$ uv run devops ci test tests/invariants/test_workflow_contracts.py` passes ## 4. State the branch measurement and the kept report Reads: `CONTRIBUTING.md`, `docs/TESTING.md`, `docs/SDLC.md`, `pyproject.toml`. Changes: `CONTRIBUTING.md`, `docs/TESTING.md`, `docs/SDLC.md`. -- [ ] 4.1 Change the floor sentence of `CONTRIBUTING.md` and of `docs/TESTING.md` to say that `devops ci` enforces 85% of the lines and branches of `src/`, keeping the figure once in each document so the test that reads `fail_under` still finds one statement per file. In `docs/SDLC.md`, state that `[tool.coverage.run]` sets `branch = true` and that the job `Tests & Coverage` keeps `.data/coverage.xml` as an artifact with a 7-day retention, beside the step that writes it +- [x] 4.1 Change the floor sentence of `CONTRIBUTING.md` and of `docs/TESTING.md` to say that `devops ci` enforces 85% of the lines and branches of `src/`, keeping the figure once in each document so the test that reads `fail_under` still finds one statement per file. In `docs/SDLC.md`, state that `[tool.coverage.run]` sets `branch = true` and that the job `Tests & Coverage` keeps `.data/coverage.xml` as an artifact with a 7-day retention, beside the step that writes it - Verification: `$ uv run devops ci` passes -- [ ] 4.2 Edit `CONTRIBUTING.md`, `docs/TESTING.md` and `docs/SDLC.md` to state that the floor counts lines and branches and that the job keeps the XML report for 7 days +- [x] 4.2 Edit `CONTRIBUTING.md`, `docs/TESTING.md` and `docs/SDLC.md` to state that the floor counts lines and branches and that the job keeps the XML report for 7 days - Verification: `$ uv run devops ci docs` passes ## Criteria diff --git a/openspec/specs/ci-coverage-reports/spec.md b/openspec/specs/ci-coverage-reports/spec.md index 883c50876..a96dcbdac 100644 --- a/openspec/specs/ci-coverage-reports/spec.md +++ b/openspec/specs/ci-coverage-reports/spec.md @@ -29,3 +29,18 @@ The `Changed-Line Coverage` step of the `test` job in `.github/workflows/ci.yml` #### Scenario: The combined data comes first - **WHEN** the steps of the `test` job are read - **THEN** the step that runs `coverage combine` comes before the step `Changed-Line Coverage` + +### Requirement: The aggregate job keeps the coverage XML as an artifact +The job `test`, named `Tests & Coverage`, in `.github/workflows/ci.yml` SHALL upload `.data/coverage.xml` with `actions/upload-artifact` in a step after `Changed-Line Coverage`, pinned by a full commit SHA, with `retention-days: 7` and `include-hidden-files: true`. + +#### Scenario: The job uploads the report +- **WHEN** a test reads the steps of the job `test` in `.github/workflows/ci.yml` +- **THEN** one step after `Changed-Line Coverage` uses `actions/upload-artifact` pinned by a 40-character commit SHA with `path: .data/coverage.xml`, `include-hidden-files: true` and `retention-days: 7` + +#### Scenario: The report survives a failed changed-line gate +- **WHEN** the `Changed-Line Coverage` step exits non-zero +- **THEN** the upload step carries `if: always()` and `if-no-files-found: warn`, so it runs and a missing report warns instead of failing + +#### Scenario: A run that writes no report warns +- **WHEN** a push to main or a plan-only pull request runs the job +- **THEN** the upload step writes no artifact and adds no failure, because `if-no-files-found: warn` holds it to a warning diff --git a/openspec/specs/coverage-branch-measurement/spec.md b/openspec/specs/coverage-branch-measurement/spec.md new file mode 100644 index 000000000..3c183088d --- /dev/null +++ b/openspec/specs/coverage-branch-measurement/spec.md @@ -0,0 +1,17 @@ +# coverage-branch-measurement Specification + +## Purpose +How the coverage gate measures branches as well as lines, so a conditional whose other side has no test fails the gate instead of passing it. + +## Requirements + +### Requirement: The coverage gate measures branches +The `[tool.coverage.run]` table of `pyproject.toml` SHALL set `branch = true`, so coverage.py measures both sides of each conditional in the gate, in the shards and in `devops ci coverage`. + +#### Scenario: The coverage configuration enables branch measurement +- **WHEN** a test loads `pyproject.toml` and reads the `[tool.coverage.run]` table +- **THEN** the table sets `branch = true`, `data_file = ".data/.coverage"` and `parallel = true` + +#### Scenario: The floor stays the owner's figure +- **WHEN** a test reads the `[tool.coverage.report]` table of `pyproject.toml` +- **THEN** the table sets `fail_under = 85` diff --git a/pyproject.toml b/pyproject.toml index 113d61451..7b39b2b39 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -234,6 +234,7 @@ filterwarnings = [ [tool.coverage.run] data_file = ".data/.coverage" parallel = true +branch = true [tool.coverage.report] # The floor of the whole tree (#2356). The changed lines of a pull request must be 90% covered: diff --git a/tests/invariants/test_agents_md_claims.py b/tests/invariants/test_agents_md_claims.py index b5998abfb..85111d7e7 100644 --- a/tests/invariants/test_agents_md_claims.py +++ b/tests/invariants/test_agents_md_claims.py @@ -625,6 +625,14 @@ def _coverage_floors_stated(document: Path) -> list[int]: return [int(figure) for figure in _COVERAGE_FLOOR_STATEMENT.findall(text)] +def test_the_coverage_run_table_measures_branches() -> None: + """`[tool.coverage.run]` measures both sides of each conditional in every row of the gate, + and it keeps the data file that the aggregate job combines (#2163).""" + run = tomllib.loads(_PYPROJECT.read_text(encoding="utf-8"))["tool"]["coverage"]["run"] + + assert run == {"data_file": ".data/.coverage", "parallel": True, "branch": True} + + def test_the_contributor_guide_and_the_test_rules_state_the_coverage_floor_of_pyproject() -> None: """Two documents give the figure of the coverage floor, once each, and it is the `fail_under` of `pyproject.toml`. Every other document names the gate without a figure, so a diff --git a/tests/invariants/test_workflow_contracts.py b/tests/invariants/test_workflow_contracts.py index bbea21ddb..a097fb2b4 100644 --- a/tests/invariants/test_workflow_contracts.py +++ b/tests/invariants/test_workflow_contracts.py @@ -1521,6 +1521,7 @@ def test_actionlint_digest_pinned_in_dockerfile_and_ci() -> None: FULL_SUITE_STEP = "Tests & Coverage Quality Gate" PLAN_SUITE_STEP = "Plan-Only Invariant Tests" COVERAGE_STEP = "Changed-Line Coverage" +UPLOAD_COVERAGE_XML_STEP = "Upload Coverage XML" SHARDS_STEP = "Require Passing Test Shards" COMBINE_STEP = "Combine and Check Coverage" # Only the exact text `false` selects the reduced run: a skipped filter step leaves no output. @@ -1946,3 +1947,36 @@ def test_the_plan_check_job_fills_the_tiktoken_cache_from_the_committed_encoding True, True, ) + + +def test_the_test_job_uploads_the_coverage_xml_report() -> None: + """The aggregate job writes `.data/coverage.xml` for diff-cover, and `actions/upload-artifact` + keeps the report as an artifact for 7 days so a reviewer can inspect per-file figures (#2163). + The step runs with `if: always()` on pull requests outside plan, and warns if no report exists.""" + steps = _load_workflow(WORKFLOWS_DIR / "ci.yml")["jobs"]["test"]["steps"] + names = [s.get("name") for s in steps] + assert COVERAGE_STEP in names + assert UPLOAD_COVERAGE_XML_STEP in names + assert names.index(COVERAGE_STEP) < names.index(UPLOAD_COVERAGE_XML_STEP) + + coverage_step = steps[names.index(COVERAGE_STEP)] + coverage_run = coverage_step.get("run", "") + assert "uv run coverage xml" in coverage_run + assert "diff-cover .data/coverage.xml" in coverage_run + assert coverage_run.index("uv run coverage xml") < coverage_run.index( + "diff-cover .data/coverage.xml" + ) + + upload_step = steps[names.index(UPLOAD_COVERAGE_XML_STEP)] + assert re.fullmatch(r"actions/upload-artifact@[0-9a-f]{40}", upload_step.get("uses", "")) + assert ( + upload_step.get("if") + == "always() && github.event_name == 'pull_request' && steps.changes.outputs.outside_plan != 'false'" + ) + assert upload_step.get("with") == { + "name": "coverage-xml", + "path": ".data/coverage.xml", + "include-hidden-files": True, + "if-no-files-found": "warn", + "retention-days": 7, + }