Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions compose2pod/emit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand All @@ -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.
"""
Expand Down Expand Up @@ -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))


Expand Down
124 changes: 124 additions & 0 deletions planning/changes/2026-07-13.08-public-run-tokens.md
Original file line number Diff line number Diff line change
@@ -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 <Expand>`, 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.
99 changes: 73 additions & 26 deletions tests/test_emit.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
image_for,
referenced_variables,
run_flags,
run_tokens,
)
from compose2pod.exceptions import UnsupportedComposeError
from compose2pod.parsing import validate
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand Down