From 37324ae3dfb0e50daaf752dc863a880559fa4637 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Fri, 8 Dec 2023 16:52:57 +0100
Subject: [PATCH 1/3] test: use `skip_if_platform_not_linux` helper where
 possible

Rather than re-implementing these checks, we can use this test
framework's helper (introduced in commit
c934087b627f7d368458781944f990b0eb479634, PR #24358) called in a test's
`skip_test_if_missing_module` method instead.
---
 test/functional/feature_bind_extra.py | 10 +++-------
 test/functional/rpc_bind.py           | 11 ++++-------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/test/functional/feature_bind_extra.py b/test/functional/feature_bind_extra.py
index 4a94d2ce7b..5cd031f852 100755
--- a/test/functional/feature_bind_extra.py
+++ b/test/functional/feature_bind_extra.py
@@ -7,15 +7,12 @@
 that bind happens on the expected ports.
 """
 
-import sys
-
 from test_framework.netutil import (
     addr_to_hex,
     get_bind_addrs,
 )
 from test_framework.test_framework import (
     BitcoinTestFramework,
-    SkipTest,
 )
 from test_framework.util import (
     assert_equal,
@@ -32,12 +29,11 @@ def set_test_params(self):
         self.bind_to_localhost_only = False
         self.num_nodes = 2
 
-    def setup_network(self):
+    def skip_test_if_missing_module(self):
         # Due to OS-specific network stats queries, we only run on Linux.
-        self.log.info("Checking for Linux")
-        if not sys.platform.startswith('linux'):
-            raise SkipTest("This test can only be run on Linux.")
+        self.skip_if_platform_not_linux()
 
+    def setup_network(self):
         loopback_ipv4 = addr_to_hex("127.0.0.1")
 
         # Start custom ports by reusing unused p2p ports
diff --git a/test/functional/rpc_bind.py b/test/functional/rpc_bind.py
index 43cd23fc7a..3106419e5c 100755
--- a/test/functional/rpc_bind.py
+++ b/test/functional/rpc_bind.py
@@ -4,8 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test running bitcoind with the -rpcbind and -rpcallowip options."""
 
-import sys
-
 from test_framework.netutil import all_interfaces, addr_to_hex, get_bind_addrs, test_ipv6_local
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
 from test_framework.util import assert_equal, assert_raises_rpc_error, get_rpc_proxy, rpc_port, rpc_url
@@ -17,6 +15,10 @@ def set_test_params(self):
         self.num_nodes = 1
         self.supports_cli = False
 
+    def skip_test_if_missing_module(self):
+        # due to OS-specific network stats queries, this test works only on Linux
+        self.skip_if_platform_not_linux()
+
     def setup_network(self):
         self.add_nodes(self.num_nodes, None)
 
@@ -61,14 +63,9 @@ def run_allowip_test(self, allow_ips, rpchost, rpcport):
         self.stop_nodes()
 
     def run_test(self):
-        # due to OS-specific network stats queries, this test works only on Linux
         if sum([self.options.run_ipv4, self.options.run_ipv6, self.options.run_nonloopback]) > 1:
             raise AssertionError("Only one of --ipv4, --ipv6 and --nonloopback can be set")
 
-        self.log.info("Check for linux")
-        if not sys.platform.startswith('linux'):
-            raise SkipTest("This test can only be run on linux.")
-
         self.log.info("Check for ipv6")
         have_ipv6 = test_ipv6_local()
         if not have_ipv6 and not (self.options.run_ipv4 or self.options.run_nonloopback):

From 4c65ac96f8b021c107783adce3e8afe4f8edee6e Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Fri, 8 Dec 2023 17:30:19 +0100
Subject: [PATCH 2/3] test: detect OS consistently using `platform.system()`

---
 test/functional/feature_config_args.py                   | 6 +++---
 test/functional/feature_init.py                          | 4 ++--
 test/functional/feature_notifications.py                 | 9 +++++----
 .../functional/feature_remove_pruned_files_on_startup.py | 3 ++-
 test/functional/test_framework/p2p.py                    | 3 ++-
 test/functional/test_framework/test_node.py              | 4 ++--
 test/functional/test_framework/util.py                   | 6 +++---
 test/functional/test_runner.py                           | 5 +++--
 test/functional/wallet_multiwallet.py                    | 4 ++--
 9 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
