From fafd3da1badbe2cb8e2a84c58f58dee433c02610 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Fri, 18 Apr 2025 13:25:33 -0700 Subject: [PATCH 1/6] foreground programs --- devservices/utils/supervisor.py | 12 +++++++++ tests/utils/test_supervisor.py | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 579111d..9e1622a 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -132,3 +132,15 @@ def stop_program(self, program_name: str) -> None: raise SupervisorProcessError( f"Failed to stop program {program_name}: {e.faultString}" ) + + def foreground_program(self, program_name: str) -> None: + try: + subprocess.run( + ["supervisorctl", "-c", self.config_file_path, "fg", program_name], + check=True, + stderr=subprocess.DEVNULL, + ) + except subprocess.CalledProcessError as e: + raise SupervisorError(f"Failed to foreground {program_name}: {str(e)}") + except KeyboardInterrupt: + pass diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index 28db186..41d0706 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -187,3 +187,49 @@ def test_extend_config_file( """ ) + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +def test_foreground_program_success( + mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + supervisor_manager.foreground_program("test_program") + mock_subprocess_run.assert_called_once_with( + [ + "supervisorctl", + "-c", + supervisor_manager.config_file_path, + "fg", + "test_program", + ], + check=True, + stderr=subprocess.DEVNULL, + ) + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +def test_foreground_program_failure( + mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_subprocess_run.side_effect = subprocess.CalledProcessError(1, "supervisorctl") + with pytest.raises(SupervisorError, match="Failed to foreground test_program"): + supervisor_manager.foreground_program("test_program") + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +def test_foreground_program_keyboard_interrupt( + mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_subprocess_run.side_effect = KeyboardInterrupt() + supervisor_manager.foreground_program("test_program") + mock_subprocess_run.assert_called_once_with( + [ + "supervisorctl", + "-c", + supervisor_manager.config_file_path, + "fg", + "test_program", + ], + check=True, + stderr=subprocess.DEVNULL, + ) From 388affa598edd617d1cf54231e64aba0139a9dd6 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Fri, 18 Apr 2025 14:46:16 -0700 Subject: [PATCH 2/6] tail logs instead --- devservices/utils/supervisor.py | 44 ++++++++++++++-- tests/utils/test_supervisor.py | 90 ++++++++++++++++++++++++++++----- 2 files changed, 117 insertions(+), 17 deletions(-) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 9e1622a..c752771 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -12,6 +12,11 @@ from devservices.exceptions import SupervisorConnectionError from devservices.exceptions import SupervisorError from devservices.exceptions import SupervisorProcessError +from devservices.utils.console import Console + +# Supervisor process state 20 is RUNNING +# https://supervisord.org/subprocess.html#process-states +PROGRAM_RUNNING_STATE = 20 class UnixSocketHTTPConnection(http.client.HTTPConnection): @@ -101,6 +106,21 @@ def _get_rpc_client(self) -> xmlrpc.client.ServerProxy: f"Failed to connect to supervisor XML-RPC server: {e.errmsg}" ) + def _check_program_running(self, program_name: str) -> bool: + try: + client = self._get_rpc_client() + process_info = client.supervisor.getProcessInfo(program_name) + if not isinstance(process_info, dict): + return False + + state = process_info.get("state") + if not isinstance(state, int): + return False + return state == PROGRAM_RUNNING_STATE + except xmlrpc.client.Fault: + # If we can't get the process info, assume it's not running + return False + def start_supervisor_daemon(self) -> None: try: subprocess.run(["supervisord", "-c", self.config_file_path], check=True) @@ -118,6 +138,8 @@ def stop_supervisor_daemon(self) -> None: raise SupervisorError(f"Failed to stop supervisor: {e.faultString}") def start_program(self, program_name: str) -> None: + if self._check_program_running(program_name): + return try: self._get_rpc_client().supervisor.startProcess(program_name) except xmlrpc.client.Fault as e: @@ -126,6 +148,8 @@ def start_program(self, program_name: str) -> None: ) def stop_program(self, program_name: str) -> None: + if not self._check_program_running(program_name): + return try: self._get_rpc_client().supervisor.stopProcess(program_name) except xmlrpc.client.Fault as e: @@ -133,14 +157,26 @@ def stop_program(self, program_name: str) -> None: f"Failed to stop program {program_name}: {e.faultString}" ) - def foreground_program(self, program_name: str) -> None: + def tail_program_logs(self, program_name: str) -> None: + if not self._check_program_running(program_name): + console = Console() + console.failure(f"Process {program_name} is not running") + return + try: + # Use supervisorctl tail command subprocess.run( - ["supervisorctl", "-c", self.config_file_path, "fg", program_name], + [ + "supervisorctl", + "-c", + self.config_file_path, + "tail", + "-f", + program_name, + ], check=True, - stderr=subprocess.DEVNULL, ) except subprocess.CalledProcessError as e: - raise SupervisorError(f"Failed to foreground {program_name}: {str(e)}") + raise SupervisorError(f"Failed to tail logs for {program_name}: {str(e)}") except KeyboardInterrupt: pass diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index 41d0706..9116f53 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -72,6 +72,22 @@ def test_get_rpc_client_failure( assert transport_arg.socket_path == supervisor_manager.socket_path +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_check_program_running_success( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + assert supervisor_manager._check_program_running("test_program") + + +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_check_program_running_failure( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + assert not supervisor_manager._check_program_running("test_program") + + @mock.patch("devservices.utils.supervisor.subprocess.run") def test_start_supervisor_daemon_success( mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager @@ -123,6 +139,7 @@ def test_stop_supervisor_daemon_failure( def test_start_program_success( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} supervisor_manager.start_program("test_program") supervisor_manager._get_rpc_client().supervisor.startProcess.assert_called_once_with( "test_program" @@ -133,6 +150,7 @@ def test_start_program_success( def test_start_program_failure( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} mock_rpc_client.return_value.supervisor.startProcess.side_effect = ( xmlrpc.client.Fault(1, "Error") ) @@ -140,10 +158,20 @@ def test_start_program_failure( supervisor_manager.start_program("test_program") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_start_program_already_running( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + supervisor_manager.start_program("test_program") + mock_rpc_client.supervisor.startProcess.assert_not_called() + + @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") def test_stop_program_success( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} supervisor_manager.stop_program("test_program") supervisor_manager._get_rpc_client().supervisor.stopProcess.assert_called_once_with( "test_program" @@ -154,6 +182,7 @@ def test_stop_program_success( def test_stop_program_failure( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} mock_rpc_client.return_value.supervisor.stopProcess.side_effect = ( xmlrpc.client.Fault(1, "Error") ) @@ -161,6 +190,15 @@ def test_stop_program_failure( supervisor_manager.stop_program("test_program") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_stop_program_not_running( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + supervisor_manager.stop_program("test_program") + mock_rpc_client.supervisor.stopProcess.assert_not_called() + + def test_extend_config_file( supervisor_manager: SupervisorManager, tmp_path: Path ) -> None: @@ -190,10 +228,14 @@ def test_extend_config_file( @mock.patch("devservices.utils.supervisor.subprocess.run") -def test_foreground_program_success( - mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def tail_program_logs_success( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, ) -> None: - supervisor_manager.foreground_program("test_program") + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + supervisor_manager.tail_program_logs("test_program") mock_subprocess_run.assert_called_once_with( [ "supervisorctl", @@ -203,33 +245,55 @@ def test_foreground_program_success( "test_program", ], check=True, - stderr=subprocess.DEVNULL, ) @mock.patch("devservices.utils.supervisor.subprocess.run") -def test_foreground_program_failure( - mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def tail_program_logs_not_running( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, + capsys: pytest.CaptureFixture[str], ) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + supervisor_manager.tail_program_logs("test_program") + captured = capsys.readouterr() + assert "Process test_program is not running" in captured.out + mock_subprocess_run.assert_not_called() + + +@mock.patch("devservices.utils.supervisor.subprocess.run") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def tail_program_logs_failure( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} mock_subprocess_run.side_effect = subprocess.CalledProcessError(1, "supervisorctl") - with pytest.raises(SupervisorError, match="Failed to foreground test_program"): - supervisor_manager.foreground_program("test_program") + with pytest.raises(SupervisorError, match="Failed to tail logs for test_program"): + supervisor_manager.tail_program_logs("test_program") @mock.patch("devservices.utils.supervisor.subprocess.run") -def test_foreground_program_keyboard_interrupt( - mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def tail_program_logs_keyboard_interrupt( + mock_rpc_client: mock.MagicMock, + mock_subprocess_run: mock.MagicMock, + supervisor_manager: SupervisorManager, ) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} mock_subprocess_run.side_effect = KeyboardInterrupt() - supervisor_manager.foreground_program("test_program") + supervisor_manager.tail_program_logs("test_program") mock_subprocess_run.assert_called_once_with( [ "supervisorctl", "-c", supervisor_manager.config_file_path, - "fg", + "tail", + "-f", "test_program", ], check=True, - stderr=subprocess.DEVNULL, ) From 5ef51560b84b0cf0f5183dc6eb69dcb0847fcdd2 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Mon, 21 Apr 2025 15:31:51 -0700 Subject: [PATCH 3/6] add some more tests --- tests/utils/test_supervisor.py | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index 9116f53..df68d71 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -81,13 +81,35 @@ def test_check_program_running_success( @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_check_program_running_failure( +def test_check_program_running_program_not_running( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} assert not supervisor_manager._check_program_running("test_program") +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_check_program_running_typing_error( + mock_rpc_client: mock.MagicMock, + supervisor_manager: SupervisorManager, + capsys: pytest.CaptureFixture[str], +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = 1 + assert not supervisor_manager._check_program_running("test_program") + mock_rpc_client.return_value.supervisor.getProcessInfo.side_effect = {"state": "20"} + assert not supervisor_manager._check_program_running("test_program") + + +@mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") +def test_check_program_running_failure( + mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager +) -> None: + mock_rpc_client.return_value.supervisor.getProcessInfo.side_effect = ( + xmlrpc.client.Fault(1, "Error") + ) + assert not supervisor_manager._check_program_running("test_program") + + @mock.patch("devservices.utils.supervisor.subprocess.run") def test_start_supervisor_daemon_success( mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager @@ -250,7 +272,7 @@ def tail_program_logs_success( @mock.patch("devservices.utils.supervisor.subprocess.run") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def tail_program_logs_not_running( +def test_tail_program_logs_not_running( mock_rpc_client: mock.MagicMock, mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, @@ -265,7 +287,7 @@ def tail_program_logs_not_running( @mock.patch("devservices.utils.supervisor.subprocess.run") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def tail_program_logs_failure( +def test_tail_program_logs_failure( mock_rpc_client: mock.MagicMock, mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, @@ -278,7 +300,7 @@ def tail_program_logs_failure( @mock.patch("devservices.utils.supervisor.subprocess.run") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def tail_program_logs_keyboard_interrupt( +def test_tail_program_logs_keyboard_interrupt( mock_rpc_client: mock.MagicMock, mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, From 6a8d4ff32a68285b276a54aadb191357c8d3aa89 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Wed, 23 Apr 2025 15:14:10 -0700 Subject: [PATCH 4/6] address some comments --- devservices/utils/supervisor.py | 30 ++++++++++---- tests/utils/test_supervisor.py | 73 ++++++++++++++++++++++----------- 2 files changed, 73 insertions(+), 30 deletions(-) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index c752771..fb97838 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -6,6 +6,7 @@ import socket import subprocess import xmlrpc.client +from enum import IntEnum from devservices.constants import DEVSERVICES_SUPERVISOR_CONFIG_DIR from devservices.exceptions import SupervisorConfigError @@ -16,7 +17,22 @@ # Supervisor process state 20 is RUNNING # https://supervisord.org/subprocess.html#process-states -PROGRAM_RUNNING_STATE = 20 + + +class SupervisorProcessState(IntEnum): + """Supervisor process states. + + https://supervisord.org/subprocess.html#process-states + """ + + STOPPED = 0 + STARTING = 10 + RUNNING = 20 + BACKOFF = 30 + STOPPING = 40 + EXITED = 100 + FATAL = 200 + UNKNOWN = 1000 class UnixSocketHTTPConnection(http.client.HTTPConnection): @@ -106,7 +122,7 @@ def _get_rpc_client(self) -> xmlrpc.client.ServerProxy: f"Failed to connect to supervisor XML-RPC server: {e.errmsg}" ) - def _check_program_running(self, program_name: str) -> bool: + def _is_program_running(self, program_name: str) -> bool: try: client = self._get_rpc_client() process_info = client.supervisor.getProcessInfo(program_name) @@ -116,7 +132,7 @@ def _check_program_running(self, program_name: str) -> bool: state = process_info.get("state") if not isinstance(state, int): return False - return state == PROGRAM_RUNNING_STATE + return state == SupervisorProcessState.RUNNING except xmlrpc.client.Fault: # If we can't get the process info, assume it's not running return False @@ -138,7 +154,7 @@ def stop_supervisor_daemon(self) -> None: raise SupervisorError(f"Failed to stop supervisor: {e.faultString}") def start_program(self, program_name: str) -> None: - if self._check_program_running(program_name): + if self._is_program_running(program_name): return try: self._get_rpc_client().supervisor.startProcess(program_name) @@ -148,7 +164,7 @@ def start_program(self, program_name: str) -> None: ) def stop_program(self, program_name: str) -> None: - if not self._check_program_running(program_name): + if not self._is_program_running(program_name): return try: self._get_rpc_client().supervisor.stopProcess(program_name) @@ -158,9 +174,9 @@ def stop_program(self, program_name: str) -> None: ) def tail_program_logs(self, program_name: str) -> None: - if not self._check_program_running(program_name): + if not self._is_program_running(program_name): console = Console() - console.failure(f"Process {program_name} is not running") + console.failure(f"Program {program_name} is not running") return try: diff --git a/tests/utils/test_supervisor.py b/tests/utils/test_supervisor.py index df68d71..91f38df 100644 --- a/tests/utils/test_supervisor.py +++ b/tests/utils/test_supervisor.py @@ -13,6 +13,7 @@ from devservices.exceptions import SupervisorError from devservices.exceptions import SupervisorProcessError from devservices.utils.supervisor import SupervisorManager +from devservices.utils.supervisor import SupervisorProcessState from devservices.utils.supervisor import UnixSocketTransport @@ -73,41 +74,47 @@ def test_get_rpc_client_failure( @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_check_program_running_success( +def test_is_program_running_success( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} - assert supervisor_manager._check_program_running("test_program") + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } + assert supervisor_manager._is_program_running("test_program") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_check_program_running_program_not_running( +def test_is_program_running_program_not_running( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} - assert not supervisor_manager._check_program_running("test_program") + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.STOPPED + } + assert not supervisor_manager._is_program_running("test_program") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_check_program_running_typing_error( +def test_is_program_running_typing_error( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager, capsys: pytest.CaptureFixture[str], ) -> None: mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = 1 - assert not supervisor_manager._check_program_running("test_program") - mock_rpc_client.return_value.supervisor.getProcessInfo.side_effect = {"state": "20"} - assert not supervisor_manager._check_program_running("test_program") + assert not supervisor_manager._is_program_running("test_program") + mock_rpc_client.return_value.supervisor.getProcessInfo.side_effect = { + "state": [SupervisorProcessState.STOPPED] + } + assert not supervisor_manager._is_program_running("test_program") @mock.patch("devservices.utils.supervisor.xmlrpc.client.ServerProxy") -def test_check_program_running_failure( +def test_is_program_running_failure( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: mock_rpc_client.return_value.supervisor.getProcessInfo.side_effect = ( xmlrpc.client.Fault(1, "Error") ) - assert not supervisor_manager._check_program_running("test_program") + assert not supervisor_manager._is_program_running("test_program") @mock.patch("devservices.utils.supervisor.subprocess.run") @@ -161,7 +168,9 @@ def test_stop_supervisor_daemon_failure( def test_start_program_success( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.STOPPED + } supervisor_manager.start_program("test_program") supervisor_manager._get_rpc_client().supervisor.startProcess.assert_called_once_with( "test_program" @@ -172,7 +181,9 @@ def test_start_program_success( def test_start_program_failure( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.STOPPED + } mock_rpc_client.return_value.supervisor.startProcess.side_effect = ( xmlrpc.client.Fault(1, "Error") ) @@ -184,7 +195,9 @@ def test_start_program_failure( def test_start_program_already_running( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } supervisor_manager.start_program("test_program") mock_rpc_client.supervisor.startProcess.assert_not_called() @@ -193,7 +206,9 @@ def test_start_program_already_running( def test_stop_program_success( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } supervisor_manager.stop_program("test_program") supervisor_manager._get_rpc_client().supervisor.stopProcess.assert_called_once_with( "test_program" @@ -204,7 +219,9 @@ def test_stop_program_success( def test_stop_program_failure( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } mock_rpc_client.return_value.supervisor.stopProcess.side_effect = ( xmlrpc.client.Fault(1, "Error") ) @@ -216,7 +233,9 @@ def test_stop_program_failure( def test_stop_program_not_running( mock_rpc_client: mock.MagicMock, supervisor_manager: SupervisorManager ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.STOPPED + } supervisor_manager.stop_program("test_program") mock_rpc_client.supervisor.stopProcess.assert_not_called() @@ -256,7 +275,9 @@ def tail_program_logs_success( mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } supervisor_manager.tail_program_logs("test_program") mock_subprocess_run.assert_called_once_with( [ @@ -278,10 +299,12 @@ def test_tail_program_logs_not_running( supervisor_manager: SupervisorManager, capsys: pytest.CaptureFixture[str], ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 0} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.STOPPED + } supervisor_manager.tail_program_logs("test_program") captured = capsys.readouterr() - assert "Process test_program is not running" in captured.out + assert "Program test_program is not running" in captured.out mock_subprocess_run.assert_not_called() @@ -292,7 +315,9 @@ def test_tail_program_logs_failure( mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } mock_subprocess_run.side_effect = subprocess.CalledProcessError(1, "supervisorctl") with pytest.raises(SupervisorError, match="Failed to tail logs for test_program"): supervisor_manager.tail_program_logs("test_program") @@ -305,7 +330,9 @@ def test_tail_program_logs_keyboard_interrupt( mock_subprocess_run: mock.MagicMock, supervisor_manager: SupervisorManager, ) -> None: - mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = {"state": 20} + mock_rpc_client.return_value.supervisor.getProcessInfo.return_value = { + "state": SupervisorProcessState.RUNNING + } mock_subprocess_run.side_effect = KeyboardInterrupt() supervisor_manager.tail_program_logs("test_program") mock_subprocess_run.assert_called_once_with( From af4195073172e59c22dedf59acae67d55a14a505 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Wed, 23 Apr 2025 15:14:47 -0700 Subject: [PATCH 5/6] delete unused comment --- devservices/utils/supervisor.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index fb97838..3e1f556 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -15,9 +15,6 @@ from devservices.exceptions import SupervisorProcessError from devservices.utils.console import Console -# Supervisor process state 20 is RUNNING -# https://supervisord.org/subprocess.html#process-states - class SupervisorProcessState(IntEnum): """Supervisor process states. From c5769bb4e2fb3d159d4922d41213636d8df2d250 Mon Sep 17 00:00:00 2001 From: Hubert Deng Date: Fri, 25 Apr 2025 15:08:53 -0700 Subject: [PATCH 6/6] add newline in comment --- devservices/utils/supervisor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devservices/utils/supervisor.py b/devservices/utils/supervisor.py index 3e1f556..8a25135 100644 --- a/devservices/utils/supervisor.py +++ b/devservices/utils/supervisor.py @@ -17,7 +17,8 @@ class SupervisorProcessState(IntEnum): - """Supervisor process states. + """ + Supervisor process states. https://supervisord.org/subprocess.html#process-states """