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

1. 
   theStack commented at 5:12 pm on December 8, 2023: contributor

   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 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

   0$ 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.

2. 
   DrahtBot commented at 5:12 pm on December 8, 2023: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	kevkevinpal, hebasto, pablomartin4btc, achow101

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29182 (test: randomize perturbed files excluding ldb by L0laL33tz)
   • #22729 (Make it possible to disable Tor binds and abort startup on bind failure by vasild)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label Tests on Dec 8, 2023
4. 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.

   37324ae3df
5. test: detect OS consistently using `platform.system()` 4c65ac96f8
6. doc: test: mention OS detection preferences in style guideline 878d914777
7. theStack force-pushed on Dec 8, 2023
8. DrahtBot added the label CI failed on Dec 8, 2023
9. DrahtBot removed the label CI failed on Dec 8, 2023
10. 
    kevkevinpal commented at 7:25 pm on December 8, 2023: contributor

    ACK 878d914

    ---

    Ran this command to make sure we were not missing any, only seeing os.name show up for posix in test_framework.py

    0$ grep -nr "os\.name" ./test && grep -nr "sys\.platform" ./test
    1
    2./test/functional/README.md:40:- Use `platform.system()` for detecting the running operating system and `os.name` to
    3./test/functional/test_framework/test_framework.py:918:        if os.name != 'posix':
    

    ---

    I am running on MacOs 14.1.2 (23B92) and got the following from running the code provided

    0$ python3 -c "import os; import sys; import platform; print(os.name, sys.platform, platform.system())"
    1
    2posix darwin Darwin
    

11. 
    hebasto commented at 2:51 pm on December 10, 2023: member

    Concept ACK.
12. 
    hebasto commented at 4:17 pm on December 10, 2023: member

    The following table shows values for common operating systems, found via

    0$ python3 -c "import os; import sys; import platform; print(os.name, sys.platform, platform.system())"
    

    OS	os.name	sys.platform	platform.system()
    MacOS*	posix	darwin	Darwin
    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.

    Confirming outputs for macOS and Windows.

13. hebasto approved
14. 
    hebasto commented at 5:57 pm on December 10, 2023: member

    ACK 878d914777a03a04ecb84217152e8b7fd73a5062, I have reviewed the code and it looks OK.
15. 
    pablomartin4btc commented at 6:55 pm on January 10, 2024: member

    tACK 878d914777a03a04ecb84217152e8b7fd73a5062

    Tested on both Linux (Ubuntu 22.04 & WSL) and Windows.

    As @kevkevinpal pointed it out, there’s still usage of os.name in test_framework.py, not sure if you want to cover that as well.

16. 
    achow101 commented at 5:16 pm on January 11, 2024: member

    ACK 878d914777a03a04ecb84217152e8b7fd73a5062
17. achow101 merged this on Jan 11, 2024
18. achow101 closed this on Jan 11, 2024

    ---

19. theStack deleted the branch on Jan 11, 2024

Contributors
theStack DrahtBot kevkevinpal hebasto pablomartin4btc achow101

Labels
Tests