Merge bitcoin/bitcoin#29034: test: detect OS in functional tests consistently using platform.system()

878d914777 doc: test: mention OS detection preferences in style guideline (Sebastian Falbesoner)
4c65ac96f8 test: detect OS consistently using `platform.system()` (Sebastian Falbesoner)
37324ae3df test: use `skip_if_platform_not_linux` helper where possible (Sebastian Falbesoner)

Pull request description:

  There are at least three ways to detect the operating system in Python3:
  - `os.name` (https://docs.python.org/3.9/library/os.html#os.name)
  - `sys.platform` (https://docs.python.org/3.9/library/sys.html#sys.platform)
  - `platform.system()` (https://docs.python.org/3.9/library/platform.html#platform.system)

  We are currently using all of them in functional tests (both in individual tests and shared test framework code), which seems a bit messy. This PR consolidates into using `platform.system()`, as it appears to be one most consistent and easy to read (see also [IRC discussion](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-08#989301;) and table below). `sys.platform` is inconsistent as it has the major version number encoded for BSD systems, which doesn't make much sense for e.g. OpenBSD, where there is no concept of major versions, but instead the version is simply increased by 0.1 on each release.

  Note that `os.name` is still useful to detect whether we are running a POSIX system (see `BitcoinTestFramework.skip_if_platform_not_posix`), so for this use-case it is kept as only exception. The following table shows values for common operating systems, found via
  ```
  $ python3 -c "import os; import sys; import platform; print(os.name, sys.platform, platform.system())"
  ```

  |     OS       | os.name | sys.platform | platform.system()  |
  |--------------|---------|--------------|--------------------|
  | Linux 6.2.0  |  posix  |   linux      |      Linux         |
  | MacOS*       |  posix  |   darwin     |      Darwin        |
  | OpenBSD 7.4  |  posix  |   openbsd7   |      OpenBSD       |
  | Windows*     |  nt     |   win32      |      Windows       |

  \* = I neither have a MacOS nor a Windows machine available, so I extracted the values from documentation and our current code. Also I'm relying on CI for testing the relevant code-paths. Having reviewers to this this locally would be very appreciated, if this gets Concept ACKed.

ACKs for top commit:
  kevkevinpal:
    ACK [878d914](878d914777)
  achow101:
    ACK 878d914777
  hebasto:
    ACK 878d914777, I have reviewed the code and it looks OK.
  pablomartin4btc:
    tACK 878d914777

Tree-SHA512: 24513d493e47f572028c843260b81c47c2c29bfb701991050255c9f9529cd19065ecbc7b3b6e15619da7f3f608b4825c345ce6fee30d8fd1eaadbd08cff400fc
This commit is contained in:
Ava Chow
2024-01-11 12:08:55 -05:00
12 changed files with 35 additions and 34 deletions

View File

@@ -24,6 +24,7 @@ import asyncio
from collections import defaultdict
from io import BytesIO
import logging
import platform
import struct
import sys
import threading
@@ -592,7 +593,7 @@ class NetworkThread(threading.Thread):
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()

View File

@@ -12,6 +12,7 @@ import http.client
import json
import logging
import os
import platform
import re
import subprocess
import tempfile
@@ -19,7 +20,6 @@ import time
import urllib.parse
import collections
import shlex
import sys
from pathlib import Path
from .authproxy import (
@@ -566,7 +566,7 @@ class TestNode():
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

View File

@@ -13,8 +13,8 @@ import json
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"