diff --git a/src/python/pants/pantsd/pants_daemon.py b/src/python/pants/pantsd/pants_daemon.py index 82749ce00ac..1326cd8ed75 100644 --- a/src/python/pants/pantsd/pants_daemon.py +++ b/src/python/pants/pantsd/pants_daemon.py @@ -170,24 +170,20 @@ def _pantsd_logging(self) -> Iterator[None]: self._logger.debug("Logging reinitialized in pantsd context") yield - def _write_nailgun_port(self): - """Write the nailgun port to a well known file.""" - self.write_socket(self._server.port()) - - def _initialize_pid(self): - """Writes out our pid and metadata. + def _initialize_metadata(self) -> None: + """Writes out our pid and other metadata. - Once written, does a one-time read of the pid to confirm that we haven't raced another - process starting. + Order matters a bit here, because technically all that is necessary to connect is the port, + and Services are lazily initialized by the core when a connection is established. Our pid + needs to be on disk before that happens. """ # Write the pidfile. The SchedulerService will monitor it after a grace period. - pid = os.getpid() - self._logger.debug(f"pantsd running with PID: {pid}") - self.write_pid(pid=pid) - self.write_metadata_by_name( - "pantsd", self.FINGERPRINT_KEY, ensure_text(self.options_fingerprint) - ) + self.write_pid() + self.write_process_name() + self.write_fingerprint(ensure_text(self.options_fingerprint)) + self._logger.debug(f"pantsd running with PID: {self.pid}") + self.write_socket(self._server.port()) def run_sync(self): """Synchronously run pantsd.""" @@ -208,12 +204,7 @@ def run_sync(self): # Set the process name in ps output to 'pantsd' vs './pants compile src/etc:: -ldebug'. set_process_title(f"pantsd [{self._build_root}]") - # Write our pid and the server's port to .pids. Order matters a bit here, because - # technically all that is necessary to connect is the port, and Services are lazily - # initialized by the core when a connection is established. Our pid needs to be on - # disk before that happens. - self._initialize_pid() - self._write_nailgun_port() + self._initialize_metadata() # Check periodically whether the core is valid, and exit if it is not. while self._core.is_valid(): diff --git a/src/python/pants/pantsd/process_manager.py b/src/python/pants/pantsd/process_manager.py index f073d8565b7..a521ac645c7 100644 --- a/src/python/pants/pantsd/process_manager.py +++ b/src/python/pants/pantsd/process_manager.py @@ -4,12 +4,11 @@ import logging import os import signal -import subprocess import sys import time import traceback from abc import ABCMeta -from contextlib import contextmanager +from hashlib import sha256 from typing import Callable, Optional, cast import psutil @@ -20,24 +19,18 @@ from pants.option.scope import GLOBAL_SCOPE from pants.process.lock import OwnerPrintingInterProcessFileLock from pants.util.dirutil import read_file, rm_rf, safe_file_dump, safe_mkdir -from pants.util.memo import memoized_property +from pants.util.memo import memoized_classproperty, memoized_property logger = logging.getLogger(__name__) -@contextmanager -def swallow_psutil_exceptions(): - """A contextmanager that swallows standard psutil access exceptions.""" - try: - yield - except (psutil.AccessDenied, psutil.NoSuchProcess): - # This masks common, but usually benign psutil process access exceptions that might be seen - # when accessing attributes/methods on psutil.Process objects. - pass - - class ProcessMetadataManager: - """"Manages contextual, on-disk process metadata.""" + """Manages contextual, on-disk process metadata. + + Metadata is stored under a per-host fingerprinted directory, and a nested per-named-process + directory. The per-host directory defends against attempting to use process metadata that has + been mounted into virtual machines or docker images. + """ class MetadataError(Exception): pass @@ -53,6 +46,25 @@ def __init__(self, metadata_base_dir: str) -> None: super().__init__() self._metadata_base_dir = metadata_base_dir + @memoized_classproperty + def host_fingerprint(cls) -> str: + """A fingerprint that attempts to identify the potential scope of a live process. + + See the class pydoc. + + In the absence of kernel hotswapping, a new uname means a restart or virtual machine, both + of which mean that process metadata is invalid. Additionally, docker generates a random + hostname per instance, which improves the reliability of this hash. + + TODO: It would be nice to be able to use `uptime` (e.g. https://crates.io/crates/uptime_lib) + to identify reboots, but it's more challenging than it should be because it would involve + subtracting from the current time, which might hit aliasing issues. + """ + hasher = sha256() + for component in os.uname(): + hasher.update(component.encode()) + return hasher.hexdigest()[:12] + @staticmethod def _maybe_cast(item, caster): """Given a casting function, attempt to cast to that type while masking common cast @@ -139,23 +151,19 @@ def file_waiter(): return cls._deadline_until(file_waiter, ongoing_msg, completed_msg, timeout=timeout) - @staticmethod - def _get_metadata_dir_by_name(name, metadata_base_dir): + @classmethod + def _get_metadata_dir_by_name(cls, name: str, metadata_base_dir: str) -> str: """Retrieve the metadata dir by name. This should always live outside of the workdir to survive a clean-all. """ - return os.path.join(metadata_base_dir, name) - - def _maybe_init_metadata_dir_by_name(self, name): - """Initialize the metadata directory for a named identity if it doesn't exist.""" - safe_mkdir(self.__class__._get_metadata_dir_by_name(name, self._metadata_base_dir)) + return os.path.join(metadata_base_dir, cls.host_fingerprint, name) - def _metadata_file_path(self, name, metadata_key): + def _metadata_file_path(self, name, metadata_key) -> str: return self.metadata_file_path(name, metadata_key, self._metadata_base_dir) @classmethod - def metadata_file_path(cls, name, metadata_key, metadata_base_dir): + def metadata_file_path(cls, name, metadata_key, metadata_base_dir) -> str: return os.path.join(cls._get_metadata_dir_by_name(name, metadata_base_dir), metadata_key) def read_metadata_by_name(self, name, metadata_key, caster=None): @@ -172,14 +180,14 @@ def read_metadata_by_name(self, name, metadata_key, caster=None): except (IOError, OSError): return None - def write_metadata_by_name(self, name, metadata_key, metadata_value): + def write_metadata_by_name(self, name, metadata_key, metadata_value) -> None: """Write process metadata using a named identity. :param string name: The ProcessMetadataManager identity/name (e.g. 'pantsd'). :param string metadata_key: The metadata key (e.g. 'pid'). :param string metadata_value: The metadata value (e.g. '1729'). """ - self._maybe_init_metadata_dir_by_name(name) + safe_mkdir(self._get_metadata_dir_by_name(name, self._metadata_base_dir)) file_path = self._metadata_file_path(name, metadata_key) safe_file_dump(file_path, metadata_value) @@ -201,7 +209,7 @@ def await_metadata_by_name( self._wait_for_file(file_path, ongoing_msg, completed_msg, timeout=timeout) return self.read_metadata_by_name(name, metadata_key, caster) - def purge_metadata_by_name(self, name): + def purge_metadata_by_name(self, name) -> None: """Purge a processes metadata directory. :raises: `ProcessManager.MetadataError` when OSError is encountered on metadata dir removal. @@ -222,10 +230,10 @@ class ProcessManager(ProcessMetadataManager): Not intended to be thread-safe. """ - class InvalidCommandOutput(Exception): + class NonResponsiveProcess(Exception): pass - class NonResponsiveProcess(Exception): + class NotStarted(Exception): pass class ExecutionError(Exception): @@ -242,42 +250,26 @@ def __repr__(self): KILL_WAIT_SEC = 5 KILL_CHAIN = (signal.SIGTERM, signal.SIGKILL) - def __init__( - self, - name, - metadata_base_dir: str, - pid=None, - socket=None, - process_name=None, - ): + SOCKET_KEY = "socket" + PROCESS_NAME_KEY = "process_name" + PID_KEY = "pid" + FINGERPRINT_KEY = "fingerprint" + + def __init__(self, name: str, metadata_base_dir: str): """ :param string name: The process identity/name (e.g. 'pantsd' or 'ng_Zinc'). - :param int pid: The process pid. Overrides fetching of the self.pid @property. - :param string socket: The socket metadata. Overrides fetching of the self.socket @property. - :param string process_name: The process name for cmdline executable name matching. :param str metadata_base_dir: The overridden base directory for process metadata. """ super().__init__(metadata_base_dir) self._name = name.lower().strip() - self._pid = pid - self._socket = socket - self._process_name = process_name + # TODO: Extract process spawning code. self._buildroot = get_buildroot() - self._process = None @property def name(self): """The logical name/label of the process.""" return self._name - @property - def process_name(self): - """The logical process name. - - If defined, this is compared to exe_name for stale pid checking. - """ - return self._process_name - @memoized_property def lifecycle_lock(self): """An identity-keyed inter-process lock for safeguarding lifecycle and other operations.""" @@ -290,53 +282,46 @@ def lifecycle_lock(self): ) @property - def cmdline(self): - """The process commandline. e.g. ['/usr/bin/python2.7', 'pants.pex']. - - :returns: The command line or else `None` if the underlying process has died. - """ - with swallow_psutil_exceptions(): - process = self._as_process() - if process: - return process.cmdline() - return None + def fingerprint(self): + """The fingerprint of the current process. - @property - def cmd(self): - """The first element of the process commandline e.g. '/usr/bin/python2.7'. + This reads the current fingerprint from the `ProcessManager` metadata. - :returns: The first element of the process command line or else `None` if the underlying - process has died. + :returns: The fingerprint of the running process as read from ProcessManager metadata or `None`. + :rtype: string """ - return (self.cmdline or [None])[0] + return self.read_metadata_by_name(self.name, self.FINGERPRINT_KEY) @property def pid(self): """The running processes pid (or None).""" - return self._pid or self.read_metadata_by_name(self._name, "pid", int) + return self.read_metadata_by_name(self._name, self.PID_KEY, int) + + @property + def process_name(self): + """The process name, to be compared to the psutil exe_name for stale pid checking.""" + return self.read_metadata_by_name(self._name, self.PROCESS_NAME_KEY, str) @property def socket(self): """The running processes socket/port information (or None).""" - return self._socket or self.read_metadata_by_name(self._name, "socket", int) + return self.read_metadata_by_name(self._name, self.SOCKET_KEY, int) - @classmethod - def get_subprocess_output(cls, command, ignore_stderr=True, **kwargs): - """Get the output of an executed command. + def has_current_fingerprint(self, fingerprint): + """Determines if a new fingerprint is the current fingerprint of the running process. - :param command: An iterable representing the command to execute (e.g. ['ls', '-al']). - :param ignore_stderr: Whether or not to ignore stderr output vs interleave it with stdout. - :raises: `ProcessManager.ExecutionError` on `OSError` or `CalledProcessError`. - :returns: The output of the command. + :param string fingerprint: The new fingerprint to compare to. + :rtype: bool """ - if ignore_stderr is False: - kwargs.setdefault("stderr", subprocess.STDOUT) + return fingerprint == self.fingerprint - try: - return subprocess.check_output(command, **kwargs).decode().strip() - except (OSError, subprocess.CalledProcessError) as e: - subprocess_output = getattr(e, "output", "").strip() - raise cls.ExecutionError(str(e), subprocess_output) + def needs_restart(self, fingerprint): + """Determines if the current ProcessManager needs to be started or restarted. + + :param string fingerprint: The new fingerprint to compare to. + :rtype: bool + """ + return self.is_dead() or not self.has_current_fingerprint(fingerprint) def await_pid(self, timeout: float) -> int: """Wait up to a given timeout for a process to write pid metadata.""" @@ -344,7 +329,7 @@ def await_pid(self, timeout: float) -> int: int, self.await_metadata_by_name( self._name, - "pid", + self.PID_KEY, f"{self._name} to start", f"{self._name} started", timeout, @@ -358,7 +343,7 @@ def await_socket(self, timeout: float) -> int: int, self.await_metadata_by_name( self._name, - "socket", + self.SOCKET_KEY, f"{self._name} socket to be opened", f"{self._name} socket opened", timeout, @@ -366,14 +351,22 @@ def await_socket(self, timeout: float) -> int: ), ) - def write_pid(self, pid=None): - """Write the current processes PID to the pidfile location.""" - pid = pid or os.getpid() - self.write_metadata_by_name(self._name, "pid", str(pid)) + def write_pid(self, pid: Optional[int] = None): + """Write the current process's PID.""" + pid = os.getpid() if pid is None else pid + self.write_metadata_by_name(self._name, self.PID_KEY, str(pid)) + + def write_process_name(self, process_name: Optional[str] = None): + """Write the current process's name.""" + process_name = process_name or self._as_process().name() + self.write_metadata_by_name(self._name, self.PROCESS_NAME_KEY, process_name) - def write_socket(self, socket_info): + def write_socket(self, socket_info: int): """Write the local processes socket information (TCP port or UNIX socket).""" - self.write_metadata_by_name(self._name, "socket", str(socket_info)) + self.write_metadata_by_name(self._name, self.SOCKET_KEY, str(socket_info)) + + def write_fingerprint(self, fingerprint: str) -> None: + self.write_metadata_by_name(self._name, self.FINGERPRINT_KEY, fingerprint) def _as_process(self): """Returns a psutil `Process` object wrapping our pid. @@ -385,10 +378,12 @@ def _as_process(self): :returns: a psutil Process object or else None if we have no pid. :rtype: :class:`psutil.Process` :raises: :class:`psutil.NoSuchProcess` if the process identified by our pid has died. + :raises: :class:`self.NotStarted` if no pid has been recorded for this process. """ - if self._process is None and self.pid: - self._process = psutil.Process(self.pid) - return self._process + pid = self.pid + if not pid: + raise self.NotStarted() + return psutil.Process(pid) def is_dead(self): """Return a boolean indicating whether the process is dead or not.""" @@ -417,7 +412,7 @@ def is_alive(self, extended_check=None): # Extended checking. (extended_check and not extended_check(process)) ) - except (psutil.NoSuchProcess, psutil.AccessDenied): + except (self.NotStarted, psutil.NoSuchProcess, psutil.AccessDenied): # On some platforms, accessing attributes of a zombie'd Process results in NoSuchProcess. return False @@ -522,63 +517,7 @@ def post_fork_parent(self): """Post-fork parent callback for subclasses.""" -class FingerprintedProcessManager(ProcessManager): - """A `ProcessManager` subclass that provides a general strategy for process fingerprinting.""" - - FINGERPRINT_KEY = "fingerprint" - FINGERPRINT_CMD_KEY: Optional[str] = None - FINGERPRINT_CMD_SEP = "=" - - @property - def fingerprint(self): - """The fingerprint of the current process. - - This can either read the current fingerprint from the running process's psutil.Process.cmdline - (if the managed process supports that) or from the `ProcessManager` metadata. - - :returns: The fingerprint of the running process as read from the process table, ProcessManager - metadata or `None`. - :rtype: string - """ - return self.parse_fingerprint(self.cmdline) or self.read_metadata_by_name( - self.name, self.FINGERPRINT_KEY - ) - - def parse_fingerprint(self, cmdline, key=None, sep=None): - """Given a psutil.Process.cmdline, parse and return a fingerprint. - - :param list cmdline: The psutil.Process.cmdline of the current process. - :param string key: The key for fingerprint discovery. - :param string sep: The key/value separator for fingerprint discovery. - :returns: The parsed fingerprint or `None`. - :rtype: string or `None` - """ - key = key or self.FINGERPRINT_CMD_KEY - if key: - sep = sep or self.FINGERPRINT_CMD_SEP - cmdline = cmdline or [] - for cmd_part in cmdline: - if cmd_part.startswith("{}{}".format(key, sep)): - return cmd_part.split(sep)[1] - - def has_current_fingerprint(self, fingerprint): - """Determines if a new fingerprint is the current fingerprint of the running process. - - :param string fingerprint: The new fingerprint to compare to. - :rtype: bool - """ - return fingerprint == self.fingerprint - - def needs_restart(self, fingerprint): - """Determines if the current ProcessManager needs to be started or restarted. - - :param string fingerprint: The new fingerprint to compare to. - :rtype: bool - """ - return self.is_dead() or not self.has_current_fingerprint(fingerprint) - - -class PantsDaemonProcessManager(FingerprintedProcessManager, metaclass=ABCMeta): +class PantsDaemonProcessManager(ProcessManager, metaclass=ABCMeta): """An ABC for classes that interact with pantsd's metadata. This is extended by both a pantsd client handle, and by the server: the client reads process diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test.py b/tests/python/pants_test/pantsd/pantsd_integration_test.py index 6dd41c0d30b..e96124be11d 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test.py @@ -2,6 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import datetime +import glob import os import re import signal @@ -14,7 +15,7 @@ from pants.testutil.pants_integration_test import read_pantsd_log, temporary_workdir from pants.util.contextutil import environment_as, temporary_dir, temporary_file -from pants.util.dirutil import rm_rf, safe_file_dump, safe_mkdir, safe_open, touch +from pants.util.dirutil import rm_rf, safe_file_dump, safe_mkdir, safe_open, safe_rmtree, touch from pants_test.pantsd.pantsd_integration_test_base import PantsDaemonIntegrationTestBase @@ -284,7 +285,7 @@ def test_pantsd_pid_deleted(self): ctx.checker.assert_running() subprocess_dir = ctx.pantsd_config["GLOBAL"]["pants_subprocessdir"] - os.unlink(os.path.join(subprocess_dir, "pantsd", "pid")) + safe_rmtree(subprocess_dir) ctx.checker.assert_stopped() @@ -298,7 +299,7 @@ def test_pantsd_pid_change(self): ctx.checker.assert_running() subprocess_dir = ctx.pantsd_config["GLOBAL"]["pants_subprocessdir"] - pidpath = os.path.join(subprocess_dir, "pantsd", "pid") + (pidpath,) = glob.glob(os.path.join(subprocess_dir, "*", "pantsd", "pid")) with open(pidpath, "w") as f: f.write("9") diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test_base.py b/tests/python/pants_test/pantsd/pantsd_integration_test_base.py index 57c2e389ec2..6cfc236940d 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test_base.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test_base.py @@ -8,7 +8,6 @@ from dataclasses import dataclass from typing import Any, Callable, Dict, Iterator, Optional, Tuple -import psutil from colors import bold, cyan, magenta from pants.pantsd.process_manager import ProcessManager @@ -47,21 +46,22 @@ def attempts( class PantsDaemonMonitor(ProcessManager): def __init__(self, metadata_base_dir: str): super().__init__(name="pantsd", metadata_base_dir=metadata_base_dir) + self._started = False def _log(self): - print(magenta(f"PantsDaemonMonitor: pid is {self._pid} is_alive={self.is_alive()}")) + print(magenta(f"PantsDaemonMonitor: pid is {self.pid} is_alive={self.is_alive()}")) def assert_started_and_stopped(self, timeout: int = 30) -> None: """Asserts that pantsd was alive (it wrote a pid file), but that it stops afterward.""" - self._process = None - self._pid = self.await_pid(timeout) + self.await_pid(timeout) + self._started = True self.assert_stopped() def assert_started(self, timeout=30): - self._process = None - self._pid = self.await_pid(timeout) + self.await_pid(timeout) + self._started = True self._check_pantsd_is_alive() - return self._pid + return self.pid def assert_pantsd_runner_started(self, client_pid, timeout=12): return self.await_metadata_by_name( @@ -76,10 +76,10 @@ def assert_pantsd_runner_started(self, client_pid, timeout=12): def _check_pantsd_is_alive(self): self._log() assert ( - self._pid is not None + self._started ), "cannot assert that pantsd is running. Try calling assert_started before calling this method." assert self.is_alive(), "pantsd was not alive." - return self._pid + return self.pid def current_memory_usage(self): """Return the current memory usage of the pantsd process (which must be running) @@ -87,10 +87,10 @@ def current_memory_usage(self): :return: memory usage in bytes """ self.assert_running() - return psutil.Process(self._pid).memory_info()[0] + return self._as_process().memory_info()[0] def assert_running(self): - if not self._pid: + if not self._started: return self.assert_started() else: return self._check_pantsd_is_alive() @@ -98,12 +98,11 @@ def assert_running(self): def assert_stopped(self): self._log() assert ( - self._pid is not None + self._started ), "cannot assert pantsd stoppage. Try calling assert_started before calling this method." for _ in attempts("pantsd should be stopped!"): if self.is_dead(): break - return self._pid @dataclass(frozen=True) diff --git a/tests/python/pants_test/pantsd/test_process_manager.py b/tests/python/pants_test/pantsd/test_process_manager.py index 25fde1ab399..b71f27d94c5 100644 --- a/tests/python/pants_test/pantsd/test_process_manager.py +++ b/tests/python/pants_test/pantsd/test_process_manager.py @@ -4,8 +4,6 @@ import errno import logging import os -import subprocess -import sys import unittest import unittest.mock from contextlib import contextmanager @@ -48,18 +46,9 @@ def test_get_metadata_dir_by_name(self): self.pmm = ProcessMetadataManager(metadata_base_dir=self.BUILDROOT) self.assertEqual( self.pmm._get_metadata_dir_by_name(self.NAME, self.BUILDROOT), - os.path.join(self.BUILDROOT, self.NAME), + os.path.join(self.BUILDROOT, self.pmm.host_fingerprint, self.NAME), ) - def test_maybe_init_metadata_dir_by_name(self): - with unittest.mock.patch( - "pants.pantsd.process_manager.safe_mkdir", **PATCH_OPTS - ) as mock_mkdir: - self.pmm._maybe_init_metadata_dir_by_name(self.NAME) - mock_mkdir.assert_called_once_with( - self.pmm._get_metadata_dir_by_name(self.NAME, self.SUBPROCESS_DIR) - ) - def test_readwrite_metadata_by_name(self): with temporary_dir() as tmpdir, unittest.mock.patch( "pants.pantsd.process_manager.get_buildroot", return_value=tmpdir @@ -129,71 +118,9 @@ def test_purge_metadata_error(self): class TestProcessManager(unittest.TestCase): - SUBPROCESS_DIR = safe_mkdtemp() - def setUp(self): super().setUp() - # N.B. We pass in `metadata_base_dir` here because ProcessManager (itself a non-task/non- - # subsystem) depends on an initialized `GlobalOptions` subsystem for the value of - # `--pants-subprocessdir` in the default case. This is normally provided by subsystem - # dependencies in a typical pants run (and integration tests), but not in unit tests. - # Thus, passing this parameter here short-circuits the subsystem-reliant path for the - # purposes of unit testing without requiring adhoc subsystem initialization. - self.pm = ProcessManager("test", metadata_base_dir=self.SUBPROCESS_DIR) - - def test_process_properties(self): - with unittest.mock.patch.object( - ProcessManager, "_as_process", **PATCH_OPTS - ) as mock_as_process: - mock_as_process.return_value = fake_process( - name="name", cmdline=["cmd", "line"], status="status" - ) - self.assertEqual(self.pm.cmdline, ["cmd", "line"]) - self.assertEqual(self.pm.cmd, "cmd") - - def test_process_properties_cmd_indexing(self): - with unittest.mock.patch.object( - ProcessManager, "_as_process", **PATCH_OPTS - ) as mock_as_process: - mock_as_process.return_value = fake_process(cmdline="") - self.assertEqual(self.pm.cmd, None) - - def test_process_properties_none(self): - with unittest.mock.patch.object(ProcessManager, "_as_process", **PATCH_OPTS) as mock_asproc: - mock_asproc.return_value = None - self.assertEqual(self.pm.cmdline, None) - self.assertEqual(self.pm.cmd, None) - - def test_get_subprocess_output(self): - test_str = "333" - self.assertEqual(self.pm.get_subprocess_output(["echo", "-n", test_str]), test_str) - - def test_get_subprocess_output_interleaved(self): - cmd_payload = "import sys; " + ( - 'sys.stderr.write("9"); sys.stderr.flush(); sys.stdout.write("3"); sys.stdout.flush();' - * 3 - ) - cmd = [sys.executable, "-c", cmd_payload] - - self.assertEqual(self.pm.get_subprocess_output(cmd), "333") - self.assertEqual(self.pm.get_subprocess_output(cmd, ignore_stderr=False), "939393") - self.assertEqual(self.pm.get_subprocess_output(cmd, stderr=subprocess.STDOUT), "939393") - - def test_get_subprocess_output_interleaved_bash(self): - cmd_payload = 'printf "9">&2; printf "3";' * 3 - cmd = ["/bin/bash", "-c", cmd_payload] - - self.assertEqual(self.pm.get_subprocess_output(cmd), "333") - self.assertEqual(self.pm.get_subprocess_output(cmd, ignore_stderr=False), "939393") - self.assertEqual(self.pm.get_subprocess_output(cmd, stderr=subprocess.STDOUT), "939393") - - def test_get_subprocess_output_oserror_exception(self): - with self.assertRaises(ProcessManager.ExecutionError): - self.pm.get_subprocess_output(["i_do_not_exist"]) - - def test_get_subprocess_output_failure_exception(self): - with self.assertRaises(ProcessManager.ExecutionError): - self.pm.get_subprocess_output(["false"]) + self.pm = ProcessManager("test", metadata_base_dir=safe_mkdtemp()) def test_await_pid(self): with unittest.mock.patch.object(ProcessManager, "await_metadata_by_name") as mock_await: @@ -228,19 +155,20 @@ def test_as_process(self): sentinel = 3333 with unittest.mock.patch("psutil.Process", **PATCH_OPTS) as mock_proc: mock_proc.return_value = sentinel - self.pm._pid = sentinel + self.pm.write_pid(sentinel) self.assertEqual(self.pm._as_process(), sentinel) def test_as_process_no_pid(self): fake_pid = 3 with unittest.mock.patch("psutil.Process", **PATCH_OPTS) as mock_proc: mock_proc.side_effect = psutil.NoSuchProcess(fake_pid) - self.pm._pid = fake_pid + self.pm.write_pid(fake_pid) with self.assertRaises(psutil.NoSuchProcess): self.pm._as_process() def test_as_process_none(self): - self.assertEqual(self.pm._as_process(), None) + with self.assertRaises(self.pm.NotStarted): + self.pm._as_process() def test_is_alive_neg(self): with unittest.mock.patch.object( @@ -285,7 +213,7 @@ def test_is_alive_stale_pid(self): mock_as_process.return_value = fake_process( name="not_test", pid=3, status=psutil.STATUS_IDLE ) - self.pm._process_name = "test" + self.pm.write_process_name("test") self.assertFalse(self.pm.is_alive()) mock_as_process.assert_called_with(self.pm) @@ -316,7 +244,7 @@ def test_purge_metadata_alive_but_forced(self): def test_kill(self): with unittest.mock.patch("os.kill", **PATCH_OPTS) as mock_kill: - self.pm._pid = 42 + self.pm.write_pid(42) self.pm._kill(0) mock_kill.assert_called_once_with(42, 0)