-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: Treat bitcoin-wallet
binary in the same way as others
#27554
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
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
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.
lgtm
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.
ACK ef01337
Nice little refactoring (modulo being able to now set BITCOINWALLET
which is new behaviour), less code duplication and functions with smaller scope.
Updated ef01337 -> de8b7ab (pr27554.03 -> pr27554.04, diff):
|
This change factors out the repeated code into a new `set_binary_paths` function.
This change makes the `bitcoin-wallet` binary path customizable in the same way how it can be done now with other ones, including `bitcoind`, `bitcoin-cli` and `bitcoin-util`.
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.
re-ACK f6d7636
…BITCOINUTIL` and `BITCOINTX` 4f2f615 test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX` (Hennadii Stepanov) Pull request description: This PR is a continuation of changes to our testing frameworks (bitcoin/bitcoin#27554, bitcoin/bitcoin#27561) that allow them to work correctly in a multi-config build environment that is possible for [upcoming](bitcoin/bitcoin#25797) CMake-based build system. That means that built for different configurations binaries (e.g., "Debug" and "Release") can coexist in separated directories. The commit has been pulled from hebasto/bitcoin#15 and it seems [useful](hebasto/bitcoin#15 (comment)) by itself as: > I believe the rationale for allowing to drop in the executables via env var is to allow to test the guix-produced, or other third-party-produced executables... The current implementation of the `test/functional/test_framework/test_framework.py` script uses the same approach: https://github.com/bitcoin/bitcoin/blob/09351f51d279612973ecd76811dc075dff08209f/test/functional/test_framework/test_framework.py#L231-L246 ACKs for top commit: MarcoFalke: lgtm ACK 4f2f615 TheCharlatan: ACK 4f2f615 stickies-v: ACK 4f2f615 Tree-SHA512: 99ee9a727b266700649d8f2ec528dfaeb04a1e48f0cb1d4eeaece65917165be647c10c4548429a9e8b30d094597f67e860c1db03ac689ebb409b223ce1b63aa9
…TIL` and `BITCOINTX` 4f2f615 test: Make `util/test_runner.py` honor `BITCOINUTIL` and `BITCOINTX` (Hennadii Stepanov) Pull request description: This PR is a continuation of changes to our testing frameworks (bitcoin#27554, bitcoin#27561) that allow them to work correctly in a multi-config build environment that is possible for [upcoming](bitcoin#25797) CMake-based build system. That means that built for different configurations binaries (e.g., "Debug" and "Release") can coexist in separated directories. The commit has been pulled from hebasto#15 and it seems [useful](hebasto#15 (comment)) by itself as: > I believe the rationale for allowing to drop in the executables via env var is to allow to test the guix-produced, or other third-party-produced executables... The current implementation of the `test/functional/test_framework/test_framework.py` script uses the same approach: https://github.com/bitcoin/bitcoin/blob/09351f51d279612973ecd76811dc075dff08209f/test/functional/test_framework/test_framework.py#L231-L246 ACKs for top commit: MarcoFalke: lgtm ACK 4f2f615 TheCharlatan: ACK 4f2f615 stickies-v: ACK 4f2f615 Tree-SHA512: 99ee9a727b266700649d8f2ec528dfaeb04a1e48f0cb1d4eeaece65917165be647c10c4548429a9e8b30d094597f67e860c1db03ac689ebb409b223ce1b63aa9
This change makes the `bitcoin-wallet` binary path customizable in the same way how it can be done now with other ones, including `bitcoind`, `bitcoin-cli` and `bitcoin-util`. Github-Pull: bitcoin#27554 Rebased-From: f6d7636
This PR makes the
bitcoin-wallet
binary path customizable in the same way how it can be done now with other ones, includingbitcoind
,bitcoin-cli
andbitcoin-util
.