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

test: move accounting_tests and rpc_wallet_tests to wallet/test #7905

Merged
merged 4 commits into from Apr 19, 2016

Conversation

Projects
None yet
4 participants
@laanwj
Member

laanwj commented Apr 18, 2016

Move the two other wallet tests to where they belong.

@jonasschnelli I'm surprised you didn't stumble on this one yet

test: move accounting_tests and rpc_wallet_tests to wallet/test
Move the two other wallet tests to where they belong.
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Apr 18, 2016

Member

Yes! This should have been done long ago. Thanks for fixing.
utACK de39c95

Member

jonasschnelli commented Apr 18, 2016

Yes! This should have been done long ago. Thanks for fixing.
utACK de39c95

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 18, 2016

Member

With this, I guess we also want a wallet-specific test fixture, to get rid of the ENABLE_WALLETs in test_bitcoin.cpp?

Member

laanwj commented Apr 18, 2016

With this, I guess we also want a wallet-specific test fixture, to get rid of the ENABLE_WALLETs in test_bitcoin.cpp?

test: Create test fixture for wallet
Removes all the `#ifdef ENABLE_WALLET` from `test_bitcoin` by
making the wallet tests use their own fixture.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 18, 2016

Member

Done. Latest commit removes all wallet-related setup from test_bitcoin.cpp. Wallet tests set up and tear down their own environment.

These references to the wallet remain in src/test, this probably signifies some code movement is still necessary:

multisig_tests.cpp:#include "wallet/wallet_ismine.h"
script_P2SH_tests.cpp:#include "wallet/wallet_ismine.h"
Member

laanwj commented Apr 18, 2016

Done. Latest commit removes all wallet-related setup from test_bitcoin.cpp. Wallet tests set up and tear down their own environment.

These references to the wallet remain in src/test, this probably signifies some code movement is still necessary:

multisig_tests.cpp:#include "wallet/wallet_ismine.h"
script_P2SH_tests.cpp:#include "wallet/wallet_ismine.h"
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Apr 18, 2016

Member
Member

sipa commented Apr 18, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 18, 2016

Member

@sipa What about keeping it separate and just moving, e.g. script/ismine.cpp and script/ismine.h?

Member

laanwj commented Apr 18, 2016

@sipa What about keeping it separate and just moving, e.g. script/ismine.cpp and script/ismine.h?

laanwj added some commits Apr 18, 2016

wallet_ismine.h → script/ismine.h
Removes conditional dependency of `src/test` on wallet.

Makes multisig and P2SH tests complete without wallet built-in.
test: Rename wallet.dat to wallet_test.dat
Indicate that the file name is not hardcoded, and a little bit of safety
so that it never nukes the main wallet.

Suggestion by Marco Falke.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 18, 2016

Member

moving, e.g. script/ismine.cpp and script/ismine.h?

Done. All conditional wallet stuff is gone now from src/test.

Member

laanwj commented Apr 18, 2016

moving, e.g. script/ismine.cpp and script/ismine.h?

Done. All conditional wallet stuff is gone now from src/test.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Apr 18, 2016

Member

Confirmed move only besides b30fb42

utACK b30fb42

Member

MarcoFalke commented Apr 18, 2016

Confirmed move only besides b30fb42

utACK b30fb42

@laanwj laanwj merged commit b30fb42 into bitcoin:master Apr 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 19, 2016

Merge #7905: test: move accounting_tests and rpc_wallet_tests to wall…
…et/test

b30fb42 test: Rename wallet.dat to wallet_test.dat (Wladimir J. van der Laan)
a25a4f5 wallet_ismine.h → script/ismine.h (Wladimir J. van der Laan)
f4eae2d test: Create test fixture for wallet (Wladimir J. van der Laan)
de39c95 test: move accounting_tests and rpc_wallet_tests to wallet/test (Wladimir J. van der Laan)

@laanwj laanwj referenced this pull request Apr 28, 2016

Closed

Remaining instances of ENABLE_WALLET in `libbitcoin_server.a` #7965

14 of 14 tasks complete

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7905: test: move accounting_tests and rpc_wallet_tests to wall…
…et/test

b30fb42 test: Rename wallet.dat to wallet_test.dat (Wladimir J. van der Laan)
a25a4f5 wallet_ismine.h → script/ismine.h (Wladimir J. van der Laan)
f4eae2d test: Create test fixture for wallet (Wladimir J. van der Laan)
de39c95 test: move accounting_tests and rpc_wallet_tests to wallet/test (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7905: test: move accounting_tests and rpc_wallet_tests to wall…
…et/test

b30fb42 test: Rename wallet.dat to wallet_test.dat (Wladimir J. van der Laan)
a25a4f5 wallet_ismine.h → script/ismine.h (Wladimir J. van der Laan)
f4eae2d test: Create test fixture for wallet (Wladimir J. van der Laan)
de39c95 test: move accounting_tests and rpc_wallet_tests to wallet/test (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Dec 20, 2017

Merge #7905: test: move accounting_tests and rpc_wallet_tests to wall…
…et/test

b30fb42 test: Rename wallet.dat to wallet_test.dat (Wladimir J. van der Laan)
a25a4f5 wallet_ismine.h → script/ismine.h (Wladimir J. van der Laan)
f4eae2d test: Create test fixture for wallet (Wladimir J. van der Laan)
de39c95 test: move accounting_tests and rpc_wallet_tests to wallet/test (Wladimir J. van der Laan)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment