[sapphire] Replace serve_path() kwarg optional_files with required_files

This enables optimizations and code simplifications both internal
and external to Sapphire.

grizzly/common/runner.py

@@ -283,7 +283,7 @@ def run(
             testcase.data_path,
             continue_cb=self._keep_waiting,
             forever=wait_for_callback,
-            optional_files=tuple(testcase.optional),
+            required_files=tuple(testcase.required),
             server_map=server_map,
         )
         duration = time() - serve_start

grizzly/common/storage.py

@@ -534,6 +534,19 @@ def purge_optional(self, keep):
         for idx in reversed(to_remove):
             self._files.optional.pop(idx).data_file.unlink()
 
+    @property
+    def required(self):
+        """Get file paths of required files.
+
+        Args:
+            None
+
+        Yields:
+            str: File path of each file.
+        """
+        for test in self._files.required:
+            yield test.file_name
+
     @staticmethod
     def sanitize_path(path):
         """Sanitize given path for use as a URI path.

grizzly/common/test_runner.py

@@ -67,7 +67,9 @@ def test_runner_02(mocker):
     target.check_result.return_value = Result.NONE
     serv_files = ["a.bin"]
     server.serve_path.return_value = (Served.ALL, serv_files)
-    testcase = mocker.Mock(spec_set=TestCase, entry_point=serv_files[0], optional=[])
+    testcase = mocker.Mock(
+        spec_set=TestCase, entry_point=serv_files[0], required=serv_files
+    )
     # single run/iteration relaunch (not idle exit)
     target.is_idle.return_value = False
     runner = Runner(server, target, relaunch=1)
@@ -139,7 +141,7 @@ def test_runner_03(mocker, srv_result, served):
     server.serve_path.return_value = (srv_result, served)
     target = mocker.Mock(spec_set=Target)
     target.check_result.return_value = Result.NONE
-    testcase = mocker.Mock(spec_set=TestCase, entry_point="x", optional=[])
+    testcase = mocker.Mock(spec_set=TestCase, entry_point="x", required=["x"])
     runner = Runner(server, target)
     result = runner.run([], ServerMap(), testcase)
     assert runner.initial
@@ -166,7 +168,7 @@ def test_runner_04(mocker, ignore, status, idle, check_result):
     """test reporting timeout"""
     server = mocker.Mock(spec_set=Sapphire)
     target = mocker.Mock(spec_set=Target)
-    testcase = mocker.Mock(spec_set=TestCase, entry_point="a.bin", optional=[])
+    testcase = mocker.Mock(spec_set=TestCase, entry_point="a.bin", required=["a.bin"])
     serv_files = ["a.bin", "/another/file.bin"]
     server.serve_path.return_value = (Served.TIMEOUT, serv_files)
     target.check_result.return_value = Result.FOUND
@@ -202,7 +204,7 @@ def test_runner_05(mocker, served, attempted, target_result, status):
     target = mocker.Mock(spec_set=Target, launch_timeout=10)
     target.check_result.return_value = target_result
     target.monitor.is_healthy.return_value = False
-    testcase = mocker.Mock(spec_set=TestCase, entry_point="a.bin", optional=[])
+    testcase = mocker.Mock(spec_set=TestCase, entry_point="a.bin", required=["a.bin"])
     runner = Runner(server, target)
     runner.launch("http://a/")
     result = runner.run([], ServerMap(), testcase)
@@ -225,7 +227,7 @@ def test_runner_06(mocker):
     result = runner.run(
         [],
         ServerMap(),
-        mocker.Mock(spec_set=TestCase, entry_point=serv_files[0], optional=[]),
+        mocker.Mock(spec_set=TestCase, entry_point=serv_files[0], required=serv_files),
     )
     assert result.status == Result.NONE
     assert result.attempted

grizzly/common/test_storage.py

@@ -33,6 +33,7 @@ def test_testcase_01(tmp_path):
         assert not any(tcase.contents)
         assert tcase.pop_assets() is None
         assert not any(tcase.optional)
+        assert not any(tcase.required)
         tcase.dump(tmp_path)
         assert not any(tmp_path.iterdir())
         tcase.dump(tmp_path, include_details=True)
@@ -88,6 +89,7 @@ def test_testcase_03(tmp_path, file_paths):
             src_file.write_text("data")
             tcase.add_from_file(src_file, file_name=file_path, required=True)
             assert file_path in tcase.contents
+            assert file_path in tcase.required
             assert file_path not in tcase.optional
 
 
@@ -113,25 +115,25 @@ def test_testcase_05():
         tcase.add_from_bytes(b"foo", "testfile2.bin", required=False)
         tcase.add_from_bytes(b"foo", "testfile3.bin", required=False)
         tcase.add_from_bytes(b"foo", "not_served.bin", required=False)
-        assert len(tuple(tcase.optional)) == 3
+        assert len(tcase._files.optional) == 3
         # nothing to remove - with required
         tcase.purge_optional(chain(["testfile1.bin"], tcase.optional))
-        assert len(tuple(tcase.optional)) == 3
+        assert len(tcase._files.optional) == 3
         # nothing to remove - use relative path (forced)
         tcase.purge_optional(x.file_name for x in tcase._files.optional)
-        assert len(tuple(tcase.optional)) == 3
+        assert len(tcase._files.optional) == 3
         # nothing to remove - use absolute path
         tcase.purge_optional(x.data_file.as_posix() for x in tcase._files.optional)
-        assert len(tuple(tcase.optional)) == 3
+        assert len(tcase._files.optional) == 3
         # remove not_served.bin
         tcase.purge_optional(["testfile2.bin", "testfile3.bin"])
-        assert len(tuple(tcase.optional)) == 2
+        assert len(tcase._files.optional) == 2
         assert "testfile2.bin" in tcase.optional
         assert "testfile3.bin" in tcase.optional
         assert "not_served.bin" not in tcase.optional
         # remove remaining optional
         tcase.purge_optional(["testfile1.bin"])
-        assert not any(tcase.optional)
+        assert not tcase._files.optional
 
 
 def test_testcase_06():
@@ -478,6 +480,7 @@ def test_testcase_19():
                 assert not dst_file.samefile(src_file)
             assert dst.env_vars == {"foo": "bar"}
             assert not set(src.optional) ^ set(dst.optional)
+            assert not set(src.required) ^ set(dst.required)
 
 
 @mark.parametrize(

sapphire/connection_manager.py

@@ -14,7 +14,8 @@
 
 
 class ConnectionManager:
-    SHUTDOWN_DELAY = 0.5  # allow extra time before closing socket if needed
+    # allow extra time before closing socket if needed
+    SHUTDOWN_DELAY = 0.5
 
     __slots__ = (
         "_deadline",
@@ -117,7 +118,7 @@ def serve(self, timeout, continue_cb=None, shutdown_delay=SHUTDOWN_DELAY):
         Returns:
             bool: True unless the timeout is exceeded.
         """
