shell: support extra output files in experimental_test_shell_command

docs/notes/2.26.x.md

@@ -65,7 +65,10 @@ The Pants repo now uses Ruff format in lieu of Black. This was not a drop-in rep
 
 #### Shell
 
-The `experiemental_test_shell_command` target type may now be used with the `test` goal's `--debug` flag to execute the test interactively.
+The `experiemental_test_shell_command` target type learned several new features:
+
+- It now supports the `test` goal's `--debug` flag to execute the test interactively.
+- It gained the output-related fields from `shell_command`; specifically, the `output_files`, `output_directories`, `root_output_directory`, and `outputs_match_mode` fields. Captured output will be written by the `test` goal as "extra output" in a directory under `dist/` based on the target name.
 
 #### Terraform
 

src/python/pants/backend/adhoc/adhoc_tool_test.py

@@ -3,6 +3,7 @@
 
 from __future__ import annotations
 
+import time
 from pathlib import Path
 from textwrap import dedent
 
@@ -17,13 +18,11 @@
 from pants.core.target_types import ArchiveTarget, FilesGeneratorTarget
 from pants.core.target_types import rules as core_target_type_rules
 from pants.core.util_rules import archive, source_files
-from pants.core.util_rules.adhoc_process_support import AdhocProcessRequest
 from pants.core.util_rules.environments import LocalWorkspaceEnvironmentTarget
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
 from pants.engine.fs import EMPTY_SNAPSHOT, DigestContents
 from pants.engine.internals.scheduler import ExecutionError
-from pants.engine.process import Process
 from pants.engine.target import (
     GeneratedSources,
     GenerateSourcesRequest,
@@ -45,7 +44,6 @@ def rule_runner() -> PythonRuleRunner:
             *run_python_source_rules(),
             *run_system_binary_rules(),
             QueryRule(GeneratedSources, [GenerateFilesFromAdhocToolRequest]),
-            QueryRule(Process, [AdhocProcessRequest]),
             QueryRule(SourceFiles, [SourceFilesRequest]),
             QueryRule(TransitiveTargets, [TransitiveTargetsRequest]),
         ],
@@ -373,6 +371,7 @@ def test_adhoc_tool_workspace_invalidation_sources(rule_runner: PythonRuleRunner
 
     # Update the hash-only source file's content. The adhoc_tool should be re-executed now.
     (Path(rule_runner.build_root) / "src" / "a-file").write_text("xyzzy")
+    time.sleep(0.1)  # wait for invalidation to occur in engine
     result3 = execute_adhoc_tool(rule_runner, address)
     assert result1.snapshot != result3.snapshot
 

src/python/pants/backend/shell/goals/test.py

@@ -25,14 +25,18 @@
     TestResult,
     TestSubsystem,
 )
+from pants.core.util_rules.adhoc_process_support import (
+    AdhocProcessRequest,
+    FallibleAdhocProcessResult,
+    prepare_adhoc_process,
+)
+from pants.core.util_rules.adhoc_process_support import rules as adhoc_process_support_rules
+from pants.core.util_rules.adhoc_process_support import run_prepared_adhoc_process
 from pants.core.util_rules.environments import EnvironmentField
+from pants.engine.fs import EMPTY_DIGEST, Snapshot
 from pants.engine.internals.graph import resolve_target
-from pants.engine.process import (
-    InteractiveProcess,
-    ProcessCacheScope,
-    ProcessWithRetries,
-    execute_process_with_retry,
-)
+from pants.engine.intrinsics import digest_to_snapshot
+from pants.engine.process import InteractiveProcess, ProcessCacheScope
 from pants.engine.rules import collect_rules, implicitly, rule
 from pants.engine.target import Target, WrappedTargetRequest
 from pants.util.frozendict import FrozenDict
