Add support for running integration tests via cargo test #420
Conversation
Future commits will remove the hacky integration test setup. There are still some annoying warnings present for ununsed functions when they are really used. rust-lang/rust#46379 Disambiguiate rand feature with dev dependancy https://users.rust-lang.org/t/features-and-dependencies-cannot-have-the-same-name/47746
sanket1729
force-pushed
the
int_test_rework
branch
from
3864ae4 to
139324a
Compare
|
|
||
| #[test] | ||
| fn tests_from_cpp() { | ||
| let cl = &setup::setup().client; |
There was a problem hiding this comment.
I am a little puzzled in understanding how this test could work, since once BitcoinD struct goes out of scope, the node is stopped
There was a problem hiding this comment.
setup returns BitcoinD. It never goes out of scope.
| [dev-dependencies] | ||
| bitcoind = {version = "0.26.1", features=["22_0"]} | ||
| actual-rand = { package = "rand", version = "0.8.4"} | ||
| bitcoin = { version = "0.28", features = ["rand"]} |
There was a problem hiding this comment.
since there is rand anyway, can't we use sign_schnorr_with_aux_rand and remove the feature rand so that we don't have both bitcoin and bitcoin-with-rand?
There was a problem hiding this comment.
Adressing in a follow-up PR.
There was a problem hiding this comment.
ACK 139324a
I did not read the code as carefully as Riccardo evidently did -- but on a light reading it all looks good (or at least, it all looks like it does integration testing and looks like it's confined to test-only parts of the codebase, and it succeeded on my local CI).
92050de Remove bitcoin rand feature from integration testing (sanket1729) Pull request description: Addresses [feedback ](#420 (comment)) #420 's review ACKs for top commit: apoelstra: ACK 92050de RCasatta: ACK 92050de Tree-SHA512: ad3dcc3631eb3ffd6de30d577d8a08756cbb4a2ff1670b21f4902422a2104bbfc8b161583a1ba3dd7368160e2dd51bac1d68c9332fc73a3a72d3c6b3ddcdabb4
There are still some annoying warnings present for unused functions when they are used.
rust-lang/rust#46379
Later commits will remove the hacky integration test setup.
Running
cargo testnow should also run integration testsOur testing infrastructure now takes a long time to build because we have more than 100 dependencies (transitive). But all of these are dev dependencies, so our library isn't getting bloated.
Fixes #361