index dcea662089..9e13a3deef 100755
--- a/test/functional/feature_config_args.py
+++ b/test/functional/feature_config_args.py
@@ -6,8 +6,8 @@
 
 import os
 from pathlib import Path
+import platform
 import re
-import sys
 import tempfile
 import time
 
@@ -116,7 +116,7 @@ def test_config_file_parser(self):
     def test_config_file_log(self):
         # Disable this test for windows currently because trying to override
         # the default datadir through the environment does not seem to work.
-        if sys.platform == "win32":
+        if platform.system() == "Windows":
             return
 
         self.log.info('Test that correct configuration path is changed when configuration file changes the datadir')
@@ -339,7 +339,7 @@ def test_ignored_conf(self):
     def test_ignored_default_conf(self):
         # Disable this test for windows currently because trying to override
         # the default datadir through the environment does not seem to work.
-        if sys.platform == "win32":
+        if platform.system() == "Windows":
             return
 
         self.log.info('Test error is triggered when bitcoin.conf in the default data directory sets another datadir '
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py
index 142d75a851..268009b0f4 100755
--- a/test/functional/feature_init.py
+++ b/test/functional/feature_init.py
@@ -3,8 +3,8 @@
 # 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."""
-import os
 from pathlib import Path
+import platform
 import shutil
 
 from test_framework.test_framework import BitcoinTestFramework, SkipTest
@@ -36,7 +36,7 @@ def run_test(self):
         # and other approaches (like below) don't work:
         #
         #   os.kill(node.process.pid, signal.CTRL_C_EVENT)
-        if os.name == 'nt':
+        if platform.system() == 'Windows':
             raise SkipTest("can't SIGTERM on Windows")
 
         self.stop_node(0)
diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py
index adf6c13973..d2b5315d31 100755
--- a/test/functional/feature_notifications.py
+++ b/test/functional/feature_notifications.py
@@ -4,6 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the -alertnotify, -blocknotify and -walletnotify options."""
 import os
+import platform
 
 from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE
 from test_framework.descriptors import descsum_create
@@ -14,13 +15,13 @@
 
 # Linux allow all characters other than \x00
 # Windows disallow control characters (0-31) and /\?%:|"<>
-FILE_CHAR_START = 32 if os.name == 'nt' else 1
+FILE_CHAR_START = 32 if platform.system() == 'Windows' else 1
 FILE_CHAR_END = 128
-FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if os.name == 'nt' else '/'
+FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/'
 UNCONFIRMED_HASH_STRING = 'unconfirmed'
 
 def notify_outputname(walletname, txid):
-    return txid if os.name == 'nt' else f'{walletname}_{txid}'
+    return txid if platform.system() == 'Windows' else f'{walletname}_{txid}'
 
 
 class NotificationsTest(BitcoinTestFramework):
@@ -181,7 +182,7 @@ def expect_wallet_notify(self, tx_details):
                 # Universal newline ensures '\n' on 'nt'
                 assert_equal(text[-1], '\n')
                 text = text[:-1]
-                if os.name == 'nt':
+                if platform.system() == 'Windows':
                     # On Windows, echo as above will append a whitespace
                     assert_equal(text[-1], ' ')
                     text = text[:-1]
diff --git a/test/functional/feature_remove_pruned_files_on_startup.py b/test/functional/feature_remove_pruned_files_on_startup.py
index c128587949..4ee653142a 100755
--- a/test/functional/feature_remove_pruned_files_on_startup.py
+++ b/test/functional/feature_remove_pruned_files_on_startup.py
@@ -4,6 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test removing undeleted pruned blk files on startup."""
 
+import platform
 import os
 from test_framework.test_framework import BitcoinTestFramework
 
@@ -32,7 +33,7 @@ def run_test(self):
         self.nodes[0].pruneblockchain(600)
 
         # Windows systems will not remove files with an open fd
-        if os.name != 'nt':
+        if platform.system() != 'Windows':
             assert not os.path.exists(blk0)
             assert not os.path.exists(rev0)
             assert not os.path.exists(blk1)
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index b1ed97b794..34fe467d23 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -24,6 +24,7 @@
 from collections import defaultdict
 from io import BytesIO
 import logging
+import platform
 import struct
 import sys
 import threading
@@ -592,7 +593,7 @@ def __init__(self):
 
         NetworkThread.listeners = {}
         NetworkThread.protos = {}
-        if sys.platform == 'win32':
+        if platform.system() == 'Windows':
             asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
         NetworkThread.network_event_loop = asyncio.new_event_loop()
 
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 90c1213deb..c6465f0346 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -12,6 +12,7 @@
 import json
 import logging
 import os
+import platform
 import re
 import subprocess
 import tempfile
@@ -19,7 +20,6 @@
 import urllib.parse
 import collections
 import shlex
-import sys
 from pathlib import Path
 
 from .authproxy import (
@@ -567,7 +567,7 @@ def test_success(cmd):
                 cmd, shell=True,
                 stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL) == 0
 
-        if not sys.platform.startswith('linux'):
+        if platform.system() != 'Linux':
             self.log.warning("Can't profile with perf; only available on Linux platforms")
             return None
 
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index c65e3e38e6..b4b05b1597 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -13,8 +13,8 @@
 import logging
 import os
 import pathlib
+import platform
 import re
-import sys
 import time
 
 from . import coverage
@@ -414,12 +414,12 @@ def get_temp_default_datadir(temp_dir: pathlib.Path) -> tuple[dict, pathlib.Path
     """Return os-specific environment variables that can be set to make the
     GetDefaultDataDir() function return a datadir path under the provided
     temp_dir, as well as the complete path it would return."""
-    if sys.platform == "win32":
+    if platform.system() == "Windows":
         env = dict(APPDATA=str(temp_dir))
         datadir = temp_dir / "Bitcoin"
     else:
         env = dict(HOME=str(temp_dir))
-        if sys.platform == "darwin":
+        if platform.system() == "Darwin":
             datadir = temp_dir / "Library/Application Support/Bitcoin"
         else:
             datadir = temp_dir / ".bitcoin"
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 6016a482f8..245c3b4b06 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -17,6 +17,7 @@
 import configparser
 import datetime
 import os
+import platform
 import time
 import shutil
 import signal
@@ -42,8 +43,8 @@
     CROSS = "x "
     CIRCLE = "o "
 
-if os.name != 'nt' or sys.getwindowsversion() >= (10, 0, 14393): #type:ignore
-    if os.name == 'nt':
+if platform.system() != 'Windows' or sys.getwindowsversion() >= (10, 0, 14393): #type:ignore
+    if platform.system() == 'Windows':
         import ctypes
         kernel32 = ctypes.windll.kernel32  # type: ignore
         ENABLE_VIRTUAL_TERMINAL_PROCESSING = 4
diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py
index 10bc516d8f..ee866ee59b 100755
--- a/test/functional/wallet_multiwallet.py
+++ b/test/functional/wallet_multiwallet.py
@@ -9,9 +9,9 @@
 from decimal import Decimal
 from threading import Thread
 import os
+import platform
 import shutil
 import stat
-import sys
 import time
 
 from test_framework.authproxy import JSONRPCException
@@ -143,7 +143,7 @@ def wallet_file(name):
 
         # should raise rpc error if wallet path can't be created
         err_code = -4 if self.options.descriptors else -1
-        assert_raises_rpc_error(err_code, "filesystem error:" if sys.platform != 'win32' else "create_directories:", self.nodes[0].createwallet, "w8/bad")
+        assert_raises_rpc_error(err_code, "filesystem error:" if platform.system() != 'Windows' else "create_directories:", self.nodes[0].createwallet, "w8/bad")
 
         # check that all requested wallets were created
         self.stop_node(0)

From 878d914777a03a04ecb84217152e8b7fd73a5062 Mon Sep 17 00:00:00 2001
From: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Date: Fri, 8 Dec 2023 17:39:23 +0100
Subject: [PATCH 3/3] doc: test: mention OS detection preferences in style
 guideline

---
 test/functional/README.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/functional/README.md b/test/functional/README.md
index 1bd618a0c3..a4994f2e7c 100644
--- a/test/functional/README.md
+++ b/test/functional/README.md
@@ -37,6 +37,10 @@ don't have test cases for.
   `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of
   the subclass, then locally-defined helper methods, then the `run_test()` method.
 - Use `f'{x}'` for string formatting in preference to `'{}'.format(x)` or `'%s' % x`.
+- Use `platform.system()` for detecting the running operating system and `os.name` to
+  check whether it's a POSIX system (see also the `skip_if_platform_not_{linux,posix}`
+  methods in the `BitcoinTestFramework` class, which can be used to skip a whole test
+  depending on the platform).
 
 #### Naming guidelines