Write test reports to a standard location under the distdir. (#14929)

Now users can turn reporting on with --report, without having
to fiddle with the location, but can still modify that location
if necessary. This is similar to how coverage reports work.

Also fixes an issue where we hardcoded `dist/` as the default for
coverage reports, instead of using distdir.

Note that user can, as before, set a custom path outside the
distdir (as they can for coverage reports), but this change makes
it more likely the reports will go to dist/, which is probably
gitignored, instead of to some arbitrary location that may not be.

Basically it makes it easier to turn out reports without creating
complications, and makes use of dist/ more standardized.

[ci skip-rust]

[ci skip-build-wheels]

src/python/pants/backend/python/goals/coverage_py.py

@@ -32,6 +32,7 @@
     FilesystemCoverageReport,
 )
 from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest
+from pants.core.util_rules.distdir import DistDir
 from pants.engine.addresses import Address
 from pants.engine.fs import (
     EMPTY_DIGEST,
@@ -141,9 +142,9 @@ class CoverageSubsystem(PythonToolBase):
     )
     _output_dir = StrOption(
         "--output-dir",
-        default=str(PurePath("dist", "coverage", "python")),
+        default=str(PurePath("{distdir}", "coverage", "python")),
         advanced=True,
-        help="Path to write the Pytest Coverage report to. Must be relative to build root.",
+        help="Path to write the Pytest Coverage report to. Must be relative to the build root.",
     )
     config = FileOption(
         "--config",
@@ -191,9 +192,8 @@ class CoverageSubsystem(PythonToolBase):
         ),
     )
 
-    @property
-    def output_dir(self) -> PurePath:
-        return PurePath(self._output_dir)
+    def output_dir(self, distdir: DistDir) -> PurePath:
+        return PurePath(self._output_dir.format(distdir=distdir.relpath))
 
     @property
     def config_request(self) -> ConfigFilesRequest:
