Merge bitcoin/bitcoin#30724: test: add test for specifying custom pid…

…file via `-pid`

04e4d52 test: add test for specifying custom pidfile via `-pid` (Sebastian Falbesoner)
b832ffe refactor: introduce default pid file name constant in tests (tdb3)

Pull request description:

  This small PR adds test coverage for the `-pid` command line option, which allows to overrule the pid filename (`bitcoind.pid` by default). One can specify either a relative path (within the datadir) or an absolute one; the latter is tested using `self.options.tmpdir`. Note that the functional test file `feature_init.py` so far only contained a stress test; with this new sub-test added, both the description and the test name are adapted to be more generic.

ACKs for top commit:
  achow101:
    ACK 04e4d52
  tdb3:
    ACK 04e4d52
  ryanofsky:
    Code review ACK 04e4d52
  naiyoma:
    Tested ACK [https://github.com/bitcoin/bitcoin/pull/30724/commits/04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a](https://github.com/bitcoin/bitcoin/pull/30724/commits/04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a)

Tree-SHA512: b2bc8a790e5d187e2c84345f344f65a176b62caecd9797c3b9edf10294c741c33a24e535be640b56444b91dcf9c65c7dd152cdffd8b1c1d9ca68e5e3c6ad1e99

test/functional/feature_filelock.py

@@ -7,7 +7,10 @@
 import string
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.test_node import ErrorMatch
+from test_framework.test_node import (
+    BITCOIN_PID_FILENAME_DEFAULT,
+    ErrorMatch,
+)
 
 class FilelockTest(BitcoinTestFramework):
     def add_options(self, parser):
@@ -33,7 +36,7 @@ def run_test(self):
         self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir")
         cookie_file = datadir / ".cookie"
         assert cookie_file.exists()  # should not be deleted during the second bitcoind instance shutdown
-        pid_file = datadir / "bitcoind.pid"
+        pid_file = datadir / BITCOIN_PID_FILENAME_DEFAULT
         assert pid_file.exists()
 
         if self.is_wallet_compiled():

test/functional/feature_init.py

@@ -2,17 +2,20 @@
 # Copyright (c) 2021-present The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Stress tests related to node initialization."""
+"""Tests related to node initialization."""
 from pathlib import Path
 import platform
 import shutil
 
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
-from test_framework.test_node import ErrorMatch
+from test_framework.test_node import (
+    BITCOIN_PID_FILENAME_DEFAULT,
+    ErrorMatch,
+)
 from test_framework.util import assert_equal
 
 
-class InitStressTest(BitcoinTestFramework):
+class InitTest(BitcoinTestFramework):
     """
     Ensure that initialization can be interrupted at a number of points and not impair
     subsequent starts.
@@ -25,7 +28,7 @@ def set_test_params(self):
         self.setup_clean_chain = False
         self.num_nodes = 1
 
-    def run_test(self):
+    def init_stress_test(self):
         """
         - test terminating initialization after seeing a certain log line.
         - test removing certain essential files to test startup error paths.
@@ -147,6 +150,31 @@ def check_clean_start():
             shutil.move(node.chain_path / "blocks_bak", node.chain_path / "blocks")
             shutil.move(node.chain_path / "chainstate_bak", node.chain_path / "chainstate")
 
+    def init_pid_test(self):
+        BITCOIN_PID_FILENAME_CUSTOM = "my_fancy_bitcoin_pid_file.foobar"
+
+        self.log.info("Test specifying custom pid file via -pid command line option")
+        custom_pidfile_relative = BITCOIN_PID_FILENAME_CUSTOM
+        self.log.info(f"-> path relative to datadir ({custom_pidfile_relative})")
+        self.restart_node(0, [f"-pid={custom_pidfile_relative}"])
+        datadir = self.nodes[0].chain_path
+        assert not (datadir / BITCOIN_PID_FILENAME_DEFAULT).exists()
+        assert (datadir / custom_pidfile_relative).exists()
+        self.stop_node(0)
+        assert not (datadir / custom_pidfile_relative).exists()
+
+        custom_pidfile_absolute = Path(self.options.tmpdir) / BITCOIN_PID_FILENAME_CUSTOM
+        self.log.info(f"-> absolute path ({custom_pidfile_absolute})")
+        self.restart_node(0, [f"-pid={custom_pidfile_absolute}"])
+        assert not (datadir / BITCOIN_PID_FILENAME_DEFAULT).exists()
+        assert custom_pidfile_absolute.exists()
+        self.stop_node(0)
+        assert not custom_pidfile_absolute.exists()
+
+    def run_test(self):
+        self.init_pid_test()
+        self.init_stress_test()
+
 
 if __name__ == '__main__':
-    InitStressTest(__file__).main()
+    InitTest(__file__).main()

test/functional/test_framework/test_node.py

@@ -48,6 +48,7 @@
 NUM_XOR_BYTES = 8
 # The null blocks key (all 0s)
 NULL_BLK_XOR_KEY = bytes([0] * NUM_XOR_BYTES)
+BITCOIN_PID_FILENAME_DEFAULT = "bitcoind.pid"
 
 
 class FailedToStartError(Exception):