Report coverage in CI, and count only what a test can reach

.editorconfig

@@ -11,7 +11,7 @@ trim_trailing_whitespace = true
 indent_style = space
 indent_size = 4
 
-[*.{json,yml,yaml,csproj,props,targets,slnx,resx,webmanifest}]
+[*.{json,yml,yaml,csproj,props,targets,slnx,resx,runsettings,webmanifest}]
 indent_size = 2
 
 [*.{js,css,html,razor}]

.github/workflows/ci.yml

@@ -1,7 +1,9 @@
 name: CI
 
 # The whole pipeline for a change, in one file and in one shape: build it, test it, publish it,
-# then drive a browser over the thing that was published. The commit is compiled exactly once —
+# then drive a browser over the thing that was published and judge what the tests covered. The
+# coverage report comes out of the gate's own test run, so the number is about the code that
+# passed rather than a second run of it. The commit is compiled exactly once —
 # `dotnet publish --no-build` reuses what `dotnet build` produced and what `dotnet test` ran
 # against, so the app the browsers see is the app the unit tests passed on, not a rebuild of it.
 
@@ -60,7 +62,23 @@ jobs:
 
       - run: dotnet restore
       - run: dotnet build --no-restore --configuration Release
-      - run: dotnet test --no-build --configuration Release --verbosity normal
+
+      # The collector rides along with the run this gate already does — one test run, not a second
+      # one for the number. coverage.runsettings decides which files it may count (the same file
+      # scripts/coverage.sh passes), and the Coverage job below is what judges the report.
+      - name: Test, with the coverage collector attached
+        run: >-
+          dotnet test --no-build --configuration Release --verbosity normal
+          --collect:"XPlat Code Coverage" --settings coverage.runsettings
+          --results-directory artifacts/coverage
+
+      # Handed to the Coverage job below, and kept for the week a reviewer might want to open the
+      # Cobertura report itself rather than the summary that job writes.
+      - uses: actions/upload-artifact@v7
+        with:
+          name: coverage
+          path: artifacts/coverage/
+          retention-days: 7
 
       # The one compile in this pipeline ends here. `--no-build` publishes what the two steps above
       # already produced; without it this would be a second compile of the same commit.
@@ -79,14 +97,54 @@ jobs:
       - name: Drop the native libraries for other architectures
         run: find artifacts/app/runtimes -mindepth 1 -maxdepth 1 ! -name linux-x64 -exec rm -rf {} +
 
-      # A day, because nothing reads this after the run that made it — unlike the reports below,
-      # which are uploaded for a person to open.
+      # A day, because nothing reads this after the run that made it — unlike the reports, which
+      # are uploaded for a person to open.
       - uses: actions/upload-artifact@v7
         with:
           name: app
           path: artifacts/app/
           retention-days: 1
 
+  # Whether the code this branch changed is tested. Not the repository's overall percentage, which
+  # a review cannot act on — Core sits above 95%, so a solution-wide 80% gate would pass with an
+  # entirely untested new service in the diff. scripts/coverage.mjs takes the added and rewritten
+  # lines, keeps the ones the instrumenter counted as coverable, and holds them to an 80% floor;
+  # docs/testing.md has the reasoning. The result is written to this run's summary page, so the
+  # number is read without opening a log or downloading the report.
+  #
+  # Advisory, like the two browser jobs: it goes red under the floor but `main`'s ruleset requires
+  # "Build and test" alone, so it reports rather than blocks. It needs no compiler — the report was
+  # produced by the test run above — so it is node and a git history, and it is over in seconds.
+  coverage:
+    name: Coverage
+    needs: build
+    if: github.event_name != 'workflow_call'
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      # The whole history, unlike every other job here: judging the change means diffing against
+      # the merge base, and the single-commit checkout the others get has nothing to diff against.
+      - uses: actions/checkout@v7
+        with:
+          fetch-depth: 0
+
+      - uses: actions/setup-node@v7
+        with:
+          node-version: 22
+
+      - uses: actions/download-artifact@v7
+        with:
+          name: coverage
+          path: artifacts/coverage
+
+      # The base branch rather than a hardcoded main, so a pull request onto a release branch is
+      # judged on what it actually changed. workflow_dispatch has no pull request and falls back to
+      # the repository's default branch.
+      - name: Judge the lines this change touched
+        run: node scripts/coverage.mjs
+        env:
+          COVERAGE_BASE: origin/${{ github.event.pull_request.base.ref || github.event.repository.default_branch }}
+
   # What the app does: the public/admin split, the dialogs, the match-day journey, both languages,
   # the phone layout. See docs/testing.md.
   #

CLAUDE.md

