-
Notifications
You must be signed in to change notification settings - Fork 38.7k
test_framework: detect failure of bitcoind startup #7744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f22f8ed to
8869c3b
Compare
|
Not ready to merge yet. I've tested this with a -salvagewallet loop: Even though reporting is better, it still hangs after "Stopping nodes", going to investigate why. |
d4066d4 to
9c28f02
Compare
|
Should be fixed now. The problem was that start_nodes can fail with part of the nodes already started. |
Replace the `bitcoin-cli -rpcwait` after spawning bitcoind with our own loop that detects when bitcoind exits prematurely. And if one node fails to start, stop the others. This prevents a hang in such a case (see bitcoin#7463).
9c28f02 to
018b60c
Compare
|
|
| try: | ||
| for i in range(num_nodes): | ||
| rpcs.append(start_node(i, dirname, extra_args[i], rpchost, binary=binary[i])) | ||
| except: # If one node failed to start, stop the others |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
Good idea! |
|
utACK 018b60c |
018b60c test_framework: detect failure of bitcoind startup (Wladimir J. van der Laan)
| return datadir | ||
|
|
||
| def rpc_url(i, rpchost=None): | ||
| return "http://rt:rt@%s:%d" % (rpchost or '127.0.0.1', rpc_port(i)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: rpcbind_test.py no longer works after this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably re-add that test to one of the standard sequence, otherwise it will just bitrot
Github-Pull: bitcoin#7744 Rebased-From: 018b60c
This is a merge of bitcoin/bitcoin#7744 / MVF-Core PR#16. Instead of freezing up qa tests, wallet.py will fail on some 32-bit platforms where wallet salvage operation sometimes fails due to unknown reasons. See also: bitcoin/bitcoin#7463 about the -salvagewallet problem. Core has decided to disable the salvage part of the test in bitcoin/bitcoin#8038 , we may implement a Skip() of this test on 32-bit or disable that test step in a similar way. The wallet salvage problem is definitely unrelated to any fork programming.
Backport RPC test harness PRs Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6548 - bitcoin/bitcoin#6804 - Just the coverage backend, not the flag to enable it. - bitcoin/bitcoin#7744 - bitcoin/bitcoin#9832 - Excludes `wallet-hd.py` change (missing bitcoin/bitcoin#8309). Part of #2074.
Backport RPC test harness PRs Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6548 - bitcoin/bitcoin#6804 - Just the coverage backend, not the flag to enable it for all RPC tests. - bitcoin/bitcoin#7744 - bitcoin/bitcoin#9832 - Excludes `wallet-hd.py` change (missing bitcoin/bitcoin#8309). Part of #2074.
Replace the
bitcoin-cli -rpcwaitafter spawning bitcoind with our own loop that detects when bitcoind exits prematurely (before RPC is up).This prevents a hang in such a case (see #7463).