Make check output more useful for Go and Java (Cherry-pick of #13379)

src/python/pants/backend/go/goals/check.py

@@ -2,7 +2,6 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 from dataclasses import dataclass
-from typing import cast
 
 from pants.backend.go.target_types import GoFirstPartyPackageSourcesField
 from pants.backend.go.util_rules.build_pkg import (
@@ -15,6 +14,7 @@
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import FieldSet
 from pants.engine.unions import UnionRule
+from pants.util.logging import LogLevel
 
 
 @dataclass(frozen=True)
@@ -28,7 +28,7 @@ class GoCheckRequest(CheckRequest):
     field_set_type = GoCheckFieldSet
 
 
-@rule
+@rule(desc="Check Go compilation", level=LogLevel.DEBUG)
 async def check_go(request: GoCheckRequest) -> CheckResults:
     build_requests = await MultiGet(
         Get(FallibleBuildGoPackageRequest, BuildGoPackageTargetRequest(field_set.address))
@@ -46,30 +46,16 @@ async def check_go(request: GoCheckRequest) -> CheckResults:
         Get(FallibleBuiltGoPackage, BuildGoPackageRequest, request) for request in valid_requests
     )
 
-    # TODO: Update `build_pkg.py` to use streaming workunits to log compilation results, which has
-    #  the benefit of other contexts like `test.py` using it. Switch this to only preserve the
-    #  exit code.
-    check_results = [
-        *(
-            CheckResult(
-                result.exit_code,
-                "",
-                cast(str, result.stderr),
-                partition_description=result.import_path,
-            )
-            for result in invalid_requests
+    # NB: We don't pass stdout/stderr as it will have already been rendered as streaming.
+    exit_code = next(
+        (
+            result.exit_code  # type: ignore[attr-defined]
+            for result in (*build_results, *invalid_requests)
+            if result.exit_code != 0  # type: ignore[attr-defined]
         ),
-        *(
-            CheckResult(
-                result.exit_code,
-                result.stdout or "",
-                result.stderr or "",
-                partition_description=result.import_path,
-            )
-            for result in build_results
-        ),
-    ]
-    return CheckResults(check_results, checker_name="go")
+        0,
+    )
+    return CheckResults([CheckResult(exit_code, "", "")], checker_name="go compile")
 
 
 def rules():

src/python/pants/backend/go/goals/check_test.py

@@ -77,9 +77,4 @@ def test_check(rule_runner: RuleRunner) -> None:
     results = rule_runner.request(
         CheckResults, [GoCheckRequest(GoCheckFieldSet.create(tgt) for tgt in targets)]
     ).results
-    assert set(results) == {
-        CheckResult(0, "", "", "example.com/greeter/good"),
-        CheckResult(
-            1, "", "bad/f.go:1:1: expected 'package', found invalid\n", "example.com/greeter/bad"
-        ),
-    }
+    assert set(results) == {CheckResult(1, "", "")}

src/python/pants/backend/go/util_rules/build_pkg.py

@@ -26,12 +26,13 @@
 from pants.backend.go.util_rules.sdk import GoSdkProcess
 from pants.backend.go.util_rules.third_party_pkg import ThirdPartyPkgInfo, ThirdPartyPkgInfoRequest
 from pants.build_graph.address import Address
-from pants.engine.engine_aware import EngineAwareParameter
+from pants.engine.engine_aware import EngineAwareParameter, EngineAwareReturnType
 from pants.engine.fs import AddPrefix, Digest, MergeDigests
 from pants.engine.process import FallibleProcessResult
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import Dependencies, DependenciesRequest, UnexpandedTargets, WrappedTarget
 from pants.util.frozendict import FrozenDict
+from pants.util.logging import LogLevel
 from pants.util.strutil import path_safe
 
 
@@ -58,7 +59,7 @@ def debug_hint(self) -> str | None:
 
 
 @dataclass(frozen=True)
-class FallibleBuildGoPackageRequest(EngineAwareParameter):
+class FallibleBuildGoPackageRequest(EngineAwareParameter, EngineAwareReturnType):
     """Request to build a package, but fallible if determining the request metadata failed.
 
     When creating "synthetic" packages, use `GoPackageRequest` directly. This type is only intended
@@ -70,9 +71,25 @@ class FallibleBuildGoPackageRequest(EngineAwareParameter):
     exit_code: int = 0
     stderr: str | None = None
 
+    def level(self) -> LogLevel:
+        return LogLevel.ERROR if self.exit_code != 0 else LogLevel.DEBUG
+
+    def message(self) -> str:
+        message = self.import_path
+        message += (
+            " succeeded." if self.exit_code == 0 else f" failed (exit code {self.exit_code})."
+        )
+        if self.stderr:
+            message += f"\n{self.stderr}"
+        return message
+
+    def cacheable(self) -> bool:
+        # Failed compile outputs should be re-rendered in every run.
+        return self.exit_code == 0
+
 
 @dataclass(frozen=True)
-class FallibleBuiltGoPackage:
+class FallibleBuiltGoPackage(EngineAwareReturnType):
     """Fallible version of `BuiltGoPackage` with error details."""
 
     output: BuiltGoPackage | None
@@ -81,6 +98,24 @@ class FallibleBuiltGoPackage:
     stdout: str | None = None
     stderr: str | None = None
 
+    def level(self) -> LogLevel:
+        return LogLevel.ERROR if self.exit_code != 0 else LogLevel.DEBUG
+
+    def message(self) -> str:
+        message = self.import_path
+        message += (
+            " succeeded." if self.exit_code == 0 else f" failed (exit code {self.exit_code})."
+        )
+        if self.stdout:
+            message += f"\n{self.stdout}"
+        if self.stderr:
+            message += f"\n{self.stderr}"
+        return message
+
+    def cacheable(self) -> bool:
+        # Failed compile outputs should be re-rendered in every run.
+        return self.exit_code == 0
+
 
 @dataclass(frozen=True)
 class BuiltGoPackage:
@@ -93,7 +128,9 @@ class BuiltGoPackage:
     import_paths_to_pkg_a_files: FrozenDict[str, str]
 
 
-@rule
+# NB: We must have a description for the streaming of this rule to work properly
+# (triggered by `FallibleBuiltGoPackage` subclassing `EngineAwareReturnType`).
+@rule(desc="Compile with Go")
 async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPackage:
     maybe_built_deps = await MultiGet(
         Get(FallibleBuiltGoPackage, BuildGoPackageRequest, build_request)
@@ -227,7 +264,9 @@ def debug_hint(self) -> str:
         return str(self.address)
 
 
-@rule
+# NB: We must have a description for the streaming of this rule to work properly
+# (triggered by `FallibleBuildGoPackageRequest` subclassing `EngineAwareReturnType`).
+@rule(desc="Set up Go compilation request")
 async def setup_build_go_package_target_request(
     request: BuildGoPackageTargetRequest,
 ) -> FallibleBuildGoPackageRequest:

src/python/pants/backend/java/compile/javac.py

@@ -231,7 +231,7 @@ async def compile_java_source(
     )
 
 
-@rule(desc="Check compilation for javac", level=LogLevel.DEBUG)
+@rule(desc="Check javac compilation", level=LogLevel.DEBUG)
 async def javac_check(request: JavacCheckRequest) -> CheckResults:
     coarsened_targets = await Get(
         CoarsenedTargets, Addresses(field_set.address for field_set in request.field_sets)
@@ -242,20 +242,9 @@ async def javac_check(request: JavacCheckRequest) -> CheckResults:
         for t in coarsened_targets
     )
 
-    # NB: We return CheckResults with exit codes for the root targets, but we do not pass
-    # stdout/stderr because it will already have been rendered as streaming.
-    return CheckResults(
-        [
-            CheckResult(
-                result.exit_code,
-                stdout="",
-                stderr="",
-                partition_description=str(coarsened_target),
-            )
-            for result, coarsened_target in zip(results, coarsened_targets)
-        ],
-        checker_name="javac",
-    )
+    # NB: We don't pass stdout/stderr as it will have already been rendered as streaming.
+    exit_code = next((result.exit_code for result in results if result.exit_code != 0), 0)
+    return CheckResults([CheckResult(exit_code, "", "")], checker_name="javac")
 
 
 def rules():

src/python/pants/backend/java/compile/javac_test.py

@@ -15,7 +15,7 @@
 from pants.backend.java.target_types import JavaSourcesGeneratorTarget
 from pants.backend.java.target_types import rules as target_types_rules
 from pants.build_graph.address import Address
-from pants.core.goals.check import CheckResults
+from pants.core.goals.check import CheckResult, CheckResults
 from pants.core.util_rules import archive, config_files, source_files
 from pants.core.util_rules.archive import UnzipBinary
 from pants.core.util_rules.external_tool import rules as external_tool_rules
@@ -196,11 +196,8 @@ def test_compile_no_deps(rule_runner: RuleRunner) -> None:
                 [JavacCheckRequest.field_set_type.create(coarsened_target.representative)]
             )
         ],
-    )
-
-    assert len(check_results.results) == 1
-    check_result = check_results.results[0]
-    assert check_result.partition_description == str(coarsened_target)
+    ).results
+    assert set(check_results) == {CheckResult(0, "", "")}
 
 
 @maybe_skip_jdk_test

src/python/pants/jvm/compile.py

@@ -71,7 +71,7 @@ def prep_output(s: bytes) -> str:
         )
 
     def level(self) -> LogLevel:
-        return LogLevel.ERROR if self.exit_code != 0 else LogLevel.INFO
+        return LogLevel.ERROR if self.exit_code != 0 else LogLevel.DEBUG
 
     def message(self) -> str:
         message = self.description