@@ -80,21 +84,33 @@ async def test_shell_command(
         cache_scope=(
             ProcessCacheScope.PER_SESSION if test_subsystem.force else ProcessCacheScope.SUCCESSFUL
         ),
-        env=FrozenDict(
+        env_vars=FrozenDict(
             {
                 **test_extra_env.env,
-                **shell_process.env,
+                **shell_process.env_vars,
             }
         ),
     )
 
-    shell_result = await execute_process_with_retry(
-        ProcessWithRetries(shell_process, test_subsystem.attempts_default)
-    )
+    results: list[FallibleAdhocProcessResult] = []
+    for _ in range(test_subsystem.attempts_default):
+        result = await run_prepared_adhoc_process(
+            **implicitly({shell_process: AdhocProcessRequest})
+        )  # noqa: PNT30: retry loop
+        results.append(result)
+        if result.process_result.exit_code == 0:
+            break
+
+    extra_output: Snapshot | None = None
+    if results[-1].adjusted_digest != EMPTY_DIGEST:
+        extra_output = await digest_to_snapshot(results[-1].adjusted_digest)
+
     return TestResult.from_fallible_process_result(
-        process_results=shell_result.results,
+        process_results=tuple(r.process_result for r in results),
         address=field_set.address,
         output_setting=test_subsystem.output,
+        extra_output=extra_output,
+        log_extra_output=extra_output is not None,
     )
 
 
@@ -108,12 +124,14 @@ async def test_shell_command_interactively(
         **implicitly(),
     )
 
-    shell_process = await prepare_process_request_from_target(
-        ShellCommandProcessFromTargetRequest(wrapped_tgt.target), **implicitly()
+    prepared_request = await prepare_adhoc_process(
+        **implicitly(ShellCommandProcessFromTargetRequest(wrapped_tgt.target))
     )
 
     # This is probably not strictly necessary given the use of `InteractiveProcess` but good to be correct in any event.
-    shell_process = dataclasses.replace(shell_process, cache_scope=ProcessCacheScope.PER_SESSION)
+    shell_process = dataclasses.replace(
+        prepared_request.process, cache_scope=ProcessCacheScope.PER_SESSION
+    )
 
     return TestDebugRequest(
         InteractiveProcess.from_process(
@@ -127,4 +145,5 @@ def rules():
         *collect_rules(),
         *shell_command.rules(),
         *ShellTestRequest.rules(),
+        *adhoc_process_support_rules(),
     )

src/python/pants/backend/shell/goals/test_test.py

@@ -19,6 +19,8 @@
 from pants.core.goals import package
 from pants.core.goals.test import TestDebugRequest, TestResult, get_filtered_environment
 from pants.core.util_rules import archive, source_files
+from pants.engine.fs import DigestContents, FileContent
+from pants.engine.internals.scheduler import ExecutionError
 from pants.engine.rules import QueryRule
 from pants.engine.target import Target
 from pants.testutil.rule_runner import RuleRunner, mock_console
@@ -51,7 +53,7 @@ def rule_runner() -> RuleRunner:
 
 
 @pytest.mark.platform_specific_behavior
-def test_shell_command_as_test(rule_runner: RuleRunner) -> None:
+def test_basic_usage_of_test_shell_command(rule_runner: RuleRunner) -> None:
     rule_runner.write_files(
         {
             "BUILD": dedent(
@@ -123,3 +125,133 @@ def run_test(test_target: Target) -> TestResult:
     with mock_console(rule_runner.options_bootstrapper):
         fail_debug_result = rule_runner.run_interactive_process(fail_debug_request.process)
         assert fail_debug_result.exit_code == 1
+
+
+@pytest.mark.platform_specific_behavior
+def test_extra_outputs_support(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "BUILD": dedent(
+                """\
+                shell_sources(name="src")
+
+                experimental_test_shell_command(
+                  name="test",
+                  execution_dependencies=[":src"],
+                  tools=["echo", "mkdir"],
+                  command="./test.sh msg.txt message",
+                  output_files=["world.txt"],
+                  output_directories=["some-dir"],
+                )
+                """
+            ),
+            "test.sh": dedent(
+                """\
+                mkdir -p some-dir
+                echo "xyzzy" > some-dir/foo.txt
+                echo "hello" > world.txt
+                """
+            ),
+        }
+    )
+    (Path(rule_runner.build_root) / "test.sh").chmod(0o555)
+
+    def test_batch_for_target(test_target: Target) -> ShellTestRequest.Batch:
+        return ShellTestRequest.Batch("", (TestShellCommandFieldSet.create(test_target),), None)
+
+    def run_test(test_target: Target) -> TestResult:
+        return rule_runner.request(TestResult, [test_batch_for_target(test_target)])
+
+    result = run_test(rule_runner.get_target(Address("", target_name="test")))
+    assert result.extra_output is not None
+    digest_contents = rule_runner.request(DigestContents, [result.extra_output.digest])
+    digest_contents_sorted = sorted(digest_contents, key=lambda x: x.path)
+    assert len(digest_contents_sorted) == 2
+    assert digest_contents_sorted[0] == FileContent("some-dir/foo.txt", b"xyzzy\n")
+    assert digest_contents_sorted[1] == FileContent("world.txt", b"hello\n")
+
+
+def test_outputs_match_mode_support(rule_runner: RuleRunner) -> None:
+    rule_runner.write_files(
+        {
+            "BUILD": dedent(
+                """\
+            experimental_test_shell_command(
+                name="allow_empty",
+                command="true",
+                output_files=["non-existent-file"],
+                output_directories=["non-existent-dir"],
+                outputs_match_mode="allow_empty",
+            )
+            experimental_test_shell_command(
+                name="all_with_present_file",
+                command="touch some-file",
+                tools=["touch"],
+                output_files=["some-file"],
+                output_directories=["some-directory"],
+                outputs_match_mode="all",
+            )
+            experimental_test_shell_command(
+                name="all_with_present_directory",
+                command="mkdir some-directory",
+                tools=["mkdir"],
+                output_files=["some-file"],
+                output_directories=["some-directory"],
+                outputs_match_mode="all",
+            )
+            experimental_test_shell_command(
+                name="at_least_one_with_present_file",
+                command="touch some-file",
+                tools=["touch"],
+                output_files=["some-file"],
+                output_directories=["some-directory"],
+                outputs_match_mode="at_least_one",
+            )
+            experimental_test_shell_command(
+                name="at_least_one_with_present_directory",
+                command="mkdir some-directory && touch some-directory/foo.txt",
+                tools=["mkdir", "touch"],
+                output_files=["some-file"],
+                output_directories=["some-directory"],
+                outputs_match_mode="at_least_one",
+            )
+            """
+            )
+        }
+    )
+
+    def test_batch_for_target(test_target: Target) -> ShellTestRequest.Batch:
+        return ShellTestRequest.Batch("", (TestShellCommandFieldSet.create(test_target),), None)
+
+    def run_test(address: Address) -> TestResult:
+        test_target = rule_runner.get_target(address)
+        return rule_runner.request(TestResult, [test_batch_for_target(test_target)])
+
+    def assert_result(
+        address: Address,
+        expected_contents: dict[str, str],
+    ) -> None:
+        result = run_test(address)
+        if expected_contents:
+            assert result.extra_output
+            assert result.extra_output.files == tuple(expected_contents)
+
+            contents = rule_runner.request(DigestContents, [result.extra_output.digest])
+            for fc in contents:
+                assert fc.content == expected_contents[fc.path].encode()
+
+    assert_result(Address("", target_name="allow_empty"), {})
+
+    with pytest.raises(ExecutionError) as exc_info:
+        run_test(Address("", target_name="all_with_present_file"))
+    assert "some-directory" in str(exc_info)
+
+    with pytest.raises(ExecutionError) as exc_info:
+        run_test(Address("", target_name="all_with_present_directory"))
+    assert "some-file" in str(exc_info)
+
+    assert_result(Address("", target_name="at_least_one_with_present_file"), {"some-file": ""})
+    assert_result(
+        Address("", target_name="at_least_one_with_present_directory"),
+        {"some-directory/foo.txt": ""},
+    )

src/python/pants/backend/shell/target_types.py

@@ -500,6 +500,10 @@ class ShellCommandTestTarget(Target):
         EnvironmentField,
         SkipShellCommandTestsField,
         ShellCommandWorkdirField,
+        ShellCommandOutputFilesField,
+        ShellCommandOutputDirectoriesField,
+        ShellCommandOutputRootDirField,
+        ShellCommandOutputsMatchMode,
     )
     help = help_text(
         """

src/python/pants/backend/shell/util_rules/shell_command.py

@@ -36,18 +36,16 @@
 from pants.core.target_types import FileSourceField
 from pants.core.util_rules.adhoc_process_support import (
     AdhocProcessRequest,
-    AdhocProcessResult,
     ExtraSandboxContents,
     MergeExtraSandboxContents,
     ResolveExecutionDependenciesRequest,
+    convert_fallible_adhoc_process_result,
     merge_extra_sandbox_contents,
     parse_relative_directory,
-    prepare_adhoc_process,
     prepare_env_vars,
     resolve_execution_environment,
 )
 from pants.core.util_rules.adhoc_process_support import rules as adhoc_process_support_rules
-from pants.core.util_rules.adhoc_process_support import run_adhoc_process
 from pants.core.util_rules.environments import (
     EnvironmentNameRequest,
     EnvironmentTarget,
@@ -89,12 +87,15 @@ class ShellCommandProcessFromTargetRequest:
     target: Target
 
 
-async def _prepare_process_request_from_target(
-    shell_command: Target,
+@rule
+async def prepare_process_request_from_target(
+    request: ShellCommandProcessFromTargetRequest,
     shell_setup: ShellSetup.EnvironmentAware,
     bash: BashBinary,
     env_target: EnvironmentTarget,
 ) -> AdhocProcessRequest:
+    shell_command = request.target
+
     description = f"the `{shell_command.alias}` at `{shell_command.address}`"
 
     working_directory = shell_command[ShellCommandWorkdirField].value
@@ -110,7 +111,7 @@ async def _prepare_process_request_from_target(
             shell_command.get(ShellCommandExecutionDependenciesField).value,
             shell_command.get(ShellCommandRunnableDependenciesField).value,
         ),
-        bash,
+        **implicitly(),
     )
 
     dependencies_digest = execution_environment.digest
@@ -214,29 +215,6 @@ async def _prepare_process_request_from_target(
     )
 
 
-@rule
-async def run_adhoc_result_from_target(
-    request: ShellCommandProcessFromTargetRequest,
-    shell_setup: ShellSetup.EnvironmentAware,
-    bash: BashBinary,
-    env_target: EnvironmentTarget,
-) -> AdhocProcessResult:
-    scpr = await _prepare_process_request_from_target(request.target, shell_setup, bash, env_target)
-    return await run_adhoc_process(scpr)
-
-
-@rule
-async def prepare_process_request_from_target(
-    request: ShellCommandProcessFromTargetRequest,
-    shell_setup: ShellSetup.EnvironmentAware,
-    bash: BashBinary,
-    env_target: EnvironmentTarget,
-) -> Process:
-    # Needed to support `experimental_test_shell_command`
-    scpr = await _prepare_process_request_from_target(request.target, shell_setup, bash, env_target)
-    return await prepare_adhoc_process(scpr, **implicitly())
-
-
 class RunShellCommand(RunFieldSet):
     required_fields = (
         ShellCommandCommandField,
@@ -255,7 +233,7 @@ async def shell_command_in_sandbox(
         **implicitly(),
     )
 
-    adhoc_result = await run_adhoc_result_from_target(
+    adhoc_result = await convert_fallible_adhoc_process_result(
         **implicitly(
             {
                 environment_name: EnvironmentName,