-        assert self._job.pending
+        assert self._job.pending or self._job.forever
         assert self._socket.gettimeout() is not None
         assert shutdown_delay >= 0
         assert timeout >= 0

sapphire/core.py

@@ -186,13 +186,13 @@ def serve_path(
         path,
         continue_cb=None,
         forever=False,
-        optional_files=None,
+        required_files=None,
         server_map=None,
     ):
-        """Serve files in path. On completion a list served files and a status
+        """Serve files in path. On completion a of list served files and a status
         code will be returned.
         The status codes include:
-            - Served.ALL: All files excluding files in optional_files were served
+            - Served.ALL: All required files were served
             - Served.NONE: No files were served
             - Served.REQUEST: Some files were requested
 
@@ -202,8 +202,8 @@ def serve_path(
                                     This must be a callable that returns a bool.
             forever (bool): Continue to handle requests even after all files have
                             been served. This is meant to be used with continue_cb.
-            optional_files (list(str)): Files that do not need to be served in order
-                                        to exit the serve loop.
+            required_files (list(str)): Files that need to be served in order to exit
+                                        the serve loop.
             server_map (ServerMap):
 
         Returns:
@@ -216,17 +216,13 @@ def serve_path(
             path,
             auto_close=self._auto_close,
             forever=forever,
-            optional_files=optional_files,
+            required_files=required_files,
             server_map=server_map,
         )
-        if not job.pending:
-            job.finish()
-            LOG.debug("nothing to serve")
-            return (Served.NONE, tuple())
         with ConnectionManager(job, self._socket, limit=self._max_workers) as mgr:
-            was_timeout = not mgr.serve(self.timeout, continue_cb=continue_cb)
-        LOG.debug("%s, timeout: %r", job.status, was_timeout)
-        return (Served.TIMEOUT if was_timeout else job.status, tuple(job.served))
+            timed_out = not mgr.serve(self.timeout, continue_cb=continue_cb)
+        LOG.debug("%s, timed out: %r", job.status, timed_out)
+        return (Served.TIMEOUT if timed_out else job.status, tuple(job.served))
 
     @classmethod
     def main(cls, args):
@@ -240,10 +236,6 @@ def main(cls, args):
                     gethostname() if args.remote else "127.0.0.1",
                     serv.port,
                 )
