qa: Use `sys.executable` when invoking other Python scripts #31541

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:241219-qa-signers changing 2 files +6 −17
  1. hebasto commented at 4:06 PM on December 19, 2024: member

    This PR fixes the rpc_signer.py and wallet_signer.py functional tests on systems where python3 is not available in the PATH, causing the shebang #!/usr/bin/env python3 to fail.

    Here are logs on NetBSD 10.0:

    • without this PR:
    $ python3.12 ./build/test/functional/test_runner.py rpc_signer.py wallet_signer.py
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_160538
Remaining jobs: [rpc_signer.py, wallet_signer.py --descriptors]
1/2 - rpc_signer.py failed, Duration: 1 s

stdout:
2024-12-19T16:05:40.012000Z TestFramework (INFO): PRNG seed is: 1833166631173850775
2024-12-19T16:05:40.012000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1
2024-12-19T16:05:40.754000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 160, in try_rpc
    fun(*args, **kwds)
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(/home/hebasto/dev/bitcoin/test/functional/mocks/signer.py enumerate) returned 127: env: python3: No such file or directory
 (-1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_framework.py", line 135, in main
    self.run_test()
  File "/home/hebasto/dev/bitcoin/build/test/functional/rpc_signer.py", line 72, in run_test
    assert_raises_rpc_error(-1, 'fingerprint not found',
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 151, in assert_raises_rpc_error
    assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 166, in try_rpc
    raise AssertionError(
AssertionError: Expected substring not found in error message:
substring: 'fingerprint not found'
error message: 'RunCommandParseJSON error: process(/home/hebasto/dev/bitcoin/test/functional/mocks/signer.py enumerate) returned 127: env: python3: No such file or directory
'.
2024-12-19T16:05:40.756000Z TestFramework (INFO): Stopping nodes
2024-12-19T16:05:40.873000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1
2024-12-19T16:05:40.873000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1/test_framework.log
2024-12-19T16:05:40.873000Z TestFramework (ERROR): 
2024-12-19T16:05:40.873000Z TestFramework (ERROR): Hint: Call /home/hebasto/dev/bitcoin/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1' to consolidate all logs
2024-12-19T16:05:40.873000Z TestFramework (ERROR): 
2024-12-19T16:05:40.873000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-12-19T16:05:40.873000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-12-19T16:05:40.873000Z TestFramework (ERROR): 


stderr:


Remaining jobs: [wallet_signer.py --descriptors]
2/2 - wallet_signer.py --descriptors failed, Duration: 1 s

stdout:
2024-12-19T16:05:40.014000Z TestFramework (INFO): PRNG seed is: 7530764367977090686
2024-12-19T16:05:40.014000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0
2024-12-19T16:05:40.526000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_framework.py", line 135, in main
    self.run_test()
  File "/home/hebasto/dev/bitcoin/build/test/functional/wallet_signer.py", line 66, in run_test
    self.test_valid_signer()
  File "/home/hebasto/dev/bitcoin/build/test/functional/wallet_signer.py", line 83, in test_valid_signer
    self.nodes[1].createwallet(wallet_name='hww', disable_private_keys=True, descriptors=True, external_signer=True)
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_node.py", line 935, in createwallet
    return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors, load_on_startup, external_signer)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(/home/hebasto/dev/bitcoin/test/functional/mocks/signer.py enumerate) returned 127: env: python3: No such file or directory
 (-1)
2024-12-19T16:05:40.528000Z TestFramework (INFO): Stopping nodes
2024-12-19T16:05:40.645000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0
2024-12-19T16:05:40.646000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0/test_framework.log
2024-12-19T16:05:40.646000Z TestFramework (ERROR): 
2024-12-19T16:05:40.646000Z TestFramework (ERROR): Hint: Call /home/hebasto/dev/bitcoin/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0' to consolidate all logs
2024-12-19T16:05:40.646000Z TestFramework (ERROR): 
2024-12-19T16:05:40.646000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-12-19T16:05:40.646000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-12-19T16:05:40.646000Z TestFramework (ERROR): 


stderr:



TEST                           | STATUS    | DURATION

rpc_signer.py                  | ✖ Failed  | 1 s
wallet_signer.py --descriptors | ✖ Failed  | 1 s

ALL                            | ✖ Failed  | 2 s (accumulated) 
Runtime: 1 s
    • with this PR:
    $ python3.12 ./build/test/functional/test_runner.py rpc_signer.py wallet_signer.py   
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_160011
Remaining jobs: [rpc_signer.py, wallet_signer.py --descriptors]
1/2 - rpc_signer.py passed, Duration: 2 s
Remaining jobs: [wallet_signer.py --descriptors]
2/2 - wallet_signer.py --descriptors passed, Duration: 3 s

TEST                           | STATUS    | DURATION

rpc_signer.py                  | ✓ Passed  | 2 s
wallet_signer.py --descriptors | ✓ Passed  | 3 s

ALL                            | ✓ Passed  | 5 s (accumulated) 
Runtime: 3 s
  2. hebasto added the label Tests on Dec 19, 2024
  3. DrahtBot commented at 4:06 PM on December 19, 2024: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31541.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK maflcko, stickies-v
    Concept ACK tdb3, BrandonOdiwuor

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

  4. hebasto force-pushed on Dec 19, 2024
  5. DrahtBot added the label CI failed on Dec 19, 2024
  6. DrahtBot commented at 4:15 PM on December 19, 2024: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Debug: https://github.com/bitcoin/bitcoin/runs/34664908840</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  7. maflcko commented at 5:08 PM on December 19, 2024: member

    lgtm. even without netbsd, this is required to pass down the python version that was selected to run the tests. This is also used in all other places in the tests, like https://github.com/bitcoin/bitcoin/blob/bb57017b2945d5e0bbd95c7f1a9369a8ab7c6fcd/test/functional/test_runner.py#L632

  8. qa: Use `sys.executable` when invoking other Python scripts
    This change fixes tests on systems where `python3` is not available
in the `PATH`, causing the shebang `#!/usr/bin/env python3` to fail.
    d38ade7bc4
  9. hebasto force-pushed on Dec 19, 2024
  10. hebasto marked this as ready for review on Dec 19, 2024
  11. hebasto commented at 5:12 PM on December 19, 2024: member

    lgtm. even without netbsd

    Thanks! Rebased on the master branch.

  12. DrahtBot removed the label CI failed on Dec 19, 2024
  13. hebasto commented at 7:55 PM on December 19, 2024: member

    Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.

  14. fanquake commented at 11:05 AM on December 20, 2024: member

    Tracked in hebasto/bitcoin-core-nightly#6.

    Unless there's a good reason not too, it'd be good to track issues in our issue tracker, rather than some other repository. Otherwise they are not included in stats tracking, the metadata archives, Core Check dashboards, overall less discoverable etc

  15. maflcko commented at 11:15 AM on December 20, 2024: member

    I think in this case it is fine, because each issue has a pull request with an extended description, so it will end up in the metadata backup. But anything should be fine by me.

  16. maflcko commented at 11:16 AM on December 20, 2024: member

    lgtm ACK d38ade7bc409207bf425e05ee10b633b0aef4c36

    In theory, the shebang could be removed from the mocks files, to better enforce this, but this could make one of the linters sad, so could be done in a follow-up.

  17. hebasto commented at 11:51 AM on December 20, 2024: member

    Friendly ping Python connoisseur @stickies-v :)

  18. tdb3 commented at 1:32 PM on January 2, 2025: contributor

    Concept ACK

    Ran both tests with test_runner on NetBSD 10.1 and Ubuntu 24.04. Both passed. Encountered a failure with tool_wallet on NetBSD that I want to check out before full ACK.

  19. hebasto commented at 4:29 PM on January 2, 2025: member

    Encountered a failure with tool_wallet on NetBSD that I want to check out before full ACK.

    The issue with tool_wallet is fixed in #31543. It could be worked around by setting the LD_LIBRARY_PATH environment variable.

  20. maflcko added this to the milestone 29.0 on Jan 2, 2025
  21. maflcko removed this from the milestone 29.0 on Jan 2, 2025
  22. BrandonOdiwuor commented at 9:46 AM on January 7, 2025: contributor

    Concept ACK

  23. in test/functional/wallet_signer.py:25 in d38ade7bc4 
      23 | @@ -24,24 +24,15 @@ def add_options(self, parser):
  24 |  
  25 |      def mock_signer_path(self):
    stickies-v commented at 10:42 AM on January 16, 2025:

    nit: these functions don't return paths. mock_signer_command would be more appropriate. Alternatively, they could keep returning paths, and prepending the python binary could be a responsibility of the callsite (potentially through py_command() helper function.

  24. in test/functional/rpc_signer.py:24 in d38ade7bc4 
      20 | @@ -20,10 +21,7 @@
  21 |  class RPCSignerTest(BitcoinTestFramework):
  22 |      def mock_signer_path(self):
  23 |          path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'signer.py')
  24 | -        if platform.system() == "Windows":
  25 | -            return "py -3 " + path
  26 | -        else:
  27 | -            return path
  28 | +        return sys.executable + " " + path
    stickies-v commented at 10:58 AM on January 16, 2025:

    note: from https://docs.python.org/3/library/sys.html#sys.executable, there are no guarantees that sys.executable returns a Python path

    A string giving the absolute path of the executable binary for the Python interpreter, on systems where this makes sense. If Python is unable to retrieve the real path to its executable, sys.executable will be an empty string or None.

    It seems like this shouldn't be an issue in normal circumstances, and I'm not sure if this should be considered user error if/when it happens, but it's something to be aware of. Perhaps wrapping this line in a py_command(cmd) function and adding a warning log if sys.executable is empty might make future debugging easier?

    maflcko commented at 11:35 AM on January 16, 2025:

    note: from https://docs.python.org/3/library/sys.html#sys.executable, there are no guarantees that sys.executable returns a Python path

    For the purposes of functional test, it is guaranteed, because it is already used and enforced elsewhere. You can check this with git grep 'sys.exec' test/functional/.

    stickies-v commented at 11:47 AM on January 16, 2025:

    Ah yes, that's true, test_runner itself shouldn't even work in that case.

  25. stickies-v approved
  26. stickies-v commented at 11:01 AM on January 16, 2025: contributor

    ACK d38ade7bc409207bf425e05ee10b633b0aef4c36 . I have a minor concern about sys.executable not being guaranteed to return a valid Python path, but this patch seems good enough as is so no blocker.

    The mock scripts (e.g. mock/multi_signers.py) are executed from within functional tests such as wallet_signer.py. The mocks contain a shebang (#!/usr/bin/env python3), but shebangs are not well supported on Windows, which is why prior to this commit, py -3 was prefixed to the path.

    Prior to this PR, on any platform, because the binary to be used to run these mocks (py on Windows, /usr/bin/env python3 on any other) is hardcoded, it can be different from the Python binary used to run the functional test. This can lead to to test failures or anomalies because of unexpected environment issues, or because the hardcoded binaries are not in fact available on the user's system.

    Fix this by running the mock scripts with the same binary as the test runner.

  27. DrahtBot requested review from tdb3 on Jan 16, 2025
  28. DrahtBot requested review from BrandonOdiwuor on Jan 16, 2025
  29. hebasto commented at 8:44 AM on January 23, 2025: member

    cc @ryanofsky

  30. maflcko commented at 10:46 AM on January 23, 2025: member

    test-only change with two acks looks RFM?

  31. fanquake merged this on Jan 23, 2025
  32. fanquake closed this on Jan 23, 2025
  33. hebasto deleted the branch on Jan 23, 2025
  34. hebasto referenced this in commit 2c4c6333d9 on Jan 23, 2025
  35. sedited referenced this in commit e1405325c9 on Feb 3, 2025
  36. stickies-v referenced this in commit d760fd3dda on Mar 17, 2025
  37. stickies-v referenced this in commit cc83553352 on Mar 17, 2025
  38. stickies-v referenced this in commit 2614933f06 on Mar 17, 2025
  39. stickies-v referenced this in commit b70418c5fc on Mar 17, 2025
  40. stickies-v referenced this in commit 69f8a1fe50 on Mar 17, 2025
  41. bug-castercv502 referenced this in commit bf2e6853b0 on Sep 28, 2025
  42. bitcoin locked this on Jan 23, 2026
Contributors
hebastoDrahtBotmaflckofanquaketdb3BrandonOdiwuorstickies-v


BrandonOdiwuortdb3

Labels
Tests
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-22 18:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me