@@ -186,9 +186,11 @@ admin matrix; it is not part of this repository.)
 - Work on a feature branch. `main` takes pull requests only, and the merge button stays disabled
   until **Build and test** is green and every review thread is resolved
   (`.github/rulesets/main-build-and-test.json`).
-- `ci.yml` runs `dotnet build -c Release` + `dotnet test` on every pull request. `fly-deploy.yml`
-  *calls that same workflow* as the gate its deploy job depends on, then smoke-checks `/health`
-  until it reports the commit that was just built.
+- `ci.yml` runs `dotnet build -c Release` + `dotnet test` on every pull request. That test run
+  carries the coverage collector, and an advisory **Coverage** job judges the lines the pull
+  request changed against the 80% floor and writes the numbers to the run's summary page.
+  `fly-deploy.yml` *calls that same workflow* as the gate its deploy job depends on, then
+  smoke-checks `/health` until it reports the commit that was just built.
 - Merging to `main` *proposes* a deploy; it does not perform one. The deploy job runs in the
   `production` environment, which has a required reviewer, so the run waits at *Waiting* until the
   maintainer approves it. There is no staging environment — that approval is the last look.

coverage.runsettings

@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- What the coverage collector is allowed to count. Used by scripts/coverage.sh and by the
+     `dotnet test` step in .github/workflows/ci.yml, so a local run and a CI run judge the same set
+     of lines. See docs/testing.md. -->
+<RunSettings>
+  <DataCollectionRunSettings>
+    <DataCollectors>
+      <DataCollector friendlyName="XPlat Code Coverage">
+        <Configuration>
+          <Format>cobertura</Format>
+
+          <!-- Modules. The report is Core-only today because the test project references Core
+               alone, and UI and Web are covered by tests/ui and visual-check.sh rather than by
+               unit tests. Naming them anyway keeps that true by construction: adding a reference
+               to UI for one helper would otherwise drop every Razor component into the number at
+               0%, and the first thing anyone would do with that number is stop reading it. -->
+          <Exclude>[FootballFormation.UI]*,[FootballFormation.Web]*</Exclude>
+
+          <!-- Files. A .razor compiles to a generated class whose lines map back to markup, and a
+               .razor.cs is the other half of that same partial class — neither is reachable
+               without rendering a component, which no test here does. Migrations and the model
+               snapshot are scaffolded, and a Down() is never executed by the suite and never will
+               be. DesignTimeDbContextFactory exists for `dotnet ef` and runs in no test. Counting
+               any of them measures how much scaffolding a change happened to touch, not whether
+               the change is tested. -->
+          <ExcludeByFile>**/Migrations/*.cs,**/DesignTimeDbContextFactory.cs,**/*.razor,**/*.razor.cs,**/*.g.cs,**/*.designer.cs</ExcludeByFile>
+
+          <!-- Anything a person deliberately marked. These are coverlet's defaults, restated
+               because setting the element replaces them. CompilerGeneratedAttribute is not one of
+               them and stays out on purpose: it does not just tag lambdas and iterator state
+               machines, it is also how the compiler marks every async method body and every
+               auto-property, so it would take most of ServiceOperation and DatabaseSafety and five
+               model files out of the report along with them. -->
+          <ExcludeByAttribute>Obsolete,GeneratedCodeAttribute,ExcludeFromCodeCoverageAttribute</ExcludeByAttribute>
+        </Configuration>
+      </DataCollector>
+    </DataCollectors>
+  </DataCollectionRunSettings>
+</RunSettings>

docs/testing.md

@@ -91,23 +91,45 @@ COVERAGE_SKIP_TEST=1 scripts/coverage.sh   # re-judge the last run without re-ru
 
 `coverlet.collector` writes a Cobertura report into `artifacts/coverage/`, and `coverage.mjs`
 answers the only question a review can act on: **is the code this branch changed covered?** The
