From aa82ef7501116ed2274cd2ee7b9ce6bd97d40ab1 Mon Sep 17 00:00:00 2001 From: "D. V. Wiebe" Date: Thu, 25 Jun 2026 14:06:02 -0700 Subject: [PATCH 1/4] fix(endpoint): Fixes for endpoint validation Two things: * restore the `Endpoint.timestamp_path` assignment inadvertently deleted * Validate keys in the endpoing "call" dictionary. --- coco/endpoint.py | 5 ++--- tests/test_endpoint.py | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/coco/endpoint.py b/coco/endpoint.py index 189acf8..5b52e2a 100644 --- a/coco/endpoint.py +++ b/coco/endpoint.py @@ -90,6 +90,7 @@ def __init__(self, name, conf, forwarder, state): self.after = [] self._load_internal_forward(conf.get("before"), self.before) self._load_internal_forward(conf.get("after"), self.after) + self.timestamp_path = conf.get("timestamp") def _load_internal_forward(self, dict_, list_): """ @@ -914,9 +915,7 @@ def validate_endpoint(config: dict, groups: dict, all_endpoints: set, state) -> if "call" in config: call = config["call"] - # Check type - if not isinstance(call, dict): - raise click.ClickException(f"expected mapping for 'call' in {location}") + _validate_dict(call, "call", ("coco", "forward"), location) # Unless external forwards are _explicitly_ disabled, an endpoint must # have a group. diff --git a/tests/test_endpoint.py b/tests/test_endpoint.py index 770c65c..f5f02a1 100644 --- a/tests/test_endpoint.py +++ b/tests/test_endpoint.py @@ -110,6 +110,24 @@ def test_ext_forward_no_group(set_endpoint, cocod): assert "group" in result.output +def test_call_dict(set_endpoint, cocod): + """Call must be a dict""" + set_endpoint( + { + "call": True, + } + ) + result = cocod(1, ["--check-config"]) + assert "call" in result.output + + +def test_call_keys(set_endpoint, cocod): + """Check allowed keys in "call.""" + set_endpoint({"call": {"something": "bad"}}) + result = cocod(1, ["--check-config"]) + assert "call" in result.output + + def test_call_types(set_endpoint, cocod): """Check typing for calls. From 34b7bc6ba1b9d804f6b688a360a77e2e281eb5a3 Mon Sep 17 00:00:00 2001 From: "D. V. Wiebe" Date: Thu, 25 Jun 2026 14:07:33 -0700 Subject: [PATCH 2/4] test(rest_server): Fix return of request body It was getting doubly JSON encoded. --- tests/rest_server.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/rest_server.py b/tests/rest_server.py index b8f40f4..8158eaf 100644 --- a/tests/rest_server.py +++ b/tests/rest_server.py @@ -94,15 +94,15 @@ def handle_route(self): # Read the body from a POST if self.command == "POST": body = self.rfile.read(int(self.headers["Content-Length"])) + else: + body = "" # If we're accepting all routes, reply with something generic if self.server.rest_server.any_route: - if self.command == "GET": - # if this is a GET, send back just the default stuff - response = {} - else: - # Otherwise send back what we were given - response = body.decode() + response = {} + if self.command == "POST": + # For a POST, send back what we were given + response["body"] = json.loads(body) # Add generic response data response["path"] = path From 7ab62181487789fa4023002199823be06c404248 Mon Sep 17 00:00:00 2001 From: "D. V. Wiebe" Date: Thu, 25 Jun 2026 14:08:07 -0700 Subject: [PATCH 3/4] test(coco_runner): add `port` and `targets` properties Also some doc updates. --- tests/coco_runner.py | 54 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/tests/coco_runner.py b/tests/coco_runner.py index 5313d58..8e01779 100644 --- a/tests/coco_runner.py +++ b/tests/coco_runner.py @@ -1,4 +1,29 @@ -"""pytest fixture to run the coco daemon and client.""" +"""pytest fixture to run the coco daemon and client. + +Using the coco_runner fixture is slow. It can take more than a second to +run a test. This time is almost all due to daemon start-up delay, so +running multiple client calls with the same runner instance can speed things +up. i.e.: + + def test_all(coco_runner): + coco_runner.client("call1") + coco_runner.client("call2") + [...] + coco_runner.client("callN") + +will run almost N times faster than: + + def test1(coco_runner): + coco_runner.client("call1") + + def test2(coco_runner): + coco_runner.client("call2") + + [...] + + def testN(coco_runner): + coco_runner.client("callN") +""" import json import multiprocessing @@ -20,7 +45,12 @@ def coco_runner(tmp_path, rest_server): """Yields a function to create a CocoRunner. Arguments to the returned function are passed to the CocoRunner - constructor, except for "arena", which is ignored, if given. + constructor, except for "arena", and "rest_server", which are + ignored, if given. + + Calling the returned function more than once will result in a + RuntimeError. (Running more than one CocoRunner instance + in the same test is not possible.) Ensures the coco runner has stopped after the test completes. """ @@ -28,16 +58,18 @@ def coco_runner(tmp_path, rest_server): # The runner instance is stored here runner = None - def _create_runner(config={}, **kwargs): + def _create_runner(**kwargs): nonlocal tmp_path, rest_server, runner + if runner: + raise RuntimeError("coco_runner already initialized") + # Add the fixtures to kwargs, overwriting the caller's # values if given kwargs["arena"] = tmp_path kwargs["rest_server"] = rest_server - if runner is None: - runner = CocoRunner(**kwargs) + runner = CocoRunner(**kwargs) return runner @@ -158,6 +190,16 @@ def __init__(self, *, arena, rest_server, no_daemon=False, daemon_args=[]): # Rest server farm self._farm = [] + @property + def port(self) -> int | None: + """The daemon port, if running.""" + return self._daemon_port + + @property + def targets(self) -> list: + """List of rest_server targets.""" + return self._farm + def add_config(self, **extra_config): """Update the daemon's config. @@ -205,7 +247,7 @@ def add_targets(self, group, count): def add_endpoint(self, name, endpoint_def): """Add an endpoint to the daemon. - Be sure to also add the target group to the config with `add_group`, + Be sure to also add the target group to the config with `add_targets`, if you want things to work. Must be called before starting the daemon. From 7c6f6a150a74c6c26d18f4a2dbf2988d9ef85de7 Mon Sep 17 00:00:00 2001 From: "D. V. Wiebe" Date: Thu, 25 Jun 2026 16:54:15 -0700 Subject: [PATCH 4/4] test(coco_runner): Automatically instantiate the runner Instead of returning a function to instantiate the runner, now `coco_runner` just does it directly. If you need to set daemon command-line arguments, use `start_daemon`. If you want to avoid starting the daemon when running the client, use the `no_daemon` keyword argument with the (still unimplemented) `client` method. --- tests/coco_runner.py | 74 +++++++++++++++--------------------------- tests/test_core.py | 18 ++++------ tests/test_fixtures.py | 5 ++- 3 files changed, 35 insertions(+), 62 deletions(-) diff --git a/tests/coco_runner.py b/tests/coco_runner.py index 8e01779..9a7dd31 100644 --- a/tests/coco_runner.py +++ b/tests/coco_runner.py @@ -42,42 +42,19 @@ def testN(coco_runner): # Can't use pyfakefs here, so we rely on tmp_path instead @pytest.fixture def coco_runner(tmp_path, rest_server): - """Yields a function to create a CocoRunner. - - Arguments to the returned function are passed to the CocoRunner - constructor, except for "arena", and "rest_server", which are - ignored, if given. - - Calling the returned function more than once will result in a - RuntimeError. (Running more than one CocoRunner instance - in the same test is not possible.) + """Yields a CocoRunner instance. Ensures the coco runner has stopped after the test completes. """ - # The runner instance is stored here - runner = None - - def _create_runner(**kwargs): - nonlocal tmp_path, rest_server, runner - - if runner: - raise RuntimeError("coco_runner already initialized") - - # Add the fixtures to kwargs, overwriting the caller's - # values if given - kwargs["arena"] = tmp_path - kwargs["rest_server"] = rest_server - - runner = CocoRunner(**kwargs) + # Create the runner + runner = CocoRunner(arena=tmp_path, rest_server=rest_server) - return runner + yield runner - yield _create_runner - - # Ensure runner terminates - if runner: - runner.stop() + # Ensure runner terminates. If the test already called stop, it's + # harmless to call it again here. + runner.stop() def _daemon_main(conf_path, args, pipe): @@ -126,24 +103,19 @@ class CocoRunner: The daemon is run in a subprocess (not a thread, because Sanic doesn't work like that). + In general, you shouldn't instantiate this directly. Instead, let the + `coco_runner` pytest fixture do it for you. + Attributes ---------- arena : pathlib.Path An empty temporary directory to be used by cocod rest_server : callable The rest_server fixture - no_daemon : bool - If True, don't start the daemon before invoking the client. It - can still be started manually by calling `start_daemon`. - daemon_args : list, optional - Command-line arguments to the daemon, if any. The "--testing" flag - will always be appended to these arguments and does not need to be - listed here. """ - def __init__(self, *, arena, rest_server, no_daemon=False, daemon_args=[]): + def __init__(self, *, arena, rest_server): self.rest_server = rest_server - self.no_daemon = no_daemon # Populate the arena self.storage_path = arena / "storage" @@ -169,9 +141,6 @@ def __init__(self, *, arena, rest_server, no_daemon=False, daemon_args=[]): # Process for coco daemon self._daemon_proc = None - - # Arguments for the daemon - self._daemon_args = ["--testing", *daemon_args] self._daemon_port = None # Return value from the daemon @@ -284,8 +253,13 @@ def _start_redis(self): # return the port return self._redis_port - def start_daemon(self): - """Start function for the coco daemon. Runs in a subprocess.""" + def start_daemon(self, *args): + """Start function for the coco daemon. Runs in a subprocess. + + If arguments are given, they are used as commandline arguments + to the daemon. This is in addition to the "--testing" flag, + which is always used when starting the daemon. + """ # If it's already running, do nothing if self._daemon_proc: @@ -312,7 +286,7 @@ def start_daemon(self): # Create a daemon subprocess self._daemon_proc = context.Process( target=_daemon_main, - args=(str(self.config_file), self._daemon_args, self._daemon_send), + args=(str(self.config_file), ["--testing", *args], self._daemon_send), ) # Start it @@ -333,13 +307,17 @@ def start_daemon(self): # even after it successfully fetches the port. sleep(0.2) - def invoke(self, args): + def client(self, *args, no_daemon=False): """Invoke the coco client. Parameters ---------- - args : list - Arguments to the coco client. + *args : str + Positional arguments are used as commandline arguments. + no_daemon : bool, optional + If True, don't start the daemon before running the + client. If the daemon is already running, this + won't stop it. """ raise NotImplementedError("coco not supported yet!") diff --git a/tests/test_core.py b/tests/test_core.py index 5d5b9ee..9c4ae98 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -128,13 +128,12 @@ def test_comet(mock_comet, coco_runner): from coco import __version__ # Configure comet in the coco_runner - runner = coco_runner() - runner.add_config( + coco_runner.add_config( comet_broker={"enabled": True, "host": "127.0.0.1", "port": mock_comet.port} ) # Start the daemon - runner.start_daemon() + coco_runner.start_daemon() # Check that everything was registered. The counts are 2 here # because the "start" state is separate from the "config" state. @@ -144,11 +143,11 @@ def test_comet(mock_comet, coco_runner): # Check the coco config was sent mock_comet.assert_hit_received( "/send-state", - {"state": {"version": __version__, "config_state": runner.config}}, + {"state": {"version": __version__, "config_state": coco_runner.config}}, ) # Check for no error from daemon - runner.stop() + coco_runner.stop() def test_comet_check_config(mock_comet, coco_runner): @@ -157,20 +156,17 @@ def test_comet_check_config(mock_comet, coco_runner): We don't want cocod registering its start in this case. """ - # Set up daemon to be invoked with --check-config - runner = coco_runner(daemon_args=("--check-config",)) - # Configure comet in the coco_runner - runner.add_config( + coco_runner.add_config( comet_broker={"enabled": True, "host": "127.0.0.1", "port": mock_comet.port} ) # Start the daemon - runner.start_daemon() + coco_runner.start_daemon("--check-config") # Check that comet wasn't called. assert mock_comet.hit_count("/register-state") == 0 assert mock_comet.hit_count("/send-state") == 0 # Check for no error from daemon - runner.stop() + coco_runner.stop() diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py index 4081bef..5257c3b 100644 --- a/tests/test_fixtures.py +++ b/tests/test_fixtures.py @@ -6,7 +6,6 @@ def test_runner(coco_runner): This tests the coco runner fixture to make sure it can start up and shut down cleanly. This is not a test of the production code.""" - runner = coco_runner() - runner.start_daemon() - result = runner.stop() + coco_runner.start_daemon() + result = coco_runner.stop() assert result["exit_code"] == 0