Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Feb 28, 2019

Link against BasicTestingSetup in the fuzz tests, so we can fuzz against validation.

Also include a commit to remove test_bitcoin_main.cpp. That file may or may not overwrite globals in the link stage depending on the link order. This is confusing and useless anyway: The unit tests should never std::exit in the middle of the run (especially with success as exit code), since it will skip all test modules afterward.

Also include a commit to remove some unused forward declarations and move the main_tests to validation_tests, since main was long ago split into net_processing and validation.

@practicalswift
Copy link
Contributor

Concept ACK

Nice cleanup: -242 lines of code

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 28, 2019

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15382 ([util] add runCommandParseJSON by Sjors)
  • #14464 (refactor: make checkqueue manage the threads by itself (also removed some boost dependencies) by ken2812221)
  • #10102 ([experimental] Multiprocess bitcoin by ryanofsky)

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.

@fanquake fanquake added the Tests label Feb 28, 2019
@maflcko maflcko changed the title test: Remove useless test_bitcoin_main.cpp fuzz: Link BasicTestingSetup (shared with unit tests) Mar 6, 2019
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK faa9b88. Looks good, bunch of nice simple make & test cleanups.

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Mar 6, 2019
…ests)

faa9b88 fuzz: Link BasicTestingSetup (shared with unit tests) (MarcoFalke)
fa85468 test: Move main_tests to validation_tests (MarcoFalke)
fa02b22 test: Remove useless test_bitcoin_main.cpp (MarcoFalke)
fab2daa test: Add missing LIBBITCOIN_ZMQ to test_test_bitcoin_LDADD (MarcoFalke)

Pull request description:

  Link against BasicTestingSetup in the fuzz tests, so we can fuzz against validation.

  Also include a commit to remove test_bitcoin_main.cpp. That file may or may not overwrite globals in the link stage depending on the link order. This is confusing and useless anyway: The unit tests should never `std::exit` in the middle of the run (especially with success as exit code), since it will skip all test modules afterward.

  Also include a commit to remove some unused forward declarations and move the main_tests to validation_tests, since main was long ago split into net_processing and validation.

Tree-SHA512: bdd34c87505450ec106d632f6664aadcbdac7c198172a77da55fab75b274f869ae1a8d06573ba2aff4cb186be9c7a34b7697894ab6f9c82b392f769c9135f36c
@maflcko maflcko merged commit faa9b88 into bitcoin:master Mar 6, 2019
@maflcko maflcko deleted the 1903-testMain branch March 6, 2019 20:18
maflcko pushed a commit that referenced this pull request Apr 15, 2019
faf4000 scripted-diff: Bump copyright headers in test, bench (MarcoFalke)
fa82190 scripted-diff: Rename test_bitcoin to test/setup_common (MarcoFalke)
fa8685d test: Use test_bitcoin setup in bench, Add test utils (MarcoFalke)
666696b test: Have segwit always active in (Basic)TestingSetup (MarcoFalke)

Pull request description:

  Now that the fuzz tests can use the BasicTestingSetup [1], do the same for bench.

  Also move some duplicate code to a common "test/util" module.

  [1]:  fuzz: Link BasicTestingSetup (shared with unit tests) #15504