-                status = serv.serve_path(args.path)[0]
-            if status == Served.ALL:
-                LOG.info("All test case content was served")
-            else:
-                LOG.warning("Failed to serve all test content")
+                serv.serve_path(args.path, forever=True)
         except KeyboardInterrupt:
             LOG.warning("Ctrl+C detected. Shutting down...")

sapphire/job.py

@@ -68,7 +68,7 @@ def __init__(
         wwwroot,
         auto_close=-1,
         forever=False,
-        optional_files=None,
+        required_files=None,
         server_map=None,
     ):
         self._complete = Event()
@@ -82,25 +82,26 @@ def __init__(
         self.forever = forever
         self.server_map = server_map
         self.worker_complete = Event()
-        self._build_pending(optional_files)
+        self._build_pending(required_files)
+        if not self._pending.files and not self.forever:
+            raise RuntimeError("Empty Job")
 
-    def _build_pending(self, optional_files):
+    def _build_pending(self, required_files):
         # build file list to track files that must be served
         # this is intended to only be called once by __init__()
-        for entry in self._wwwroot.rglob("*"):
-            location = entry.relative_to(self._wwwroot).as_posix()
-            # do not add optional files to queue of required files
-            if optional_files and location in optional_files:
-                continue
-            if "?" in str(entry):
-                LOG.warning("Path cannot contain '?', skipping '%s'", entry)
-                continue
-            if entry.is_file():
-                self._pending.files.add(str(entry.resolve()))
-                LOG.debug("required: %r", location)
+        assert not self._complete.is_set()
+        assert not self._pending.files
+        assert not self._served.files
+        if required_files:
+            for required in required_files:
+                assert "?" not in required
+                entry = self._wwwroot / required
+                if entry.is_file():
+                    self._pending.files.add(str(entry.resolve()))
+                    LOG.debug("required: %r", entry)
         # if nothing was found check if the path exists
         if not self._pending.files and not self._wwwroot.is_dir():
-            raise OSError(f"'{self._wwwroot}' does not exist")
+            raise OSError(f"wwwroot '{self._wwwroot}' does not exist")
         if self.server_map:
             for redirect, resource in self.server_map.redirect.items():
                 if resource.required:
@@ -110,11 +111,7 @@ def _build_pending(self, optional_files):
                 if resource.required:
                     self._pending.files.add(dyn_resp)
                     LOG.debug("required: %r -> %r", dyn_resp, resource.target)
-        LOG.debug(
-            "job has %d required and %d optional file(s)",
-            len(self._pending.files),
-            len(optional_files) if optional_files else 0,
-        )
+        LOG.debug("job has %d required file(s)", len(self._pending.files))
 
     @classmethod
     def lookup_mime(cls, url):
@@ -144,11 +141,6 @@ def lookup_resource(self, path):
                 # file name is too long to look up so ignore it
                 return None
             raise  # pragma: no cover
-        except ValueError:  # pragma: no cover
-            # this is for compatibility with python versions < 3.8
-            # is_file() will raise if the path contains characters unsupported
-            # at the OS level
-            pass
         # look for path in server map
         if self.server_map is not None:
             if path in self.server_map.redirect:
@@ -160,11 +152,7 @@ def lookup_resource(self, path):
                 LOG.debug("checking include %r", inc)
                 file = path[len(inc) :].lstrip("/")
                 local = Path(self.server_map.include[inc].target) / file
-                try:
-                    if not local.is_file():
-                        continue
-                except ValueError:  # pragma: no cover
-                    # python versions < 3.8 compatibility
+                if not local.is_file():
                     continue
                 # file exists, look up resource
                 return Resource(
@@ -212,7 +200,7 @@ def pending(self):
             return len(self._pending.files)
 
     def remove_pending(self, file_name):
-        # return True when all file have been removed
+        # return True when all files have been removed
         with self._pending.lock:
             if self._pending.files:
                 self._pending.files.discard(file_name)

sapphire/test_connection_manager.py

@@ -17,7 +17,7 @@
 def test_connection_manager_01(mocker, tmp_path, timeout):
     """test basic ConnectionManager"""
     (tmp_path / "testfile").write_bytes(b"test")
-    job = Job(tmp_path)
+    job = Job(tmp_path, required_files=["testfile"])
     clnt_sock = mocker.Mock(spec_set=socket)
     clnt_sock.recv.return_value = b"GET /testfile HTTP/1.1"
     serv_sock = mocker.Mock(spec_set=socket)
@@ -37,7 +37,7 @@ def test_connection_manager_02(mocker, tmp_path, worker_limit):
     (tmp_path / "test1").touch()
     (tmp_path / "test2").touch()
     (tmp_path / "test3").touch()
-    job = Job(tmp_path)
+    job = Job(tmp_path, required_files=["test1", "test2", "test3"])
     clnt_sock = mocker.Mock(spec_set=socket)
     clnt_sock.recv.side_effect = (
         b"GET /test1 HTTP/1.1",
@@ -60,8 +60,8 @@ def test_connection_manager_02(mocker, tmp_path, worker_limit):
 
 def test_connection_manager_03(mocker, tmp_path):
     """test ConnectionManager re-raise worker exceptions"""
-    (tmp_path / "test1").touch()
-    job = Job(tmp_path)
+    (tmp_path / "file").touch()
+    job = Job(tmp_path, required_files=["file"])
     clnt_sock = mocker.Mock(spec_set=socket)
     clnt_sock.recv.side_effect = Exception("worker exception")
     serv_sock = mocker.Mock(spec_set=socket)
@@ -76,8 +76,8 @@ def test_connection_manager_03(mocker, tmp_path):
 
 def test_connection_manager_04(mocker, tmp_path):
     """test ConnectionManager.serve() with callback"""
-    (tmp_path / "test1").touch()
-    job = Job(tmp_path)
+    (tmp_path / "file").touch()
+    job = Job(tmp_path, required_files=["file"])
     with ConnectionManager(job, mocker.Mock(spec_set=socket), poll=0.01) as mgr:
         # invalid callback
         with raises(TypeError, match="continue_cb must be callable"):
@@ -92,13 +92,12 @@ def test_connection_manager_04(mocker, tmp_path):
 def test_connection_manager_05(mocker, tmp_path):
     """test ConnectionManager.serve() with timeout"""
     mocker.patch("sapphire.connection_manager.time", autospec=True, side_effect=count())
-    (tmp_path / "test1").touch()
-    job = Job(tmp_path)
+    (tmp_path / "file").touch()
     clnt_sock = mocker.Mock(spec_set=socket)
     clnt_sock.recv.return_value = b""
     serv_sock = mocker.Mock(spec_set=socket)
     serv_sock.accept.return_value = (clnt_sock, None)
-    job = Job(tmp_path)
+    job = Job(tmp_path, required_files=["file"])
     with ConnectionManager(job, serv_sock, poll=0.01) as mgr:
         assert not mgr.serve(10)
         assert job.is_complete()
@@ -108,11 +107,11 @@ def test_connection_manager_06(mocker, tmp_path):
     """test ConnectionManager.serve() worker fails to exit"""
     mocker.patch("sapphire.worker.Thread", autospec=True)
     mocker.patch("sapphire.connection_manager.time", autospec=True, side_effect=count())
-    (tmp_path / "test1").touch()
+    (tmp_path / "file").touch()
     clnt_sock = mocker.Mock(spec_set=socket)
     serv_sock = mocker.Mock(spec_set=socket)
     serv_sock.accept.return_value = (clnt_sock, None)
-    job = Job(tmp_path)
+    job = Job(tmp_path, required_files=["file"])
     mocker.patch.object(job, "worker_complete")
     with ConnectionManager(job, serv_sock) as mgr:
         with raises(RuntimeError, match="Failed to close workers"):