-
Notifications
You must be signed in to change notification settings - Fork 38.7k
QA: stop nodes after RPC tests, even with --nocleanup #6032
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
|
I was also surprised by the bitcoinds still running when specifying |
`--nocleanup` should provide a way to preserve test data, but should not have an impact on whether nodes are to be stopped after the test execution. In particular, when currently running RPC tests with `--nocleanup`, then it may result in several active `bitcoind` processes, which are not terminated properly.
There could be a Assuming there were such an option, additional reporting of the ports, or alternatively providing options to specify ports, or maybe even an option to run the QT client instead of bitcoind, would be helpful. I'm not sure how other people use, or want to use, the tests, so it would be interesting to learn more about potential use cases. :) |
|
You don't have to explicitly know the ports, you can use |
qa/rpc-tests/test_framework.py
Outdated
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.
Please report the exception w/ stack trace here (traceback.print_exc()). Ignoring exceptions, especially in tests, can cause useful diagnostic information to et lost.
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.
I was thinking: if the client crashed earlier, then stopping the nodes is going to fail, too, and this was the part I wanted to hide. I'm going to take a look at it. :)
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.
I might be wrong, but it looks like there doesn't seem to be an exception raised to begin with. I removed the catch completely in the updated version.
With `--noshutdown`, the nodes are not stopped explicitly. `--noshutdown` implies `--nocleanup`, to prevent removing datadirs, which are still in use.
7d1bf7c to
688da79
Compare
|
utACK |
|
ACK |
|
Concept ACK. I didn't even know --nocleanup existed. |
|
After-the-fact ACK. --nocleanup is indispensable when something actually breaks, which is often when developing a test, but the zombie bitcoind's were a nuisance. |
--nocleanupshould provide a way to preserve test data, but should not have an impact on whether nodes are to be stopped after the test execution.In particular, when currently running RPC tests with
--nocleanup, then it may result in several activebitcoind, which are not stopped properly.Shutdown errors are ignored, given that a failure likely relates to already terminated processes, caused during the actual testing.Edit:
Further, with
--noshutdown, the nodes are not stopped explicitly.--noshutdownimplies--nocleanup, to prevent removing datadirs, which are still in use.