ACKs for commit faf400:
  jonatack:
    ACK faf4000

Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 16, 2020
Summary:
Partial backport of core [[bitcoin/bitcoin#15504 | PR15504]]:
bitcoin/bitcoin@fa02b22

Test Plan:
  ninja check
  make check # autotools build

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5481
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 19, 2020
Summary:
Completes backport of core [[bitcoin/bitcoin#15504 | PR15504]]:
bitcoin/bitcoin@faa9b88

Depends on D5481 and D4629.

Test Plan:
  export CC=clang CXX=clang++
  ../configure --enable-fuzz --with-sanitizers=fuzzer,address \
    --disable-wallet \
    --disable-bench \
    --with-utils=no \
    --with-daemon=no \
    --with-libs=no \
    --with-gui=no \
    --with-seeder=no
  make
  mkdir -p test/fuzz
  cp ../test/fuzz/test_runner.py test/fuzz/
  ./test/fuzz/test_runner.py -l DEBUG <path_to_corpus>

  cmake -GNinja .. -DENABLE_SANITIZERS="address;fuzzer" \
    -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
  ninja bitcoin-fuzzers link-fuzz-test_runner.py
  ./test/fuzz/test_runner.py -l DEBUG <path_to_corpus>

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5484
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request May 19, 2020
Summary:
Partial backport of core [[bitcoin/bitcoin#15504 | PR15504]]:
bitcoin/bitcoin@fa02b22

Test Plan:
  ninja check
  make check # autotools build

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5481
kwvg added a commit to kwvg/dash that referenced this pull request Aug 2, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 5, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 5, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 5, 2021
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Aug 6, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 8, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Aug 11, 2021
5tefan pushed a commit to 5tefan/dash that referenced this pull request Aug 12, 2021
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Oct 7, 2021
…unit tests

faf4000 scripted-diff: Bump copyright headers in test, bench (MarcoFalke)
fa82190 scripted-diff: Rename test_bitcoin to test/setup_common (MarcoFalke)
fa8685d test: Use test_bitcoin setup in bench, Add test utils (MarcoFalke)
666696b test: Have segwit always active in (Basic)TestingSetup (MarcoFalke)

Pull request description:

  Now that the fuzz tests can use the BasicTestingSetup [1], do the same for bench.

  Also move some duplicate code to a common "test/util" module.

  [1]:  fuzz: Link BasicTestingSetup (shared with unit tests) bitcoin#15504

ACKs for commit faf400:
  jonatack:
    ACK bitcoin@faf4000

Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4

# Conflicts:
#	build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
#	build_msvc/test_bitcoin/test_bitcoin.vcxproj
#	src/Makefile.bench.include
#	src/Makefile.qttest.include
#	src/Makefile.test.include
#	src/bench/bench.cpp
#	src/bench/bench_bitcoin.cpp
#	src/bench/block_assemble.cpp
#	src/bench/duplicate_inputs.cpp
#	src/qt/test/addressbooktests.cpp
#	src/qt/test/rpcnestedtests.cpp
#	src/qt/test/wallettests.cpp
#	src/test/README.md
#	src/test/addrman_tests.cpp
#	src/test/allocator_tests.cpp
#	src/test/amount_tests.cpp
#	src/test/arith_uint256_tests.cpp
#	src/test/base32_tests.cpp
#	src/test/base58_tests.cpp
#	src/test/base64_tests.cpp
#	src/test/bech32_tests.cpp
#	src/test/bip32_tests.cpp
#	src/test/blockchain_tests.cpp
#	src/test/blockencodings_tests.cpp
#	src/test/blockfilter_tests.cpp
#	src/test/bloom_tests.cpp
#	src/test/bswap_tests.cpp
#	src/test/checkqueue_tests.cpp
#	src/test/coins_tests.cpp
#	src/test/compress_tests.cpp
#	src/test/crypto_tests.cpp
#	src/test/cuckoocache_tests.cpp
#	src/test/dbwrapper_tests.cpp
#	src/test/denialofservice_tests.cpp
#	src/test/descriptor_tests.cpp
#	src/test/flatfile_tests.cpp
#	src/test/fs_tests.cpp
#	src/test/getarg_tests.cpp
#	src/test/hash_tests.cpp
#	src/test/key_io_tests.cpp
#	src/test/key_properties.cpp
#	src/test/key_tests.cpp
#	src/test/limitedmap_tests.cpp
#	src/test/mempool_tests.cpp
#	src/test/merkle_tests.cpp
#	src/test/merkleblock_tests.cpp
#	src/test/miner_tests.cpp
#	src/test/multisig_tests.cpp
#	src/test/net_tests.cpp
#	src/test/netbase_tests.cpp
#	src/test/pmt_tests.cpp
#	src/test/policyestimator_tests.cpp
#	src/test/pow_tests.cpp
#	src/test/prevector_tests.cpp
#	src/test/raii_event_tests.cpp
#	src/test/random_tests.cpp
#	src/test/reverselock_tests.cpp
#	src/test/rpc_tests.cpp
#	src/test/sanity_tests.cpp
#	src/test/scheduler_tests.cpp
#	src/test/script_p2sh_tests.cpp
#	src/test/script_standard_tests.cpp
#	src/test/script_tests.cpp
#	src/test/scriptnum_tests.cpp
#	src/test/serialize_tests.cpp
#	src/test/setup_common.cpp
#	src/test/setup_common.h
#	src/test/sighash_tests.cpp
#	src/test/sigopcount_tests.cpp
#	src/test/skiplist_tests.cpp
#	src/test/streams_tests.cpp
#	src/test/sync_tests.cpp
#	src/test/test_bitcoin.cpp
#	src/test/test_bitcoin.h
#	src/test/test_dash.cpp
#	src/test/test_dash.h
#	src/test/timedata_tests.cpp
#	src/test/torcontrol_tests.cpp
#	src/test/transaction_tests.cpp
#	src/test/txindex_tests.cpp
#	src/test/txvalidation_tests.cpp
#	src/test/txvalidationcache_tests.cpp
#	src/test/uint256_tests.cpp
#	src/test/util_tests.cpp
#	src/test/validation_block_tests.cpp
#	src/test/validation_tests.cpp
#	src/test/versionbits_tests.cpp
#	src/wallet/test/coinselector_tests.cpp
#	src/wallet/test/db_tests.cpp
#	src/wallet/test/init_test_fixture.h
#	src/wallet/test/init_tests.cpp
#	src/wallet/test/psbt_wallet_tests.cpp
#	src/wallet/test/wallet_crypto_tests.cpp
#	src/wallet/test/wallet_test_fixture.h
#	src/wallet/test/wallet_tests.cpp
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Oct 7, 2021
…unit tests

faf4000 scripted-diff: Bump copyright headers in test, bench (MarcoFalke)
fa82190 scripted-diff: Rename test_bitcoin to test/setup_common (MarcoFalke)
fa8685d test: Use test_bitcoin setup in bench, Add test utils (MarcoFalke)
666696b test: Have segwit always active in (Basic)TestingSetup (MarcoFalke)

Pull request description:

  Now that the fuzz tests can use the BasicTestingSetup [1], do the same for bench.

  Also move some duplicate code to a common "test/util" module.

  [1]:  fuzz: Link BasicTestingSetup (shared with unit tests) bitcoin#15504

ACKs for commit faf400:
  jonatack:
    ACK bitcoin@faf4000

Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4

# Conflicts:
#	build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
#	build_msvc/test_bitcoin/test_bitcoin.vcxproj
#	src/Makefile.bench.include
#	src/Makefile.qttest.include
#	src/Makefile.test.include
#	src/bench/bench.cpp
#	src/bench/bench_bitcoin.cpp
#	src/bench/block_assemble.cpp
#	src/bench/duplicate_inputs.cpp
#	src/qt/test/addressbooktests.cpp
#	src/qt/test/rpcnestedtests.cpp
#	src/qt/test/wallettests.cpp
#	src/test/README.md
#	src/test/addrman_tests.cpp
#	src/test/allocator_tests.cpp
#	src/test/amount_tests.cpp
#	src/test/arith_uint256_tests.cpp
#	src/test/base32_tests.cpp
#	src/test/base58_tests.cpp
#	src/test/base64_tests.cpp
#	src/test/bech32_tests.cpp
#	src/test/bip32_tests.cpp
#	src/test/blockchain_tests.cpp
#	src/test/blockencodings_tests.cpp
#	src/test/blockfilter_tests.cpp
#	src/test/bloom_tests.cpp
#	src/test/bswap_tests.cpp
#	src/test/checkqueue_tests.cpp
#	src/test/coins_tests.cpp
#	src/test/compress_tests.cpp
#	src/test/crypto_tests.cpp
#	src/test/cuckoocache_tests.cpp
#	src/test/dbwrapper_tests.cpp
#	src/test/denialofservice_tests.cpp
#	src/test/descriptor_tests.cpp
#	src/test/flatfile_tests.cpp
#	src/test/fs_tests.cpp
#	src/test/getarg_tests.cpp
#	src/test/hash_tests.cpp
#	src/test/key_io_tests.cpp
#	src/test/key_properties.cpp
#	src/test/key_tests.cpp
#	src/test/limitedmap_tests.cpp
#	src/test/mempool_tests.cpp
#	src/test/merkle_tests.cpp
#	src/test/merkleblock_tests.cpp
#	src/test/miner_tests.cpp
#	src/test/multisig_tests.cpp
#	src/test/net_tests.cpp
#	src/test/netbase_tests.cpp
#	src/test/pmt_tests.cpp
#	src/test/policyestimator_tests.cpp
#	src/test/pow_tests.cpp
#	src/test/prevector_tests.cpp
#	src/test/raii_event_tests.cpp
#	src/test/random_tests.cpp
#	src/test/reverselock_tests.cpp
#	src/test/rpc_tests.cpp
#	src/test/sanity_tests.cpp
#	src/test/scheduler_tests.cpp
#	src/test/script_p2sh_tests.cpp
#	src/test/script_standard_tests.cpp
#	src/test/script_tests.cpp
#	src/test/scriptnum_tests.cpp
#	src/test/serialize_tests.cpp
#	src/test/setup_common.cpp
#	src/test/setup_common.h
#	src/test/sighash_tests.cpp
#	src/test/sigopcount_tests.cpp
#	src/test/skiplist_tests.cpp
#	src/test/streams_tests.cpp
#	src/test/sync_tests.cpp
#	src/test/test_bitcoin.cpp
#	src/test/test_bitcoin.h
#	src/test/test_dash.cpp
#	src/test/test_dash.h
#	src/test/timedata_tests.cpp
#	src/test/torcontrol_tests.cpp
#	src/test/transaction_tests.cpp
#	src/test/txindex_tests.cpp
#	src/test/txvalidation_tests.cpp
#	src/test/txvalidationcache_tests.cpp
#	src/test/uint256_tests.cpp
#	src/test/util_tests.cpp
#	src/test/validation_block_tests.cpp
#	src/test/validation_tests.cpp
#	src/test/versionbits_tests.cpp
#	src/wallet/test/coinselector_tests.cpp
#	src/wallet/test/db_tests.cpp
#	src/wallet/test/init_test_fixture.h
#	src/wallet/test/init_tests.cpp
#	src/wallet/test/psbt_wallet_tests.cpp
#	src/wallet/test/wallet_crypto_tests.cpp
#	src/wallet/test/wallet_test_fixture.h
#	src/wallet/test/wallet_tests.cpp
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Oct 12, 2021
…unit tests

faf4000 scripted-diff: Bump copyright headers in test, bench (MarcoFalke)
fa82190 scripted-diff: Rename test_bitcoin to test/setup_common (MarcoFalke)
fa8685d test: Use test_bitcoin setup in bench, Add test utils (MarcoFalke)
666696b test: Have segwit always active in (Basic)TestingSetup (MarcoFalke)

Pull request description:

  Now that the fuzz tests can use the BasicTestingSetup [1], do the same for bench.

  Also move some duplicate code to a common "test/util" module.

  [1]:  fuzz: Link BasicTestingSetup (shared with unit tests) bitcoin#15504

ACKs for commit faf400:
  jonatack:
    ACK bitcoin@faf4000

Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4

# Conflicts:
#	build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
#	build_msvc/test_bitcoin/test_bitcoin.vcxproj
#	src/Makefile.bench.include
#	src/Makefile.qttest.include
#	src/Makefile.test.include
#	src/bench/bench.cpp
#	src/bench/bench_bitcoin.cpp
#	src/bench/block_assemble.cpp
#	src/bench/duplicate_inputs.cpp
#	src/qt/test/addressbooktests.cpp
#	src/qt/test/rpcnestedtests.cpp
#	src/qt/test/wallettests.cpp
#	src/test/README.md
#	src/test/addrman_tests.cpp
#	src/test/allocator_tests.cpp
#	src/test/amount_tests.cpp
#	src/test/arith_uint256_tests.cpp
#	src/test/base32_tests.cpp
#	src/test/base58_tests.cpp
#	src/test/base64_tests.cpp
#	src/test/bech32_tests.cpp
#	src/test/bip32_tests.cpp
#	src/test/blockchain_tests.cpp
#	src/test/blockencodings_tests.cpp
#	src/test/blockfilter_tests.cpp
#	src/test/bloom_tests.cpp
#	src/test/bswap_tests.cpp
#	src/test/checkqueue_tests.cpp
#	src/test/coins_tests.cpp
#	src/test/compress_tests.cpp
#	src/test/crypto_tests.cpp
#	src/test/cuckoocache_tests.cpp
#	src/test/dbwrapper_tests.cpp
#	src/test/denialofservice_tests.cpp
#	src/test/descriptor_tests.cpp
#	src/test/flatfile_tests.cpp
#	src/test/fs_tests.cpp
#	src/test/getarg_tests.cpp
#	src/test/hash_tests.cpp
#	src/test/key_io_tests.cpp
#	src/test/key_properties.cpp
#	src/test/key_tests.cpp
#	src/test/limitedmap_tests.cpp
#	src/test/mempool_tests.cpp
#	src/test/merkle_tests.cpp
#	src/test/merkleblock_tests.cpp
#	src/test/miner_tests.cpp
#	src/test/multisig_tests.cpp
#	src/test/net_tests.cpp
#	src/test/netbase_tests.cpp
#	src/test/pmt_tests.cpp
#	src/test/policyestimator_tests.cpp
#	src/test/pow_tests.cpp
#	src/test/prevector_tests.cpp
#	src/test/raii_event_tests.cpp
#	src/test/random_tests.cpp
#	src/test/reverselock_tests.cpp
#	src/test/rpc_tests.cpp
#	src/test/sanity_tests.cpp
#	src/test/scheduler_tests.cpp
#	src/test/script_p2sh_tests.cpp
#	src/test/script_standard_tests.cpp
#	src/test/script_tests.cpp
#	src/test/scriptnum_tests.cpp
#	src/test/serialize_tests.cpp
#	src/test/setup_common.cpp
#	src/test/setup_common.h
#	src/test/sighash_tests.cpp
#	src/test/sigopcount_tests.cpp
#	src/test/skiplist_tests.cpp
#	src/test/streams_tests.cpp
#	src/test/sync_tests.cpp
#	src/test/test_bitcoin.cpp
#	src/test/test_bitcoin.h
#	src/test/test_dash.cpp
#	src/test/test_dash.h
#	src/test/timedata_tests.cpp
#	src/test/torcontrol_tests.cpp
#	src/test/transaction_tests.cpp
#	src/test/txindex_tests.cpp
#	src/test/txvalidation_tests.cpp
#	src/test/txvalidationcache_tests.cpp
#	src/test/uint256_tests.cpp
#	src/test/util_tests.cpp
#	src/test/validation_block_tests.cpp
#	src/test/validation_tests.cpp
#	src/test/versionbits_tests.cpp
#	src/wallet/test/coinselector_tests.cpp
#	src/wallet/test/db_tests.cpp
#	src/wallet/test/init_test_fixture.h
#	src/wallet/test/init_tests.cpp
#	src/wallet/test/psbt_wallet_tests.cpp
#	src/wallet/test/wallet_crypto_tests.cpp
#	src/wallet/test/wallet_test_fixture.h
#	src/wallet/test/wallet_tests.cpp
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants