test: move more utility functions into test utility library #17593

pull mzumsande wants to merge 3 commits into bitcoin:master from mzumsande:201911_test_moveutil changing 12 files +135 −113

mzumsande commented at 1:35 PM on November 25, 2019: member
This disbands test/util.h and test/util.cpp and moves the content into the test utility library recently created in #17542, so that all test utility functions are in one place.

The content of the original files are split into three modules:
1. string helper functions go to test/util/str
2. mining helper functions go to the newly created test/util/mining
3. wallet helper functions go to the newly created test/util/wallet
[test] move string helper functions into test library 2cb4e8bdc7
fanquake added the label Tests on Nov 25, 2019
in src/Makefile.test_util.include:13 in 3b50882ac9 outdated
```
   9 | @@ -10,22 +10,25 @@ EXTRA_LIBRARIES += \
  10 |  TEST_UTIL_H = \
  11 |      test/util/blockfilter.h \
  12 |      test/util/logging.h \
  13 | +    test/util/mining_utils.h \
```
MarcoFalke commented at 1:41 PM on November 25, 2019:

I think we do without the _utils suffix. No strong opinion, though.

mzumsande commented at 1:54 PM on November 25, 2019:

I used the suffix because we already have wallet.h twice. No strong opinion either though, so I am happy to change it after the CIs are through.

MarcoFalke commented at 2:06 PM on November 25, 2019:

Yeah, we use the include guards e.g. BITCOIN_TEST_UTIL_WALLET_UTILS_H to protect against duplicate includes and the <> import to protect against including the wrong path. So it should be fine to have three wallet.h

The travis CI passed and the appveyor ci is failing on master currently, so no need to wait for that.

mzumsande commented at 3:44 PM on November 25, 2019:

Ok, I renamed and rebased. As to move-only, note that I removed one include from util.cpp (validationinterface) that I could not allocate to any of the helper functions.
MarcoFalke approved
MarcoFalke commented at 1:42 PM on November 25, 2019: member

Concept ACK

Will check if it is move-only and then merge
mzumsande force-pushed on Nov 25, 2019
[test] move mining helper functions into test library f613e5dfda
[test] move wallet helper functions into test library 78e283e656
mzumsande force-pushed on Nov 25, 2019
DrahtBot commented at 5:13 PM on November 25, 2019: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #17577 (refactor: deduplicate the message sign/verify code by vasild)
- #17529 (rpc: Faster getblock using PureBlock by MarcoFalke)
- #17261 (Make ScriptPubKeyMan an actual interface and the wallet to have multiple by achow101)
- #16895 (External signer multisig support by Sjors)
- #16549 ([WIP] UI external signer support (e.g. hardware wallet) by Sjors)
- #16546 ([WIP] External signer support - Wallet Box edition by Sjors)
- #16528 ([WIP] Native Descriptor Wallets (take 2) by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
MarcoFalke commented at 8:31 PM on December 16, 2019: member

re-run ci
MarcoFalke closed this on Dec 16, 2019
MarcoFalke reopened this on Dec 16, 2019