Fix the preflight summary counts not adding up

The origin split describes every check that ran, so hanging it off the passed
count read as arithmetic that does not work: "10 passed (6 static and 5
dynamic)" when 11 checks ran and one failed. The line now reports what ran and
then what came of it -- "11 checks ran (6 static, 5 dynamic) — 10 passed, 0
blocking, 2 warnings."

Also pins the end-to-end guarantee for the case that motivated the severity
change on the server: an error-severity service check that fails stops the
render and names the service and the fix, rather than letting the render spend
its budget failing every test.

Author: VitjanZ

env_check/report.py

@@ -59,7 +59,7 @@ def print_report(report: PreflightReport, verbose: bool = False) -> None:
 
     if not has_findings and not verbose:
         console.info(
-            f"✓ {HEADER}: {len(report.passed)} checks passed ({report.describe_composition()}).",
+            f"✓ {HEADER}: {len(report.results)} checks passed ({report.describe_composition()}).",
             color=SUCCESS_COLOR,
         )
         _print_skipped(report, verbose)
@@ -84,7 +84,8 @@ def print_report(report: PreflightReport, verbose: bool = False) -> None:
         _print_advisory(advisory)
 
     console.info(
-        f"\n  {len(report.passed)} passed ({report.describe_composition()}), "
+        f"\n  {len(report.results)} checks ran ({report.describe_composition()}) — "
+        f"{len(report.passed)} passed, "
         f"{len(blocking) + len(blocking_advisories)} blocking, "
         f"{len(warnings) + len(warning_advisories)} warnings.\n"
     )

env_check/types.py

@@ -123,5 +123,9 @@ def count_by_origin(self, origin: str) -> int:
         return sum(1 for result in self.results if result.check.origin == origin)
 
     def describe_composition(self) -> str:
-        """Describe how many checks came from each origin, for the log."""
-        return f"{self.count_by_origin(ORIGIN_STATIC)} static and " f"{self.count_by_origin(ORIGIN_DYNAMIC)} dynamic"
+        """Describe how many of the checks that ran came from each origin.
+
+        This counts every result, passed or not, so it belongs next to the number
+        of checks that ran rather than the number that passed.
+        """
+        return f"{self.count_by_origin(ORIGIN_STATIC)} static, {self.count_by_origin(ORIGIN_DYNAMIC)} dynamic"

tests/test_env_check_origin.py

@@ -81,7 +81,7 @@ def test_a_report_counts_each_origin_separately(self):
 
         assert report.count_by_origin(ORIGIN_STATIC) == 1
         assert report.count_by_origin(ORIGIN_DYNAMIC) == 2
-        assert report.describe_composition() == "1 static and 2 dynamic"
+        assert report.describe_composition() == "1 static, 2 dynamic"
 
 
 class TestOriginInOutput:
@@ -93,7 +93,29 @@ def test_the_summary_names_both_kinds(self, capsys):
 
         print_report(report)
 
-        assert "1 static and 0 dynamic" in capsys.readouterr().out
+        assert "1 static, 0 dynamic" in capsys.readouterr().out
+
+    def test_the_counts_add_up_when_a_check_fails(self, capsys):
+        """The origin split describes every check that ran, not just the ones that passed."""
+        passed = CheckSpec(
+            id="git", type="command_available", severity=SEVERITY_ERROR, description="git", origin=ORIGIN_STATIC
+        )
+        failed = CheckSpec(
+            id="svc",
+            type="tcp_service_reachable",
+            severity=SEVERITY_WARNING,
+            description="storage service",
+            origin=ORIGIN_DYNAMIC,
+        )
+        report = PreflightReport(
+            results=[CheckResult(passed, STATUS_PASSED, "found"), CheckResult(failed, STATUS_FAILED, "refused")]
+        )
+
+        print_report(report)
+        output = capsys.readouterr().out
+
+        assert "2 checks ran (1 static, 1 dynamic)" in output
+        assert "1 passed" in output
 
     def test_a_finding_says_which_kind_of_check_produced_it(self, capsys):
         check = CheckSpec(

tests/test_env_check_render_wiring.py

@@ -118,3 +118,38 @@ def test_render_module_checks_the_environment_before_doing_any_work(monkeypatch,
         renderer.render_module()
 
     assert rendered == []
+
+
+def test_an_unreachable_local_service_stops_the_render(monkeypatch):
+    """A service the tests depend on is a blocking finding, not a note to skim past.
+
+    The whole point of the preflight is that a render which cannot pass a single
+    test never starts. This mirrors a real plan: the test requirements name a
+    storage service on localhost:6001 that the tests do not mock, the service is
+    down, and the render must stop with the reason rather than spend its budget
+    failing every conformance test.
+    """
+    check = CheckSpec(
+        id="storage-service",
+        type="tcp_service_reachable",
+        severity=SEVERITY_ERROR,
+        description="Storage Service is reachable on port 6001",
+        args={"host": "localhost", "port": 6001},
+        reason="The test requirements say the tests talk to the storage service unmocked.",
+        remediation={"default": "Start the storage service on localhost:6001."},
+    )
+    report = PreflightReport(
+        results=[CheckResult(check, STATUS_FAILED, "localhost:6001 is not reachable ([Errno 61] Connection refused)")]
+    )
+
+    monkeypatch.setattr(module_renderer_module, "run_environment_preflight", lambda *a, **kw: report)
+    monkeypatch.setattr(module_renderer_module, "print_report", lambda *a, **kw: None)
+
+    event_bus = RecordingEventBus()
+    with pytest.raises(EnvironmentCheckFailed) as raised:
+        make_renderer(event_bus)._verify_environment()
+
+    message = str(raised.value)
+    assert "Storage Service is reachable on port 6001" in message
+    assert "Start the storage service on localhost:6001." in message
+    assert event_bus.published[-1].passed is False