diff --git a/cli/localci/core/executor.py b/cli/localci/core/executor.py index 26b5c80..34f1e39 100644 --- a/cli/localci/core/executor.py +++ b/cli/localci/core/executor.py @@ -146,7 +146,32 @@ def summary_line(self) -> str: class ActCommand: """Builder for ``act`` CLI command. - Constructs the full command-line invocation for act. + Constructs the full command-line invocation for act. :meth:`build` is pure: + it only reads ``self`` to assemble an argv list. + + **Executor mutation contract.** When passed to :meth:`JobExecutor.run`, the + executor mutates this instance in place during + :meth:`JobExecutor._execute_process`: + + * :attr:`env_file` and :attr:`_executor_owned_env_file` are set when + :attr:`env` is non-empty (a ``0600`` temp file is created). + * When materializing that file, if a caller-supplied, unowned + :attr:`env_file` already exists on disk, its contents are copied into + the new temp file before :attr:`env` entries are written; the original + caller file is left on disk. + * :attr:`secret_file` and :attr:`_executor_owned_secret_file` are set when + :attr:`secrets` is non-empty (a ``0600`` temp file is created). + + The ownership flags tell :meth:`JobExecutor._cleanup_temp_files` (called from + :meth:`JobExecutor.run`'s ``finally`` block) which temp files the executor + created and must delete. Caller-provided ``env_file`` / ``secret_file`` paths + without the ownership flag are left on disk. + + **Single-use per execution.** Build a fresh :class:`ActCommand` for each + call to :meth:`JobExecutor.run`. Reusing the same instance across runs is + unsupported: stale paths, ownership flags, and merged env state from a prior + run produce undefined behavior. Sharing one instance across concurrent + executions is likewise unsupported. """ # Required @@ -440,6 +465,15 @@ def run( ------- JobResult Complete execution result with status, output, and timing. + + Notes + ----- + Mutates *cmd* in place: :meth:`_execute_process` writes + :attr:`ActCommand.env_file` / :attr:`ActCommand.secret_file` and the + ``_executor_owned_*`` flags on the passed-in instance. Temp files the + executor creates are removed in this method's ``finally`` block via + :meth:`_cleanup_temp_files`. See :class:`ActCommand` for the full + mutation contract. """ result = JobResult( job_id=cmd.job_id, @@ -551,6 +585,13 @@ def _execute_process( the secret file; :attr:`ActCommand.secrets` is also injected into the subprocess environment for act's GitHub auth. + **Mutates *act_cmd* in place.** When :attr:`ActCommand.env` or + :attr:`ActCommand.secrets` is non-empty, this method creates temp files + and sets :attr:`ActCommand.env_file` / :attr:`ActCommand.secret_file` + plus the corresponding ``_executor_owned_*`` flags on *act_cmd*. The + caller's object is the cleanup target for :meth:`_cleanup_temp_files`. + Do not reuse *act_cmd* across calls; see :class:`ActCommand`. + Returns ``(exit_code, stdout, stderr)``. """ workdir = act_cmd.workdir or Path(".") diff --git a/cli/tests/integration/test_act_command_mutation.py b/cli/tests/integration/test_act_command_mutation.py new file mode 100644 index 0000000..fce1f01 --- /dev/null +++ b/cli/tests/integration/test_act_command_mutation.py @@ -0,0 +1,107 @@ +"""Integration tests for ActCommand mutation contract (JobExecutor side effects).""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from localci.core.command_builder import ActCommandBuilder +from localci.core.executor import ActCommand, JobExecutor, JobStatus +from localci.core.workflow import MatrixEntry, WorkflowAnalyzer + +from .conftest import INTEGRATION_JOB_ID, INTEGRATION_TIMEOUT + +pytestmark = pytest.mark.integration + + +def _workflow_path(name: str, project_dir: Path) -> Path: + return project_dir / ".github/workflows" / name + + +def _build_act_command( + workflow_name: str, + project_dir: Path, + act_runner_image: str, +) -> tuple[MatrixEntry, ActCommand]: + workflow_path = _workflow_path(workflow_name, project_dir) + analyzer = WorkflowAnalyzer() + workflow = analyzer.analyze(workflow_path) + entry = workflow.jobs[INTEGRATION_JOB_ID].matrix[0] + + builder = ActCommandBuilder( + workflow_file=workflow_path, + project_dir=project_dir, + job_id=INTEGRATION_JOB_ID, + default_secrets={"GITHUB_TOKEN": "local-ci-test-token"}, + ) + cmd = builder.build( + entry, + image_tag=act_runner_image, + # Minimal fixture matrix has no cc/cxx; supply env so executor + # materializes an executor-owned env_file (mutation contract). + extra_env={"LOCALCI_TEST_ENV": "mutation-contract"}, + ) + return entry, cmd + + +def test_act_command_mutation_and_cleanup( + integration_project: tuple[Path, Path], + act_runner_image: str, +) -> None: + """End-to-end: executor mutates ActCommand during run and cleans temp files.""" + project, logs_dir = integration_project + entry, cmd = _build_act_command("test.yml", project, act_runner_image) + + assert cmd.env, "fixture must populate env so executor materializes env_file" + assert cmd.secrets, ( + "fixture must populate secrets so executor materializes secret_file" + ) + assert cmd.env_file is None + assert cmd.secret_file is None + assert not cmd._executor_owned_env_file + assert not cmd._executor_owned_secret_file + + mutation_during_run = False + + def on_output(_line: str) -> None: + nonlocal mutation_during_run + if mutation_during_run: + return + env_ready = ( + cmd.env_file is not None + and cmd._executor_owned_env_file + and cmd.env_file.exists() + ) + secret_ready = ( + cmd.secret_file is not None + and cmd._executor_owned_secret_file + and cmd.secret_file.exists() + ) + if env_ready and secret_ready: + mutation_during_run = True + + executor = JobExecutor(logs_dir=logs_dir) + result = executor.run( + cmd, + matrix_index=entry.index, + matrix_name=entry.name, + timeout=INTEGRATION_TIMEOUT, + stream_output=False, + on_output=on_output, + ) + + assert result.status == JobStatus.PASSED + assert mutation_during_run, ( + "temp files and ownership flags must be set before act output" + ) + assert cmd._executor_owned_env_file + assert cmd._executor_owned_secret_file + assert cmd.env_file is not None + assert cmd.secret_file is not None + assert not cmd.env_file.exists(), ( + "executor-owned env file must be removed after run" + ) + assert not cmd.secret_file.exists(), ( + "executor-owned secret file must be removed after run" + )