From 7371685ffbeee1ebf43ca0d78b26e707a4ce3ef1 Mon Sep 17 00:00:00 2001 From: Jericho Tolentino <68654047+jericht@users.noreply.github.com> Date: Wed, 1 May 2024 02:33:39 +0000 Subject: [PATCH] add --working-dir option to replace --connection-file Signed-off-by: Jericho Tolentino <68654047+jericht@users.noreply.github.com> --- .../_background/backend_runner.py | 31 +++- .../_background/frontend_runner.py | 42 +++++- src/openjd/adaptor_runtime/_entrypoint.py | 57 +++++-- src/openjd/adaptor_runtime/_http/__init__.py | 4 +- src/openjd/adaptor_runtime/_http/sockets.py | 140 +++++++++++------- .../application_ipc/_adaptor_server.py | 10 +- .../integ/AdaptorExample/adaptor.py | 7 + .../integ/background/test_background_mode.py | 2 +- .../integ/test_integration_entrypoint.py | 3 +- .../unit/background/test_backend_runner.py | 16 +- .../unit/background/test_frontend_runner.py | 62 ++++---- .../adaptor_runtime/unit/http/test_sockets.py | 87 ++++++----- .../adaptor_runtime/unit/test_entrypoint.py | 24 ++- .../test_integration_client_interface.py | 2 +- 14 files changed, 317 insertions(+), 170 deletions(-) diff --git a/src/openjd/adaptor_runtime/_background/backend_runner.py b/src/openjd/adaptor_runtime/_background/backend_runner.py index 50bb7b1..658083b 100644 --- a/src/openjd/adaptor_runtime/_background/backend_runner.py +++ b/src/openjd/adaptor_runtime/_background/backend_runner.py @@ -13,7 +13,7 @@ from .server_response import ServerResponseGenerator from .._osname import OSName from ..adaptors import AdaptorRunner -from .._http import SocketDirectories +from .._http import SocketPaths from .._utils import secure_open if OSName.is_posix(): @@ -33,15 +33,34 @@ class BackendRunner: Class that runs the backend logic in background mode. """ + _connection_file_path: str + _working_dir: str | None + def __init__( self, adaptor_runner: AdaptorRunner, - connection_file_path: str, *, + # TODO: Deprecate connection_file_path + connection_file_path: str | None = None, + working_dir: str | None = None, log_buffer: LogBuffer | None = None, ) -> None: self._adaptor_runner = adaptor_runner - self._connection_file_path = connection_file_path + + if (connection_file_path and working_dir) or not (connection_file_path or working_dir): + raise RuntimeError( + "Exactly one of 'connection_file_path' or 'working_dir' must be provided, but got:" + f" connection_file_path={connection_file_path} working_dir={working_dir}" + ) + + if working_dir: + self._working_dir = working_dir + self._connection_file_path = os.path.join(working_dir, "connection.json") + else: + assert connection_file_path # for mypy + self._working_dir = None + self._connection_file_path = connection_file_path + self._log_buffer = log_buffer self._server: Optional[Union[BackgroundHTTPServer, WinBackgroundNamedPipeServer]] = None signal.signal(signal.SIGINT, self._sigint_handler) @@ -79,8 +98,10 @@ def run(self) -> None: shutdown_event: Event = Event() if OSName.is_posix(): # pragma: is-windows - server_path = SocketDirectories.for_os().get_process_socket_path( - "runtime", create_dir=True + server_path = SocketPaths.for_os().get_process_socket_path( + "runtime", + base_dir=self._working_dir, + create_dir=True, ) else: # pragma: is-posix server_path = NamedPipeHelper.generate_pipe_name("AdaptorNamedPipe") diff --git a/src/openjd/adaptor_runtime/_background/frontend_runner.py b/src/openjd/adaptor_runtime/_background/frontend_runner.py index 23e9234..37fb3a0 100644 --- a/src/openjd/adaptor_runtime/_background/frontend_runner.py +++ b/src/openjd/adaptor_runtime/_background/frontend_runner.py @@ -42,23 +42,43 @@ class FrontendRunner: Class that runs the frontend logic in background mode. """ + _connection_file_path: str + _working_dir: str | None + def __init__( self, - connection_file_path: str, *, + # TODO: Deprecate this option, replace with working_dir + connection_file_path: str | None = None, + working_dir: str | None = None, timeout_s: float = 5.0, heartbeat_interval: float = 1.0, ) -> None: """ Args: - connection_file_path (str): Absolute path to the connection file. + connection_file_path (Optional[str]): DEPRECATED. Please use 'working_dir' instead. Absolute path to the connection file. + working_dir (Optional[str]): Working directory for the runtime. A connection.json file must exist in this directory. timeout_s (float, optional): Timeout for HTTP requests, in seconds. Defaults to 5. heartbeat_interval (float, optional): Interval between heartbeats, in seconds. Defaults to 1. """ self._timeout_s = timeout_s self._heartbeat_interval = heartbeat_interval - self._connection_file_path = connection_file_path + + if (connection_file_path and working_dir) or not (connection_file_path or working_dir): + raise RuntimeError( + "Expected exactly one of 'connection_file_path' or 'working_dir', but got: " + f"connection_file_path={connection_file_path} working_dir={working_dir}" + ) + + if working_dir: + self._working_dir = working_dir + self._connection_file_path = os.path.join(working_dir, "connection.json") + else: + assert connection_file_path # for mypy + self._working_dir = None + self._connection_file_path = connection_file_path + self._canceled = Event() signal.signal(signal.SIGINT, self._sigint_handler) if OSName.is_posix(): # pragma: is-windows @@ -111,14 +131,26 @@ def init( [ "daemon", "_serve", - "--connection-file", - self._connection_file_path, "--init-data", json.dumps(init_data), "--path-mapping-rules", json.dumps(path_mapping_data), ] ) + if self._working_dir: + args.extend( + [ + "--working-dir", + self._working_dir, + ] + ) + else: + args.extend( + [ + "--connection-file", + self._connection_file_path, + ] + ) try: process = subprocess.Popen( args, diff --git a/src/openjd/adaptor_runtime/_entrypoint.py b/src/openjd/adaptor_runtime/_entrypoint.py index 6c4ab8c..4f373cf 100644 --- a/src/openjd/adaptor_runtime/_entrypoint.py +++ b/src/openjd/adaptor_runtime/_entrypoint.py @@ -53,7 +53,8 @@ "file://path/to/file.json" ), "show_config": "Prints the adaptor runtime configuration, then the program exits.", - "connection_file": "The file path to the connection file for use in background mode.", + "connection_file": "DEPRECATED. Please use 'working_dir' instead.\n\nThe file path to the connection file for use in background mode.", + "working_dir": "The path to the directory to use for runtime files. This directory must not already exist.", } _DIR = os.path.dirname(os.path.realpath(__file__)) @@ -300,9 +301,25 @@ def _handle_daemon( integration_data: _IntegrationData, reentry_exe: Optional[Path] = None, ): - connection_file = parsed_args.connection_file - if not os.path.isabs(connection_file): + # Validate args + connection_file = None + if hasattr(parsed_args, "connection_file"): + connection_file = parsed_args.connection_file + + working_dir = None + if hasattr(parsed_args, "working_dir"): + working_dir = parsed_args.working_dir + + if (connection_file and working_dir) or not (connection_file or working_dir): + raise RuntimeError( + "Expected exactly one of 'connection_file' or 'working_dir' to be provided, " + f"but got args: {parsed_args}" + ) + + if connection_file and not os.path.isabs(connection_file): connection_file = os.path.abspath(connection_file) + if working_dir and not os.path.isabs(working_dir): + working_dir = os.path.abspath(working_dir) subcommand = parsed_args.subcommand if hasattr(parsed_args, "subcommand") else None if subcommand == "_serve": @@ -318,14 +335,18 @@ def _handle_daemon( # forever until a shutdown is requested backend = BackendRunner( AdaptorRunner(adaptor=adaptor), - connection_file, + connection_file_path=connection_file, + working_dir=working_dir, log_buffer=log_buffer, ) backend.run() else: # This process is running in frontend mode. Create the frontend runner and send # the appropriate request to the backend. - frontend = FrontendRunner(connection_file) + frontend = FrontendRunner( + connection_file_path=connection_file, + working_dir=working_dir, + ) if subcommand == "start": adaptor_module = sys.modules.get(self.adaptor_class.__module__) if adaptor_module is None: @@ -349,10 +370,12 @@ def _handle_daemon( def _parse_args(self) -> Tuple[ArgumentParser, Namespace]: parser = self._build_argparser() try: - return parser, parser.parse_args(sys.argv[1:]) + parsed_args = parser.parse_args(sys.argv[1:]) except Exception as e: _logger.error(f"Error parsing command line arguments: {e}") raise + else: + return parser, parsed_args def _build_argparser(self) -> ArgumentParser: parser = ArgumentParser( @@ -412,9 +435,15 @@ def _build_argparser(self) -> ArgumentParser: connection_file = ArgumentParser(add_help=False) connection_file.add_argument( "--connection-file", - default="", help=_CLI_HELP_TEXT["connection_file"], - required=True, + required=False, + ) + + working_dir = ArgumentParser(add_help=False) + working_dir.add_argument( + "--working-dir", + help=_CLI_HELP_TEXT["working_dir"], + required=False, ) bg_parser = subparser.add_parser("daemon", help="Runs the adaptor in a daemon mode.") @@ -427,11 +456,15 @@ def _build_argparser(self) -> ArgumentParser: ) # "Hidden" command that actually runs the adaptor runtime in background mode - bg_subparser.add_parser("_serve", parents=[init_data, path_mapping_rules, connection_file]) + bg_subparser.add_parser( + "_serve", parents=[init_data, path_mapping_rules, connection_file, working_dir] + ) - bg_subparser.add_parser("start", parents=[init_data, path_mapping_rules, connection_file]) - bg_subparser.add_parser("run", parents=[run_data, connection_file]) - bg_subparser.add_parser("stop", parents=[connection_file]) + bg_subparser.add_parser( + "start", parents=[init_data, path_mapping_rules, connection_file, working_dir] + ) + bg_subparser.add_parser("run", parents=[run_data, connection_file, working_dir]) + bg_subparser.add_parser("stop", parents=[connection_file, working_dir]) return parser diff --git a/src/openjd/adaptor_runtime/_http/__init__.py b/src/openjd/adaptor_runtime/_http/__init__.py index 834c7e8..70b2f0d 100644 --- a/src/openjd/adaptor_runtime/_http/__init__.py +++ b/src/openjd/adaptor_runtime/_http/__init__.py @@ -1,6 +1,6 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. from .request_handler import HTTPResponse, RequestHandler, ResourceRequestHandler -from .sockets import SocketDirectories +from .sockets import SocketPaths -__all__ = ["HTTPResponse", "RequestHandler", "ResourceRequestHandler", "SocketDirectories"] +__all__ = ["HTTPResponse", "RequestHandler", "ResourceRequestHandler", "SocketPaths"] diff --git a/src/openjd/adaptor_runtime/_http/sockets.py b/src/openjd/adaptor_runtime/_http/sockets.py index f057f6d..aed857b 100644 --- a/src/openjd/adaptor_runtime/_http/sockets.py +++ b/src/openjd/adaptor_runtime/_http/sockets.py @@ -16,17 +16,17 @@ # Max PID on 64-bit systems is 4194304 (2^22) _PID_MAX_LENGTH = 7 -_PID_MAX_LENGTH_PADDED = _PID_MAX_LENGTH + 1 # 1 char for path seperator -class SocketDirectories(abc.ABC): +class SocketPaths(abc.ABC): """ - Base class for determining the base directory for sockets used in the Adaptor Runtime. + Base class for determining the paths for sockets used in the Adaptor Runtime. """ @staticmethod def for_os(osname: OSName = OSName()): # pragma: no cover - """_summary_ + """ + Gets the SocketPaths class for a specific OS. Args: osname (OSName, optional): The OS to get socket directories for. @@ -41,12 +41,20 @@ def for_os(osname: OSName = OSName()): # pragma: no cover raise UnsupportedPlatformException(osname) return klass() - def get_process_socket_path(self, namespace: str | None = None, *, create_dir: bool = False): + def get_process_socket_path( + self, + namespace: str | None = None, + *, + base_dir: str | None = None, + create_dir: bool = False, + ): """ Gets the path for this process' socket in the given namespace. Args: namespace (Optional[str]): The optional namespace (subdirectory) where the sockets go. + base_dir (Optional[str]): The base directory to create sockets in. Defaults to user's home + directory, then the temp directory. create_dir (bool): Whether to create the socket directory. Default is false. Raises: @@ -60,21 +68,29 @@ def get_process_socket_path(self, namespace: str | None = None, *, create_dir: b len(socket_name) <= _PID_MAX_LENGTH ), f"PID too long. Only PIDs up to {_PID_MAX_LENGTH} digits are supported." - return self.get_socket_path(socket_name, namespace, create_dir=create_dir) + return self.get_socket_path( + socket_name, + namespace, + base_dir=base_dir, + create_dir=create_dir, + ) def get_socket_path( self, base_socket_name: str, namespace: str | None = None, *, + base_dir: str | None = None, create_dir: bool = False, ) -> str: """ - Gets the base directory for sockets used in Adaptor IPC + Gets the path for a socket used in Adaptor IPC Args: base_socket_name (str): The name of the socket namespace (Optional[str]): The optional namespace (subdirectory) where the sockets go + base_dir (Optional[str]): The base directory to create sockets in. Defaults to user's home + directory, then the temp directory. create_dir (bool): Whether to create the directory or not. Default is false. Raises: @@ -103,39 +119,57 @@ def gen_socket_path(dir: str, base_name: str): reasons: list[str] = [] - # First try home directory - home_dir = os.path.expanduser("~") - socket_dir = os.path.join(home_dir, rel_path) - socket_path = gen_socket_path(socket_dir, base_socket_name) - try: - self.verify_socket_path(socket_path) - except NonvalidSocketPathException as e: - reasons.append(f"Cannot create sockets directory in the home directory because: {e}") - else: - mkdir(socket_dir) - return socket_path - - # Last resort is the temp directory - temp_dir = tempfile.gettempdir() - socket_dir = os.path.join(temp_dir, rel_path) - socket_path = gen_socket_path(socket_dir, base_socket_name) - try: - self.verify_socket_path(socket_path) - except NonvalidSocketPathException as e: - reasons.append(f"Cannot create sockets directory in the temp directory because: {e}") + if base_dir: + # Only try to use the provided base directory + socket_dir = os.path.join(base_dir, rel_path) + socket_path = gen_socket_path(socket_dir, base_socket_name) + try: + self.verify_socket_path(socket_path) + except NonvalidSocketPathException as e: + reasons.append( + f"Cannot create socket in the base directory at '{socket_dir}' because: {e}" + ) + else: + mkdir(socket_dir) + return socket_path else: - # Also check that the sticky bit is set on the temp dir - if not os.stat(temp_dir).st_mode & stat.S_ISVTX: + # First try home directory + home_dir = os.path.expanduser("~") + socket_dir = os.path.join(home_dir, rel_path) + socket_path = gen_socket_path(socket_dir, base_socket_name) + try: + self.verify_socket_path(socket_path) + except NonvalidSocketPathException as e: reasons.append( - f"Cannot use temporary directory {temp_dir} because it does not have the " - "sticky bit (restricted deletion flag) set" + f"Cannot create socket in the home directory at '{socket_dir}' because: {e}" ) else: mkdir(socket_dir) return socket_path + # Last resort is the temp directory + temp_dir = tempfile.gettempdir() + socket_dir = os.path.join(temp_dir, rel_path) + socket_path = gen_socket_path(socket_dir, base_socket_name) + try: + self.verify_socket_path(socket_path) + except NonvalidSocketPathException as e: + reasons.append( + f"Cannot create socket in the temp directory at '{socket_dir}' because: {e}" + ) + else: + # Also check that the sticky bit is set on the temp dir + if not os.stat(temp_dir).st_mode & stat.S_ISVTX: + reasons.append( + f"Cannot use temporary directory {temp_dir} because it does not have the " + "sticky bit (restricted deletion flag) set" + ) + else: + mkdir(socket_dir) + return socket_path + raise NoSocketPathFoundException( - "Failed to find a suitable base directory to create sockets in for the following " + "Failed to find a suitable socket path for the following " f"reasons: {os.linesep.join(reasons)}" ) @@ -151,53 +185,55 @@ def verify_socket_path(self, path: str) -> None: # pragma: no cover pass -class LinuxSocketDirectories(SocketDirectories): +class LinuxSocketPaths(SocketPaths): """ Specialization for socket paths in Linux systems. """ # This is based on the max length of socket names to 108 bytes # See unix(7) under "Address format" - _socket_path_max_length = 108 - _socket_dir_max_length = _socket_path_max_length - _PID_MAX_LENGTH_PADDED + # In practice, only 107 bytes are accepted (one byte for null terminator) + _socket_name_max_length = 108 - 1 def verify_socket_path(self, path: str) -> None: - path_length = len(path.encode("utf-8")) - if path_length > self._socket_dir_max_length: + socket_name = os.path.basename(path) + socket_name_length = len(socket_name.encode("utf-8")) + if socket_name_length > self._socket_name_max_length: raise NonvalidSocketPathException( - "Socket base directory path too big. The maximum allowed size is " - f"{self._socket_dir_max_length} bytes, but the directory has a size of " - f"{path_length}: {path}" + "Socket name too long. The maximum allowed size is " + f"{self._socket_name_max_length} bytes, but the name has a size of " + f"{socket_name_length}: {socket_name}" ) -class MacOSSocketDirectories(SocketDirectories): +class MacOSSocketPaths(SocketPaths): """ Specialization for socket paths in macOS systems. """ # This is based on the max length of socket names to 104 bytes # See https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/un.h#L79 - _socket_path_max_length = 104 - _socket_dir_max_length = _socket_path_max_length - _PID_MAX_LENGTH_PADDED + # In practice, only 103 bytes are accepted (one byte for null terminator) + _socket_name_max_length = 104 - 1 def verify_socket_path(self, path: str) -> None: - path_length = len(path.encode("utf-8")) - if path_length > self._socket_dir_max_length: + socket_name = os.path.basename(path) + socket_name_length = len(socket_name.encode("utf-8")) + if socket_name_length > self._socket_name_max_length: raise NonvalidSocketPathException( - "Socket base directory path too big. The maximum allowed size is " - f"{self._socket_dir_max_length} bytes, but the directory has a size of " - f"{path_length}: {path}" + "Socket name too long. The maximum allowed size is " + f"{self._socket_name_max_length} bytes, but the name has a size of " + f"{socket_name_length}: {socket_name}" ) -_os_map: dict[str, type[SocketDirectories]] = { - OSName.LINUX: LinuxSocketDirectories, - OSName.MACOS: MacOSSocketDirectories, +_os_map: dict[str, type[SocketPaths]] = { + OSName.LINUX: LinuxSocketPaths, + OSName.MACOS: MacOSSocketPaths, } def _get_socket_directories_cls( osname: OSName, -) -> type[SocketDirectories] | None: # pragma: no cover +) -> type[SocketPaths] | None: # pragma: no cover return _os_map.get(osname, None) diff --git a/src/openjd/adaptor_runtime/application_ipc/_adaptor_server.py b/src/openjd/adaptor_runtime/application_ipc/_adaptor_server.py index 70a88af..794d2ca 100644 --- a/src/openjd/adaptor_runtime/application_ipc/_adaptor_server.py +++ b/src/openjd/adaptor_runtime/application_ipc/_adaptor_server.py @@ -8,7 +8,7 @@ from socketserver import UnixStreamServer # type: ignore[attr-defined] from typing import TYPE_CHECKING -from .._http import SocketDirectories +from .._http import SocketPaths from ._http_request_handler import AdaptorHTTPRequestHandler if TYPE_CHECKING: # pragma: no cover because pytest will think we should test for this. @@ -32,8 +32,14 @@ def __init__( self, actions_queue: ActionsQueue, adaptor: BaseAdaptor, + *, + working_dir: str | None = None, ) -> None: # pragma: no cover - socket_path = SocketDirectories.for_os().get_process_socket_path("dcc", create_dir=True) + socket_path = SocketPaths.for_os().get_process_socket_path( + "dcc", + base_dir=working_dir, + create_dir=True, + ) super().__init__(socket_path, AdaptorHTTPRequestHandler) self.actions_queue = actions_queue diff --git a/test/openjd/adaptor_runtime/integ/AdaptorExample/adaptor.py b/test/openjd/adaptor_runtime/integ/AdaptorExample/adaptor.py index 42a6a43..6c5ea48 100644 --- a/test/openjd/adaptor_runtime/integ/AdaptorExample/adaptor.py +++ b/test/openjd/adaptor_runtime/integ/AdaptorExample/adaptor.py @@ -3,6 +3,7 @@ import logging import os import sys +import tempfile import threading import time @@ -45,12 +46,15 @@ def on_start(self) -> None: # This example initializes a server thread to interact with a client application, showing command exchange and # execution. _logger.info("on_start") + + self.working_dir = tempfile.TemporaryDirectory() # Initialize the server thread to manage actions self.server = AdaptorServer( # actions_queue will be used for storing the actions. In the client application, it will keep polling the # actions from this queue and run actions actions_queue=self.actions, adaptor=self, + working_dir=self.working_dir.name, ) # The server will keep running until `stop` is called self.server_thread = threading.Thread( @@ -156,6 +160,9 @@ def on_cleanup(self) -> None: if self.server_thread.is_alive(): _logger.error("Failed to shutdown the AdaptorExample server") + if hasattr(self, "working_dir"): + self.working_dir.cleanup() + def on_cancel(self): """ Handles the cancellation or termination of a running task, typically triggered by external signals. diff --git a/test/openjd/adaptor_runtime/integ/background/test_background_mode.py b/test/openjd/adaptor_runtime/integ/background/test_background_mode.py index f62ff90..21fce78 100644 --- a/test/openjd/adaptor_runtime/integ/background/test_background_mode.py +++ b/test/openjd/adaptor_runtime/integ/background/test_background_mode.py @@ -72,7 +72,7 @@ def initialized_setup( caplog: pytest.LogCaptureFixture, ) -> Generator[tuple[FrontendRunner, psutil.Process], None, None]: caplog.set_level(0) - frontend = FrontendRunner(connection_file_path, timeout_s=5.0) + frontend = FrontendRunner(connection_file_path=connection_file_path, timeout_s=5.0) frontend.init(sys.modules[AdaptorExample.__module__]) conn_settings = _load_connection_settings(connection_file_path) diff --git a/test/openjd/adaptor_runtime/integ/test_integration_entrypoint.py b/test/openjd/adaptor_runtime/integ/test_integration_entrypoint.py index ca2eb05..50a1ef2 100644 --- a/test/openjd/adaptor_runtime/integ/test_integration_entrypoint.py +++ b/test/openjd/adaptor_runtime/integ/test_integration_entrypoint.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import pathlib import os import re import sys @@ -32,7 +33,7 @@ class TestCommandAdaptorRun: """ def test_runs_command_adaptor( - self, capfd: pytest.CaptureFixture, caplog: pytest.LogCaptureFixture + self, capfd: pytest.CaptureFixture, caplog: pytest.LogCaptureFixture, tmp_path: pathlib.Path ): # GIVEN caplog.set_level(INFO) diff --git a/test/openjd/adaptor_runtime/unit/background/test_backend_runner.py b/test/openjd/adaptor_runtime/unit/background/test_backend_runner.py index c4788b7..127a1c6 100644 --- a/test/openjd/adaptor_runtime/unit/background/test_backend_runner.py +++ b/test/openjd/adaptor_runtime/unit/background/test_backend_runner.py @@ -23,7 +23,7 @@ class TestBackendRunner: @pytest.fixture(autouse=True) def socket_path(self, tmp_path: pathlib.Path) -> Generator[str, None, None]: if OSName.is_posix(): - with patch.object(backend_runner.SocketDirectories, "get_process_socket_path") as mock: + with patch.object(backend_runner.SocketPaths, "get_process_socket_path") as mock: path = os.path.join(tmp_path, "socket", "1234") mock.return_value = path @@ -67,10 +67,10 @@ def test_run( ): # GIVEN caplog.set_level("DEBUG") - conn_file_path = "/path/to/conn_file" + conn_dir = "/path/to/conn_dir" connection_settings = {"socket": socket_path} adaptor_runner = Mock() - runner = BackendRunner(adaptor_runner, conn_file_path) + runner = BackendRunner(adaptor_runner, working_dir=conn_dir) # WHEN open_mock: MagicMock @@ -93,6 +93,7 @@ def test_run( ) mock_thread.assert_called_once() mock_thread.return_value.start.assert_called_once() + conn_file_path = f"{conn_dir}/connection.json" open_mock.assert_called_once_with(conn_file_path, open_mode="w") mock_json_dump.assert_called_once_with( ConnectionSettings(socket_path), @@ -114,7 +115,7 @@ def test_run_raises_when_http_server_fails_to_start( caplog.set_level("DEBUG") exc = Exception() mock_server_cls.side_effect = exc - runner = BackendRunner(Mock(), "") + runner = BackendRunner(Mock(), connection_file_path="/tmp/connection.json") # WHEN with pytest.raises(Exception) as raised_exc: @@ -144,9 +145,9 @@ def test_run_raises_when_writing_connection_file_fails( caplog.set_level("DEBUG") err = OSError() open_mock.side_effect = err - conn_file_path = "/path/to/conn_file" + conn_dir = "/path/to/conn_dir" adaptor_runner = Mock() - runner = BackendRunner(adaptor_runner, conn_file_path) + runner = BackendRunner(adaptor_runner, working_dir=conn_dir) # WHEN with pytest.raises(OSError) as raised_err: @@ -164,6 +165,7 @@ def test_run_raises_when_writing_connection_file_fails( ] mock_thread.assert_called_once() mock_thread.return_value.start.assert_called_once() + conn_file_path = f"{conn_dir}/connection.json" open_mock.assert_called_once_with(conn_file_path, open_mode="w") mock_thread.return_value.join.assert_called_once() if OSName.is_posix(): @@ -180,7 +182,7 @@ def test_signal_hook(self, mock_submit, signal_mock: MagicMock) -> None: # GIVEN conn_file_path = "/path/to/conn_file" adaptor_runner = Mock() - runner = BackendRunner(adaptor_runner, conn_file_path) + runner = BackendRunner(adaptor_runner, connection_file_path=conn_file_path) server_mock = MagicMock() submit_mock = MagicMock() server_mock.submit = submit_mock diff --git a/test/openjd/adaptor_runtime/unit/background/test_frontend_runner.py b/test/openjd/adaptor_runtime/unit/background/test_frontend_runner.py index 1f2ecc5..2da4261 100644 --- a/test/openjd/adaptor_runtime/unit/background/test_frontend_runner.py +++ b/test/openjd/adaptor_runtime/unit/background/test_frontend_runner.py @@ -93,7 +93,7 @@ def test_initializes_backend_process( conn_file_path = "/path" init_data = {"init": "data"} path_mapping_data: dict = {} - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN runner.init(adaptor_module, init_data, path_mapping_data, reentry_exe) @@ -113,24 +113,24 @@ def test_initializes_backend_process( adaptor_module.__package__, "daemon", "_serve", - "--connection-file", - conn_file_path, "--init-data", json.dumps(init_data), "--path-mapping-rules", json.dumps(path_mapping_data), + "--connection-file", + conn_file_path, ] else: expected_args = [ str(reentry_exe), "daemon", "_serve", - "--connection-file", - conn_file_path, "--init-data", json.dumps(init_data), "--path-mapping-rules", json.dumps(path_mapping_data), + "--connection-file", + conn_file_path, ] mock_Popen.assert_called_once_with( expected_args, @@ -148,7 +148,7 @@ def test_raises_when_adaptor_module_not_package(self): # GIVEN adaptor_module = ModuleType("") adaptor_module.__package__ = None - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN with pytest.raises(Exception) as raised_exc: @@ -167,7 +167,7 @@ def test_raises_when_connection_file_exists( adaptor_module = ModuleType("") adaptor_module.__package__ = "package" conn_file_path = "/path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with pytest.raises(FileExistsError) as raised_err: @@ -196,7 +196,7 @@ def test_raises_when_failed_to_create_backend_process( adaptor_module = ModuleType("") adaptor_module.__package__ = "package" conn_file_path = "/path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with pytest.raises(Exception) as raised_exc: @@ -231,7 +231,7 @@ def test_raises_when_connection_file_wait_times_out( adaptor_module = ModuleType("") adaptor_module.__package__ = "package" conn_file_path = "/path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with pytest.raises(TimeoutError) as raised_err: @@ -267,7 +267,7 @@ def test_sends_heartbeat( if OSName.is_windows(): mock_send_request.return_value = {"body": '{"key1": "value1"}'} mock_response = mock_send_request.return_value - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN response = runner._heartbeat() @@ -295,7 +295,7 @@ def test_sends_heartbeat_with_ack_id( if OSName.is_windows(): mock_send_request.return_value = {"body": '{"key1": "value1"}'} mock_response = mock_send_request.return_value - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN response = runner._heartbeat(ack_id) @@ -338,7 +338,9 @@ def test_heartbeats_until_complete( mock_event.wait = MagicMock() mock_event.is_set = MagicMock(return_value=False) heartbeat_interval = 1 - runner = FrontendRunner("", heartbeat_interval=heartbeat_interval) + runner = FrontendRunner( + connection_file_path="/tmp/connection.json", heartbeat_interval=heartbeat_interval + ) # WHEN runner._heartbeat_until_state_complete(state) @@ -367,7 +369,7 @@ def test_raises_when_adaptor_fails(self, mock_heartbeat: MagicMock) -> None: failed=False, ), ] - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN with pytest.raises(AdaptorFailedException) as raised_exc: @@ -385,7 +387,7 @@ class TestShutdown: @patch.object(FrontendRunner, "_send_request") def test_sends_shutdown(self, mock_send_request: MagicMock): # GIVEN - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN runner.shutdown() @@ -407,7 +409,7 @@ def test_sends_run( ): # GIVEN run_data = {"run": "data"} - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN runner.run(run_data) @@ -429,7 +431,7 @@ def test_sends_start( mock_heartbeat_until_state_complete: MagicMock, ): # GIVEN - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN runner.start() @@ -451,7 +453,7 @@ def test_sends_end( mock_heartbeat_until_state_complete: MagicMock, ): # GIVEN - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN runner.stop() @@ -471,7 +473,7 @@ def test_sends_cancel( mock_send_request: MagicMock, ): # GIVEN - runner = FrontendRunner("") + runner = FrontendRunner(connection_file_path="/tmp/connection.json") # WHEN runner.cancel() @@ -502,7 +504,7 @@ def test_sends_request(self, mock_request: MagicMock, mock_getresponse: MagicMoc method = "GET" path = "/path" conn_file_path = "/conn/file/path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN response = runner._send_request(method, path) @@ -529,7 +531,7 @@ def test_raises_when_request_fails( method = "GET" path = "/path" conn_file_path = "/conn/file/path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with pytest.raises(http_client.HTTPException) as raised_exc: @@ -559,7 +561,7 @@ def test_raises_when_error_response_received( method = "GET" path = "/path" conn_file_path = "/conn/file/path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with pytest.raises(HTTPError) as raised_err: @@ -585,7 +587,7 @@ def test_formats_query_string(self, mock_request: MagicMock, mock_getresponse: M path = "/path" conn_file_path = "/conn/file/path" params = {"first param": 1, "second_param": ["one", "two three"]} - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN response = runner._send_request(method, path, params=params) @@ -606,7 +608,7 @@ def test_sends_body(self, mock_request: MagicMock, mock_getresponse: MagicMock): path = "/path" conn_file_path = "/conn/file/path" json = {"the": "body"} - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN response = runner._send_request(method, path, json_body=json) @@ -648,7 +650,7 @@ def test_sends_request( path = "/path" conn_file_path = r"C:\conn\file\path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with patch.object( @@ -680,7 +682,7 @@ def test_raises_when_request_fails( method = "GET" path = "/path" conn_file_path = r"C:\conn\file\path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with patch.object( @@ -709,7 +711,7 @@ def test_raises_when_error_response_received( method = "GET" path = "/path" conn_file_path = r"C:\conn\file\path" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with patch.object( @@ -747,7 +749,7 @@ def test_formats_query_string( path = "/path" conn_file_path = r"C:\conn\file\path" params = {"first param": 1, "second_param": ["one", "two three"]} - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with patch.object( @@ -777,7 +779,7 @@ def test_sends_body( path = "/path" conn_file_path = r"C:\conn\file\path" json_body = {"the": "body"} - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN with patch.object( @@ -805,7 +807,7 @@ def test_hook(self, signal_mock: MagicMock, cancel_mock: MagicMock) -> None: # GIVEN conn_file_path = "/path/to/conn_file" - runner = FrontendRunner(conn_file_path) + runner = FrontendRunner(connection_file_path=conn_file_path) # WHEN runner._sigint_handler(MagicMock(), MagicMock()) @@ -949,7 +951,7 @@ def test_connection_settings_lazy_loads(mock_load_connection_settings: MagicMock filepath = "/path" expected = ConnectionSettings("/socket") mock_load_connection_settings.return_value = expected - runner = FrontendRunner(filepath) + runner = FrontendRunner(connection_file_path=filepath) # Assert the internal connection settings var is not set yet assert not hasattr(runner, "_connection_settings") diff --git a/test/openjd/adaptor_runtime/unit/http/test_sockets.py b/test/openjd/adaptor_runtime/unit/http/test_sockets.py index c0cb336..bf497cc 100644 --- a/test/openjd/adaptor_runtime/unit/http/test_sockets.py +++ b/test/openjd/adaptor_runtime/unit/http/test_sockets.py @@ -10,23 +10,23 @@ import openjd.adaptor_runtime._http.sockets as sockets from openjd.adaptor_runtime._http.sockets import ( - LinuxSocketDirectories, - MacOSSocketDirectories, + LinuxSocketPaths, + MacOSSocketPaths, NonvalidSocketPathException, NoSocketPathFoundException, - SocketDirectories, + SocketPaths, ) -class SocketDirectoriesStub(SocketDirectories): +class SocketPathsStub(SocketPaths): def verify_socket_path(self, path: str) -> None: pass -class TestSocketDirectories: +class TestSocketPaths: class TestGetProcessSocketPath: """ - Tests for SocketDirectories.get_process_socket_path() + Tests for SocketPaths.get_process_socket_path() """ @patch.object(sockets.os, "getpid", return_value=1234) @@ -36,7 +36,7 @@ def test_gets_path( ) -> None: # GIVEN namespace = "my-namespace" - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN result = subject.get_process_socket_path(namespace) @@ -48,7 +48,7 @@ def test_gets_path( @patch.object(sockets.os, "getpid", return_value="a" * (sockets._PID_MAX_LENGTH + 1)) def test_asserts_max_pid_length(self, mock_getpid: MagicMock): # GIVEN - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN with pytest.raises(AssertionError) as raised_err: @@ -62,7 +62,7 @@ def test_asserts_max_pid_length(self, mock_getpid: MagicMock): class TestGetSocketPath: """ - Tests for SocketDirectories.get_socket_path() + Tests for SocketPaths.get_socket_path() """ @pytest.fixture(autouse=True) @@ -100,7 +100,7 @@ def test_gets_home_dir( home_dir: str, ) -> None: # GIVEN - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN result = subject.get_socket_path("sock") @@ -110,7 +110,7 @@ def test_gets_home_dir( assert result.startswith(home_dir) @patch.object(sockets.os, "stat") - @patch.object(SocketDirectoriesStub, "verify_socket_path") + @patch.object(SocketPathsStub, "verify_socket_path") def test_gets_temp_dir( self, mock_verify_socket_path: MagicMock, @@ -122,7 +122,7 @@ def test_gets_temp_dir( exc = NonvalidSocketPathException() mock_verify_socket_path.side_effect = [exc, None] # Raise exc only once mock_stat.return_value.st_mode = stat.S_ISVTX - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN result = subject.get_socket_path("sock") @@ -144,7 +144,7 @@ def test_gets_temp_dir( ) def test_create_dir(self, mock_makedirs: MagicMock, create: bool) -> None: # GIVEN - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN result = subject.get_socket_path("sock", create_dir=create) @@ -160,7 +160,7 @@ def test_create_dir(self, mock_makedirs: MagicMock, create: bool) -> None: def test_uses_namespace(self) -> None: # GIVEN namespace = "my-namespace" - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN result = subject.get_socket_path("sock", namespace) @@ -168,11 +168,11 @@ def test_uses_namespace(self) -> None: # THEN assert os.path.dirname(result).endswith(namespace) - @patch.object(SocketDirectoriesStub, "verify_socket_path") + @patch.object(SocketPathsStub, "verify_socket_path") def test_raises_when_no_valid_path_found(self, mock_verify_socket_path: MagicMock) -> None: # GIVEN mock_verify_socket_path.side_effect = NonvalidSocketPathException() - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN with pytest.raises(NoSocketPathFoundException) as raised_exc: @@ -180,12 +180,11 @@ def test_raises_when_no_valid_path_found(self, mock_verify_socket_path: MagicMoc # THEN assert raised_exc.match( - "Failed to find a suitable base directory to create sockets in for the following " - "reasons: " + "Failed to find a suitable socket path for the following reasons: " ) assert mock_verify_socket_path.call_count == 2 - @patch.object(SocketDirectoriesStub, "verify_socket_path") + @patch.object(SocketPathsStub, "verify_socket_path") @patch.object(sockets.os, "stat") def test_raises_when_no_tmpdir_sticky_bit( self, @@ -196,7 +195,7 @@ def test_raises_when_no_tmpdir_sticky_bit( # GIVEN mock_verify_socket_path.side_effect = [NonvalidSocketPathException(), None] mock_stat.return_value.st_mode = 0 - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN with pytest.raises(NoSocketPathFoundException) as raised_exc: @@ -219,7 +218,7 @@ def test_handles_socket_name_collisions( sock_name = "sock" existing_sock_names = [sock_name, f"{sock_name}_1", f"{sock_name}_2"] mock_exists.side_effect = ([True] * len(existing_sock_names)) + [False] - subject = SocketDirectoriesStub() + subject = SocketPathsStub() # WHEN result = subject.get_socket_path(sock_name) @@ -229,22 +228,21 @@ def test_handles_socket_name_collisions( mock_exists.call_count == len(existing_sock_names) + 1 -class TestLinuxSocketDirectories: +class TestLinuxSocketPaths: @pytest.mark.parametrize( argnames=["path"], argvalues=[ ["a"], - ["a" * 100], + ["a" * 107], ], - ids=["one byte", "100 bytes"], + ids=["one byte", "107 bytes"], ) - def test_accepts_paths_within_100_bytes(self, path: str): + def test_accepts_paths_within_107_bytes(self, path: str): """ - Verifies the function accepts paths up to 100 bytes (108 byte max - 8 byte padding - for socket name portion (path sep + PID)) + Verifies the function accepts paths up to 100 bytes (108 byte max - 1 byte null terminator) """ # GIVEN - subject = LinuxSocketDirectories() + subject = LinuxSocketPaths() try: # WHEN @@ -255,11 +253,11 @@ def test_accepts_paths_within_100_bytes(self, path: str): # THEN pass # success - def test_rejects_paths_over_100_bytes(self): + def test_rejects_paths_over_107_bytes(self): # GIVEN - length = 101 + length = 108 path = "a" * length - subject = LinuxSocketDirectories() + subject = LinuxSocketPaths() # WHEN with pytest.raises(NonvalidSocketPathException) as raised_exc: @@ -267,28 +265,27 @@ def test_rejects_paths_over_100_bytes(self): # THEN assert raised_exc.match( - "Socket base directory path too big. The maximum allowed size is " - f"{subject._socket_dir_max_length} bytes, but the directory has a size of " + "Socket name too long. The maximum allowed size is " + f"{subject._socket_name_max_length} bytes, but the name has a size of " f"{length}: {path}" ) -class TestMacOSSocketDirectories: +class TestMacOSSocketPaths: @pytest.mark.parametrize( argnames=["path"], argvalues=[ ["a"], - ["a" * 96], + ["a" * 103], ], - ids=["one byte", "96 bytes"], + ids=["one byte", "103 bytes"], ) - def test_accepts_paths_within_100_bytes(self, path: str): + def test_accepts_paths_within_103_bytes(self, path: str): """ - Verifies the function accepts paths up to 96 bytes (104 byte max - 8 byte padding - for socket name portion (path sep + PID)) + Verifies the function accepts paths up to 103 bytes (104 byte max - 1 byte null terminator) """ # GIVEN - subject = MacOSSocketDirectories() + subject = MacOSSocketPaths() try: # WHEN @@ -299,11 +296,11 @@ def test_accepts_paths_within_100_bytes(self, path: str): # THEN pass # success - def test_rejects_paths_over_96_bytes(self): + def test_rejects_paths_over_103_bytes(self): # GIVEN - length = 97 + length = 104 path = "a" * length - subject = MacOSSocketDirectories() + subject = MacOSSocketPaths() # WHEN with pytest.raises(NonvalidSocketPathException) as raised_exc: @@ -311,7 +308,7 @@ def test_rejects_paths_over_96_bytes(self): # THEN assert raised_exc.match( - "Socket base directory path too big. The maximum allowed size is " - f"{subject._socket_dir_max_length} bytes, but the directory has a size of " + "Socket name too long. The maximum allowed size is " + f"{subject._socket_name_max_length} bytes, but the name has a size of " f"{length}: {path}" ) diff --git a/test/openjd/adaptor_runtime/unit/test_entrypoint.py b/test/openjd/adaptor_runtime/unit/test_entrypoint.py index 8ae766a..e238300 100644 --- a/test/openjd/adaptor_runtime/unit/test_entrypoint.py +++ b/test/openjd/adaptor_runtime/unit/test_entrypoint.py @@ -214,7 +214,14 @@ def test_creates_adaptor_with_init_data(self, mock_adaptor_cls: MagicMock): # GIVEN init_data = {"init": "data"} with patch.object( - runtime_entrypoint.sys, "argv", ["Adaptor", "run", "--init-data", json.dumps(init_data)] + runtime_entrypoint.sys, + "argv", + [ + "Adaptor", + "run", + "--init-data", + json.dumps(init_data), + ], ): entrypoint = EntryPoint(mock_adaptor_cls) @@ -487,7 +494,8 @@ def test_runs_background_serve( ) mock_init.assert_called_once_with( mock_adaptor_runner.return_value, - conn_file, + connection_file_path=conn_file, + working_dir=None, log_buffer=mock_log_buffer.return_value, ) mock_run.assert_called_once() @@ -555,7 +563,7 @@ def test_background_start_raises_when_adaptor_module_not_loaded( # THEN assert raised_err.match(f"Adaptor module is not loaded: {FakeAdaptor.__module__}") - mock_magic_init.assert_called_once_with(conn_file) + mock_magic_init.assert_called_once_with(connection_file_path=conn_file, working_dir=None) @pytest.mark.parametrize( argnames="reentry_exe", @@ -598,7 +606,7 @@ def test_runs_background_start( # THEN mock_magic_init.assert_called_once_with(mock_adaptor_module, {}, {}, reentry_exe) - mock_magic_start.assert_called_once_with(conn_file) + mock_magic_start.assert_called_once_with(connection_file_path=conn_file, working_dir=None) mock_start.assert_called_once_with() @patch.object(FrontendRunner, "__init__", return_value=None) @@ -629,7 +637,7 @@ def test_runs_background_stop( entrypoint.start() # THEN - mock_magic_init.assert_called_once_with(conn_file) + mock_magic_init.assert_called_once_with(connection_file_path=conn_file, working_dir=None) mock_end.assert_called_once() mock_shutdown.assert_called_once_with() @@ -662,7 +670,7 @@ def test_runs_background_run( entrypoint.start() # THEN - mock_magic_init.assert_called_once_with(conn_file) + mock_magic_init.assert_called_once_with(connection_file_path=conn_file, working_dir=None) mock_run.assert_called_once_with(run_data) @patch.object(FrontendRunner, "__init__", return_value=None) @@ -729,7 +737,9 @@ def test_makes_connection_file_path_absolute( # THEN mock_isabs.assert_any_call(conn_file) mock_abspath.assert_any_call(conn_file) - mock_runner.assert_called_once_with(mock_abspath.return_value) + mock_runner.assert_called_once_with( + connection_file_path=mock_abspath.return_value, working_dir=None + ) class TestLoadData: diff --git a/test/openjd/adaptor_runtime_client/integ/test_integration_client_interface.py b/test/openjd/adaptor_runtime_client/integ/test_integration_client_interface.py index 6fc2bd1..1dd0940 100644 --- a/test/openjd/adaptor_runtime_client/integ/test_integration_client_interface.py +++ b/test/openjd/adaptor_runtime_client/integ/test_integration_client_interface.py @@ -53,7 +53,7 @@ def test_graceful_shutdown(self) -> None: # Ensure the process actually shutdown assert client_subprocess.returncode is not None - def test_client_in_thread_does_not_do_graceful_shutdown(self): + def test_client_in_thread_does_not_do_graceful_shutdown(self) -> None: """Ensures that a client running in a thread does not crash by attempting to register a signal, since they can only be created in the main thread. This means the graceful shutdown is effectively ignored."""