PostgresNode::is_started is read-only and is not used internally (#305)

PostgresNode::is_started provides information about a manually started node.

It is "set" in start method and is "reset" in stop, kill methods.

These methods do not verify this flag and do not use for other aims - for example to ignore the second start or stop of a node.

So we cannot stop and start our node twice.

An updated implementation of kill method raises InvalidOperationException when our node is not running.

This patch allows to use testgres in code where a node started or stopped explicitly, not via start/stop/kill methods.

Author: dmitry-lipetsk

src/node.py

@@ -160,11 +160,14 @@ class PostgresNode(object):
     # a max number of node start attempts
     _C_MAX_START_ATEMPTS = 5
 
+    _C_PM_PID__IS_NOT_DETECTED = -1
+
     _name: typing.Optional[str]
     _port: typing.Optional[int]
     _should_free_port: bool
     _os_ops: OsOperations
     _port_manager: typing.Optional[PortManager]
+    _manually_started_pm_pid: typing.Optional[int]
 
     def __init__(self,
                  name=None,
@@ -251,7 +254,7 @@ def __init__(self,
         self.pg_log_name = self.pg_log_file
 
         # Node state
-        self.is_started = False
+        self._manually_started_pm_pid = None
 
     def __enter__(self):
         return self
@@ -380,6 +383,14 @@ def pid(self) -> int:
         assert type(x.pid) == int  # noqa: E721
         return x.pid
 
+    @property
+    def is_started(self) -> bool:
+        if self._manually_started_pm_pid is None:
+            return False
+
+        assert type(self._manually_started_pm_pid) == int  # noqa: E721
+        return True
+
     @property
     def auxiliary_pids(self) -> typing.Dict[ProcessType, typing.List[int]]:
         """
@@ -995,9 +1006,6 @@ def start(self, params=[], wait=True, exec_env=None):
         assert exec_env is None or type(exec_env) == dict  # noqa: E721
         assert __class__._C_MAX_START_ATEMPTS > 1
 
-        if self.is_started:
-            return self
-
         if self._port is None:
             raise InvalidOperationException("Can't start PostgresNode. Port is not defined.")
 
@@ -1016,8 +1024,11 @@ def LOCAL__start_node():
             if error and 'does not exist' in error:
                 raise Exception(error)
 
-        def LOCAL__raise_cannot_start_node(from_exception, msg):
-            assert isinstance(from_exception, Exception)
+        def LOCAL__raise_cannot_start_node(
+            from_exception: typing.Optional[Exception],
+            msg: str
+        ):
+            assert from_exception is None or isinstance(from_exception, Exception)
             assert type(msg) == str  # noqa: E721
             files = self._collect_special_files()
             raise_from(StartNodeException(msg, files), from_exception)
@@ -1076,7 +1087,17 @@ def LOCAL__raise_cannot_start_node__std(from_exception):
                     continue
                 break
         self._maybe_start_logger()
-        self.is_started = True
+
+        if not wait:
+            # Postmaster process is starting in background
+            self._manually_started_pm_pid = __class__._C_PM_PID__IS_NOT_DETECTED
+        else:
+            self._manually_started_pm_pid = self._get_node_state().pid
+            if self._manually_started_pm_pid is None:
+                LOCAL__raise_cannot_start_node(None, "Cannot detect postmaster pid.")
+
+        assert type(self._manually_started_pm_pid) == int  # noqa: E721
+
         return self
 
     def stop(self, params=[], wait=True):
@@ -1090,9 +1111,6 @@ def stop(self, params=[], wait=True):
         Returns:
             This instance of :class:`.PostgresNode`.
         """
-        if not self.is_started:
-            return self
-
         _params = [
             self._get_bin_path("pg_ctl"),
             "-D", self.data_dir,
@@ -1102,8 +1120,9 @@ def stop(self, params=[], wait=True):
 
         execute_utility2(self.os_ops, _params, self.utils_log_file)
 
+        self._manually_started_pm_pid = None
+
         self._maybe_stop_logger()
-        self.is_started = False
         return self
 
     def kill(self, someone=None):
@@ -1114,14 +1133,27 @@ def kill(self, someone=None):
                 someone: A key to the auxiliary process in the auxiliary_pids dictionary.
                          If None, the main PostgreSQL node process will be killed. Defaults to None.
         """
-        if self.is_started:
-            assert isinstance(self._os_ops, OsOperations)
-            sig = signal.SIGKILL if os.name != 'nt' else signal.SIGBREAK
-            if someone is None:
-                self._os_ops.kill(self.pid, sig)
-            else:
-                self._os_ops.kill(self.auxiliary_pids[someone][0], sig)
-            self.is_started = False
+        x = self._get_node_state()
+        assert type(x) == utils.PostgresNodeState  # noqa: E721
+
+        if x.node_status != NodeStatus.Running:
+            RaiseError.node_err__cant_kill(x.node_status)
+            assert False
+
+        assert x.node_status == NodeStatus.Running
+        assert type(x.pid) == int  # noqa: E721
+        sig = signal.SIGKILL if os.name != 'nt' else signal.SIGBREAK
+        if someone is None:
+            self._os_ops.kill(x.pid, sig)
+            self._manually_started_pm_pid = None
+        else:
+            childs = self._get_child_processes(x.pid)
+            for c in childs:
+                assert type(c) == ProcessProxy  # noqa: E721
+                if c.ptype == someone:
+                    self._os_ops.kill(c.process.pid, sig)
+                continue
+        return
 
     def restart(self, params=[]):
         """

src/raise_error.py

@@ -45,6 +45,21 @@ def node_err__cant_enumerate_child_processes(
 
         raise InvalidOperationException(msg)
 
+    @staticmethod
+    def node_err__cant_kill(
+        node_status: NodeStatus
+    ):
+        assert type(node_status) == NodeStatus  # noqa: E721
+
+        msg = "Can't kill server process. {}.".format(
+            __class__._map_node_status_to_reason(
+                node_status,
+                None,
+            )
+        )
+
+        raise InvalidOperationException(msg)
+
     @staticmethod
     def _map_node_status_to_reason(
         node_status: NodeStatus,

tests/test_raise_error.py

@@ -7,7 +7,7 @@
 
 
 class TestRaiseError:
-    class tagData001__NodeErr_CantEnumerateChildProcesses:
+    class tagTestData001:
         node_status: NodeStatus
         expected_msg: str
 
@@ -29,12 +29,12 @@ def sign(self) -> str:
             msg = "status: {}".format(self.node_status)
             return msg
 
-    sm_Data001: typing.List[tagData001__NodeErr_CantEnumerateChildProcesses] = [
-        tagData001__NodeErr_CantEnumerateChildProcesses(
+    sm_Data001: typing.List[tagTestData001] = [
+        tagTestData001(
             NodeStatus.Uninitialized,
             "Can't enumerate node child processes. Node is not initialized.",
         ),
-        tagData001__NodeErr_CantEnumerateChildProcesses(
+        tagTestData001(
             NodeStatus.Stopped,
             "Can't enumerate node child processes. Node is not running.",
         ),
@@ -44,16 +44,16 @@ def sign(self) -> str:
         params=sm_Data001,
         ids=[x.sign for x in sm_Data001],
     )
-    def data001(self, request: pytest.FixtureRequest) -> tagData001__NodeErr_CantEnumerateChildProcesses:
+    def data001(self, request: pytest.FixtureRequest) -> tagTestData001:
         assert isinstance(request, pytest.FixtureRequest)
-        assert type(request.param).__name__ == "tagData001__NodeErr_CantEnumerateChildProcesses"
+        assert type(request.param).__name__ == "tagTestData001"
         return request.param
 
     def test_001__node_err__cant_enumerate_child_processes(
         self,
-        data001: tagData001__NodeErr_CantEnumerateChildProcesses,
+        data001: tagTestData001,
     ):
-        assert type(data001) == __class__.tagData001__NodeErr_CantEnumerateChildProcesses  # noqa: E721
+        assert type(data001) == __class__.tagTestData001  # noqa: E721
 
         with pytest.raises(expected_exception=InvalidOperationException) as x:
             RaiseError.node_err__cant_enumerate_child_processes(
@@ -63,3 +63,38 @@ def test_001__node_err__cant_enumerate_child_processes(
         assert x is not None
         assert str(x.value) == data001.expected_msg
         return
+
+    sm_Data002: typing.List[tagTestData001] = [
+        tagTestData001(
+            NodeStatus.Uninitialized,
+            "Can't kill server process. Node is not initialized.",
+        ),
+        tagTestData001(
+            NodeStatus.Stopped,
+            "Can't kill server process. Node is not running.",
+        ),
+    ]
+
+    @pytest.fixture(
+        params=sm_Data002,
+        ids=[x.sign for x in sm_Data002],
+    )
+    def data002(self, request: pytest.FixtureRequest) -> tagTestData001:
+        assert isinstance(request, pytest.FixtureRequest)
+        assert type(request.param).__name__ == "tagTestData001"
+        return request.param
+
+    def test_002__node_err__cant_kill(
+        self,
+        data002: tagTestData001,
+    ):
+        assert type(data002) == __class__.tagTestData001  # noqa: E721
+
+        with pytest.raises(expected_exception=InvalidOperationException) as x:
+            RaiseError.node_err__cant_kill(
+                data002.node_status
+            )
+
+        assert x is not None
+        assert str(x.value) == data002.expected_msg
+        return