diff --git a/compose2pod/emit.py b/compose2pod/emit.py index 837d44e..2392caa 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -178,7 +178,7 @@ def _collect_vars(tokens: list[Token], names: set[str]) -> None: names.update(variable_names(token.value)) -def _run_tokens(name: str, services: dict[str, Any], options: EmitOptions) -> list[Token]: +def run_tokens(name: str, services: dict[str, Any], options: EmitOptions) -> list[Token]: svc = services[name] tokens = run_flags(name, svc, options.pod, options.project_dir) entrypoint = entrypoint_tokens(svc) @@ -195,10 +195,10 @@ def _run_tokens(name: str, services: dict[str, Any], options: EmitOptions) -> li return tokens -def _emit_target(lines: list[str], run_tokens: list[Token], options: EmitOptions) -> None: +def _emit_target(lines: list[str], tokens: list[Token], options: EmitOptions) -> None: target_ctr = shlex.quote(f"{options.pod}-{options.target}") lines.append("rc=0") - lines.append(f"podman run {_render(run_tokens)} || rc=$?") + lines.append(f"podman run {_render(tokens)} || rc=$?") for artifact in options.artifacts: source, destination = artifact.split(":", 1) lines.append(f"podman cp {target_ctr}:{shlex.quote(source)} {shlex.quote(destination)} || true") @@ -219,7 +219,7 @@ def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript: """Walk the target's dependency closure once, building the script and its variables. Every token source is visited a single time and feeds both outputs: each - service's `_run_tokens` and the `pod_create_flags` render into lines *and* + service's `run_tokens` and the `pod_create_flags` render into lines *and* have their `Expand` variables collected, so the script and the variable list cannot disagree about what the script expands at run time. """ @@ -256,14 +256,14 @@ def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript: attempts = max(HEALTHY_WAIT_BUDGET_SECONDS // interval, 1) lines.append(f"wait_healthy {shlex.quote(f'{options.pod}-{dep}')} {attempts} {interval}") waited.add(dep) - run_tokens = _run_tokens(name, services, options) - _collect_vars(run_tokens, names) + tokens = run_tokens(name, services, options) + _collect_vars(tokens, names) if name == options.target: - _emit_target(lines, run_tokens, options) + _emit_target(lines, tokens, options) elif name in completion_gated: - lines.append(f"podman run --rm {_render(run_tokens)}") + lines.append(f"podman run --rm {_render(tokens)}") else: - lines.append(f"podman run -d {_render(run_tokens)}") + lines.append(f"podman run -d {_render(tokens)}") return PlannedScript(script="\n".join(lines) + "\n", variables=sorted(names)) diff --git a/planning/changes/2026-07-13.08-public-run-tokens.md b/planning/changes/2026-07-13.08-public-run-tokens.md new file mode 100644 index 0000000..4a3ec9f --- /dev/null +++ b/planning/changes/2026-07-13.08-public-run-tokens.md @@ -0,0 +1,124 @@ +--- +summary: Rename emit.py's _run_tokens to run_tokens (matching the referenced_variables/command_tokens precedent of package-internal-but-testable names) and add direct tests against its returned token list, replacing four of five full-script string-search ordering tests with one end-to-end smoke test. +--- + +# Design: give run_tokens a direct test seam + +## Summary + +`compose2pod/emit.py`'s `_run_tokens` holds the real subtlety of composing a +service's run command — how `entrypoint` and `command` interact, and in what +order their tokens land — but it is private and has no test importing it +directly. Every ordering assertion today goes through `emit_script()`'s full +pipeline and searches the rendered, shell-quoted script string for substrings +and their relative `.index()` positions. This change renames `_run_tokens` to +`run_tokens` (dropping the underscore, matching the exact precedent already +set by `referenced_variables`/`command_tokens`/`entrypoint_tokens` in this +same file — package-internal, not in `__all__`, but directly testable) and +adds tests that assert on its returned `list[Token]` directly. No return-type +change, no new production consumer, no behavior change. + +## Motivation + +`image_for`, `command_tokens`, and `entrypoint_tokens` are each under 8 lines +and individually unit-tested (`tests/test_emit.py`'s `TestImageAndCommand`, +`TestEntrypoint`). The actual composition logic — string-form entrypoint runs +via `sh -c` and drops the service command (matching Docker semantics), +list-form entrypoint prepends only its first token as `--entrypoint` and +appends the rest positionally after the image, a `--command` override lands +*after* the entrypoint's own tokens rather than replacing them — lives +entirely in `_run_tokens` (13 lines), which no test addresses directly. It is +only reachable by rendering a full script via `TestEmitScript._single()` and +then asserting on substrings and index-ordering in the shell-quoted output +(`test_list_entrypoint_prepends_to_command`, +`test_string_entrypoint_ignores_service_command`, +`test_command_override_still_applies_with_entrypoint`, +`test_list_entrypoint_composes_with_command_override`, +`test_empty_list_entrypoint_is_a_noop`). + +This is a genuine testability gap, not organizational preference: a bug in +`_run_tokens`'s ordering would surface as a failed string-position assertion +on a shell-quoted line, not a clear "wrong token at position N," and writing +a new ordering test requires understanding `shlex.quote`'s escaping alongside +the actual logic under test. + +The originally considered fix — restructuring `_run_tokens` to return +structured pieces (a dataclass: entrypoint flag, image, argv) before final +flattening — was evaluated and rejected as over-scoped: `_run_tokens` has +exactly one caller (`_plan`), so a structured return type would exist only to +serve tests, not a second real consumer ("two adapters = a real seam" does +not hold here). `_run_tokens` already returns `list[Token]`, a directly +comparable value — the only reason today's tests use string search is that +they go through `emit_script()`, the wrong entry point, not because the +return type itself is inadequate. + +## Design + +**Rename.** `_run_tokens` → `run_tokens` in `compose2pod/emit.py`. Same +signature (`name: str, services: dict[str, Any], options: EmitOptions) -> +list[Token]`), same body, same single caller (`_plan`, updated to call +`run_tokens(...)`). Not added to `compose2pod/__init__.py`'s `__all__` — +matches `referenced_variables`'s existing precedent (package-internal, +name-public, test-importable). + +**New direct tests** in `tests/test_emit.py`, a new `TestRunTokens` class, +importing `run_tokens` and asserting on the returned `list[Token]` (no +`_render`/shell-quoting involved). One test per ordering rule, replacing the +logic currently pinned only through `TestEmitScript`'s string search: + +- List entrypoint prepends its first token as `--entrypoint`, appends the + rest after the image, before the service's own `command_tokens`. +- String entrypoint routes through `/bin/sh -c `, and the service's + own `command` key is dropped entirely (not appended). +- A target-service `command` override lands *after* the entrypoint's tokens + (proving it's positional to `sh -c`, not concatenated as a new command). +- A target-service `command` override composes with a list-form entrypoint + (override tokens land after the entrypoint's remaining tokens). +- An empty-list entrypoint (`entrypoint: []`) is a no-op — no `--entrypoint` + flag is added, and the service's own `command` still applies. + +**Existing `TestEmitScript` tests thinned to one smoke test.** Four of the +five string-search tests +(`test_list_entrypoint_prepends_to_command`, +`test_string_entrypoint_ignores_service_command`, +`test_empty_list_entrypoint_is_a_noop`, +`test_list_entrypoint_composes_with_command_override`) are removed — their +ordering-rule coverage moves to `TestRunTokens`, which asserts the same +behavior more precisely. `test_command_override_still_applies_with_entrypoint` +stays, unchanged, as the one remaining end-to-end test proving the full +pipeline (`run_tokens` → `_render` → `shlex` quoting) still assembles a +correct, properly-quoted script — the composed case (entrypoint + command +override) it covers is the most demanding of the five, so it carries the most +end-to-end confidence per test kept. + +## Non-goals + +- Changing `run_tokens`'s return type or signature — the existing + `list[Token]` is already a fine test surface; only its visibility and test + coverage change. +- Adding `run_tokens` to `compose2pod/__init__.py`'s `__all__` — no evidence + any external caller needs it; matches `referenced_variables`'s existing + scope. +- Touching `image_for`/`command_tokens`/`entrypoint_tokens` — already public, + already directly tested, not part of this friction. +- A glossary entry for `run_tokens` — it's a single-file internal seam, not + vocabulary shared across modules (unlike `Token`/`Expand`, which candidate + #1 added to the glossary because five modules import them). + +## Testing + +`just test-ci` at 100% line coverage: the new `TestRunTokens` tests cover +every ordering rule directly; the one retained `TestEmitScript` test confirms +end-to-end rendering is unaffected. `just lint-ci` clean, `just +check-planning`. + +## Risk + +- **Coverage gap from removing four tests before the replacements land** + (low x low): mitigated by writing `TestRunTokens` first and confirming it + covers every branch the removed tests exercised, before deleting them — + `just test-ci`'s 100% gate would fail if any branch lost coverage in the + gap. +- **`run_tokens` renamed but a stale `_run_tokens` reference survives + somewhere** (low x low): `_plan` is the only call site; a `git grep` sweep + after the rename confirms zero remaining references. diff --git a/tests/test_emit.py b/tests/test_emit.py index 9677dea..ecd68c3 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -15,6 +15,7 @@ image_for, referenced_variables, run_flags, + run_tokens, ) from compose2pod.exceptions import UnsupportedComposeError from compose2pod.parsing import validate @@ -289,6 +290,78 @@ def test_string_form_becomes_shell(self) -> None: def test_missing_entrypoint_is_empty(self) -> None: assert entrypoint_tokens({"image": "x"}) == [] + def test_empty_list_is_empty(self) -> None: + # An empty list means "no override", not "an entrypoint of zero tokens" -- + # the same convention `command: []` follows. + assert entrypoint_tokens({"entrypoint": []}) == [] + + +class TestRunTokens: + def _options(self, command: str = "") -> EmitOptions: + return EmitOptions( + target="app", + ci_image="ci", + command=command, + pod="p", + project_dir="/proj", + artifacts=[], + allow_exit_codes=[], + ) + + def test_list_entrypoint_prepends_to_command(self) -> None: + svc = {"image": "x", "entrypoint": ["prog", "--flag"], "command": ["run"]} + tokens = run_tokens("app", {"app": svc}, self._options()) + assert tokens[-5:] == [ + "--entrypoint", + Expand(value="prog"), + Expand(value="x"), + Expand(value="--flag"), + Expand(value="run"), + ] + + def test_string_entrypoint_ignores_service_command(self) -> None: + svc = {"image": "x", "entrypoint": "serve now", "command": ["dropped"]} + tokens = run_tokens("app", {"app": svc}, self._options()) + assert tokens[-5:] == [ + "--entrypoint", + "/bin/sh", + Expand(value="x"), + "-c", + Expand(value="serve now"), + ] + assert Expand(value="dropped") not in tokens + + def test_command_override_lands_after_entrypoint(self) -> None: + svc = {"image": "x", "entrypoint": "serve"} + tokens = run_tokens("app", {"app": svc}, self._options(command="pytest tests")) + assert tokens[-7:] == [ + "--entrypoint", + "/bin/sh", + Expand(value="x"), + "-c", + Expand(value="serve"), + "pytest", + "tests", + ] + + def test_list_entrypoint_composes_with_command_override(self) -> None: + svc = {"image": "x", "entrypoint": ["prog", "--flag"]} + tokens = run_tokens("app", {"app": svc}, self._options(command="run me")) + assert tokens[-6:] == [ + "--entrypoint", + Expand(value="prog"), + Expand(value="x"), + Expand(value="--flag"), + "run", + "me", + ] + + def test_empty_list_entrypoint_is_a_noop(self) -> None: + svc = {"image": "x", "entrypoint": [], "command": ["run"]} + tokens = run_tokens("app", {"app": svc}, self._options()) + assert "--entrypoint" not in tokens + assert tokens[-2:] == [Expand(value="x"), Expand(value="run")] + class TestSecretsLifecycle: def _script(self, doc: dict) -> str: @@ -507,17 +580,6 @@ def _single(self, svc: dict, command: str = "") -> str: ) return emit_script(compose={"services": {"app": svc}}, options=options) - def test_list_entrypoint_prepends_to_command(self) -> None: - script = self._single({"image": "x", "entrypoint": ["prog", "--flag"], "command": ["run"]}) - assert '--entrypoint "prog"' in script - assert '"x" "--flag" "run"' in script - - def test_string_entrypoint_ignores_service_command(self) -> None: - script = self._single({"image": "x", "entrypoint": "serve now", "command": ["dropped"]}) - assert "--entrypoint /bin/sh" in script - assert '-c "serve now"' in script - assert "dropped" not in script - def test_command_override_still_applies_with_entrypoint(self) -> None: script = self._single({"image": "x", "entrypoint": "serve"}, command="pytest tests") assert "--entrypoint /bin/sh" in script @@ -528,21 +590,6 @@ def test_command_override_still_applies_with_entrypoint(self) -> None: # a string entrypoint still runs only `serve`, never `pytest tests`. assert target_line.index('-c "serve"') < target_line.index("pytest") - def test_empty_list_entrypoint_is_a_noop(self) -> None: - # Mirrors the existing `command: []` convention: an empty list means - # "no override", not "an entrypoint of zero tokens". - assert entrypoint_tokens({"entrypoint": []}) == [] - script = self._single({"image": "x", "entrypoint": [], "command": ["run"]}) - assert "--entrypoint" not in script - assert '"x" "run"' in script - - def test_list_entrypoint_composes_with_command_override(self) -> None: - script = self._single({"image": "x", "entrypoint": ["prog", "--flag"]}, command="run me") - target_line = next(line for line in script.splitlines() if "--entrypoint" in line) - assert target_line.index('--entrypoint "prog"') < target_line.index('"x"') - assert target_line.index('"x"') < target_line.index('"--flag"') - assert target_line.index('"--flag"') < target_line.index("run me") - def test_all_process_identity_keys_compose_on_one_service(self) -> None: svc = { "image": "x",