Polish unit tests

lib/pbench/test/unit/server/test_cache_manager.py

@@ -520,10 +520,15 @@ def mock_run(args, **_kwargs):
                 args, returncode=0, stdout="Successfully Unpacked!", stderr=None
             )
 
+        def mock_resolve(_path, _strict=False):
+            """In this scenario, there are no symlinks,
+            so resolve() should never be called."""
+            raise AssertionError("Unexpected call to Path.resolve()")
+
         with monkeypatch.context() as m:
             m.setattr(Path, "mkdir", lambda path, parents: None)
             m.setattr(subprocess, "run", mock_run)
-            m.setattr(Path, "resolve", lambda path, strict: path)
+            m.setattr(Path, "resolve", mock_resolve)
             m.setattr(Tarball, "__init__", TestCacheManager.MockTarball.__init__)
             m.setattr(Controller, "__init__", TestCacheManager.MockController.__init__)
             tb = Tarball(tar, Controller(Path("/mock/archive"), cache, make_logger))
@@ -943,7 +948,7 @@ def mock_find_dataset(_self, dataset: str) -> MockTarball:
         ),
         (
             (None, False, 0, b"", None, 2, b""),  # No tar executable
-            (None, True, 0, b"", None, 2, b""),  # Popen failure
+            ("/usr/bin/tar", True, 0, b"", None, 2, b""),  # Popen failure
             # Success, output in peek
             ("/usr/bin/tar", False, 0, b"[test]", None, 0, b""),
             ("/usr/bin/tar", False, 0, b"", 0, 0, b""),  # Success, poll() show success
@@ -998,7 +1003,9 @@ def __init__(self, contents: bytes):
                 self.loop_count = wait_cnt
 
             def close(self) -> None:
-                pass
+                raise AssertionError(
+                    "This test doesn't expect the stream to be closed."
+                )
 
             def peek(self, size=0) -> bytes:
                 if self.loop_count > 0:
@@ -1029,8 +1036,10 @@ def poll(self) -> Optional[int]:
                 self.returncode = poll_return
                 return poll_return
 
-            def __repr__(self):
-                return self.__class__.__name__
+            def kill(self) -> None:
+                raise AssertionError(
+                    "This test doesn't expect the stream to be closed."
+                )
 
         def mock_shutil_which(
             cmd: str, _mode: int = os.F_OK | os.X_OK, _path: Optional[str] = None
@@ -1044,6 +1053,10 @@ def mock_shutil_which(
 
             try:
                 got = Tarball.extract(tar, path)
+            except CacheExtractBadPath as exc:
+                assert tar_path
+                assert not popen_fail
+                assert str(exc) == f"Unable to extract {path} from {tar.name}"
             except TarballUnpackError as exc:
                 if tar_path is None:
                     msg = "External 'tar' executable not found"
@@ -1054,13 +1067,10 @@ def mock_shutil_which(
             except ValueError:
                 assert tar_path
                 assert popen_fail
-            except CacheExtractBadPath as exc:
-                assert tar_path
-                assert not popen_fail
-                assert str(exc) == f"Unable to extract {path} from {tar.name}"
             else:
                 assert tar_path
                 assert not popen_fail
+                assert peek_return or poll_return
                 assert isinstance(got, Inventory)
                 assert got.read() == stdout_contents
 
@@ -1163,9 +1173,20 @@ def seek(self, offset: int, _whence: int = io.SEEK_SET) -> int:
     @pytest.mark.parametrize(
         ("poll_val", "stdout_size", "stderr_size", "wait_timeout", "exp_calls"),
         (
+            # The subprocess completed before the close() call
             (0, 0, None, None, ["poll", "close"]),
+            # The subprocess is still running when close() is called, the wait
+            # does not time out, stdout is empty and there is no stderr.
             (None, 0, None, False, ["poll", "kill", "stdout", "wait", "close"]),
+            # The subprocess is still running when close() is called, the wait
+            # does not time out, stderr is empty and there is no stdout.
+            (None, None, 0, False, ["poll", "kill", "stdout", "wait", "close"]),
+            # The subprocess is still running when close() is called, the wait
+            # does not time out, both stdout and stderr are present and empty.
             (None, 0, 0, False, ["poll", "kill", "stdout", "stderr", "wait", "close"]),
+            # The subprocess is still running when close() is called, the wait
+            # does not time out, stdout and stderr each require one read to
+            # drain them (and a second to see that they are empty).
             (
                 None,
                 2000,
@@ -1182,6 +1203,9 @@ def seek(self, offset: int, _whence: int = io.SEEK_SET) -> int:
                     "close",
                 ],
             ),
+            # The subprocess is still running when close() is called, the wait
+            # does not time out, stdout and stderr each require two reads to
+            # drain them (and a third to see that they are empty).
             (
                 None,
                 6000,
@@ -1200,6 +1224,9 @@ def seek(self, offset: int, _whence: int = io.SEEK_SET) -> int:
                     "close",
                 ],
             ),
+            # The subprocess is still running when close() is called, the wait
+            # does not time out, stdout and stderr each require three reads to
+            # drain them (and a fourth to see that they are empty).
             (
                 None,
                 9000,
@@ -1220,6 +1247,8 @@ def seek(self, offset: int, _whence: int = io.SEEK_SET) -> int:
                     "close",
                 ],
             ),
+            # The subprocess is still running when close() is called, stdout is
+            # empty, there is no stderr, and the wait times out.
             (None, 0, None, True, ["poll", "kill", "stdout", "wait"]),
         ),
     )
@@ -1250,21 +1279,25 @@ def kill(self):
 
             def poll(self) -> Optional[int]:
                 my_calls.append("poll")
-                if self.returncode is None:
-                    self.returncode = poll_val
+                assert (
+                    self.returncode is None
+                ), "returncode is unexpectedly set...test bug?"
+                self.returncode = poll_val
                 return self.returncode
 
             def wait(self, timeout: Optional[float] = None) -> Optional[int]:
                 my_calls.append("wait")
-                if self.returncode is None:
-                    self.returncode = 0
+                assert (
+                    self.returncode is None
+                ), "returncode is unexpectedly set...test bug?"
                 if wait_timeout:
                     raise subprocess.TimeoutExpired(
                         cmd="mock_subprocess",
                         timeout=timeout,
                         output=b"I'm dead!",
                         stderr=b"No, really, I'm dead!",
                     )
+                self.returncode = 0
                 return self.returncode
 
             def __repr__(self):
@@ -1291,14 +1324,17 @@ def read(self, size: int = -1) -> bytes:
                     self.size -= size
                 return b"read"
 
-        assert stdout_size is not None, "Test bug:  stdout size must not be None"
-        my_stdout = MockBufferedReader(stdout_size, "stdout")
+        my_stdout = (
+            None if stdout_size is None else MockBufferedReader(stdout_size, "stdout")
+        )
         my_stderr = (
             None if stderr_size is None else MockBufferedReader(stderr_size, "stderr")
         )
+        my_stream = my_stdout if my_stdout is not None else my_stderr
+        assert my_stream, "Test bug:  we need at least one of stdout and stderr"
 
         # Invoke the CUT
-        stream = Inventory(my_stdout, MockPopen(my_stdout, my_stderr))
+        stream = Inventory(my_stream, MockPopen(my_stdout, my_stderr))
 
         # Test Inventory.__repr__()
         assert str(stream) == "<Stream <MockBufferedReader> from MockPopen>"