@@ -462,6 +462,7 @@ async def generate_coverage_reports(
     coverage_config: CoverageConfig,
     coverage_subsystem: CoverageSubsystem,
     process_cleanup: ProcessCleanupOption,
+    distdir: DistDir,
 ) -> CoverageReports:
     """Takes all Python test results and generates a single coverage report."""
     transitive_targets = await Get(
@@ -491,6 +492,7 @@ async def generate_coverage_reports(
     report_types = []
     result_snapshot = await Get(Snapshot, Digest, merged_coverage_data.coverage_data)
     coverage_reports: list[CoverageReport] = []
+    output_dir: PurePath = coverage_subsystem.output_dir(distdir)
     for report_type in coverage_subsystem.reports:
         if report_type == CoverageReportType.RAW:
             coverage_reports.append(
@@ -500,8 +502,8 @@ async def generate_coverage_reports(
                     coverage_insufficient=False,
                     report_type=CoverageReportType.RAW.value,
                     result_snapshot=result_snapshot,
-                    directory_to_materialize_to=coverage_subsystem.output_dir,
-                    report_file=coverage_subsystem.output_dir / ".coverage",
+                    directory_to_materialize_to=output_dir,
+                    report_file=output_dir / ".coverage",
                 )
             )
             continue
@@ -547,9 +549,7 @@ async def generate_coverage_reports(
     result_snapshots = await MultiGet(Get(Snapshot, Digest, res.output_digest) for res in results)
 
     coverage_reports.extend(
-        _get_coverage_report(
-            coverage_subsystem.output_dir, report_type, exit_code != 0, stdout, snapshot
-        )
+        _get_coverage_report(output_dir, report_type, exit_code != 0, stdout, snapshot)
         for (report_type, exit_code, stdout, snapshot) in zip(
             report_types, result_exit_codes, result_stdouts, result_snapshots
         )

src/python/pants/core/goals/test.py

@@ -60,6 +60,8 @@ class TestResult(EngineAwareReturnType):
     result_metadata: ProcessResultMetadata | None
 
     coverage_data: CoverageData | None = None
+    # TODO: Rename this to `reports`. There is no guarantee that every language will produce
+    #  XML reports, or only XML reports.
     xml_results: Snapshot | None = None
     # Any extra output (such as from plugins) that the test runner was configured to output.
     extra_output: Snapshot | None = None
@@ -328,9 +330,24 @@ def activated(cls, union_membership: UnionMembership) -> bool:
             "system supports this."
         ),
     )
+    report = BoolOption(
+        "--report", default=False, advanced=True, help="Write test reports to --report-dir."
+    )
+    default_report_path = str(PurePath("{distdir}", "test", "reports"))
+    _report_dir = StrOption(
+        "--report-dir",
+        default=default_report_path,
+        advanced=True,
+        help="Path to write test reports to. Must be relative to the build root.",
+    )
     xml_dir = StrOption(
         "--xml-dir",
         metavar="<DIR>",
+        removal_version="2.13.0.dev0",
+        removal_hint=(
+            "Set the `report` option in [test] scope to emit reports to a standard location under "
+            "dist/. Set the `report-dir` option to customize that location."
+        ),
         default=None,
         advanced=True,
         help=(
@@ -347,6 +364,9 @@ def activated(cls, union_membership: UnionMembership) -> bool:
         ),
     )
 
+    def report_dir(self, distdir: DistDir) -> PurePath:
+        return PurePath(self._report_dir.format(distdir=distdir.relpath))
+
 
 class Test(Goal):
     subsystem_cls = TestSubsystem
@@ -360,7 +380,7 @@ async def run_tests(
     test_subsystem: TestSubsystem,
     workspace: Workspace,
     union_membership: UnionMembership,
-    dist_dir: DistDir,
+    distdir: DistDir,
     run_id: RunId,
 ) -> Test:
     if test_subsystem.debug:
@@ -416,17 +436,19 @@ async def run_tests(
         if result.extra_output and result.extra_output.files:
             workspace.write_digest(
                 result.extra_output.digest,
-                path_prefix=str(dist_dir.relpath / "test" / result.address.path_safe_spec),
+                path_prefix=str(distdir.relpath / "test" / result.address.path_safe_spec),
             )
 
-    if test_subsystem.xml_dir:
-        xml_dir = test_subsystem.xml_dir
-        merged_xml_results = await Get(
+    if test_subsystem.report or test_subsystem.xml_dir:
+        report_dir = (
+            test_subsystem.report_dir(distdir) if test_subsystem.report else test_subsystem.xml_dir
+        )
+        merged_reports = await Get(
             Digest,
             MergeDigests(result.xml_results.digest for result in results if result.xml_results),
         )
-        workspace.write_digest(merged_xml_results, path_prefix=xml_dir)
-        console.print_stderr(f"\nWrote test XML to `{xml_dir}`")
+        workspace.write_digest(merged_reports, path_prefix=str(report_dir))
+        console.print_stderr(f"\nWrote test reports to {report_dir}")
 
     if test_subsystem.use_coverage:
         # NB: We must pre-sort the data for itertools.groupby() to work properly, using the same

src/python/pants/core/goals/test_test.py

@@ -148,7 +148,8 @@ def run_test_rule(
     targets: list[Target],
     debug: bool = False,
     use_coverage: bool = False,
-    xml_dir: str | None = None,
+    report: bool = False,
+    report_dir: str = TestSubsystem.default_report_path,
     output: ShowOutput = ShowOutput.ALL,
     valid_targets: bool = True,
     run_id: RunId = RunId(999),
@@ -157,7 +158,9 @@ def run_test_rule(
         TestSubsystem,
         debug=debug,
         use_coverage=use_coverage,
-        xml_dir=xml_dir,
+        report=report,
+        report_dir=report_dir,
+        xml_dir=None,
         output=output,
         extra_env_vars=[],
     )
@@ -336,18 +339,32 @@ def test_debug_target(rule_runner: RuleRunner) -> None:
     assert exit_code == 0
 
 
-def test_xml_dir(rule_runner: RuleRunner) -> None:
-    xml_dir = "dist/test-results"
+def test_report(rule_runner: RuleRunner) -> None:
     addr1 = Address("", target_name="t1")
     addr2 = Address("", target_name="t2")
     exit_code, stderr = run_test_rule(
         rule_runner,
         field_set=SuccessfulFieldSet,
         targets=[make_target(addr1), make_target(addr2)],
-        xml_dir=xml_dir,
+        report=True,
     )
     assert exit_code == 0
-    assert f"Wrote test XML to `{xml_dir}`" in stderr
+    assert "Wrote test reports to dist/test/reports" in stderr
+
+
+def test_report_dir(rule_runner: RuleRunner) -> None:
+    report_dir = "dist/test-results"
+    addr1 = Address("", target_name="t1")
+    addr2 = Address("", target_name="t2")
+    exit_code, stderr = run_test_rule(
+        rule_runner,
+        field_set=SuccessfulFieldSet,
+        targets=[make_target(addr1), make_target(addr2)],
+        report=True,
+        report_dir=report_dir,
+    )
+    assert exit_code == 0
+    assert f"Wrote test reports to {report_dir}" in stderr
 
 
 def test_coverage(rule_runner: RuleRunner) -> None: