Add coverage validation: 90% main-source line coverage gate (py_binary + py_test)

.claude/skills/coverage-status/SKILL.md

@@ -0,0 +1,62 @@
+---
+name: coverage-status
+description: Use to check the current main-source line coverage of the bazel-diff repo, find files below the 90% threshold, see how far the repo is from the gate, or produce an interactive HTML coverage report. Triggers on questions like "what's the test coverage", "is the coverage gate passing", "which files have the worst coverage", "show me a coverage report", or any other inspection of the LCOV output from bazel coverage.
+---
+
+# Checking coverage status
+
+bazel-diff has a 90% main-source line-coverage gate enforced in CI ([.github/workflows/ci.yaml](../../../.github/workflows/ci.yaml)). The enforcement code lives in [tools/coverage_check.py](../../../tools/coverage_check.py); the gate runs on every `test-jre21` matrix entry as `bazelisk run //tools:coverage-check -- bazel-out/_coverage/_coverage_report.dat`.
+
+## Fastest path: just check the current state
+
+```bash
+make coverage
+```
+
+This runs `bazel coverage --combined_report=lcov //cli/... //tools:coverage_check_test`, then `bazel run //tools:coverage-check` against the combined report. It prints a per-file table sorted worst-first, the overall percentage, and PASS/FAIL against the threshold. Exits non-zero when below.
+
+If a coverage report already exists in `bazel-out/_coverage/_coverage_report.dat` (e.g. from a prior `bazel coverage` invocation) and you only want to re-check the threshold without re-running the test suite:
+
+```bash
+make coverage-check
+```
+
+For an annotated HTML report (lines highlighted as covered/uncovered, per-file drilldown), use:
+
+```bash
+make coverage-html
+```
+
+Output lands at `coverage-html/index.html`. Requires `genhtml` from the `lcov` package (`brew install lcov` on macOS, `apt-get install lcov` on Debian/Ubuntu). The threshold gate still runs alongside HTML generation — the HTML is produced even when the gate fails, since the below-threshold case is exactly when the report is most useful.
+
+## Interpreting the output
+
+The per-file table looks like:
+
+```
+    COV%  LINES (hit/total)  FILE
+--------  -----------------  ----
+   0.00%      0 / 4          cli/src/main/kotlin/com/bazel_diff/Main.kt
+  62.67%     47 / 75         cli/src/main/kotlin/com/bazel_diff/bazel/BazelModService.kt
+  ...
+ 100.00%     23 / 23         cli/src/main/kotlin/com/bazel_diff/hash/TargetHash.kt
+
+Overall main-source line coverage: 90.37% (1455 / 1610)
+Threshold:                         90.00%
+PASS
+```
+
+- **Sort order is ascending** — worst-covered files appear first, so the gap is at the top.
+- **Test sources (`/test/`) are excluded** — Bazel's Kotlin instrumentation reports them too, but they don't count toward the threshold (otherwise well-covered tests would hide thin production-code coverage).
+- **Files with `LF:0` are dropped** — that's an "no instrumentation data" signal (e.g. Bazel's Python coverage when no toolchain coverage tool is configured), and treating it as 0% would tank the threshold.
+- **Threshold is inclusive at the boundary** — exactly 90.00% passes; 89.99% fails.
+
+## Looking at CI's number, not local
+
+Local coverage may be lower than CI's because the slow `//cli:E2ETest` is often skipped or fails on dev machines (JDK-env sandbox issues). The number CI sees comes from the `test-jre21` matrix; download the `coverage-report-jre21-*` artifact from a recent run for an exact snapshot, or just read the "Overall main-source line coverage:" line in the workflow log.
+
+## Configuration
+
+- **Threshold**: `COVERAGE_THRESHOLD` env var (default 90). Set in [.github/workflows/ci.yaml](../../../.github/workflows/ci.yaml) for CI, or on the command line to experiment: `COVERAGE_THRESHOLD=85 make coverage`.
+- **Include prefixes**: `--include` flag or `$COVERAGE_INCLUDE` env var (default `cli/src/main/,tools/coverage_check.py`). Only files starting with one of these prefixes count.
+- **Self-coverage of the checker**: `tools/coverage_check.py` is itself in the default include list, but Bazel's Python coverage emits `LF:0` for it today because the Python toolchain in `MODULE.bazel` has no `coverage_tool` configured. The 24-case py_test (`bazel test //tools:coverage_check_test`) is the actual proof the checker is exercised.

