test: skip test if any of the needed release binaries is missing #31462

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202412-test-skip_if_release_binaries_missing changing 1 files +8 −0
  1. theStack commented at 6:53 pm on December 10, 2024: contributor

    If the releases directory exists, but still only a subset of the necessary previous release binaries are available, the test fails by throwing an exception (sometimes leading to follow-up exceptions like AssertionError: [node 0] Error: no RPC connection) and printing out a stack trace, which can be confusing and at a first glance suggests that the node crashed or some alike. Improve this by checking and printing out all of the missing release binaries and skip the test in this case. Noticed while testing #30328.

    Can be tested by e.g.

    0$ rm -rf ./releases
1$ ./test/get_previous_releases -b
2$ rm -rf ./releases/v28.0/
3$ ./build/test/functional/wallet_migration.py

    master:

     0...
 12024-12-10T18:48:54.067000Z TestFramework (ERROR): Assertion failed
 2Traceback (most recent call last):
 3  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 590, in start_nodes
 4    node.start(extra_args[i], *args, **kwargs)
 5  File "/home/thestack/bitcoin/test/functional/test_framework/test_node.py", line 257, in start
 6    self.process = subprocess.Popen(self.args + extra_args, env=subp_env, stdout=stdout, stderr=stderr, cwd=cwd, **kwargs)
 7  File "/usr/lib/python3.10/subprocess.py", line 971, in __init__
 8    self._execute_child(args, executable, preexec_fn, close_fds,
 9  File "/usr/lib/python3.10/subprocess.py", line 1863, in _execute_child
10    raise child_exception_type(errno_num, err_msg, err_filename)
11FileNotFoundError: [Errno 2] No such file or directory: '/home/thestack/bitcoin/releases/v28.0/bin/bitcoind'
12
13During handling of the above exception, another exception occurred:
14
15Traceback (most recent call last):
16  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
17    self.setup()
18  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 315, in setup
19    self.setup_network()
20  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 409, in setup_network
21    self.setup_nodes()
22  File "/home/thestack/bitcoin/./build/test/functional/wallet_migration.py", line 54, in setup_nodes
23    self.start_nodes()
24  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 595, in start_nodes
25    self.stop_nodes()
26  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 610, in stop_nodes
27    node.stop_node(wait=wait, wait_until_stopped=False)
28  File "/home/thestack/bitcoin/test/functional/test_framework/test_node.py", line 396, in stop_node
29    self.stop(wait=wait)
30  File "/home/thestack/bitcoin/test/functional/test_framework/test_node.py", line 215, in __getattr__
31    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
32AssertionError: [node 0] Error: no RPC connection
332024-12-10T18:48:54.118000Z TestFramework (INFO): Stopping nodes
34Traceback (most recent call last):
35  File "/home/thestack/bitcoin/./build/test/functional/wallet_migration.py", line 1097, in <module>
36    WalletMigrationTest(__file__).main()
37  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 159, in main
38    exit_code = self.shutdown()
39  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 331, in shutdown
40    self.stop_nodes()
41  File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 610, in stop_nodes
42    node.stop_node(wait=wait, wait_until_stopped=False)
43  File "/home/thestack/bitcoin/test/functional/test_framework/test_node.py", line 396, in stop_node
44    self.stop(wait=wait)
45  File "/home/thestack/bitcoin/test/functional/test_framework/test_node.py", line 215, in __getattr__
46    assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
47AssertionError: [node 0] Error: no RPC connection
48[node 0] Cleaning up leftover process
49...

    PR:

    0...
12024-12-10T18:46:05.423000Z TestFramework (ERROR): Binary not found: /home/thestack/bitcoin/releases/v28.0/bin/bitcoind
22024-12-10T18:46:05.423000Z TestFramework (ERROR): Binary not found: /home/thestack/bitcoin/releases/v28.0/bin/bitcoin-cli
32024-12-10T18:46:05.423000Z TestFramework (WARNING): Test Skipped: At least one release binary is missing
4...
  2. test: skip test if any of the needed release binaries is missing 
    If the `releases` directory exists, but still only a subset of the
necessary previous release binaries are available, the test fails by
throwing an exception (sometimes leading to follow-up exceptions like
"AssertionError: [node 0] Error: no RPC connection") and printing out
a stack trace, which can be confusing and at a first glance suggests
that the node crashed or some alike.
Improve this by checking and printing out *all* of the missing release
binaries and skip the test in this case. Noticed while testing #30328.

Can be tested by e.g.

$ ./test/get_previous_releases -b
$ rm -rf ./releases/v28.0/
$ ./build/test/functional/wallet_migration.py
    2a1bb045c8
  3. DrahtBot commented at 6:53 pm on December 10, 2024: contributor

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

    Code Coverage & Benchmarks

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

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31375 (multiprocess: Add bitcoin wrapper executable by ryanofsky)

    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.

  4. DrahtBot added the label Tests on Dec 10, 2024
  5. maflcko commented at 7:12 pm on December 10, 2024: member

    Not sure about the implementation. This will turn an error (a missing previous release bin) into a silently passing CI.

    FileNotFoundError: [Errno 2] No such file or directory: ‘/home/thestack/bitcoin/releases/v28.0/bin/bitcoind’

    Seems pretty self-explanatory and doesn’t need more code to write another error?

    My understanding from reading the code is that the Error: no RPC connection shouldn’t happen on stop. Since stop checks for self.running, and self.running = True is only set after Popen, which throws for you.

    If this isn’t the case, it would be better to fix that bug.

  6. kevkevinpal commented at 7:38 pm on December 10, 2024: contributor

    I agree that at first glance the log might be confusing but I agree with @maflcko that we should not be silently passing the ci

    I would prefer we raise an AssertionError instead, so we keep the same functionality of the error but a better output

    Output with AssertionError

     02024-12-10T19:35:01.177000Z TestFramework (INFO): PRNG seed is: 5297365276077502746
 12024-12-10T19:35:01.178000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_thepg93c
 22024-12-10T19:35:01.178000Z TestFramework (ERROR): Binary not found: /mnt/shared_drive/DEVDIR/bitcoin/releases/v28.0/bin/bitcoind
 32024-12-10T19:35:01.178000Z TestFramework (ERROR): Binary not found: /mnt/shared_drive/DEVDIR/bitcoin/releases/v28.0/bin/bitcoin-cli
 42024-12-10T19:35:01.178000Z TestFramework (ERROR): Assertion failed
 5Traceback (most recent call last):
 6  File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
 7    self.setup()
 8  File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/test_framework.py", line 315, in setup
 9    self.setup_network()
10  File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/test_framework.py", line 409, in setup_network
11    self.setup_nodes()
12  File "/mnt/shared_drive/DEVDIR/bitcoin/./build/test/functional/wallet_migration.py", line 50, in setup_nodes
13    self.add_nodes(self.num_nodes, versions=[
14  File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/test_framework.py", line 536, in add_nodes
15    raise AssertionError("At least one release binary is missing")
16AssertionError: At least one release binary is missing
172024-12-10T19:35:01.229000Z TestFramework (INFO): Stopping nodes
182024-12-10T19:35:01.229000Z TestFramework (WARNING): Not cleaning up dir /tmp/bitcoin_func_test_thepg93c
192024-12-10T19:35:01.229000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/bitcoin_func_test_thepg93c/test_framework.log
202024-12-10T19:35:01.229000Z TestFramework (ERROR):
212024-12-10T19:35:01.230000Z TestFramework (ERROR): Hint: Call /mnt/shared_drive/DEVDIR/bitcoin/test/functional/combine_logs.py '/tmp/bitcoin_func_test_thepg93c' to consolidate all logs
222024-12-10T19:35:01.230000Z TestFramework (ERROR):
232024-12-10T19:35:01.230000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
242024-12-10T19:35:01.230000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
252024-12-10T19:35:01.230000Z TestFramework (ERROR):

    nit: in the PR description to reproduce, I get No such file or directory ./test/get_previous_releases -b -> ./test/get_previous_releases.py -b


theStack DrahtBot maflcko kevkevinpal

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: 2024-12-21 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me