-floor is **80% of the changed lines**, and the script exits non-zero under it, so it works as a
-pipeline step as it stands.
+floor is **80% of the changed lines**, and the script exits non-zero under it, which is exactly how
+CI runs it — see [the Coverage job](#one-pipeline-one-compile).
 
 **The gate is the change, not the repository, and that is the whole design.** Core is above 96%
 line coverage, so a solution-wide 80% gate would pass with an entirely untested new service in the
 diff — the number would move by tenths. The script takes the added and rewritten lines from
 `git diff --unified=0` against the merge base (uncommitted work included), keeps the ones the
 instrumenter counted as coverable, and reports per file with the uncovered line numbers.
 
-Three things are deliberately outside the number:
-
-- **`UI` and `Web` are not measured at all.** The test project references `Core` alone, and the
-  other two have no unit tests by design — `tests/ui` and `visual-check.sh` are what cover them.
-  A change there is reported as unmeasured rather than counted as a miss.
-- **Migrations are excluded.** A `Down()` is never executed by the suite and never will be;
-  counting scaffolded code would make the gate a lottery on how much of it a change touched.
-- **`DesignTimeDbContextFactory`** exists for `dotnet ef` and runs in no test.
+### What the collector may count
+
+`coverage.runsettings` at the repository root is what decides, and both `scripts/coverage.sh` and
+CI's test step pass it, so a local number and a pipeline number mean the same thing. Everything
+below is out of the report entirely — not merely out of the judgement:
+
+- **`UI` and `Web`**, by module (`[FootballFormation.UI]*`, `[FootballFormation.Web]*`) and by
+  file (`**/*.razor`, `**/*.razor.cs`). The test project references `Core` alone, so nothing from
+  either is instrumented today; naming them keeps that true the day somebody adds a reference for
+  one helper. A `.razor` compiles to a generated class whose lines map back to markup and a
+  `.razor.cs` is the other half of that same partial class — neither is reachable without rendering
+  a component, and nothing in `tests/` renders one. Those two are covered by `tests/ui` and
+  `visual-check.sh` instead, and a change there is reported as unmeasured rather than as a miss.
+- **Migrations and the model snapshot.** A `Down()` is never executed by the suite and never will
+  be, and counting scaffolded code makes the gate a lottery on how much of it a change touched.
+  Excluding them took `Core` from a comfortable 96.4% over 9,960 lines to an honest 93.3% over
+  2,509.
+- **`DesignTimeDbContextFactory`**, which exists for `dotnet ef` and runs in no test.
+- **Generated and deliberately-marked code**, by attribute — `GeneratedCode`, `ExcludeFromCodeCoverage`,
+  `Obsolete`. `CompilerGeneratedAttribute` is deliberately *not* one of them: it is not just
+  lambdas and iterator state machines, it is how the compiler marks every `async` method body and
+  every auto-property, and excluding it took `ServiceOperation.RunAdminAsync` — the write guard
+  every service call goes through — and most of `DatabaseSafety` out of the report along with the
+  scaffolding. A change that silently stopped judging the admin check would be worse than the
+  scaffolding problem this file exists to fix.
+
+`coverage.mjs` still recognises migrations and `DesignTimeDbContextFactory` by name, and treats any
+other changed `Core` file the report never mentions the same way: named under the table as excluded
+rather than dropped from the diff silently, because a reviewer has to know the change contains code
+this number says nothing about.
 
 Branch coverage sits near 75% and is reported for information, not gated — the line floor is what
 the `code-reviewer` agent enforces. And a floor is not a target: 100% of a change whose only test
@@ -160,18 +182,34 @@ Adding a spec that leans on a new app class means adding it to `SELECTORS` too.
 
 ### One pipeline, one compile
 
-Everything lives in `.github/workflows/ci.yml`, in three jobs on one chain:
+Everything lives in `.github/workflows/ci.yml`, in four jobs on one chain:
 
 ```
-Build and test ──┬── Playwright
-  (required)     └── Visual check
-                       (both advisory)
+Build and test ──┬── Coverage
+  (required)     ├── Playwright
+                 └── Visual check
+                       (all three advisory)
 ```
 
 **`Build and test`** restores, builds Release, runs `dotnet test`, then publishes — and the publish
 is `--no-build`, so it hands on exactly what the unit tests just ran against rather than compiling
 the commit a second time. It prunes the published `runtimes/` to `linux-x64` and uploads the result
-as the `app` artifact.
+as the `app` artifact. The test step carries `--collect:"XPlat Code Coverage" --settings
+coverage.runsettings`, so the report comes out of the run that is already the gate rather than out
+of a second run of the same tests, and it is uploaded as the `coverage` artifact.
+
+**`Coverage`** downloads that report and runs `scripts/coverage.mjs` over it — the same script and
+the same 80% floor as a local `scripts/coverage.sh`, with `COVERAGE_BASE` pointed at the pull
+request's base branch. It is the one job checked out with `fetch-depth: 0`, because judging a
+change means diffing against its merge base and a single-commit checkout has nothing to diff
+against. It compiles nothing and needs no browser, so it is Node and a git history and is over in
+seconds. The verdict, the whole-project line and branch numbers, and a per-file table with the
+uncovered line numbers are written to `$GITHUB_STEP_SUMMARY` — the run's own front page, so the
+result is read without opening a log or downloading the Cobertura report.
+
+Advisory, like the browser jobs below: it goes red under the floor without blocking a merge, since
+`main`'s ruleset names **Build and test** and nothing else. Promoting it is one line in
+`.github/rulesets/main-build-and-test.json` and nothing in the workflow.
 
 **`Playwright` and `Visual check`** download that artifact and start it. Neither calls a compiler;
 they install the .NET SDK only for the runtime to run `dotnet FootballFormation.Web.dll` with.