From 2736dded44e237ad80c0aa43defcdd7318869ced Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Wed, 20 May 2026 12:45:18 -0400 Subject: [PATCH 1/4] Add coverage percent badge to README, auto-updated by CI Wires up a shields.io endpoint badge sourced from coverage.json in the repo so the README always shows the latest tested main-source coverage. - tools/coverage_check.py gains a --badge-json PATH flag that writes the shields.io endpoint schema (color-banded against the 90% gate). Writing is decoupled from the threshold gate so a regression still surfaces in the badge instead of silently freezing on the last-good value. - CI's existing coverage step is split into a Generate step (--threshold 0, never gates) and the original strict Enforce step. A new Publish step commits coverage.json back to master on pushes and to the PR head branch on internal pull requests, so reviewers see the coverage delta directly in the PR diff. Skipped on fork PRs (GITHUB_TOKEN is read-only against forks) and on non-canonical matrix entries to avoid races on the same file. - coverage.json is seeded with a "pending" placeholder so the badge renders correctly before the first master-merge CI run replaces it with the real value. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yaml | 54 ++++++++++++++++- README.md | 1 + coverage.json | 1 + tools/coverage_check.py | 53 ++++++++++++++++ tools/coverage_check_test.py | 114 +++++++++++++++++++++++++++++++++++ tools/readme_template.md | 1 + 6 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 coverage.json diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2bdc730..9d7995c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,6 +9,13 @@ on: jobs: test-jre21: runs-on: ${{ matrix.os }} + # contents:write is needed by the "Publish coverage badge" step below, which + # commits the regenerated coverage.json back to master on push events and + # to the PR head branch on internal pull requests. The step itself is gated + # on the canonical matrix entry and a non-fork PR, but the permission has + # to be declared at the job level. + permissions: + contents: write strategy: fail-fast: false matrix: @@ -46,13 +53,58 @@ jobs: 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) + - name: Generate coverage badge JSON + # Decoupled from the threshold gate below so the badge still updates on + # PRs that drop coverage. Passing --threshold 0 means this step never + # fails on coverage value; the strict gate runs as its own step after + # the badge has been published. 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 }} + run: ~/go/bin/bazelisk run //tools:coverage-check -- --badge-json coverage.json --threshold 0 bazel-out/_coverage/_coverage_report.dat + - name: Publish coverage badge + # Auto-commits coverage.json back to master on pushes and back to the + # PR's head branch on internal pull requests so the README badge always + # tracks the latest tested state. Skipped on: + # - any matrix entry other than the canonical Linux + Bazel 9.x + # runner (so the entries don't race to commit the same file), and + # - fork PRs (the default GITHUB_TOKEN is read-only against forks). + if: | + matrix.os == 'ubuntu-latest' && matrix.bazel == '9.x' && ( + (github.event_name == 'push' && github.ref == 'refs/heads/master') || + (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) + ) + env: + HEAD_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || 'master' }} + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + if [ "${{ github.event_name }}" = "pull_request" ]; then + # actions/checkout fetches the merge commit for pull_request events; + # the PR's head branch isn't in the local repo yet. Stash the + # freshly-generated coverage.json, then switch to the PR head so the + # commit lands where the PR author can see it. + cp coverage.json /tmp/coverage.json + git fetch origin "$HEAD_REF" + git checkout -B "$HEAD_REF" "origin/$HEAD_REF" + cp /tmp/coverage.json coverage.json + fi + if [ -z "$(git status --porcelain coverage.json)" ]; then + echo "coverage.json unchanged; nothing to publish." + exit 0 + fi + git add coverage.json + # [skip ci] is belt-and-suspenders — GITHUB_TOKEN pushes already do + # not retrigger workflows, but the marker makes the intent explicit + # in the log. + git commit -m "ci: update coverage badge [skip ci]" + git push origin "HEAD:$HEAD_REF" + - name: Enforce coverage threshold (>= 90% main-source line coverage) + env: + 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 diff --git a/README.md b/README.md index 9c9a053..742ef08 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # bazel-diff [![Build status](https://github.com/Tinder/bazel-diff/actions/workflows/ci.yaml/badge.svg?branch=master)](https://github.com/Tinder/bazel-diff/actions/workflows/ci.yaml) +[![Coverage](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/Tinder/bazel-diff/master/coverage.json)](https://github.com/Tinder/bazel-diff/actions/workflows/ci.yaml) `bazel-diff` is a command line tool for Bazel projects that allows users to determine the exact affected set of impacted targets between two Git revisions. Using this set, users can test or build the exact modified set of targets. diff --git a/coverage.json b/coverage.json new file mode 100644 index 0000000..415cfa5 --- /dev/null +++ b/coverage.json @@ -0,0 +1 @@ +{"schemaVersion": 1, "label": "coverage", "message": "pending", "color": "lightgrey"} diff --git a/tools/coverage_check.py b/tools/coverage_check.py index 0ec5df2..ce2989a 100644 --- a/tools/coverage_check.py +++ b/tools/coverage_check.py @@ -20,6 +20,7 @@ """ import argparse +import json import os import shutil import subprocess @@ -121,6 +122,43 @@ def format_report( return "\n".join(lines) +def badge_color(pct: float) -> str: + """Pick a shields.io color band that visually tracks the project's 90% gate. + + Anything at-or-above the gate is brightgreen; below the gate degrades through + yellow/orange/red so the badge becomes a quick visual signal of how far the + main-source coverage has drifted. + """ + if pct >= 90.0: + return "brightgreen" + if pct >= 75.0: + return "yellow" + if pct >= 60.0: + return "orange" + return "red" + + +def write_badge_json(path: str, overall_pct: float) -> None: + """Write a shields.io endpoint badge JSON describing the overall coverage. + + See https://shields.io/endpoint — the schema is `{schemaVersion, label, + message, color}`. Consumed by a README badge URL of the form + `https://img.shields.io/endpoint?url=…/coverage.json`. + """ + payload = { + "schemaVersion": 1, + "label": "coverage", + "message": f"{overall_pct:.1f}%", + "color": badge_color(overall_pct), + } + parent = os.path.dirname(path) + if parent: + os.makedirs(parent, exist_ok=True) + with open(path, "w", encoding="utf-8") as f: + json.dump(payload, f) + f.write("\n") + + def generate_html_report(lcov_path: str, output_dir: str) -> str: """Render an annotated HTML coverage report from `lcov_path` into `output_dir`. @@ -198,6 +236,17 @@ def main(argv: List[str] | None = None) -> int: "only adds an additional artifact for interactive inspection." ), ) + parser.add_argument( + "--badge-json", + metavar="PATH", + help=( + "Also write a shields.io endpoint JSON describing the overall " + "main-source coverage to PATH. Consumed by the README badge " + "(https://img.shields.io/endpoint?url=…). Written regardless of " + "whether the threshold passes so a regression is visible on the " + "badge instead of being silently skipped." + ), + ) args = parser.parse_args(argv) if not os.path.isfile(args.lcov): @@ -244,6 +293,10 @@ def main(argv: List[str] | None = None) -> int: return 2 overall = total_lh / total_lf * 100.0 + + if args.badge_json: + write_badge_json(args.badge_json, overall) + # Allow a tiny epsilon so 89.99999... that displays as 90.00 still passes a 90 # threshold. Use the raw value, not the rounded one, otherwise 89.99 would slip # through. diff --git a/tools/coverage_check_test.py b/tools/coverage_check_test.py index fdfdb73..05480a6 100644 --- a/tools/coverage_check_test.py +++ b/tools/coverage_check_test.py @@ -5,6 +5,7 @@ """ import io +import json import os import subprocess import sys @@ -16,10 +17,12 @@ import coverage_check from coverage_check import ( FileCoverage, + badge_color, filter_main_source, format_report, main, parse_lcov, + write_badge_json, ) @@ -328,5 +331,116 @@ def test_html_generated_even_when_threshold_fails(self): self.assertIn("FAIL", stderr) +class BadgeColorTest(unittest.TestCase): + def test_color_bands_track_the_gate(self): + # Boundaries: <60 red, <75 orange, <90 yellow, >=90 brightgreen. + # The 90% boundary is the project's enforced gate; coverage at-or-above + # it must read brightgreen so the badge confirms the gate is met. + self.assertEqual(badge_color(0.0), "red") + self.assertEqual(badge_color(59.99), "red") + self.assertEqual(badge_color(60.0), "orange") + self.assertEqual(badge_color(74.99), "orange") + self.assertEqual(badge_color(75.0), "yellow") + self.assertEqual(badge_color(89.99), "yellow") + self.assertEqual(badge_color(90.0), "brightgreen") + self.assertEqual(badge_color(100.0), "brightgreen") + + +class WriteBadgeJsonTest(unittest.TestCase): + def _read_json(self, path: str) -> dict: + with open(path, "r", encoding="utf-8") as f: + return json.load(f) + + def test_writes_shields_endpoint_schema(self): + fd, path = tempfile.mkstemp(suffix=".json") + os.close(fd) + self.addCleanup(os.remove, path) + + write_badge_json(path, 92.345) + payload = self._read_json(path) + + # https://shields.io/endpoint requires schemaVersion=1 plus label/message/color. + self.assertEqual(payload["schemaVersion"], 1) + self.assertEqual(payload["label"], "coverage") + self.assertEqual(payload["message"], "92.3%") + self.assertEqual(payload["color"], "brightgreen") + + def test_creates_parent_directory(self): + # The CI step writes into the repo root; we shouldn't require the caller + # to pre-create intermediate directories when they pass a nested path. + tmp = tempfile.mkdtemp() + self.addCleanup(lambda: __import__("shutil").rmtree(tmp)) + path = os.path.join(tmp, "nested", "deeper", "badge.json") + + write_badge_json(path, 50.0) + + self.assertTrue(os.path.isfile(path)) + self.assertEqual(self._read_json(path)["color"], "red") + + def test_below_threshold_still_writes(self): + # write_badge_json itself doesn't gate on the threshold — the caller does. + # We want the badge to surface a regression rather than freeze on the + # last-good value, so a sub-90% number renders honestly (yellow here). + fd, path = tempfile.mkstemp(suffix=".json") + os.close(fd) + self.addCleanup(os.remove, path) + + write_badge_json(path, 80.0) + payload = self._read_json(path) + + self.assertEqual(payload["message"], "80.0%") + self.assertEqual(payload["color"], "yellow") + + +class MainBadgeJsonTest(unittest.TestCase): + def _write_lcov(self, content: str) -> str: + fd, path = tempfile.mkstemp(suffix=".dat") + with os.fdopen(fd, "w") as f: + f.write(content) + self.addCleanup(os.remove, path) + return path + + def _run_main(self, argv): + out, err = io.StringIO(), io.StringIO() + with redirect_stdout(out), redirect_stderr(err): + rc = main(argv) + return rc, out.getvalue(), err.getvalue() + + def test_badge_json_emitted_when_threshold_passes(self): + path = self._write_lcov(SIMPLE_LCOV) + fd, badge_path = tempfile.mkstemp(suffix=".json") + os.close(fd) + self.addCleanup(os.remove, badge_path) + + rc, _, _ = self._run_main( + [path, "--threshold", "50", "--badge-json", badge_path] + ) + + self.assertEqual(rc, 0) + with open(badge_path) as f: + payload = json.load(f) + # SIMPLE_LCOV main-source = 9 hit / 15 total = 60.0% + self.assertEqual(payload["message"], "60.0%") + + def test_badge_json_emitted_when_threshold_fails(self): + # A sub-threshold run must still update the badge — otherwise a regression + # silently keeps the old badge value and the README lies. The exit code + # still fails the gate; only the badge artifact is decoupled. + path = self._write_lcov(SIMPLE_LCOV) + fd, badge_path = tempfile.mkstemp(suffix=".json") + os.close(fd) + self.addCleanup(os.remove, badge_path) + + rc, _, _ = self._run_main( + [path, "--threshold", "90", "--badge-json", badge_path] + ) + + self.assertEqual(rc, 1) + with open(badge_path) as f: + payload = json.load(f) + self.assertEqual(payload["message"], "60.0%") + self.assertEqual(payload["color"], "orange") + + if __name__ == "__main__": unittest.main() diff --git a/tools/readme_template.md b/tools/readme_template.md index 52d9414..23e906c 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -1,6 +1,7 @@ # bazel-diff [![Build status](https://github.com/Tinder/bazel-diff/actions/workflows/ci.yaml/badge.svg?branch=master)](https://github.com/Tinder/bazel-diff/actions/workflows/ci.yaml) +[![Coverage](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/Tinder/bazel-diff/master/coverage.json)](https://github.com/Tinder/bazel-diff/actions/workflows/ci.yaml) `bazel-diff` is a command line tool for Bazel projects that allows users to determine the exact affected set of impacted targets between two Git revisions. Using this set, users can test or build the exact modified set of targets. From 599395a055c0d83d272684b385187e809a226b1c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 20 May 2026 17:00:20 +0000 Subject: [PATCH 2/4] ci: update coverage badge [skip ci] --- coverage.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coverage.json b/coverage.json index 415cfa5..d5f083d 100644 --- a/coverage.json +++ b/coverage.json @@ -1 +1 @@ -{"schemaVersion": 1, "label": "coverage", "message": "pending", "color": "lightgrey"} +{"schemaVersion": 1, "label": "coverage", "message": "91.0%", "color": "brightgreen"} From 72328f497aba7f0524614bc22204c4b1c43ddbe2 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Wed, 20 May 2026 13:04:44 -0400 Subject: [PATCH 3/4] Restrict coverage badge publish to master pushes only Reverts the PR auto-update path: PRs no longer get a github-actions[bot] commit updating coverage.json on each push. The badge now reflects master's tested state, full stop. The two-step Generate/Enforce split is collapsed back into the single Enforce step (which still writes the badge as a side effect via --badge-json, since the script writes before gating). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yaml | 71 ++++++++++++--------------------------- 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9d7995c..16f9ee9 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,10 +10,9 @@ jobs: test-jre21: runs-on: ${{ matrix.os }} # contents:write is needed by the "Publish coverage badge" step below, which - # commits the regenerated coverage.json back to master on push events and - # to the PR head branch on internal pull requests. The step itself is gated - # on the canonical matrix entry and a non-fork PR, but the permission has - # to be declared at the job level. + # commits the regenerated coverage.json back to master on push events. The + # step itself is gated on push + canonical matrix entry, but the permission + # has to be declared at the job level. permissions: contents: write strategy: @@ -53,60 +52,34 @@ jobs: name: coverage-report-jre21-${{ matrix.os }}-bazel-${{ matrix.bazel }} path: bazel-out/_coverage/_coverage_report.dat if-no-files-found: warn - - name: Generate coverage badge JSON - # Decoupled from the threshold gate below so the badge still updates on - # PRs that drop coverage. Passing --threshold 0 means this step never - # fails on coverage value; the strict gate runs as its own step after - # the badge has been published. + - 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 }} - run: ~/go/bin/bazelisk run //tools:coverage-check -- --badge-json coverage.json --threshold 0 bazel-out/_coverage/_coverage_report.dat - - name: Publish coverage badge - # Auto-commits coverage.json back to master on pushes and back to the - # PR's head branch on internal pull requests so the README badge always - # tracks the latest tested state. Skipped on: - # - any matrix entry other than the canonical Linux + Bazel 9.x - # runner (so the entries don't race to commit the same file), and - # - fork PRs (the default GITHUB_TOKEN is read-only against forks). - if: | - matrix.os == 'ubuntu-latest' && matrix.bazel == '9.x' && ( - (github.event_name == 'push' && github.ref == 'refs/heads/master') || - (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) - ) - env: - HEAD_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || 'master' }} + COVERAGE_THRESHOLD: '90' + run: ~/go/bin/bazelisk run //tools:coverage-check -- --badge-json coverage.json bazel-out/_coverage/_coverage_report.dat + - name: Publish coverage badge to master + # Only the canonical Linux + Bazel 9.x runner pushes the badge so the + # other matrix entries don't race to commit the same file. Skipped on + # pull_request because forks lack write access and the badge should + # only ever reflect master. + if: github.event_name == 'push' && github.ref == 'refs/heads/master' && matrix.os == 'ubuntu-latest' && matrix.bazel == '9.x' run: | - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - if [ "${{ github.event_name }}" = "pull_request" ]; then - # actions/checkout fetches the merge commit for pull_request events; - # the PR's head branch isn't in the local repo yet. Stash the - # freshly-generated coverage.json, then switch to the PR head so the - # commit lands where the PR author can see it. - cp coverage.json /tmp/coverage.json - git fetch origin "$HEAD_REF" - git checkout -B "$HEAD_REF" "origin/$HEAD_REF" - cp /tmp/coverage.json coverage.json - fi - if [ -z "$(git status --porcelain coverage.json)" ]; then + if [ -n "$(git status --porcelain coverage.json)" ]; then + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add coverage.json + # [skip ci] is belt-and-suspenders — GITHUB_TOKEN pushes already do + # not retrigger workflows, but the marker makes the intent explicit + # in the log. + git commit -m "ci: update coverage badge [skip ci]" + git push origin HEAD:master + else echo "coverage.json unchanged; nothing to publish." - exit 0 fi - git add coverage.json - # [skip ci] is belt-and-suspenders — GITHUB_TOKEN pushes already do - # not retrigger workflows, but the marker makes the intent explicit - # in the log. - git commit -m "ci: update coverage badge [skip ci]" - git push origin "HEAD:$HEAD_REF" - - name: Enforce coverage threshold (>= 90% main-source line coverage) - env: - 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() From 3c83460ab3b3863aab457584d34c474ddb416e81 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Thu, 21 May 2026 09:35:12 -0400 Subject: [PATCH 4/4] Add --writeEmptyOutput flag on get-impacted-targets Inspired by ewhauser/bazel-differ's --output-on-empty: let CI branch on file existence instead of file contents (`[ -f impacted.txt ] && bazel test --target_pattern_file=...`) without having to also probe whether the file is empty. Defaults to true so existing pipelines are unaffected. Split the interactor's execute/executeWithDistances into pure compute + write halves so the CLI can short-circuit the file open when the impacted set is empty and the user opted out of empty output. --- README.md | 13 ++- .../cli/GetImpactedTargetsCommand.kt | 56 ++++++++--- .../CalculateImpactedTargetsInteractor.kt | 96 +++++++++++++++---- .../test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 67 +++++++++++++ .../CalculateImpactedTargetsInteractorTest.kt | 19 ++++ 5 files changed, 219 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 742ef08..4695222 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,8 @@ workspace. ```terminal Missing required options: '--startingHashes=', '--finalHashes=', '--workspacePath=' Usage: bazel-diff get-impacted-targets [-v] [--[no-]excludeExternalTargets] [-- - [no-]noBazelrc] [-b=] + [no-]noBazelrc] [--[no-] + writeEmptyOutput] [-b=] [-d=] -fh= [-o=] @@ -313,6 +314,16 @@ Command-line utility to analyze the state of the bazel build graph -w, --workspacePath= Path to Bazel workspace directory. Required for module change detection. + --[no-]writeEmptyOutput + If true (default), always write the output file (or + stdout) even when no targets are impacted. Pass + --no-writeEmptyOutput to suppress the write entirely + on an empty impacted set, so CI can branch on file + existence instead of file contents (`if [ -f + impacted.txt ]; then bazel test + --target_pattern_file=...`). Only meaningful when + -o/--output is set; with stdout, nothing is written + either way. ``` diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt index 92e21d8..e226daf 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt @@ -105,6 +105,21 @@ class GetImpactedTargetsCommand : Callable { scope = CommandLine.ScopeType.LOCAL) var noBazelrc = false + @CommandLine.Option( + names = ["--writeEmptyOutput"], + negatable = true, + defaultValue = "true", + fallbackValue = "true", + description = + [ + "If true (default), always write the output file (or stdout) even when no targets " + + "are impacted. Pass --no-writeEmptyOutput to suppress the write entirely on " + + "an empty impacted set, so CI can branch on file existence instead of file " + + "contents (`if [ -f impacted.txt ]; then bazel test --target_pattern_file=...`). " + + "Only meaningful when -o/--output is set; with stdout, nothing is written either way."], + scope = CommandLine.ScopeType.LOCAL) + var writeEmptyOutput: Boolean = true + @CommandLine.Option( names = ["--excludeExternalTargets"], negatable = true, @@ -166,36 +181,42 @@ class GetImpactedTargetsCommand : Callable { com.bazel_diff.bazel.BazelModService::class.java) .isBzlmodEnabled - val outputWriter = - BufferedWriter( - when (val path = outputPath) { - null -> FileWriter(FileDescriptor.out) - else -> FileWriter(path) - }) + val interactor = CalculateImpactedTargetsInteractor() + // Compute first so we can decide whether to write at all. The interactor's + // compute step is pure (no I/O); we only open the output file when we + // actually have something to write or when --writeEmptyOutput is set (the + // default). Deferring the open avoids leaving an empty file behind when + // the user opted out of empty output -- the intended CI ergonomics. try { - if (depsMappingJSONPath != null) { - val depsMapping = deserialiser.deserializeDeps(depsMappingJSONPath!!) - CalculateImpactedTargetsInteractor() - .executeWithDistances( + val depsMapping = depsMappingJSONPath?.let { deserialiser.deserializeDeps(it) } + if (depsMapping != null) { + val computed = + interactor.computeImpactedTargetsWithDistances( fromData.hashes, toData.hashes, depsMapping, - outputWriter, targetType, fromData.moduleGraphJson, toData.moduleGraphJson, resolvedExcludeExternalTargets) + if (computed.isEmpty() && !writeEmptyOutput && outputPath != null) { + return CommandLine.ExitCode.OK + } + interactor.writeImpactedTargetsWithDistances(openOutputWriter(), computed) } else { - CalculateImpactedTargetsInteractor() - .execute( + val computed = + interactor.computeImpactedTargets( fromData.hashes, toData.hashes, - outputWriter, targetType, fromData.moduleGraphJson, toData.moduleGraphJson, resolvedExcludeExternalTargets) + if (computed.isEmpty() && !writeEmptyOutput && outputPath != null) { + return CommandLine.ExitCode.OK + } + interactor.writeImpactedTargets(openOutputWriter(), computed) } CommandLine.ExitCode.OK } catch (e: IOException) { @@ -206,6 +227,13 @@ class GetImpactedTargetsCommand : Callable { } } + private fun openOutputWriter(): BufferedWriter = + BufferedWriter( + when (val path = outputPath) { + null -> FileWriter(FileDescriptor.out) + else -> FileWriter(path) + }) + private fun validate() { if (!startingHashesJSONPath.canRead()) { throw CommandLine.ParameterException( diff --git a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt index 967f223..3490ac9 100644 --- a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt +++ b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt @@ -36,6 +36,33 @@ class CalculateImpactedTargetsInteractor : KoinComponent { toModuleGraphJson: String? = null, excludeExternalTargets: Boolean = false, ) { + val filtered = + computeImpactedTargets( + from, + to, + targetTypes, + fromModuleGraphJson, + toModuleGraphJson, + excludeExternalTargets) + writeImpactedTargets(outputWriter, filtered) + } + + /** + * Computes the sorted, filtered list of impacted target labels. + * + * Pure data step: detects module changes, computes the impacted set, applies + * target-type and external-target filters, and sorts. No I/O. Splitting compute + * from write lets callers short-circuit before opening an output file + * (see [GetImpactedTargetsCommand]'s --writeEmptyOutput handling). + */ + fun computeImpactedTargets( + from: Map, + to: Map, + targetTypes: Set?, + fromModuleGraphJson: String? = null, + toModuleGraphJson: String? = null, + excludeExternalTargets: Boolean = false, + ): List { /** This call might be faster if end hashes is a sorted map */ val typeFilter = TargetTypeFilter(targetTypes, to) @@ -56,13 +83,15 @@ class CalculateImpactedTargetsInteractor : KoinComponent { computeSimpleImpactedTargets(from, to) } - impactedTargets + return impactedTargets .filter { typeFilter.accepts(it) } .filter { !excludeExternalTargets || !it.startsWith("//external:") } .sortedWith(impactedTargetOrdering(to, from)) - .let { filtered -> - outputWriter.use { writer -> filtered.forEach { writer.write("$it\n") } } - } + } + + /** Writes [filtered] as one label per line to [outputWriter] and closes it. */ + fun writeImpactedTargets(outputWriter: Writer, filtered: List) { + outputWriter.use { writer -> filtered.forEach { writer.write("$it\n") } } } fun computeSimpleImpactedTargets( @@ -90,6 +119,34 @@ class CalculateImpactedTargetsInteractor : KoinComponent { toModuleGraphJson: String? = null, excludeExternalTargets: Boolean = false, ) { + val filtered = + computeImpactedTargetsWithDistances( + from, + to, + depEdges, + targetTypes, + fromModuleGraphJson, + toModuleGraphJson, + excludeExternalTargets) + writeImpactedTargetsWithDistances(outputWriter, filtered) + } + + /** + * Computes the sorted, filtered map of impacted target labels to distance metrics. + * + * Pure data step paralleling [computeImpactedTargets] but for the --depEdgesFile mode. + * Splitting compute from write lets callers short-circuit before opening an output + * file (see [GetImpactedTargetsCommand]'s --writeEmptyOutput handling). + */ + fun computeImpactedTargetsWithDistances( + from: Map, + to: Map, + depEdges: Map>, + targetTypes: Set?, + fromModuleGraphJson: String? = null, + toModuleGraphJson: String? = null, + excludeExternalTargets: Boolean = false, + ): Map { val typeFilter = TargetTypeFilter(targetTypes, to) // Quick check: if module graph JSON is identical, skip module change detection entirely @@ -113,22 +170,27 @@ class CalculateImpactedTargetsInteractor : KoinComponent { } val ordering = impactedTargetOrdering(to, from) - impactedTargets + return impactedTargets .filterKeys { typeFilter.accepts(it) } .filterKeys { !excludeExternalTargets || !it.startsWith("//external:") } .toSortedMap(ordering) - .let { filtered -> - outputWriter.use { writer -> - writer.write( - gson.toJson( - filtered.map { - mapOf( - "label" to it.key, - "targetDistance" to it.value.targetDistance, - "packageDistance" to it.value.packageDistance) - })) - } - } + } + + /** Writes [filtered] as a JSON list of distance-metric objects to [outputWriter] and closes it. */ + fun writeImpactedTargetsWithDistances( + outputWriter: Writer, + filtered: Map, + ) { + outputWriter.use { writer -> + writer.write( + gson.toJson( + filtered.map { + mapOf( + "label" to it.key, + "targetDistance" to it.value.targetDistance, + "packageDistance" to it.value.packageDistance) + })) + } } private fun impactedTargetOrdering( diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index d0d020d..9080386 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -1875,6 +1875,73 @@ class E2ETest { .isEqualTo(true) } + @Test + fun testGetImpactedTargets_writeEmptyOutput() { + // Validates the --[no-]writeEmptyOutput flag added on get-impacted-targets so CI can + // branch on file existence (`[ -f impacted.txt ] && bazel test --target_pattern_file=...`) + // instead of file contents. Inspired by ewhauser/bazel-differ's --output-on-empty. + val workspace = copyTestWorkspace("distance_metrics") + val outputDir = temp.newFolder() + val hashes = File(outputDir, "hashes.json") + + val cli = CommandLine(BazelDiff()) + cli.execute( + "generate-hashes", + "-w", workspace.absolutePath, + "-b", "bazel", + hashes.absolutePath) + + // Case 1: empty diff (compare hashes against itself) with --no-writeEmptyOutput + // -> the output file must NOT be created. + val omittedOutput = File(outputDir, "omitted.txt") + val omittedExitCode = cli.execute( + "get-impacted-targets", + "-w", workspace.absolutePath, + "-b", "bazel", + "-sh", hashes.absolutePath, + "-fh", hashes.absolutePath, + "--no-writeEmptyOutput", + "-o", omittedOutput.absolutePath) + assertThat(omittedExitCode).isEqualTo(0) + assertThat(omittedOutput.exists()).isEqualTo(false) + + // Case 2: empty diff with the default (--writeEmptyOutput) -> empty file is created. + val emptyOutput = File(outputDir, "empty.txt") + val emptyExitCode = cli.execute( + "get-impacted-targets", + "-w", workspace.absolutePath, + "-b", "bazel", + "-sh", hashes.absolutePath, + "-fh", hashes.absolutePath, + "-o", emptyOutput.absolutePath) + assertThat(emptyExitCode).isEqualTo(0) + assertThat(emptyOutput.exists()).isEqualTo(true) + assertThat(emptyOutput.readText()).isEqualTo("") + + // Case 3: non-empty diff with --no-writeEmptyOutput -> file IS written (flag only + // suppresses on empty). Modify the workspace and re-hash to produce a real diff. + File(workspace, "A/one.sh").appendText("foo") + val hashesAfter = File(outputDir, "hashes_after.json") + cli.execute( + "generate-hashes", + "-w", workspace.absolutePath, + "-b", "bazel", + hashesAfter.absolutePath) + + val populatedOutput = File(outputDir, "populated.txt") + val populatedExitCode = cli.execute( + "get-impacted-targets", + "-w", workspace.absolutePath, + "-b", "bazel", + "-sh", hashes.absolutePath, + "-fh", hashesAfter.absolutePath, + "--no-writeEmptyOutput", + "-o", populatedOutput.absolutePath) + assertThat(populatedExitCode).isEqualTo(0) + assertThat(populatedOutput.exists()).isEqualTo(true) + assertThat(populatedOutput.readText().isNotEmpty()).isEqualTo(true) + } + private fun copyTestWorkspace(path: String): File { val testProject = temp.newFolder() diff --git a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt index b4b4d3a..5c7e80c 100644 --- a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt @@ -41,6 +41,25 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { assertThat(impacted).containsExactlyInAnyOrder("1", "3") } + @Test + fun computeImpactedTargets_returnsEmpty_whenStartAndFinalMatch() { + // Covers the compute step that GetImpactedTargetsCommand uses to short-circuit + // file creation under --no-writeEmptyOutput. When the two hash maps are equal, + // there is no diff and the returned list must be empty so the CLI can opt out + // of writing the output file at all. + val hashes = + mapOf( + "//pkg:rule_a" to TargetHash("Rule", "r_a", "r_a"), + "//pkg:src_a" to TargetHash("SourceFile", "s_a", "s_a"), + ) + + val computed = + CalculateImpactedTargetsInteractor() + .computeImpactedTargets(from = hashes, to = hashes, targetTypes = null) + + assertThat(computed).isEmpty() + } + @Test fun testExecuteSortsByKindThenLabel() { val startHashes =