From 67a396a2cf3b0c27f6a8742b543f4c0087d3f6df Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Fri, 8 Nov 2024 09:37:48 -0800 Subject: [PATCH 1/6] feat(purge): Adding state clear functionality to purge --- devservices/commands/purge.py | 23 +++++- devservices/utils/console.py | 6 ++ devservices/utils/docker.py | 17 +++++ devservices/utils/state.py | 9 +++ tests/commands/test_purge.py | 128 +++++++++++++++++++++++++++++++--- 5 files changed, 173 insertions(+), 10 deletions(-) diff --git a/devservices/commands/purge.py b/devservices/commands/purge.py index 8a5053d..b732dc9 100644 --- a/devservices/commands/purge.py +++ b/devservices/commands/purge.py @@ -8,6 +8,9 @@ from devservices.constants import DEVSERVICES_CACHE_DIR from devservices.utils.console import Console +from devservices.utils.console import Status +from devservices.utils.docker import stop_all_running_containers +from devservices.utils.state import State def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: @@ -15,13 +18,29 @@ def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: parser.set_defaults(func=purge) -def purge(args: Namespace) -> None: +def purge(_args: Namespace) -> None: """Purge the local devservices cache.""" console = Console() + # Prompt the user to stop all running containers + should_stop_containers = console.confirm( + "Warning: Purging stops all running containers and clears devservices state. Would you like to continue?" + ) + if not should_stop_containers: + console.warning("Purge canceled") + return + if os.path.exists(DEVSERVICES_CACHE_DIR): try: shutil.rmtree(DEVSERVICES_CACHE_DIR) except PermissionError as e: console.failure(f"Failed to purge cache: {e}") exit(1) - console.success("The local devservices cache has been purged") + state = State() + state.clear_state() + with Status( + lambda: console.warning("Stopping all running containers"), + lambda: console.success("All running containers stopped"), + ): + stop_all_running_containers() + + console.success("The local devservices cache and state has been purged") diff --git a/devservices/utils/console.py b/devservices/utils/console.py index ba6ed87..226afeb 100644 --- a/devservices/utils/console.py +++ b/devservices/utils/console.py @@ -14,6 +14,7 @@ class Color: RED = "\033[0;31m" GREEN = "\033[0;32m" YELLOW = "\033[0;33m" + BLUE = "\033[0;34m" BOLD = "\033[1m" UNDERLINE = "\033[4m" NEGATIVE = "\033[7m" @@ -46,6 +47,11 @@ def warning(self, message: str, bold: bool = False) -> None: def info(self, message: str, bold: bool = False) -> None: self.print(message=message, color="", bold=bold) + def confirm(self, message: str) -> bool: + self.warning(message=message, bold=True) + response = input("(Y/n): ").strip().lower() + return response in ("y", "yes", "") + class Status: """Shows loading status in the terminal.""" diff --git a/devservices/utils/docker.py b/devservices/utils/docker.py index 6f96e9d..a1311ec 100644 --- a/devservices/utils/docker.py +++ b/devservices/utils/docker.py @@ -17,3 +17,20 @@ def check_docker_daemon_running() -> None: raise DockerDaemonNotRunningError( "Unable to connect to the docker daemon. Is the docker daemon running?" ) from e + + +def stop_all_running_containers() -> None: + running_containers = ( + subprocess.check_output(["docker", "ps", "-q"], stderr=subprocess.DEVNULL) + .decode() + .strip() + .splitlines() + ) + if len(running_containers) == 0: + return + subprocess.run( + ["docker", "stop"] + running_containers, + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) diff --git a/devservices/utils/state.py b/devservices/utils/state.py index cb41f0c..16670a1 100644 --- a/devservices/utils/state.py +++ b/devservices/utils/state.py @@ -79,3 +79,12 @@ def get_mode_for_service(self, service_name: str) -> str | None: if result is None: return None return str(result[0]) + + def clear_state(self) -> None: + cursor = self.conn.cursor() + cursor.execute( + """ + DELETE FROM started_services + """ + ) + self.conn.commit() diff --git a/tests/commands/test_purge.py b/tests/commands/test_purge.py index 04ed5c3..c28ee05 100644 --- a/tests/commands/test_purge.py +++ b/tests/commands/test_purge.py @@ -1,35 +1,147 @@ from __future__ import annotations +import builtins from argparse import Namespace from pathlib import Path from unittest import mock from devservices.commands.purge import purge +from devservices.utils.state import State -def test_purge_no_cache(tmp_path: Path) -> None: - with mock.patch( - "devservices.commands.purge.DEVSERVICES_CACHE_DIR", - str(tmp_path / ".devservices-cache"), +def fake_yes_input(_: str) -> str: + return "yes" + + +def fake_no_input(_: str) -> str: + return "no" + + +def test_purge_not_confirmed(tmp_path: Path) -> None: + with ( + mock.patch( + "devservices.commands.purge.DEVSERVICES_CACHE_DIR", + str(tmp_path / ".devservices-cache"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + mock.patch.object(builtins, "input", fake_no_input), ): args = Namespace() purge(args) -def test_purge_with_cache(tmp_path: Path) -> None: - with mock.patch( - "devservices.commands.purge.DEVSERVICES_CACHE_DIR", - str(tmp_path / ".devservices-cache"), +@mock.patch("devservices.utils.docker.subprocess.run") +@mock.patch("devservices.utils.docker.subprocess.check_output") +def test_purge_with_cache_and_state_and_no_running_containers_confirmed( + mock_check_output: mock.Mock, mock_run: mock.Mock, tmp_path: Path +) -> None: + with ( + mock.patch( + "devservices.commands.purge.DEVSERVICES_CACHE_DIR", + str(tmp_path / ".devservices-cache"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + mock.patch.object(builtins, "input", fake_yes_input), + ): + # Mock return value for "docker ps -q" + mock_check_output.return_value = b"" + # Create a cache file to test purging + cache_dir = tmp_path / ".devservices-cache" + cache_dir.mkdir(parents=True, exist_ok=True) + cache_file = tmp_path / ".devservices-cache" / "test.txt" + cache_file.write_text("This is a test cache file.") + + state = State() + state.add_started_service("test-service", "test-mode") + + assert cache_file.exists() + assert state.get_started_services() == ["test-service"] + + args = Namespace() + purge(args) + + assert not cache_file.exists() + assert state.get_started_services() == [] + + mock_check_output.assert_called_once_with( + ["docker", "ps", "-q"], stderr=mock.ANY + ) + mock_run.assert_not_called() + + +@mock.patch("devservices.utils.docker.subprocess.run") +@mock.patch("devservices.utils.docker.subprocess.check_output") +def test_purge_with_cache_and_state_and_running_containers_confirmed( + mock_check_output: mock.Mock, mock_run: mock.Mock, tmp_path: Path +) -> None: + with ( + mock.patch( + "devservices.commands.purge.DEVSERVICES_CACHE_DIR", + str(tmp_path / ".devservices-cache"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + mock.patch.object(builtins, "input", fake_yes_input), ): + # Mock return value for "docker ps -q" + mock_check_output.return_value = b"container_id" # Create a cache file to test purging cache_dir = tmp_path / ".devservices-cache" cache_dir.mkdir(parents=True, exist_ok=True) cache_file = tmp_path / ".devservices-cache" / "test.txt" cache_file.write_text("This is a test cache file.") + state = State() + state.add_started_service("test-service", "test-mode") + assert cache_file.exists() + assert state.get_started_services() == ["test-service"] args = Namespace() purge(args) assert not cache_file.exists() + assert state.get_started_services() == [] + + mock_check_output.assert_called_once_with( + ["docker", "ps", "-q"], stderr=mock.ANY + ) + mock_run.assert_called_once_with( + ["docker", "stop", "container_id"], + check=True, + stdout=mock.ANY, + stderr=mock.ANY, + ) + + +@mock.patch("devservices.utils.docker.subprocess.run") +@mock.patch("devservices.utils.docker.subprocess.check_output") +def test_purge_with_cache_and_state_and_running_containers_not_confirmed( + mock_check_output: mock.Mock, mock_run: mock.Mock, tmp_path: Path +) -> None: + with ( + mock.patch( + "devservices.commands.purge.DEVSERVICES_CACHE_DIR", + str(tmp_path / ".devservices-cache"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + mock.patch.object(builtins, "input", fake_no_input), + ): + # Mock return value for "docker ps -q" + mock_check_output.return_value = b"container_id" + # Create a cache file to test purging + cache_dir = tmp_path / ".devservices-cache" + cache_dir.mkdir(parents=True, exist_ok=True) + cache_file = tmp_path / ".devservices-cache" / "test.txt" + cache_file.write_text("This is a test cache file.") + + state = State() + state.add_started_service("test-service", "test-mode") + + args = Namespace() + purge(args) + + assert cache_file.exists() + assert state.get_started_services() == ["test-service"] + + mock_check_output.assert_not_called() + mock_run.assert_not_called() From 36669122bf73c84693d0e4b9493cc16910109ec2 Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Fri, 8 Nov 2024 16:16:33 -0800 Subject: [PATCH 2/6] Adding docker daemon check for purge --- devservices/commands/purge.py | 8 +++-- devservices/exceptions.py | 4 ++- devservices/utils/docker.py | 6 ++-- tests/commands/test_purge.py | 55 +++++++++++++++-------------------- 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/devservices/commands/purge.py b/devservices/commands/purge.py index b732dc9..9303432 100644 --- a/devservices/commands/purge.py +++ b/devservices/commands/purge.py @@ -7,6 +7,7 @@ from argparse import Namespace from devservices.constants import DEVSERVICES_CACHE_DIR +from devservices.exceptions import DockerDaemonNotRunningError from devservices.utils.console import Console from devservices.utils.console import Status from devservices.utils.docker import stop_all_running_containers @@ -39,8 +40,11 @@ def purge(_args: Namespace) -> None: state.clear_state() with Status( lambda: console.warning("Stopping all running containers"), - lambda: console.success("All running containers stopped"), + lambda: console.success("All running containers have been stopped"), ): - stop_all_running_containers() + try: + stop_all_running_containers() + except DockerDaemonNotRunningError: + console.warning("The docker daemon not running, no containers to stop") console.success("The local devservices cache and state has been purged") diff --git a/devservices/exceptions.py b/devservices/exceptions.py index b67aa50..f5a372f 100644 --- a/devservices/exceptions.py +++ b/devservices/exceptions.py @@ -46,7 +46,9 @@ class DevservicesUpdateError(BinaryInstallError): class DockerDaemonNotRunningError(Exception): """Raised when the Docker daemon is not running.""" - pass + def __str__(self) -> str: + # TODO: Provide explicit instructions on what to do + return "Unable to connect to the docker daemon. Is the docker daemon running?" class DockerComposeInstallationError(BinaryInstallError): diff --git a/devservices/utils/docker.py b/devservices/utils/docker.py index a1311ec..a1dd765 100644 --- a/devservices/utils/docker.py +++ b/devservices/utils/docker.py @@ -6,6 +6,7 @@ def check_docker_daemon_running() -> None: + """Checks if the Docker daemon is running. Raises DockerDaemonNotRunningError if not.""" try: subprocess.run( ["docker", "info"], @@ -14,12 +15,11 @@ def check_docker_daemon_running() -> None: check=True, ) except subprocess.CalledProcessError as e: - raise DockerDaemonNotRunningError( - "Unable to connect to the docker daemon. Is the docker daemon running?" - ) from e + raise DockerDaemonNotRunningError from e def stop_all_running_containers() -> None: + check_docker_daemon_running() running_containers = ( subprocess.check_output(["docker", "ps", "-q"], stderr=subprocess.DEVNULL) .decode() diff --git a/tests/commands/test_purge.py b/tests/commands/test_purge.py index c28ee05..809e03b 100644 --- a/tests/commands/test_purge.py +++ b/tests/commands/test_purge.py @@ -17,7 +17,10 @@ def fake_no_input(_: str) -> str: return "no" -def test_purge_not_confirmed(tmp_path: Path) -> None: +@mock.patch("devservices.commands.purge.stop_all_running_containers") +def test_purge_not_confirmed( + mock_stop_all_running_containers: mock.Mock, tmp_path: Path +) -> None: with ( mock.patch( "devservices.commands.purge.DEVSERVICES_CACHE_DIR", @@ -29,11 +32,12 @@ def test_purge_not_confirmed(tmp_path: Path) -> None: args = Namespace() purge(args) + mock_stop_all_running_containers.assert_not_called() + -@mock.patch("devservices.utils.docker.subprocess.run") -@mock.patch("devservices.utils.docker.subprocess.check_output") +@mock.patch("devservices.commands.purge.stop_all_running_containers") def test_purge_with_cache_and_state_and_no_running_containers_confirmed( - mock_check_output: mock.Mock, mock_run: mock.Mock, tmp_path: Path + mock_stop_all_running_containers: mock.Mock, tmp_path: Path ) -> None: with ( mock.patch( @@ -42,9 +46,10 @@ def test_purge_with_cache_and_state_and_no_running_containers_confirmed( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), mock.patch.object(builtins, "input", fake_yes_input), + mock.patch( + "devservices.utils.docker.check_docker_daemon_running", return_value=None + ), ): - # Mock return value for "docker ps -q" - mock_check_output.return_value = b"" # Create a cache file to test purging cache_dir = tmp_path / ".devservices-cache" cache_dir.mkdir(parents=True, exist_ok=True) @@ -63,16 +68,12 @@ def test_purge_with_cache_and_state_and_no_running_containers_confirmed( assert not cache_file.exists() assert state.get_started_services() == [] - mock_check_output.assert_called_once_with( - ["docker", "ps", "-q"], stderr=mock.ANY - ) - mock_run.assert_not_called() + mock_stop_all_running_containers.assert_called_once() -@mock.patch("devservices.utils.docker.subprocess.run") -@mock.patch("devservices.utils.docker.subprocess.check_output") +@mock.patch("devservices.commands.purge.stop_all_running_containers") def test_purge_with_cache_and_state_and_running_containers_confirmed( - mock_check_output: mock.Mock, mock_run: mock.Mock, tmp_path: Path + mock_stop_all_running_containers: mock.Mock, tmp_path: Path ) -> None: with ( mock.patch( @@ -81,9 +82,10 @@ def test_purge_with_cache_and_state_and_running_containers_confirmed( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), mock.patch.object(builtins, "input", fake_yes_input), + mock.patch( + "devservices.utils.docker.check_docker_daemon_running", return_value=None + ), ): - # Mock return value for "docker ps -q" - mock_check_output.return_value = b"container_id" # Create a cache file to test purging cache_dir = tmp_path / ".devservices-cache" cache_dir.mkdir(parents=True, exist_ok=True) @@ -102,21 +104,12 @@ def test_purge_with_cache_and_state_and_running_containers_confirmed( assert not cache_file.exists() assert state.get_started_services() == [] - mock_check_output.assert_called_once_with( - ["docker", "ps", "-q"], stderr=mock.ANY - ) - mock_run.assert_called_once_with( - ["docker", "stop", "container_id"], - check=True, - stdout=mock.ANY, - stderr=mock.ANY, - ) + mock_stop_all_running_containers.assert_called_once() -@mock.patch("devservices.utils.docker.subprocess.run") -@mock.patch("devservices.utils.docker.subprocess.check_output") +@mock.patch("devservices.commands.purge.stop_all_running_containers") def test_purge_with_cache_and_state_and_running_containers_not_confirmed( - mock_check_output: mock.Mock, mock_run: mock.Mock, tmp_path: Path + mock_stop_all_running_containers: mock.Mock, tmp_path: Path ) -> None: with ( mock.patch( @@ -125,9 +118,10 @@ def test_purge_with_cache_and_state_and_running_containers_not_confirmed( ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), mock.patch.object(builtins, "input", fake_no_input), + mock.patch( + "devservices.utils.docker.check_docker_daemon_running", return_value=None + ), ): - # Mock return value for "docker ps -q" - mock_check_output.return_value = b"container_id" # Create a cache file to test purging cache_dir = tmp_path / ".devservices-cache" cache_dir.mkdir(parents=True, exist_ok=True) @@ -143,5 +137,4 @@ def test_purge_with_cache_and_state_and_running_containers_not_confirmed( assert cache_file.exists() assert state.get_started_services() == ["test-service"] - mock_check_output.assert_not_called() - mock_run.assert_not_called() + mock_stop_all_running_containers.assert_not_called() From 2aa294e8f64e134005a456e1afd4cfd4cc145ee2 Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Mon, 11 Nov 2024 11:55:20 -0800 Subject: [PATCH 3/6] Adding docker tests --- tests/utils/test_docker.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/utils/test_docker.py diff --git a/tests/utils/test_docker.py b/tests/utils/test_docker.py new file mode 100644 index 0000000..9dade4b --- /dev/null +++ b/tests/utils/test_docker.py @@ -0,0 +1,29 @@ +from __future__ import annotations + +import subprocess +from unittest import mock + +from devservices.utils.docker import stop_all_running_containers + + +@mock.patch("subprocess.check_output") +@mock.patch("subprocess.run") +@mock.patch("devservices.utils.docker.check_docker_daemon_running") +def test_stop_all_running_containers( + mock_check_docker_daemon_running: mock.Mock, + mock_run: mock.Mock, + mock_check_output: mock.Mock, +) -> None: + mock_check_docker_daemon_running.return_value = None + mock_check_output.return_value = b"container1\ncontainer2\n" + stop_all_running_containers() + mock_check_docker_daemon_running.assert_called_once() + mock_check_output.assert_called_once_with( + ["docker", "ps", "-q"], stderr=subprocess.DEVNULL + ) + mock_run.assert_called_once_with( + ["docker", "stop", "container1", "container2"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) From 0118ea589a5dc45c3c65e140eceaf0b898c18ec3 Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Mon, 11 Nov 2024 14:16:48 -0800 Subject: [PATCH 4/6] Adding a test with no running containers --- tests/utils/test_docker.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/utils/test_docker.py b/tests/utils/test_docker.py index 9dade4b..1553de9 100644 --- a/tests/utils/test_docker.py +++ b/tests/utils/test_docker.py @@ -6,6 +6,24 @@ from devservices.utils.docker import stop_all_running_containers +@mock.patch("subprocess.check_output") +@mock.patch("subprocess.run") +@mock.patch("devservices.utils.docker.check_docker_daemon_running") +def test_stop_all_running_containers_none_running( + mock_check_docker_daemon_running: mock.Mock, + mock_run: mock.Mock, + mock_check_output: mock.Mock, +) -> None: + mock_check_docker_daemon_running.return_value = None + mock_check_output.return_value = b"" + stop_all_running_containers() + mock_check_docker_daemon_running.assert_called_once() + mock_check_output.assert_called_once_with( + ["docker", "ps", "-q"], stderr=subprocess.DEVNULL + ) + mock_run.assert_not_called() + + @mock.patch("subprocess.check_output") @mock.patch("subprocess.run") @mock.patch("devservices.utils.docker.check_docker_daemon_running") From a2f53a1f36ad85ad5274f1a074271a1320bf420f Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Mon, 11 Nov 2024 14:17:43 -0800 Subject: [PATCH 5/6] Addressing review feedback --- devservices/commands/purge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devservices/commands/purge.py b/devservices/commands/purge.py index 9303432..2a51da9 100644 --- a/devservices/commands/purge.py +++ b/devservices/commands/purge.py @@ -19,7 +19,7 @@ def add_parser(subparsers: _SubParsersAction[ArgumentParser]) -> None: parser.set_defaults(func=purge) -def purge(_args: Namespace) -> None: +def purge(args: Namespace) -> None: """Purge the local devservices cache.""" console = Console() # Prompt the user to stop all running containers From e0ed7db6aae30edb6ddcf5abd435928e8529e95c Mon Sep 17 00:00:00 2001 From: Ian Woodard <17186604+IanWoodard@users.noreply.github.com> Date: Mon, 11 Nov 2024 14:21:53 -0800 Subject: [PATCH 6/6] Addressing review feedback --- tests/commands/test_purge.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/commands/test_purge.py b/tests/commands/test_purge.py index 809e03b..99f4313 100644 --- a/tests/commands/test_purge.py +++ b/tests/commands/test_purge.py @@ -9,14 +9,6 @@ from devservices.utils.state import State -def fake_yes_input(_: str) -> str: - return "yes" - - -def fake_no_input(_: str) -> str: - return "no" - - @mock.patch("devservices.commands.purge.stop_all_running_containers") def test_purge_not_confirmed( mock_stop_all_running_containers: mock.Mock, tmp_path: Path @@ -27,7 +19,7 @@ def test_purge_not_confirmed( str(tmp_path / ".devservices-cache"), ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), - mock.patch.object(builtins, "input", fake_no_input), + mock.patch.object(builtins, "input", lambda _: "no"), ): args = Namespace() purge(args) @@ -45,7 +37,7 @@ def test_purge_with_cache_and_state_and_no_running_containers_confirmed( str(tmp_path / ".devservices-cache"), ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), - mock.patch.object(builtins, "input", fake_yes_input), + mock.patch.object(builtins, "input", lambda _: "yes"), mock.patch( "devservices.utils.docker.check_docker_daemon_running", return_value=None ), @@ -81,7 +73,7 @@ def test_purge_with_cache_and_state_and_running_containers_confirmed( str(tmp_path / ".devservices-cache"), ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), - mock.patch.object(builtins, "input", fake_yes_input), + mock.patch.object(builtins, "input", lambda _: "yes"), mock.patch( "devservices.utils.docker.check_docker_daemon_running", return_value=None ), @@ -117,7 +109,7 @@ def test_purge_with_cache_and_state_and_running_containers_not_confirmed( str(tmp_path / ".devservices-cache"), ), mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), - mock.patch.object(builtins, "input", fake_no_input), + mock.patch.object(builtins, "input", lambda _: "no"), mock.patch( "devservices.utils.docker.check_docker_daemon_running", return_value=None ),