From 8ab24726fec06d5287425df7aad445e3af0126da Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 23 Jun 2026 07:41:48 +0000 Subject: [PATCH] refactor(commit): introduce Freezer class hierarchy across backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a Freezer ABC (backend/freeze.py) that encapsulates the stop-commit-mark-preserved flow for all backends, following the same pattern as BottleBackend. Each backend gets its own Freezer subclass: DockerFreezer — docker commit MacosContainerFreezer — container export + image rebuild; prompts to stop if the container is running SmolmachinesFreezer — smolvm pack create --from-vm The base class owns write_committed_image, mark_preserved, and the resume hint. Subclasses implement _freeze() and optionally override _export_hint() for migration instructions. Freezer.commit(agent, bottle) is the primary entry point for use within a live launch context. Freezer.commit_slug(slug) is a convenience wrapper for cmd_commit, which no longer branches on backend names itself. get_freezer(backend_name) is the factory, analogous to get_bottle_backend(). CommitCancelled is raised by MacosContainerFreezer when the user declines the stop prompt; cmd_commit catches it and returns 0. --- bot_bottle/backend/__init__.py | 8 + bot_bottle/backend/docker/freezer.py | 22 ++ bot_bottle/backend/freeze.py | 131 +++++++++ bot_bottle/backend/macos_container/freezer.py | 50 ++++ bot_bottle/backend/smolmachines/freezer.py | 25 ++ bot_bottle/cli/commit.py | 100 +------ tests/unit/test_backend_freezer.py | 256 ++++++++++++++++++ tests/unit/test_cli_commit.py | 219 +++------------ 8 files changed, 540 insertions(+), 271 deletions(-) create mode 100644 bot_bottle/backend/docker/freezer.py create mode 100644 bot_bottle/backend/freeze.py create mode 100644 bot_bottle/backend/macos_container/freezer.py create mode 100644 bot_bottle/backend/smolmachines/freezer.py create mode 100644 tests/unit/test_backend_freezer.py diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 36bbb37..09ca6f1 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -526,6 +526,11 @@ from .docker import DockerBottleBackend # noqa: E402 # pylint: disable=wrong-i from .macos_container import MacosContainerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position from .smolmachines import SmolmachinesBottleBackend # noqa: E402 # pylint: disable=wrong-import-position +# Freezer is imported after the backend classes for the same reason — +# Freezer.commit_slug constructs ActiveAgent, so the dataclass must be +# fully defined first. +from .freeze import CommitCancelled, Freezer, get_freezer # noqa: E402 # pylint: disable=wrong-import-position + # The dict is heterogeneous: each value is a BottleBackend specialized # over its own plan type. Concrete plan types are erased here because @@ -613,9 +618,12 @@ __all__ = [ "BottleCleanupPlan", "BottlePlan", "BottleSpec", + "CommitCancelled", "ExecResult", + "Freezer", "enumerate_active_agents", "get_bottle_backend", + "get_freezer", "has_backend", "known_backend_names", ] diff --git a/bot_bottle/backend/docker/freezer.py b/bot_bottle/backend/docker/freezer.py new file mode 100644 index 0000000..7d67ac6 --- /dev/null +++ b/bot_bottle/backend/docker/freezer.py @@ -0,0 +1,22 @@ +"""DockerFreezer — snapshot a Docker bottle via `docker commit`.""" + +from __future__ import annotations + +from .. import ActiveAgent, Bottle +from ..freeze import Freezer +from .util import commit_container +from ...log import info + + +class DockerFreezer(Freezer): + """Freezes a Docker bottle by running `docker commit`.""" + + backend_name = "docker" + + def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + image_tag = f"bot-bottle-committed-{agent.slug}:latest" + commit_container(bottle.name, image_tag) + return image_tag + + def _export_hint(self, slug: str, image_ref: str) -> None: + info(f"to export for migration: docker save {image_ref} -o {slug}.tar") diff --git a/bot_bottle/backend/freeze.py b/bot_bottle/backend/freeze.py new file mode 100644 index 0000000..12aa6ad --- /dev/null +++ b/bot_bottle/backend/freeze.py @@ -0,0 +1,131 @@ +"""Freezer — snapshot a running bottle to a resumable artifact. + +Follows the same pattern as BottleBackend: a shared base class with +common post-freeze steps (write committed-image path, mark preserved, +print resume hint) and backend-specific subclasses in their respective +backend directories. + +Entry points: + Freezer.commit(agent, bottle) — for use within a live launch context + Freezer.commit_slug(slug) — for cmd_commit when no live Bottle exists + get_freezer(backend_name) — factory +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod + +from . import ActiveAgent, Bottle, ExecResult +from ..bottle_state import mark_preserved, write_committed_image +from ..log import die, info + + +class CommitCancelled(Exception): + """Raised by Freezer._freeze when the user declines a confirmation prompt.""" + + +class Freezer(ABC): + """Freezes a running bottle to a resumable artifact. + + The base class owns the shared post-commit steps: + - write_committed_image — records the artifact path in per-bottle state + - mark_preserved — prevents teardown from removing the state dir + - resume hint — printed to stderr after the snapshot + + Subclasses implement _freeze with the backend-specific snapshot + operation and optionally override _export_hint for migration hints. + """ + + backend_name: str + + def commit(self, agent: ActiveAgent, bottle: Bottle) -> None: + """Freeze `bottle` to a resumable artifact. + + Calls _freeze for the backend-specific snapshot, then writes the + committed image reference to per-bottle state and marks the bottle + preserved so the next `./cli.py resume` boots from the snapshot. + + Raises CommitCancelled if the user declines an interactive + confirmation prompt (e.g. the macos-container stop prompt). + """ + image_ref = self._freeze(agent, bottle) + write_committed_image(agent.slug, image_ref) + mark_preserved(agent.slug) + info(f"to resume from this snapshot: ./cli.py resume {agent.slug}") + self._export_hint(agent.slug, image_ref) + + @abstractmethod + def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + """Backend-specific snapshot. Returns the image tag or artifact path + stored by write_committed_image. Raises CommitCancelled if the user + declines a stop-confirmation prompt.""" + + def _export_hint(self, slug: str, image_ref: str) -> None: + """Optionally print an export-for-migration hint after committing. + Overridden by backends that provide a meaningful export command.""" + + def commit_slug(self, slug: str) -> None: + """Convenience entry for cmd_commit when no live Bottle is available. + + Constructs a minimal ActiveAgent from per-bottle state and a + name-only Bottle stub, then delegates to commit(). Freezer + subclasses must not call exec / exec_agent / cp_in on the stub.""" + from ..bottle_state import read_metadata + metadata = read_metadata(slug) + agent = ActiveAgent( + backend_name=self.backend_name, + slug=slug, + agent_name=metadata.agent_name if metadata else "", + started_at=metadata.started_at if metadata else "", + services=(), + ) + bottle: Bottle = _NamedBottle(f"bot-bottle-{slug}") + self.commit(agent, bottle) + + +class _NamedBottle(Bottle): + """Name-only Bottle stub for Freezer.commit_slug. + + Only `name` is meaningful. All runtime operations raise + NotImplementedError — Freezer._freeze implementations must only + access bottle.name.""" + + def __init__(self, name: str) -> None: + self.name = name + + def agent_argv(self, argv: list[str], *, tty: bool = True) -> list[str]: + raise NotImplementedError + + def exec_agent(self, argv: list[str], *, tty: bool = True) -> int: + raise NotImplementedError + + def exec(self, script: str, *, user: str = "node") -> ExecResult: + raise NotImplementedError + + def cp_in(self, host_path: str, container_path: str) -> None: + raise NotImplementedError + + def close(self) -> None: + pass + + +def get_freezer(backend_name: str) -> Freezer: + """Return the Freezer for the named backend. + + backend_name "" is treated as "docker" for backward compatibility + with state dirs written before the backend field was added.""" + resolved = backend_name or "docker" + if resolved == "docker": + from .docker.freezer import DockerFreezer + return DockerFreezer() + if resolved == "macos-container": + from .macos_container.freezer import MacosContainerFreezer + return MacosContainerFreezer() + if resolved == "smolmachines": + from .smolmachines.freezer import SmolmachinesFreezer + return SmolmachinesFreezer() + die( + f"commit is only supported for docker, macos-container, and " + f"smolmachines; backend {backend_name!r} has no freezer" + ) + raise AssertionError("unreachable") diff --git a/bot_bottle/backend/macos_container/freezer.py b/bot_bottle/backend/macos_container/freezer.py new file mode 100644 index 0000000..f405fe2 --- /dev/null +++ b/bot_bottle/backend/macos_container/freezer.py @@ -0,0 +1,50 @@ +"""MacosContainerFreezer — snapshot a macOS container bottle. + +Apple's `container export` requires the container to be stopped first. +When the container is running the freezer prompts the user to confirm +the stop before proceeding.""" + +from __future__ import annotations + +import sys + +from .. import ActiveAgent, Bottle +from ..freeze import CommitCancelled, Freezer +from .util import commit_container, container_is_running, stop_container +from ...log import info + + +class MacosContainerFreezer(Freezer): + """Freezes a macOS-container bottle via `container export` + image rebuild.""" + + backend_name = "macos-container" + + def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + image_tag = f"bot-bottle-committed-{agent.slug}:latest" + if container_is_running(bottle.name): + sys.stderr.write( + f"bot-bottle: bottle {agent.slug!r} is running; " + "commit will stop it. Continue? [y/N] " + ) + sys.stderr.flush() + reply = _read_tty_line().strip().lower() + if reply not in ("y", "yes"): + raise CommitCancelled + stop_container(bottle.name) + commit_container(bottle.name, image_tag) + return image_tag + + def _export_hint(self, slug: str, image_ref: str) -> None: + info( + f"to export for migration: " + f"container image save {image_ref} -o {slug}.tar" + ) + + +def _read_tty_line() -> str: + """Read one line from /dev/tty, falling back to stdin.""" + try: + with open("/dev/tty", "r", encoding="utf-8") as tty: + return tty.readline().rstrip("\n") + except OSError: + return sys.stdin.readline().rstrip("\n") diff --git a/bot_bottle/backend/smolmachines/freezer.py b/bot_bottle/backend/smolmachines/freezer.py new file mode 100644 index 0000000..d165a21 --- /dev/null +++ b/bot_bottle/backend/smolmachines/freezer.py @@ -0,0 +1,25 @@ +"""SmolmachinesFreezer — snapshot a smolmachines bottle via smolvm pack.""" + +from __future__ import annotations + +from .. import ActiveAgent, Bottle +from ..freeze import Freezer +from .smolvm import pack_create_from_vm +from ...bottle_state import bottle_state_dir +from ...log import info + + +class SmolmachinesFreezer(Freezer): + """Freezes a smolmachines bottle via `smolvm pack create --from-vm`.""" + + backend_name = "smolmachines" + + def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + output = bottle_state_dir(agent.slug) / "committed-smolmachine" + output.parent.mkdir(parents=True, exist_ok=True) + pack_create_from_vm(bottle.name, output) + artifact = output.with_name(f"{output.name}.smolmachine") + return str(artifact) + + def _export_hint(self, slug: str, image_ref: str) -> None: + info(f"to export for migration: cp {image_ref} {slug}.smolmachine") diff --git a/bot_bottle/cli/commit.py b/bot_bottle/cli/commit.py index 32497a9..d724bcc 100644 --- a/bot_bottle/cli/commit.py +++ b/bot_bottle/cli/commit.py @@ -11,49 +11,15 @@ snapshot instead of rebuilding from the Dockerfile. from __future__ import annotations import argparse -import sys -from pathlib import Path from ..backend import enumerate_active_agents -from ..backend.docker.util import commit_container as docker_commit_container -from ..backend.macos_container.util import commit_container as macos_commit_container -from ..backend.macos_container.util import container_is_running as macos_container_is_running -from ..backend.macos_container.util import stop_container as macos_stop_container -from ..backend.smolmachines.smolvm import pack_create_from_vm -from ..bottle_state import bottle_state_dir -from ..bottle_state import mark_preserved, read_metadata, write_committed_image -from ..log import die, info -from ._common import PROG, read_tty_line +from ..backend.freeze import CommitCancelled, get_freezer +from ..bottle_state import read_metadata +from ..log import die +from ._common import PROG from . import tui -_COMMITTED_IMAGE_PREFIX = "bot-bottle-committed-" -_DOCKER_BACKENDS = {"docker", ""} -_MACOS_CONTAINER_BACKEND = "macos-container" -_SMOLMACHINES_BACKEND = "smolmachines" - - -def _committed_image_tag(slug: str) -> str: - return f"{_COMMITTED_IMAGE_PREFIX}{slug}:latest" - - -def _agent_container_name(slug: str) -> str: - return f"bot-bottle-{slug}" - - -def _agent_machine_name(slug: str) -> str: - return f"bot-bottle-{slug}" - - -def _committed_smolmachine_output(slug: str) -> Path: - return bottle_state_dir(slug) / "committed-smolmachine" - - -def _committed_smolmachine_artifact(slug: str) -> Path: - output = _committed_smolmachine_output(slug) - return output.with_name(f"{output.name}.smolmachine") - - def cmd_commit(argv: list[str]) -> int: parser = argparse.ArgumentParser(prog=f"{PROG} commit", add_help=True) parser.add_argument( @@ -79,59 +45,9 @@ def cmd_commit(argv: list[str]) -> int: metadata = read_metadata(slug) backend = metadata.backend if metadata else "" - if backend in _DOCKER_BACKENDS: - container = _agent_container_name(slug) - image_tag = _committed_image_tag(slug) - docker_commit_container(container, image_tag) - write_committed_image(slug, image_tag) - mark_preserved(slug) - info(f"to resume from this snapshot: ./cli.py resume {slug}") - info(f"to export for migration: docker save {image_tag} -o {slug}.tar") + try: + get_freezer(backend).commit_slug(slug) + except CommitCancelled: return 0 - - if backend == _MACOS_CONTAINER_BACKEND: - container = _agent_container_name(slug) - image_tag = _committed_image_tag(slug) - - if macos_container_is_running(container): - sys.stderr.write( - f"bot-bottle: bottle {slug!r} is running; " - "commit will stop it. Continue? [y/N] " - ) - sys.stderr.flush() - reply = read_tty_line().strip().lower() - if reply not in ("y", "yes"): - return 0 - macos_stop_container(container) - - macos_commit_container(container, image_tag) - write_committed_image(slug, image_tag) - mark_preserved(slug) - info(f"to resume from this snapshot: ./cli.py resume {slug}") - info( - f"to export for migration: " - f"container image save {image_tag} -o {slug}.tar" - ) - return 0 - - if backend == _SMOLMACHINES_BACKEND: - machine = _agent_machine_name(slug) - output = _committed_smolmachine_output(slug) - output.parent.mkdir(parents=True, exist_ok=True) - pack_create_from_vm(machine, output) - artifact = _committed_smolmachine_artifact(slug) - write_committed_image(slug, str(artifact)) - mark_preserved(slug) - info(f"to resume from this snapshot: ./cli.py resume {slug}") - info(f"to export for migration: cp {artifact} {slug}.smolmachine") - return 0 - - if backend: - die( - f"commit is only supported for docker, macos-container, and " - f"smolmachines; " - f"bottle {slug!r} uses {backend!r}" - ) - die(f"commit cannot determine the backend for bottle {slug!r}") - return 1 + return 0 diff --git a/tests/unit/test_backend_freezer.py b/tests/unit/test_backend_freezer.py new file mode 100644 index 0000000..29fd64e --- /dev/null +++ b/tests/unit/test_backend_freezer.py @@ -0,0 +1,256 @@ +"""Unit: Freezer class hierarchy.""" + +from __future__ import annotations + +import tempfile +import unittest +from pathlib import Path +from unittest.mock import MagicMock, call, patch + +from bot_bottle import supervise, bottle_state +from bot_bottle.backend import ActiveAgent +from bot_bottle.backend.freeze import ( + CommitCancelled, + Freezer, + _NamedBottle, + get_freezer, +) +from bot_bottle.backend.docker.freezer import DockerFreezer +from bot_bottle.backend.macos_container.freezer import MacosContainerFreezer +from bot_bottle.backend.smolmachines.freezer import SmolmachinesFreezer + + +class _FakeHomeMixin: + def _setup_fake_home(self): + self._tmp = tempfile.TemporaryDirectory(prefix="freezer-test.") + original = supervise.bot_bottle_root + + def fake_root() -> Path: + return Path(self._tmp.name) / ".bot-bottle" + + supervise.bot_bottle_root = fake_root # type: ignore[assignment] + self._restore = lambda: setattr(supervise, "bot_bottle_root", original) + + def _teardown_fake_home(self): + self._restore() + self._tmp.cleanup() + + +def _make_agent(slug: str, backend: str = "docker") -> ActiveAgent: + return ActiveAgent( + backend_name=backend, + slug=slug, + agent_name="dev", + started_at="t", + services=(), + ) + + +def _make_bottle(slug: str) -> _NamedBottle: + return _NamedBottle(f"bot-bottle-{slug}") + + +class TestGetFreezer(unittest.TestCase): + def test_docker(self): + self.assertIsInstance(get_freezer("docker"), DockerFreezer) + + def test_empty_backend_gives_docker(self): + self.assertIsInstance(get_freezer(""), DockerFreezer) + + def test_macos_container(self): + self.assertIsInstance(get_freezer("macos-container"), MacosContainerFreezer) + + def test_smolmachines(self): + self.assertIsInstance(get_freezer("smolmachines"), SmolmachinesFreezer) + + def test_unknown_backend_dies(self): + with patch("bot_bottle.backend.freeze.die", side_effect=SystemExit("die")): + with self.assertRaises(SystemExit): + get_freezer("unknown-backend") + + +class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase): + """The base Freezer.commit() owns the shared post-freeze steps.""" + + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_writes_committed_image_and_marks_preserved(self): + slug = "dev-abc12" + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="docker", + )) + freezer = get_freezer("docker") + agent = _make_agent(slug, "docker") + bottle = _make_bottle(slug) + + with patch.object(freezer, "_freeze", return_value="bot-bottle-committed-dev-abc12:latest"), \ + patch("bot_bottle.backend.freeze.info"): + freezer.commit(agent, bottle) + + self.assertEqual( + "bot-bottle-committed-dev-abc12:latest", + bottle_state.read_committed_image(slug), + ) + self.assertTrue(bottle_state.is_preserved(slug)) + + def test_commit_slug_builds_correct_bottle_name(self): + slug = "dev-abc12" + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="docker", + )) + freezer = get_freezer("docker") + captured = {} + + def capture_freeze(agent, bottle): + captured["bottle_name"] = bottle.name + return "some-ref" + + with patch.object(freezer, "_freeze", side_effect=capture_freeze), \ + patch("bot_bottle.backend.freeze.info"): + freezer.commit_slug(slug) + + self.assertEqual(f"bot-bottle-{slug}", captured["bottle_name"]) + + +class TestDockerFreezer(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_commits_container_and_returns_image_tag(self): + slug = "dev-abc12" + freezer = DockerFreezer() + agent = _make_agent(slug) + bottle = _make_bottle(slug) + + with patch("bot_bottle.backend.docker.freezer.commit_container") as mock_commit, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.docker.freezer.info"): + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="docker", + )) + freezer.commit(agent, bottle) + + mock_commit.assert_called_once_with( + f"bot-bottle-{slug}", + f"bot-bottle-committed-{slug}:latest", + ) + self.assertEqual( + f"bot-bottle-committed-{slug}:latest", + bottle_state.read_committed_image(slug), + ) + self.assertTrue(bottle_state.is_preserved(slug)) + + +class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def _write_meta(self, slug: str) -> None: + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="macos-container", + )) + + def test_commits_stopped_container(self): + slug = "dev-abc12" + self._write_meta(slug) + freezer = MacosContainerFreezer() + agent = _make_agent(slug, "macos-container") + bottle = _make_bottle(slug) + + with patch("bot_bottle.backend.macos_container.freezer.container_is_running", + return_value=False), \ + patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.macos_container.freezer.info"): + freezer.commit(agent, bottle) + + mock_commit.assert_called_once_with( + f"bot-bottle-{slug}", + f"bot-bottle-committed-{slug}:latest", + ) + + def test_stops_running_container_on_yes(self): + slug = "dev-abc12" + self._write_meta(slug) + freezer = MacosContainerFreezer() + agent = _make_agent(slug, "macos-container") + bottle = _make_bottle(slug) + + with patch("bot_bottle.backend.macos_container.freezer.container_is_running", + return_value=True), \ + patch("bot_bottle.backend.macos_container.freezer._read_tty_line", + return_value="y"), \ + patch("bot_bottle.backend.macos_container.freezer.stop_container") as mock_stop, \ + patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.macos_container.freezer.info"): + freezer.commit(agent, bottle) + + mock_stop.assert_called_once_with(f"bot-bottle-{slug}") + mock_commit.assert_called_once() + + def test_raises_commit_cancelled_on_no(self): + slug = "dev-abc12" + self._write_meta(slug) + freezer = MacosContainerFreezer() + agent = _make_agent(slug, "macos-container") + bottle = _make_bottle(slug) + + with patch("bot_bottle.backend.macos_container.freezer.container_is_running", + return_value=True), \ + patch("bot_bottle.backend.macos_container.freezer._read_tty_line", + return_value="n"), \ + patch("bot_bottle.backend.macos_container.freezer.stop_container") as mock_stop, \ + patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit: + with self.assertRaises(CommitCancelled): + freezer.commit(agent, bottle) + + mock_stop.assert_not_called() + mock_commit.assert_not_called() + + +class TestSmolmachinesFreezer(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_packs_vm_and_records_artifact(self): + slug = "dev-abc12" + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="smolmachines", + )) + freezer = SmolmachinesFreezer() + agent = _make_agent(slug, "smolmachines") + bottle = _make_bottle(slug) + + with patch("bot_bottle.backend.smolmachines.freezer.pack_create_from_vm") as mock_pack, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.smolmachines.freezer.info"): + freezer.commit(agent, bottle) + + expected_output = bottle_state.bottle_state_dir(slug) / "committed-smolmachine" + mock_pack.assert_called_once_with(f"bot-bottle-{slug}", expected_output) + expected_artifact = str(expected_output.with_name("committed-smolmachine.smolmachine")) + self.assertEqual(expected_artifact, bottle_state.read_committed_image(slug)) + self.assertTrue(bottle_state.is_preserved(slug)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_cli_commit.py b/tests/unit/test_cli_commit.py index f2c9a6b..c88cc9a 100644 --- a/tests/unit/test_cli_commit.py +++ b/tests/unit/test_cli_commit.py @@ -7,15 +7,10 @@ import unittest from pathlib import Path from unittest.mock import MagicMock, patch -from bot_bottle.cli.commit import ( - cmd_commit, - _agent_container_name, - _committed_image_tag, - _committed_smolmachine_artifact, - _committed_smolmachine_output, -) +from bot_bottle.cli.commit import cmd_commit from bot_bottle import supervise from bot_bottle import bottle_state +from bot_bottle.backend.freeze import CommitCancelled class _FakeHomeMixin: @@ -34,32 +29,8 @@ class _FakeHomeMixin: self._tmp.cleanup() -class TestCommitHelpers(unittest.TestCase): - def test_committed_image_tag(self): - self.assertEqual( - "bot-bottle-committed-dev-abc12:latest", - _committed_image_tag("dev-abc12"), - ) - - def test_agent_container_name(self): - self.assertEqual( - "bot-bottle-dev-abc12", - _agent_container_name("dev-abc12"), - ) - - def test_committed_smolmachine_paths(self): - output = _committed_smolmachine_output("dev-abc12") - artifact = _committed_smolmachine_artifact("dev-abc12") - self.assertTrue(str(output).endswith( - "/.bot-bottle/state/dev-abc12/committed-smolmachine" - )) - self.assertTrue(str(artifact).endswith( - "/.bot-bottle/state/dev-abc12/committed-smolmachine.smolmachine" - )) - - class TestCmdCommitSlugArg(_FakeHomeMixin, unittest.TestCase): - """cmd_commit with an explicit slug bypasses the TUI picker.""" + """cmd_commit with an explicit slug delegates to get_freezer.""" def setUp(self): self._setup_fake_home() @@ -67,184 +38,74 @@ class TestCmdCommitSlugArg(_FakeHomeMixin, unittest.TestCase): def tearDown(self): self._teardown_fake_home() + def _write_meta(self, slug: str, backend: str) -> None: + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend=backend, + )) + def test_commits_docker_bottle(self): slug = "dev-abc12" - # Write metadata saying this is a docker bottle. - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="docker", - )) + self._write_meta(slug, "docker") - with patch( - "bot_bottle.cli.commit.docker_commit_container", - ) as mock_commit, patch( - "bot_bottle.cli.commit.info", - ): + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer rc = cmd_commit([slug]) self.assertEqual(0, rc) - mock_commit.assert_called_once_with( - f"bot-bottle-{slug}", - f"bot-bottle-committed-{slug}:latest", - ) + mock_gf.assert_called_once_with("docker") + mock_freezer.commit_slug.assert_called_once_with(slug) - def test_writes_committed_image_to_state(self): + def test_empty_backend_passed_to_get_freezer(self): + """Old state dirs without a backend field pass '' to get_freezer.""" slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="docker", - )) + self._write_meta(slug, "") - with patch("bot_bottle.cli.commit.docker_commit_container"), \ - patch("bot_bottle.cli.commit.info"): - cmd_commit([slug]) - - self.assertEqual( - f"bot-bottle-committed-{slug}:latest", - bottle_state.read_committed_image(slug), - ) - - def test_marks_bottle_preserved(self): - slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="docker", - )) - - with patch("bot_bottle.cli.commit.docker_commit_container"), \ - patch("bot_bottle.cli.commit.info"): - cmd_commit([slug]) - - self.assertTrue(bottle_state.is_preserved(slug)) - - def test_empty_backend_treated_as_docker(self): - """Old state dirs without a backend field should be treated as docker.""" - slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="", - )) - - with patch("bot_bottle.cli.commit.docker_commit_container") as mock_commit, \ - patch("bot_bottle.cli.commit.info"): + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer rc = cmd_commit([slug]) self.assertEqual(0, rc) - mock_commit.assert_called_once() + mock_gf.assert_called_once_with("") def test_commits_macos_container_bottle(self): slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="macos-container", - )) + self._write_meta(slug, "macos-container") - with patch( - "bot_bottle.cli.commit.macos_commit_container", - ) as mock_commit, patch( - "bot_bottle.cli.commit.info", - ), patch( - "bot_bottle.cli.commit.macos_container_is_running", return_value=False, - ): + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer rc = cmd_commit([slug]) self.assertEqual(0, rc) - mock_commit.assert_called_once_with( - f"bot-bottle-{slug}", - f"bot-bottle-committed-{slug}:latest", - ) + mock_gf.assert_called_once_with("macos-container") + mock_freezer.commit_slug.assert_called_once_with(slug) - def test_running_macos_container_stops_then_commits_on_yes(self): + def test_commits_smolmachines_bottle(self): slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="macos-container", - )) + self._write_meta(slug, "smolmachines") - with patch( - "bot_bottle.cli.commit.macos_container_is_running", return_value=True, - ), patch( - "bot_bottle.cli.commit.read_tty_line", return_value="y", - ), patch( - "bot_bottle.cli.commit.macos_stop_container", - ) as mock_stop, patch( - "bot_bottle.cli.commit.macos_commit_container", - ) as mock_commit, patch( - "bot_bottle.cli.commit.info", - ): + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer rc = cmd_commit([slug]) self.assertEqual(0, rc) - mock_stop.assert_called_once_with(f"bot-bottle-{slug}") - mock_commit.assert_called_once_with( - f"bot-bottle-{slug}", - f"bot-bottle-committed-{slug}:latest", - ) + mock_gf.assert_called_once_with("smolmachines") - def test_running_macos_container_aborts_on_no(self): + def test_returns_zero_on_commit_cancelled(self): slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="macos-container", - )) + self._write_meta(slug, "macos-container") - with patch( - "bot_bottle.cli.commit.macos_container_is_running", return_value=True, - ), patch( - "bot_bottle.cli.commit.read_tty_line", return_value="n", - ), patch( - "bot_bottle.cli.commit.macos_stop_container", - ) as mock_stop, patch( - "bot_bottle.cli.commit.macos_commit_container", - ) as mock_commit, patch( - "bot_bottle.cli.commit.info", - ): + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_freezer.commit_slug.side_effect = CommitCancelled + mock_gf.return_value = mock_freezer rc = cmd_commit([slug]) self.assertEqual(0, rc) - mock_stop.assert_not_called() - mock_commit.assert_not_called() - - -class TestCmdCommitSmolmachinesBackend(_FakeHomeMixin, unittest.TestCase): - def setUp(self): - self._setup_fake_home() - - def tearDown(self): - self._teardown_fake_home() - - def test_packs_smolmachines_bottle(self): - slug = "dev-abc12" - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="smolmachines", - )) - - with patch( - "bot_bottle.cli.commit.pack_create_from_vm", - ) as mock_pack, patch( - "bot_bottle.cli.commit.info", - ): - rc = cmd_commit([slug]) - - self.assertEqual(0, rc) - mock_pack.assert_called_once_with( - f"bot-bottle-{slug}", - _committed_smolmachine_output(slug), - ) - self.assertEqual( - str(_committed_smolmachine_artifact(slug)), - bottle_state.read_committed_image(slug), - ) - self.assertTrue(bottle_state.is_preserved(slug)) - - -class TestCmdCommitUnsupportedBackend(_FakeHomeMixin, unittest.TestCase): - def setUp(self): - self._setup_fake_home() - - def tearDown(self): - self._teardown_fake_home() class TestCmdCommitNoActiveBottles(_FakeHomeMixin, unittest.TestCase):