Pre-client-rewrite minor fixes

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.

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
@@ -17,35 +42,19 @@
 # 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", which is ignored, if given.
+    """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(config={}, **kwargs):
-        nonlocal tmp_path, rest_server, runner
+    # Create the runner
+    runner = CocoRunner(arena=tmp_path, rest_server=rest_server)
 
-        # Add the fixtures to kwargs, overwriting the caller's
-        # values if given
-        kwargs["arena"] = tmp_path
-        kwargs["rest_server"] = rest_server
+    yield runner
 
-        if runner is None:
-            runner = CocoRunner(**kwargs)
-
-        return 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):
@@ -94,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"
@@ -137,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
@@ -158,6 +159,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 +216,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.
@@ -242,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:
@@ -270,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
@@ -291,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!")
 

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

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()

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.
 

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