From 0fb2b319a07052a140086c75e8dbca3c66b62969 Mon Sep 17 00:00:00 2001 From: marcosf2 Date: Fri, 30 May 2025 16:51:10 -0500 Subject: [PATCH 1/4] adds tests to internal networking code --- tests/pytest/test_internal_network.py | 99 +++++++++++++++++++++++++++ tests/pytest/test_network_config.toml | 23 +++++++ 2 files changed, 122 insertions(+) create mode 100644 tests/pytest/test_internal_network.py create mode 100644 tests/pytest/test_network_config.toml diff --git a/tests/pytest/test_internal_network.py b/tests/pytest/test_internal_network.py new file mode 100644 index 00000000..3b5c74a1 --- /dev/null +++ b/tests/pytest/test_internal_network.py @@ -0,0 +1,99 @@ +import shutil +import subprocess +from pathlib import Path + +import pytest + +from pqnstack.network.client import Client +from pqnstack.network.client import ProxyInstrument +from pqnstack.network.packet import Packet +from pqnstack.network.packet import PacketIntent +from pqnstack.pqn.drivers.dummies import DummyInstrument + + +@pytest.fixture +def messaging_services(): + config_path = Path("./tests/pytest/test_network_config.toml").resolve() + uv_path = shutil.which("uv") + if not uv_path: + msg = "Could not find 'uv' executable in PATH" + raise RuntimeError(msg) + + router_process = subprocess.Popen( # noqa: S603 + [uv_path, "run", "pqn", "start-router", "--config", str(config_path)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=False, + ) + + node_process = subprocess.Popen( # noqa: S603 + [uv_path, "run", "pqn", "start-node", "--config", str(config_path)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + shell=False, + ) + + yield + + router_process.terminate() + router_process.wait() + + node_process.terminate() + node_process.wait() + + +def test_client_ping(messaging_services): # noqa: ARG001 + client = Client(host="localhost", port=5556, router_name="pqnstack-router") + response = client.ping("pqnstack-node") + + assert isinstance(response, Packet) + assert response.intent == PacketIntent.PING + assert response.source == "pqnstack-node" + assert response.destination == client.name + assert response.request == "PONG" + + +def test_getting_all_instruments(messaging_services): # noqa: ARG001 + client = Client(host="localhost", port=5556, router_name="pqnstack-router") + + response = client.get_available_devices("pqnstack-node") + + instruments_names = ["dummy1", "dummy2"] + + assert instruments_names == list(response.keys()) + # Get available devices returns the __class__ of the instrument as the value. + assert isinstance(response["dummy1"], DummyInstrument.__class__) + assert isinstance(response["dummy2"], DummyInstrument.__class__) + + +def test_proxy_instrument(messaging_services): # noqa: ARG001 + client = Client(host="localhost", port=5556, router_name="pqnstack-router") + + proxy_instrument = client.get_device("pqnstack-node", "dummy1") + assert isinstance(proxy_instrument, ProxyInstrument) + + base_int = 2 + double_int = base_int * 2 + arbitrary_int = 12 + + assert proxy_instrument.name == "dummy1" + assert proxy_instrument.param_int == base_int + assert proxy_instrument.param_str == "hello" + + assert proxy_instrument.double_int() == double_int + assert proxy_instrument.param_int == double_int + + proxy_instrument.param_int = arbitrary_int + assert proxy_instrument.param_int == arbitrary_int + + assert proxy_instrument.uppercase_str() == "HELLO" + assert proxy_instrument.param_str == "HELLO" + + # Make sure you cannot add attributes to the proxy instrument + fail_flag = False + try: + proxy_instrument.new_attr = 42 + except AttributeError: + fail_flag = True + + assert fail_flag, "Should not be able to set new attributes on the ProxyInstrument" diff --git a/tests/pytest/test_network_config.toml b/tests/pytest/test_network_config.toml new file mode 100644 index 00000000..e99c2be3 --- /dev/null +++ b/tests/pytest/test_network_config.toml @@ -0,0 +1,23 @@ +[router] +name = "pqnstack-router" +host = "localhost" +port = 5556 + +[node] +name = "pqnstack-node" +router_name = "pqnstack-router" +host = "localhost" +port = 5556 +beat_period = 2000 + +[[node.instruments]] +name = "dummy1" +import = "pqnstack.pqn.drivers.dummies.DummyInstrument" +desc = "Dummy instrument1 for testing purposes" +address = "1234" + +[[node.instruments]] +name = "dummy2" +import = "pqnstack.pqn.drivers.dummies.DummyInstrument" +desc = "Dummy instrument2 for testing purposes" +address = "1234" \ No newline at end of file From 1b2d7e5607d1b2b3f8d513ed17298b9861a36582 Mon Sep 17 00:00:00 2001 From: marcosf2 Date: Mon, 2 Jun 2025 18:12:36 -0500 Subject: [PATCH 2/4] Renames node to provider --- docs/routing.md | 8 +-- src/pqnstack/cli.py | 56 +++++++++---------- src/pqnstack/network/client.py | 26 ++++----- .../{node.py => instrument_provider.py} | 30 +++++----- src/pqnstack/network/packet.py | 2 +- src/pqnstack/network/router.py | 20 +++---- tests/messaging/blocking_client.py | 6 +- tests/messaging/client.py | 8 +-- tests/messaging/config_example.toml | 8 +-- tests/messaging/{node.py => provider.py} | 6 +- tests/pytest/test_internal_network.py | 18 +++--- tests/pytest/test_network_config.toml | 8 +-- 12 files changed, 94 insertions(+), 102 deletions(-) rename src/pqnstack/network/{node.py => instrument_provider.py} (94%) rename tests/messaging/{node.py => provider.py} (62%) diff --git a/docs/routing.md b/docs/routing.md index 85966cc7..b7cf6451 100644 --- a/docs/routing.md +++ b/docs/routing.md @@ -29,13 +29,13 @@ All network elements implement a `dispatch` function that depends on the content ensures that routing information is adequately interpreted, and that the appropriate protocol is performed by the desired node or group of nodes. The sequence of events in this case is as follows: -1. **Node A** sends a packet to the router +1. **InstrumentProvider A** sends a packet to the router 2. The router determines the validity of the sender and the receiver 3. The router extracts the signature of the packet, and separates the implementation into data, control and routing backplane functions -4. The router sends the packet to **Node B** -5. **Node B** executes code implementing classical or quantum functions -6. **Node B** prepares a response packet with `(source, destination)` being reversed from the original request +4. The router sends the packet to **InstrumentProvider B** +5. **InstrumentProvider B** executes code implementing classical or quantum functions +6. **InstrumentProvider B** prepares a response packet with `(source, destination)` being reversed from the original request 7. The router proceeds repeats steps 2-4 8. If the packet `hops` value is zero, no further routing happens diff --git a/src/pqnstack/cli.py b/src/pqnstack/cli.py index 0040b996..c8038008 100644 --- a/src/pqnstack/cli.py +++ b/src/pqnstack/cli.py @@ -7,7 +7,7 @@ import typer from pqnstack.base.errors import InvalidNetworkConfigurationError -from pqnstack.network.node import Node +from pqnstack.network.instrument_provider import InstrumentProvider from pqnstack.network.router import Router # TODO: check if this way of handling logging from a command line script is ok. @@ -40,51 +40,51 @@ def _verify_instruments_config(instruments: list[dict[str, str]]) -> dict[str, d return ins -def _load_and_parse_node_config( +def _load_and_parse_provider_config( config_path: Path | str, kwargs: dict[str, str | int], instruments: dict[str, dict[str, str]] ) -> tuple[dict[str, str | int], dict[str, dict[str, str]]]: path = Path(config_path) with path.open("rb") as f: config = tomllib.load(f) - if "node" not in config: + if "provider" not in config: msg = ( - f"Config file {config_path} does not contain a node section. Add node configuration under '[node]' section." + f"Config file {config_path} does not contain a provider section. Add provider configuration under '[provider]' section." ) raise InvalidNetworkConfigurationError(msg) - node = config["node"] - if "name" in node: - kwargs["name"] = str(node["name"]) - if "router_name" in node: - kwargs["router_name"] = str(node["router_name"]) - if "host" in node: - kwargs["host"] = str(node["host"]) - if "port" in node: - kwargs["port"] = int(node["port"]) - if "beat_period" in node: - kwargs["beat_period"] = int(node["beat_period"]) - - if "instruments" in node: - instruments = _verify_instruments_config(node["instruments"]) + provider = config["provider"] + if "name" in provider: + kwargs["name"] = str(provider["name"]) + if "router_name" in provider: + kwargs["router_name"] = str(provider["router_name"]) + if "host" in provider: + kwargs["host"] = str(provider["host"]) + if "port" in provider: + kwargs["port"] = int(provider["port"]) + if "beat_period" in provider: + kwargs["beat_period"] = int(provider["beat_period"]) + + if "instruments" in provider: + instruments = _verify_instruments_config(provider["instruments"]) return kwargs, instruments @app.command() -def start_node( # noqa: PLR0913 - name: Annotated[str | None, typer.Option(help="Name of the Node.")] = None, +def start_provider( # noqa: PLR0913 + name: Annotated[str | None, typer.Option(help="Name of the InstrumentProvider.")] = None, router_name: Annotated[ - str | None, typer.Option(help="Name of the router this node will talk to (default: 'router1').") + str | None, typer.Option(help="Name of the router this provider will talk to (default: 'router1').") ] = None, host: Annotated[ str | None, typer.Option( - help="Host address (IP) of the node (default: 'localhost'). Usually the IP address of the Router this node will talk to." + help="Host address (IP) of the provider (default: 'localhost'). Usually the IP address of the Router this provider will talk to." ), ] = None, port: Annotated[ - int | None, typer.Option(help="Port of the node (default: 5555). Has to be the same port as the Router.") + int | None, typer.Option(help="Port of the provider (default: 5555). Has to be the same port as the Router.") ] = None, beat_period: Annotated[int | None, typer.Option(help="Heartbeat period in milliseconds (default: 1000)")] = None, instruments: Annotated[ @@ -98,7 +98,7 @@ def start_node( # noqa: PLR0913 ] = None, ) -> None: """ - Start a PQN Node. + Start a PQN InstrumentProvider. Can be configured by passing arguments directly into the command line but it is recommended to use a config file if instruments will be added. """ @@ -106,7 +106,7 @@ def start_node( # noqa: PLR0913 ins: dict[str, dict[str, str]] = {} if config: - kwargs, ins = _load_and_parse_node_config(config, kwargs, ins) + kwargs, ins = _load_and_parse_provider_config(config, kwargs, ins) if name: kwargs["name"] = name @@ -123,11 +123,11 @@ def start_node( # noqa: PLR0913 ins = {**ins, **json.loads(instruments)} if "name" not in kwargs: - msg = "Node name is required" + msg = "InstrumentProvider name is required" raise InvalidNetworkConfigurationError(msg) - node = Node(**kwargs, **ins) # type: ignore[arg-type] - node.start() + provider = InstrumentProvider(**kwargs, **ins) # type: ignore[arg-type] + provider.start() def _load_and_parse_router_config(config_path: Path | str, kwargs: dict[str, str | int]) -> dict[str, str | int]: diff --git a/src/pqnstack/network/client.py b/src/pqnstack/network/client.py index 2fa844eb..3ed23d54 100644 --- a/src/pqnstack/network/client.py +++ b/src/pqnstack/network/client.py @@ -148,7 +148,7 @@ class InstrumentClientInit(NamedTuple): router_name: str timeout: int instrument_name: str - node_name: str + provider_name: str class InstrumentClient(ClientBase): @@ -158,11 +158,11 @@ def __init__(self, init_args: InstrumentClientInit) -> None: ) self.instrument_name = init_args.instrument_name - self.node_name = init_args.node_name + self.provider_name = init_args.provider_name def trigger_operation(self, operation: str, *args: Any, **kwargs: Any) -> Any: packet = self.create_control_packet( - self.node_name, self.instrument_name + ":OPERATION:" + operation, (args, kwargs) + self.provider_name, self.instrument_name + ":OPERATION:" + operation, (args, kwargs) ) response = self.ask(packet) @@ -170,14 +170,14 @@ def trigger_operation(self, operation: str, *args: Any, **kwargs: Any) -> Any: def trigger_parameter(self, parameter: str, *args: Any, **kwargs: Any) -> Any: packet = self.create_control_packet( - self.node_name, self.instrument_name + ":PARAMETER:" + parameter, (args, kwargs) + self.provider_name, self.instrument_name + ":PARAMETER:" + parameter, (args, kwargs) ) response = self.ask(packet) return response.payload def get_info(self) -> DeviceInfo: - packet = self.create_control_packet(self.node_name, self.instrument_name + ":INFO:", ((), {})) + packet = self.create_control_packet(self.provider_name, self.instrument_name + ":INFO:", ((), {})) response = self.ask(packet) if not isinstance(response.payload, DeviceInfo): @@ -194,7 +194,7 @@ class ProxyInstrumentInit(NamedTuple): router_name: str instrument_name: str timeout: int - node_name: str + provider_name: str desc: str address: str parameters: set[str] @@ -219,7 +219,7 @@ def __init__(self, init_args: ProxyInstrumentInit) -> None: self.parameters = init_args.parameters self.operations = init_args.operations - self.node_name = init_args.node_name + self.provider_name = init_args.provider_name self.router_name = init_args.router_name # The client's name is the instrument name with "_client" appended and a random 6 character string appended. @@ -236,7 +236,7 @@ def __init__(self, init_args: ProxyInstrumentInit) -> None: router_name=self.router_name, timeout=self.timeout, instrument_name=self.name, - node_name=self.node_name, + provider_name=self.provider_name, ) self.client = InstrumentClient(instrument_client_init) @@ -278,8 +278,8 @@ def ping(self, destination: str) -> Packet | None: ) return self.ask(ping_packet) - def get_available_devices(self, node_name: str) -> dict[str, str]: - packet = self.create_data_packet(node_name, "GET_DEVICES", None) + def get_available_devices(self, provider_name: str) -> dict[str, str]: + packet = self.create_data_packet(provider_name, "GET_DEVICES", None) response = self.ask(packet) if not isinstance(response.payload, dict): @@ -288,8 +288,8 @@ def get_available_devices(self, node_name: str) -> dict[str, str]: return response.payload - def get_device(self, node_name: str, device_name: str) -> DeviceDriver: - packet = self.create_data_packet(node_name, "GET_DEVICE_STRUCTURE", device_name) + def get_device(self, provider_name: str, device_name: str) -> DeviceDriver: + packet = self.create_data_packet(provider_name, "GET_DEVICE_STRUCTURE", device_name) response = self.ask(packet) @@ -309,7 +309,7 @@ def get_device(self, node_name: str, device_name: str) -> DeviceDriver: router_name=self.router_name, timeout=self.timeout, instrument_name=response.payload["name"], - node_name=node_name, + provider_name=provider_name, parameters=set(response.payload["parameters"]), operations=response.payload["operations"], ) diff --git a/src/pqnstack/network/node.py b/src/pqnstack/network/instrument_provider.py similarity index 94% rename from src/pqnstack/network/node.py rename to src/pqnstack/network/instrument_provider.py index b716c08d..36a912a6 100644 --- a/src/pqnstack/network/node.py +++ b/src/pqnstack/network/instrument_provider.py @@ -1,7 +1,3 @@ -# University of Illinois Urbana-Champaign -# Public Quantum Network -# -# NCSA/Illinois Computes import datetime import importlib import logging @@ -21,7 +17,7 @@ logger = logging.getLogger(__name__) -class Node: +class InstrumentProvider: def __init__( self, name: str, @@ -32,20 +28,20 @@ def __init__( **instruments: dict[str, Any], ) -> None: """ - Node class for PQN. + InstrumentProvider class for PQN. - A Node is the class that talks with real hardware and performs experiments. It talks to a + A InstrumentProvider is the class that talks with real hardware and performs experiments. It talks to a single `Router` instance through zqm and awaits for instructions from it. Every `beat_interval` milliseconds, sends a registration packet to the router. - This is done so if the router goes offline, the node can reconnect to the router automatically. + This is done so if the router goes offline, the provider can reconnect to the router automatically. - :param name: Name for the Node. - :param host: Hostname or IP address of the Router this node talks to. - :param port: Port of the name of the Router this node talks to. - :param router_name: Name of the Router this node talks to. + :param name: Name for the InstrumentProvider. + :param host: Hostname or IP address of the Router this provider talks to. + :param port: Port of the name of the Router this provider talks to. + :param router_name: Name of the Router this provider talks to. :param beat_period: Interval in milliseconds to send a beat to the Router. :param instruments: Instruments is a Dictionary holding the necessary instructions to initialize any hardware - the Node talks to. The keys are the names of the instruments, every key has another dictionary as its value + the InstrumentProvider talks to. The keys are the names of the instruments, every key has another dictionary as its value with all the necessary instructions to initialize the instrument. Inside of the dictionary for the specific instrument, a key called 'import' is required holding the import path for that specific instrument. Note that the name is not necessary since that is the key of the dictionary. @@ -127,7 +123,7 @@ def instantiate_instruments(self) -> None: def start(self) -> None: self.instantiate_instruments() - logger.info("Starting node %s at %s", self.name, self.address) + logger.info("Starting provider %s at %s", self.name, self.address) self.context = zmq.Context() self.socket = self.context.socket(zmq.DEALER) self.socket.setsockopt_string(zmq.IDENTITY, self.name) @@ -227,7 +223,7 @@ def _beat(self) -> None: self.socket.connect(self.address) reg_packet = create_registration_packet( - source=self.name, destination=self.router_name, payload=NetworkElementClass.NODE, hops=0 + source=self.name, destination=self.router_name, payload=NetworkElementClass.PROVIDER, hops=0 ) self.socket.send(pickle.dumps(reg_packet)) logger.info("Sent registration packet to router at %s", self.address) @@ -237,7 +233,7 @@ def _beat(self) -> None: logger.warning("Error while sending beat to router at %s", self.address) def _handle_reg_acknowledge(self) -> None: - logger.info("Node %s is connected to router at %s", self.name, self.address) + logger.info("InstrumentProvider %s is connected to router at %s", self.name, self.address) self.running = True self._beats_since_reply = 0 self._last_received_beat = datetime.datetime.now(tz=datetime.UTC) @@ -384,7 +380,7 @@ def _handle_instrument_control(self, packet: Packet) -> Packet: return self._create_control_packet(packet.source, f"{ins_name}:INFO", instrument.info()) # All the possible packet options should have been handled by now, so if we get here, something went wrong. - msg = f"Something inside node {self.name} went wrong. Check that your packet is correct and try again." + msg = f"Something inside provider {self.name} went wrong. Check that your packet is correct and try again." return self._create_error_packet(packet.source, msg) def _create_error_packet(self, destination: str, error_msg: str) -> Packet: diff --git a/src/pqnstack/network/packet.py b/src/pqnstack/network/packet.py index 15a82b77..c6b63927 100644 --- a/src/pqnstack/network/packet.py +++ b/src/pqnstack/network/packet.py @@ -14,7 +14,7 @@ class NetworkElementClass(Enum): ROUTER = auto() - NODE = auto() + PROVIDER = auto() CLIENT = auto() TELEMETRY = auto() diff --git a/src/pqnstack/network/router.py b/src/pqnstack/network/router.py index 6caad819..dca0f810 100644 --- a/src/pqnstack/network/router.py +++ b/src/pqnstack/network/router.py @@ -1,7 +1,3 @@ -# University of Illinois Urbana-Champaign -# Public Quantum Network -# -# NCSA/Illinois Computes import copy import logging import pickle @@ -27,7 +23,7 @@ def __init__(self, name: str, host: str = "localhost", port: int = 5555) -> None # FIXME, breaking this into 3 different dictionaries is probably not the way to go. self.routers: dict[str, bytes] = {} # Holds what other routers are in the network - self.nodes: dict[str, bytes] = {} + self.providers: dict[str, bytes] = {} self.clients: dict[str, bytes] = {} self.context: zmq.Context[zmq.Socket[bytes]] | None = None @@ -65,9 +61,9 @@ def handle_registration(self, identity_binary: bytes, packet: Packet) -> None: self.handle_packet_error(identity_binary, f"Router {self.name} is not the destination") return match packet.payload: - case NetworkElementClass.NODE: - self.nodes[packet.source] = identity_binary - logger.info("Node %s registered", identity_binary) + case NetworkElementClass.PROVIDER: + self.providers[packet.source] = identity_binary + logger.info("InstrumentProvider %s registered", identity_binary) case NetworkElementClass.CLIENT: self.clients[packet.source] = identity_binary logger.info("Client %s registered", identity_binary) @@ -90,11 +86,11 @@ def handle_pass_packet(self, identity_binary: bytes, packet: Packet) -> None: if packet.destination == self.name: logger.info("Packet destination is self, dropping") - elif packet.destination in self.nodes or packet.destination in self.clients: - logger.info("Packet destination is a node called %s, routing message there", packet.destination) + elif packet.destination in self.providers or packet.destination in self.clients: + logger.info("Packet destination is a provider called %s, routing message there", packet.destination) forward_packet = copy.copy(packet) forward_packet.hops += 1 - dest = self.nodes.get(packet.destination) or self.clients.get(packet.destination) + dest = self.providers.get(packet.destination) or self.clients.get(packet.destination) if dest is None: self.handle_packet_error(identity_binary, f"Destination {packet.destination} not found.") return @@ -102,7 +98,7 @@ def handle_pass_packet(self, identity_binary: bytes, packet: Packet) -> None: logger.info("Sent packet to %s", packet.destination) else: - logger.info("Packet destination is not a node will ask other routers in system") + logger.info("Packet destination is not a provider will ask other routers in system") # FIXME: This is temporary and should be replaced with the routing algorithm. self.handle_packet_error(identity_binary, "Routing not implemented yet.") diff --git a/tests/messaging/blocking_client.py b/tests/messaging/blocking_client.py index 68581b8a..db89f7e3 100644 --- a/tests/messaging/blocking_client.py +++ b/tests/messaging/blocking_client.py @@ -9,11 +9,11 @@ if __name__ == "__main__": c = Client() - # ping node - ping_reply = c.ping("node1") + # ping provider + ping_reply = c.ping("provider1") logger.info(ping_reply) - instrument = c.get_device("node1", "dummy1") + instrument = c.get_device("provider1", "dummy1") logger.info(instrument) # blocking operation diff --git a/tests/messaging/client.py b/tests/messaging/client.py index c65348cf..0c00d935 100644 --- a/tests/messaging/client.py +++ b/tests/messaging/client.py @@ -9,15 +9,15 @@ if __name__ == "__main__": c = Client() - # ping node - ping_reply = c.ping("node1") + # ping provider + ping_reply = c.ping("provider1") logger.info(ping_reply) - devices = c.get_available_devices("node1") + devices = c.get_available_devices("provider1") logger.info(devices) # Create instrument proxy - instrument = c.get_device("node1", "dummy1") + instrument = c.get_device("provider1", "dummy1") logger.info(instrument) logger.info("I should have the proxy object here: %s", type(instrument)) diff --git a/tests/messaging/config_example.toml b/tests/messaging/config_example.toml index e99c2be3..28566e73 100644 --- a/tests/messaging/config_example.toml +++ b/tests/messaging/config_example.toml @@ -3,20 +3,20 @@ name = "pqnstack-router" host = "localhost" port = 5556 -[node] -name = "pqnstack-node" +[provider] +name = "pqnstack-provider" router_name = "pqnstack-router" host = "localhost" port = 5556 beat_period = 2000 -[[node.instruments]] +[[provider.instruments]] name = "dummy1" import = "pqnstack.pqn.drivers.dummies.DummyInstrument" desc = "Dummy instrument1 for testing purposes" address = "1234" -[[node.instruments]] +[[provider.instruments]] name = "dummy2" import = "pqnstack.pqn.drivers.dummies.DummyInstrument" desc = "Dummy instrument2 for testing purposes" diff --git a/tests/messaging/node.py b/tests/messaging/provider.py similarity index 62% rename from tests/messaging/node.py rename to tests/messaging/provider.py index 4b2e9d17..2189bb51 100644 --- a/tests/messaging/node.py +++ b/tests/messaging/provider.py @@ -1,6 +1,6 @@ import logging -from pqnstack.network.node import Node +from pqnstack.network.instrument_provider import InstrumentProvider logging.basicConfig(level=logging.INFO) @@ -12,5 +12,5 @@ "address": "123456", } } - node = Node("node1", "127.0.0.1", 5555, **instruments) - node.start() + provider = InstrumentProvider("provider1", "127.0.0.1", 5555, **instruments) + provider.start() diff --git a/tests/pytest/test_internal_network.py b/tests/pytest/test_internal_network.py index 3b5c74a1..1bb1119f 100644 --- a/tests/pytest/test_internal_network.py +++ b/tests/pytest/test_internal_network.py @@ -13,7 +13,7 @@ @pytest.fixture def messaging_services(): - config_path = Path("./tests/pytest/test_network_config.toml").resolve() + config_path = Path("./test_network_config.toml").resolve() uv_path = shutil.which("uv") if not uv_path: msg = "Could not find 'uv' executable in PATH" @@ -26,8 +26,8 @@ def messaging_services(): shell=False, ) - node_process = subprocess.Popen( # noqa: S603 - [uv_path, "run", "pqn", "start-node", "--config", str(config_path)], + provider_process = subprocess.Popen( # noqa: S603 + [uv_path, "run", "pqn", "start-provider", "--config", str(config_path)], stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False, @@ -38,17 +38,17 @@ def messaging_services(): router_process.terminate() router_process.wait() - node_process.terminate() - node_process.wait() + provider_process.terminate() + provider_process.wait() def test_client_ping(messaging_services): # noqa: ARG001 client = Client(host="localhost", port=5556, router_name="pqnstack-router") - response = client.ping("pqnstack-node") + response = client.ping("pqnstack-provider") assert isinstance(response, Packet) assert response.intent == PacketIntent.PING - assert response.source == "pqnstack-node" + assert response.source == "pqnstack-provider" assert response.destination == client.name assert response.request == "PONG" @@ -56,7 +56,7 @@ def test_client_ping(messaging_services): # noqa: ARG001 def test_getting_all_instruments(messaging_services): # noqa: ARG001 client = Client(host="localhost", port=5556, router_name="pqnstack-router") - response = client.get_available_devices("pqnstack-node") + response = client.get_available_devices("pqnstack-provider") instruments_names = ["dummy1", "dummy2"] @@ -69,7 +69,7 @@ def test_getting_all_instruments(messaging_services): # noqa: ARG001 def test_proxy_instrument(messaging_services): # noqa: ARG001 client = Client(host="localhost", port=5556, router_name="pqnstack-router") - proxy_instrument = client.get_device("pqnstack-node", "dummy1") + proxy_instrument = client.get_device("pqnstack-provider", "dummy1") assert isinstance(proxy_instrument, ProxyInstrument) base_int = 2 diff --git a/tests/pytest/test_network_config.toml b/tests/pytest/test_network_config.toml index e99c2be3..28566e73 100644 --- a/tests/pytest/test_network_config.toml +++ b/tests/pytest/test_network_config.toml @@ -3,20 +3,20 @@ name = "pqnstack-router" host = "localhost" port = 5556 -[node] -name = "pqnstack-node" +[provider] +name = "pqnstack-provider" router_name = "pqnstack-router" host = "localhost" port = 5556 beat_period = 2000 -[[node.instruments]] +[[provider.instruments]] name = "dummy1" import = "pqnstack.pqn.drivers.dummies.DummyInstrument" desc = "Dummy instrument1 for testing purposes" address = "1234" -[[node.instruments]] +[[provider.instruments]] name = "dummy2" import = "pqnstack.pqn.drivers.dummies.DummyInstrument" desc = "Dummy instrument2 for testing purposes" From fca5ad0840969d07ac8a4adcf1359ccb8f139c1e Mon Sep 17 00:00:00 2001 From: marcosf2 Date: Mon, 28 Jul 2025 13:23:32 -0500 Subject: [PATCH 3/4] More robust handling of subprocess closing --- src/pqnstack/cli.py | 4 +- tests/pytest/test_internal_network.py | 75 +++++++++++++++++++++------ 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/pqnstack/cli.py b/src/pqnstack/cli.py index c8038008..a309acfe 100644 --- a/src/pqnstack/cli.py +++ b/src/pqnstack/cli.py @@ -48,9 +48,7 @@ def _load_and_parse_provider_config( config = tomllib.load(f) if "provider" not in config: - msg = ( - f"Config file {config_path} does not contain a provider section. Add provider configuration under '[provider]' section." - ) + msg = f"Config file {config_path} does not contain a provider section. Add provider configuration under '[provider]' section." raise InvalidNetworkConfigurationError(msg) provider = config["provider"] diff --git a/tests/pytest/test_internal_network.py b/tests/pytest/test_internal_network.py index 1bb1119f..8d9f4ed9 100644 --- a/tests/pytest/test_internal_network.py +++ b/tests/pytest/test_internal_network.py @@ -1,5 +1,7 @@ +import logging import shutil import subprocess +import time from pathlib import Path import pytest @@ -10,40 +12,79 @@ from pqnstack.network.packet import PacketIntent from pqnstack.pqn.drivers.dummies import DummyInstrument +logger = logging.getLogger(__name__) + @pytest.fixture def messaging_services(): - config_path = Path("./test_network_config.toml").resolve() + """Start router and provider services for testing.""" + logger.debug("Starting messaging services...") + # Get the path to the config file relative to this test file, not current working directory + test_dir = Path(__file__).parent + config_path = test_dir / "test_network_config.toml" uv_path = shutil.which("uv") if not uv_path: msg = "Could not find 'uv' executable in PATH" raise RuntimeError(msg) - router_process = subprocess.Popen( # noqa: S603 + logger.debug("Using uv path: %s, starting router and provider with config: %s", uv_path, config_path) + + # Start router process + router_process = subprocess.Popen( [uv_path, "run", "pqn", "start-router", "--config", str(config_path)], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - shell=False, ) - provider_process = subprocess.Popen( # noqa: S603 + # Give router time to start up + time.sleep(1) + + # Start provider process + provider_process = subprocess.Popen( [uv_path, "run", "pqn", "start-provider", "--config", str(config_path)], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - shell=False, ) - yield + # Give provider time to start up and connect to router + time.sleep(1) - router_process.terminate() - router_process.wait() - - provider_process.terminate() - provider_process.wait() + # Check if provider started successfully + if provider_process.poll() is not None: + stdout, stderr = provider_process.communicate() + msg = f"Provider failed to start. Exit code: {provider_process.returncode}\nStdout: {stdout.decode()}\nStderr: {stderr.decode()}" + raise RuntimeError(msg) + logger.debug("Services should be ready for testing") -def test_client_ping(messaging_services): # noqa: ARG001 - client = Client(host="localhost", port=5556, router_name="pqnstack-router") + try: + yield + finally: + logger.debug("Cleaning up messaging services...") + + # Terminate provider first (depends on router) + if provider_process.poll() is None: + provider_process.terminate() + try: + provider_process.wait(timeout=5) + except subprocess.TimeoutExpired: + provider_process.kill() + provider_process.wait() + + # Then terminate router + if router_process.poll() is None: + router_process.terminate() + try: + router_process.wait(timeout=5) + except subprocess.TimeoutExpired: + router_process.kill() + router_process.wait() + + logger.debug("All services cleaned up") + + +def test_client_ping(messaging_services): # noqa: ARG001 + client = Client(host="localhost", port=5556, router_name="pqnstack-router", timeout=1000) response = client.ping("pqnstack-provider") assert isinstance(response, Packet) @@ -53,8 +94,8 @@ def test_client_ping(messaging_services): # noqa: ARG001 assert response.request == "PONG" -def test_getting_all_instruments(messaging_services): # noqa: ARG001 - client = Client(host="localhost", port=5556, router_name="pqnstack-router") +def test_getting_all_instruments(messaging_services): # noqa: ARG001 + client = Client(host="localhost", port=5556, router_name="pqnstack-router", timeout=1000) response = client.get_available_devices("pqnstack-provider") @@ -66,8 +107,8 @@ def test_getting_all_instruments(messaging_services): # noqa: ARG001 assert isinstance(response["dummy2"], DummyInstrument.__class__) -def test_proxy_instrument(messaging_services): # noqa: ARG001 - client = Client(host="localhost", port=5556, router_name="pqnstack-router") +def test_proxy_instrument(messaging_services): # noqa: ARG001 + client = Client(host="localhost", port=5556, router_name="pqnstack-router", timeout=1000) proxy_instrument = client.get_device("pqnstack-provider", "dummy1") assert isinstance(proxy_instrument, ProxyInstrument) From 861ec002a1ba57708703266ce1b82c2a6ef3f7b6 Mon Sep 17 00:00:00 2001 From: marcosf2 Date: Mon, 28 Jul 2025 13:30:03 -0500 Subject: [PATCH 4/4] ruff fixes --- tests/pytest/test_internal_network.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/pytest/test_internal_network.py b/tests/pytest/test_internal_network.py index 8d9f4ed9..6c76e70e 100644 --- a/tests/pytest/test_internal_network.py +++ b/tests/pytest/test_internal_network.py @@ -30,7 +30,7 @@ def messaging_services(): logger.debug("Using uv path: %s, starting router and provider with config: %s", uv_path, config_path) # Start router process - router_process = subprocess.Popen( + router_process = subprocess.Popen( # Noqa: S603 # Subprocess is used for testing purposes, not in production code. [uv_path, "run", "pqn", "start-router", "--config", str(config_path)], stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -40,7 +40,7 @@ def messaging_services(): time.sleep(1) # Start provider process - provider_process = subprocess.Popen( + provider_process = subprocess.Popen( # Noqa: S603 # Subprocess is used for testing purposes, not in production code. [uv_path, "run", "pqn", "start-provider", "--config", str(config_path)], stdout=subprocess.PIPE, stderr=subprocess.PIPE,