.claude/skills/improve-coverage/SKILL.md

@@ -0,0 +1,63 @@
+---
+name: improve-coverage
+description: Use when you need to raise main-source line coverage in the bazel-diff repo, write tests for an under-covered Kotlin file, fix a CI failure on the 90% coverage gate, or pick the highest-leverage files to test next. Triggers on requests like "the coverage gate is failing, fix it", "write tests for X", "we need more coverage", or "what should I test to get to 90%".
+---
+
+# Improving coverage to clear the 90% gate
+
+bazel-diff enforces a 90% main-source line-coverage gate on every PR (see [coverage-status](../coverage-status/SKILL.md) for the inspection side). When the gate fails or you want to raise the bar, the workflow is: pick the worst-covered files, write small focused unit tests, re-run the gate locally before pushing.
+
+## 1. Pick the right files to target
+
+Run `make coverage` (or check the latest CI artifact) and look at the top of the sorted table. Prioritise files by **uncovered-lines-per-test-effort**, not by lowest percentage:
+
+- **Highest-leverage**: small files at 0% (e.g. enum classes, value objects, single-method utilities) — one short unit test usually moves the needle without much code.
+- **Highest absolute gain**: large files with moderate coverage (e.g. `BazelQueryService.kt` at 303 lines / 93%) — closing a small percentage gap covers many lines.
+- **Lowest leverage**: tiny files at 50–80% where the remaining branches are error paths needing fault injection or refactors.
+
+A worked example: the PR that added the gate ([#356](https://github.com/Tinder/bazel-diff/pull/356)) raised coverage from 88.76% → 90.37% by adding five small files of tests — `BazelTargetTypeTest`, `VersionProviderTest`, `BazelDiffTest`, `StderrLoggerTest`, `BazelTargetTest` — for a total of 26 newly-covered lines.
+
+## 2. Write the test
+
+Existing tests follow a consistent shape:
+
+- Live under `cli/src/test/kotlin/...` mirroring the main source path.
+- Use **JUnit 4** (`@Test`, `@Before`, `@After`, `org.junit.Assert.assertThrows`).
+- Use **assertk** for assertions (`assertk.assertThat`, with explicit imports for each assertion like `assertk.assertions.isEqualTo`). Forgetting `import assertk.assertions.contains` on a `String.contains` assertion produces a confusing receiver-mismatch error — import every assertion you use.
+- Use **mockito-kotlin** when mocking is required (existing examples: `cli/src/test/kotlin/com/bazel_diff/bazel/BazelClientTest.kt`).
+- Use **koin** for DI-test setup (existing pattern: `cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorIssue335Test.kt`).
+
+Tiny files (enums, value objects, small command classes) usually only need a few targeted tests. Looking at the bytes via `assertThat(BazelTargetType.entries).hasSize(N).containsExactlyInAnyOrder(...)` is enough to cover an enum's declaration lines.
+
+## 3. Register the test target in cli/BUILD
+
+Every test needs its own `kt_jvm_test` entry:
+
+```python
+kt_jvm_test(
+    name = "BazelTargetTypeTest",
+    test_class = "com.bazel_diff.bazel.BazelTargetTypeTest",
+    runtime_deps = [":cli-test-lib"],
+)
+```
+
+The `:cli-test-lib` glob picks up the new test source automatically; the explicit `kt_jvm_test` rule is what makes it executable via `bazel test //cli:<name>`.
+
+## 4. Verify locally before pushing
+
+```bash
+bazel test //cli:<YourNewTest>        # one-off run of the new test
+make coverage                          # full gate
+```
+
+The local number may be lower than CI's because `//cli:E2ETest` often fails or is excluded on dev machines (JDK-env sandbox issues). If you've added tests for a file that's also exercised by E2E (e.g. `BazelQueryService.kt`), the CI delta will be smaller than the local delta — count only the lines that weren't already covered by E2E.
+
+## 5. Things that don't work / aren't worth attempting
+
+- **`Main.kt`** — calls `exitProcess(...)` which kills the JVM; can't be tested in-process without a SecurityManager hack or refactor. Stays at 0%; the threshold tolerates it.
+- **`throw IllegalArgumentException(...)` branches in resource-loading code** like `VersionProvider.kt` — the production code resolves the classloader from `this::class.java`, with no injection seam to swap it for one missing the resource. Refactor or skip.
+- **`else -> BazelTargetType.UNKNOWN` branches** — only reachable when Bazel's `Build.Target.Discriminator` adds a new enum value the production code doesn't recognise. Triggering today would require a hand-forged proto with a reserved discriminator number, which the protobuf builder rejects.
+
+## When the gate fails on a flake, not on a coverage drop
+
+If the CI threshold step fails with `error: LCOV report not found at 'bazel-out/_coverage/_coverage_report.dat'`, that's an infrastructure issue, not a coverage regression. The fix that landed in PR #356 was to propagate `USE_BAZEL_VERSION` to the threshold step so `bazelisk run` doesn't start a different bazel server. If you see a similar mismatch resurface, check that env propagation first.

.github/workflows/ci.yaml

@@ -38,14 +38,23 @@ jobs:
       - name: Run bazel-diff tests
         env:
           USE_BAZEL_VERSION: ${{ matrix.bazel }}
-        run: ~/go/bin/bazelisk coverage --combined_report=lcov //cli/... --enable_bzlmod=true --enable_workspace=false
+        run: ~/go/bin/bazelisk coverage --combined_report=lcov //cli/... //tools:coverage_check_test --enable_bzlmod=true --enable_workspace=false
       - name: Upload coverage report
         uses: actions/upload-artifact@v4
         if: always()
         with:
           name: coverage-report-jre21-${{ matrix.os }}-bazel-${{ matrix.bazel }}
           path: bazel-out/_coverage/_coverage_report.dat
           if-no-files-found: warn
+      - name: Enforce coverage threshold (>= 90% main-source line coverage)
+        env:
+          # Must match the `Run bazel-diff tests` step above, otherwise bazelisk
+          # downloads the default bazel from .bazelversion and starts a fresh
+          # server whose output base loses track of the coverage report symlink
+          # (seen flaking on macos-latest x bazel 9.x).
+          USE_BAZEL_VERSION: ${{ matrix.bazel }}
+          COVERAGE_THRESHOLD: '90'
+        run: ~/go/bin/bazelisk run //tools:coverage-check -- bazel-out/_coverage/_coverage_report.dat
       - name: Upload test logs
         uses: actions/upload-artifact@v4
         if: always()

.gitignore

@@ -28,3 +28,10 @@ user.bazelrc
 .DS_Store
 !.bazelversion
 archives/
+# Coverage HTML report (genhtml output from `make coverage-html`).
+coverage-html/
+# Claude Code local state (per-user settings + scratch worktrees).
+# Skills under .claude/skills/ ARE checked in -- they're shared team docs --
+# so this is a precise ignore list, not a blanket .claude/ ignore.
+.claude/settings.local.json
+.claude/worktrees/

Makefile

@@ -21,3 +21,22 @@ format:
 .PHONY: generate-readme
 generate-readme:
 	bazel run //tools:generate-readme
+
+.PHONY: coverage
+coverage:
+	bazel coverage --combined_report=lcov //cli/... //tools:coverage_check_test
+	bazel run //tools:coverage-check -- bazel-out/_coverage/_coverage_report.dat
+
+.PHONY: coverage-check
+coverage-check:
+	bazel run //tools:coverage-check -- bazel-out/_coverage/_coverage_report.dat
+
+.PHONY: coverage-test
+coverage-test:
+	bazel test //tools:coverage_check_test
+
+.PHONY: coverage-html
+coverage-html:
+	bazel coverage --combined_report=lcov //cli/... //tools:coverage_check_test
+	bazel run //tools:coverage-check -- bazel-out/_coverage/_coverage_report.dat --html coverage-html
+	@echo "Open coverage-html/index.html in a browser to inspect."

README.md

@@ -532,6 +532,43 @@ To run the tests simply run
 bazel test //...
 ```
 
+## Code coverage
+
+CI enforces a minimum **90% line coverage** on production sources (`cli/src/main/...`). To
+run the same check locally:
+
+```terminal
+make coverage
+```
+
+This invokes `bazel coverage --combined_report=lcov //cli/... //tools:coverage_check_test`
+and then runs `//tools:coverage-check` against the resulting LCOV report. The check is a
+Python `py_binary` ([`tools/coverage_check.py`](tools/coverage_check.py)) that prints a
+per-file table sorted by coverage (worst first), the overall percentage, and exits
+non-zero if main-source coverage is below the threshold.
+
+If you've already produced a coverage report and just want to re-check the threshold,
+`make coverage-check` runs only the binary against `bazel-out/_coverage/_coverage_report.dat`.
+
+The enforcement logic itself is tested under `//tools:coverage_check_test` — run it
+directly with `make coverage-test` (or `bazel test //tools:coverage_check_test`).
+
+For an interactive HTML report (annotated source with covered/uncovered lines
+highlighted), use `make coverage-html`. This requires the `lcov` package
+(`brew install lcov` on macOS, `apt-get install lcov` on Debian/Ubuntu) and writes
+the report to `coverage-html/index.html`. The threshold gate still runs and still
+sets the exit code — HTML is an additional artifact, not a replacement.
+
+To experiment with a different threshold (e.g. while ratcheting up), set
+`COVERAGE_THRESHOLD`:
+
+```terminal
+COVERAGE_THRESHOLD=80 make coverage
+```
+
+The CI matrix runs the same check on every Linux/macOS test job, so a PR cannot
+land if it drops main-source line coverage below the threshold.
+
 ## Versioning
 
 We use [SemVer](http://semver.org/) for versioning. For the versions available,

cli/BUILD

@@ -108,6 +108,36 @@ kt_jvm_test(
     runtime_deps = [":cli-test-lib"],
 )
 
+kt_jvm_test(
+    name = "BazelTargetTest",
+    test_class = "com.bazel_diff.bazel.BazelTargetTest",
+    runtime_deps = [":cli-test-lib"],
+)
+
+kt_jvm_test(
+    name = "BazelTargetTypeTest",
+    test_class = "com.bazel_diff.bazel.BazelTargetTypeTest",
+    runtime_deps = [":cli-test-lib"],
+)
+
+kt_jvm_test(
+    name = "BazelDiffTest",
+    test_class = "com.bazel_diff.cli.BazelDiffTest",
+    runtime_deps = [":cli-test-lib"],
+)
+
+kt_jvm_test(
+    name = "VersionProviderTest",
+    test_class = "com.bazel_diff.cli.VersionProviderTest",
+    runtime_deps = [":cli-test-lib"],
+)
+
+kt_jvm_test(
+    name = "StderrLoggerTest",
+    test_class = "com.bazel_diff.log.StderrLoggerTest",
+    runtime_deps = [":cli-test-lib"],
+)
+
 kt_jvm_test(
     name = "BazelModServiceTest",
     test_class = "com.bazel_diff.bazel.BazelModServiceTest",

cli/src/test/kotlin/com/bazel_diff/bazel/BazelTargetTest.kt

@@ -0,0 +1,75 @@
+package com.bazel_diff.bazel
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import com.google.devtools.build.lib.query2.proto.proto2api.Build
+import org.junit.Test
+
+class BazelTargetTest {
+  // Helper that builds a Rule-shaped target with a chosen discriminator so we can
+  // exercise the `type` getter's branches without needing six separate hand-built
+  // protos. `BazelTarget.Rule` only asserts `target.hasRule()` in its init, which
+  // is satisfied by setting any Rule on the builder, so we can drive any
+  // discriminator through it.
+  private fun ruleTargetWith(d: Build.Target.Discriminator): Build.Target =
+      Build.Target.newBuilder()
+          .setType(d)
+          .setRule(Build.Rule.newBuilder().setRuleClass("dummy").setName("//:dummy"))
+          .build()
+
+  @Test
+  fun sourceFileTarget_exposesSourceFileName_andSourceFileType() {
+    val target = Build.Target.newBuilder()
+        .setType(Build.Target.Discriminator.SOURCE_FILE)
+        .setSourceFile(
+            Build.SourceFile.newBuilder()
+                .setName("//pkg:foo.txt")
+                .addSubinclude("//other:bzl.bzl"))
+        .build()
+    val bt = BazelTarget.SourceFile(target)
+    assertThat(bt.name).isEqualTo("//pkg:foo.txt")
+    assertThat(bt.sourceFileName).isEqualTo("//pkg:foo.txt")
+    assertThat(bt.subincludeList).isEqualTo(listOf("//other:bzl.bzl"))
+    assertThat(bt.type).isEqualTo(BazelTargetType.SOURCE_FILE)
+  }
+
+  @Test
+  fun ruleTarget_exposesRuleName_andRuleType() {
+    val target = Build.Target.newBuilder()
+        .setType(Build.Target.Discriminator.RULE)
+        .setRule(Build.Rule.newBuilder().setRuleClass("java_library").setName("//:foo"))
+        .build()
+    val bt = BazelTarget.Rule(target)
+    assertThat(bt.name).isEqualTo("//:foo")
+    assertThat(bt.rule.name).isEqualTo("//:foo")
+    assertThat(bt.type).isEqualTo(BazelTargetType.RULE)
+  }
+
+  @Test
+  fun generatedFileTarget_exposesNamesAndGeneratedFileType() {
+    val target = Build.Target.newBuilder()
+        .setType(Build.Target.Discriminator.GENERATED_FILE)
+        .setGeneratedFile(
+            Build.GeneratedFile.newBuilder()
+                .setName("//:gen.out")
+                .setGeneratingRule("//:gen"))
+        .build()
+    val bt = BazelTarget.GeneratedFile(target)
+    assertThat(bt.name).isEqualTo("//:gen.out")
+    assertThat(bt.generatedFileName).isEqualTo("//:gen.out")
+    assertThat(bt.generatingRuleName).isEqualTo("//:gen")
+    assertThat(bt.type).isEqualTo(BazelTargetType.GENERATED_FILE)
+  }
+
+  @Test
+  fun packageGroupDiscriminator_mapsToPackageGroupType() {
+    val bt = BazelTarget.Rule(ruleTargetWith(Build.Target.Discriminator.PACKAGE_GROUP))
+    assertThat(bt.type).isEqualTo(BazelTargetType.PACKAGE_GROUP)
+  }
+
+  @Test
+  fun environmentGroupDiscriminator_mapsToEnvironmentGroupType() {
+    val bt = BazelTarget.Rule(ruleTargetWith(Build.Target.Discriminator.ENVIRONMENT_GROUP))
+    assertThat(bt.type).isEqualTo(BazelTargetType.ENVIRONMENT_GROUP)
+  }
+}

cli/src/test/kotlin/com/bazel_diff/bazel/BazelTargetTypeTest.kt

@@ -0,0 +1,32 @@
+package com.bazel_diff.bazel
+
+import assertk.assertThat
+import assertk.assertions.containsExactlyInAnyOrder
+import assertk.assertions.hasSize
+import assertk.assertions.isEqualTo
+import org.junit.Test
+
+class BazelTargetTypeTest {
+  @Test
+  fun enumDeclaresExpectedValues() {
+    // Reading every constant forces the JVM to initialise the whole enum, which
+    // is what gives this otherwise-trivial declaration any line coverage.
+    val all = BazelTargetType.entries
+    assertThat(all).hasSize(6)
+    assertThat(all).containsExactlyInAnyOrder(
+        BazelTargetType.RULE,
+        BazelTargetType.SOURCE_FILE,
+        BazelTargetType.GENERATED_FILE,
+        BazelTargetType.PACKAGE_GROUP,
+        BazelTargetType.ENVIRONMENT_GROUP,
+        BazelTargetType.UNKNOWN,
+    )
+  }
+
+  @Test
+  fun valueOfRoundTrip() {
+    for (t in BazelTargetType.entries) {
+      assertThat(BazelTargetType.valueOf(t.name)).isEqualTo(t)
+    }
+  }
+}