Skip to content

test: Unify testing setups for fuzz, bench, and unit tests#15788

Merged
maflcko merged 4 commits intobitcoin:masterfrom
maflcko:1904-testUnify
Apr 15, 2019
Merged

test: Unify testing setups for fuzz, bench, and unit tests#15788
maflcko merged 4 commits intobitcoin:masterfrom
maflcko:1904-testUnify

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Apr 10, 2019

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

@practicalswift
Copy link
Contributor

practicalswift commented Apr 11, 2019

Concept ACK

Nice!

scripted-diff: Rename test_bitcoin to test/setup_common
fa82190 
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e 's/test_bitcoin\.(h|cpp)/setup_common.\1/g' $(git grep -l test_bitcoin)
git mv ./src/test/test_bitcoin.h   ./src/test/setup_common.h
git mv ./src/test/test_bitcoin.cpp ./src/test/setup_common.cpp
sed -i -e 's/BITCOIN_TEST_TEST_BITCOIN_H/BITCOIN_TEST_SETUP_COMMON_H/g' ./src/test/setup_common.h
-END VERIFY SCRIPT-
@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 11, 2019 •
edited
Loading

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15778 ([wallet] Move maxtxfee from node to wallet by jnewbery)
  • #15104 (Tests: Add unit testing for the CompressScript function by mmachicao)
  • #15052 (Tests: Contract testing for the procedure AddTimeData and related fixes by mmachicao)
  • #14430 (Add more property based tests for basic bitcoin data structures by Christewart)
  • #14193 (validation: Add missing mempool locks by MarcoFalke)
  • #13751 (Utils and libraries: Drops the boost/algorithm/string/split.hpp dependency by 251Labs)

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.

scripted-diff: Bump copyright headers in test, bench
faf4000 
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-
jonatack
jonatack reviewed Apr 12, 2019
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK faf4000

Reviewed changes, recompiled, ran unit tests and benchmarks. Todo: Run fuzzing.

@jonatack
Copy link
Member

jonatack commented Apr 12, 2019 •
edited
Loading

FWIW I haven't managed to get the fuzzing to run as per doc/fuzzing.md so cannot confirm or infirm that aspect for this PR. Gist here. It's surely something I'm doing wrong. Pointers welcome.

@maflcko
Copy link
Member Author

maflcko commented Apr 12, 2019

Fuzzing shouldn't be changed by this pr, but it would help if you posted your configure line (and config log) as well as the output of make. See also doc/fuzzing

@maflcko maflcko merged commit faf4000 into bitcoin:master Apr 15, 2019
maflcko pushed a commit that referenced this pull request Apr 15, 2019
Merge #15788: test: Unify testing setups for fuzz, bench, and unit tests
78295e9 
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
@maflcko maflcko deleted the 1904-testUnify branch April 15, 2019 15:30
@jamesob jamesob mentioned this pull request Apr 18, 2019
Merged
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Apr 18, 2019
Merge bitcoin#15843: tests: fix outdated include in blockfilter_index…
693c743 
…_tests

89e8df1 tests: fix outdate include in blockfilter_index_tests (James O'Beirne)

Pull request description:

  Build is currently failing due to bad merge of bitcoin#15788 and bitcoin#14121.

ACKs for commit 89e8df:
  fanquake:
    tACK 89e8df1

Tree-SHA512: d3fea861f80d660b4a2827ca7241237311b68de4175d3db938a9a1d538e1325822410c98d84ba0734208af8163fbcc42cf2732788311ea22f3834c95eeb330b8
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 18, 2020
@Fabcien
Bump copyright headers in bench
17f8996 
Summary:
Original script:
```
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@faf4000

Bench files only for ease of review.

Test Plan: Check nothing but core's copyright year changed.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5495
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 18, 2020
@Fabcien
Bump copyright headers in tests (part1)
213a370 
Summary:
Original script:
```
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@faf4000

Subset of test files only for ease of review.

Test Plan: Check nothing but core's copyright year changed.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5496
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 18, 2020
@Fabcien
Bump copyright headers in tests (part2)
a117335 
Summary:
Original script:
```
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@faf4000

Subset of test files only for ease of review.

Test Plan: Check nothing but core's copyright year changed.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5497
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 19, 2020
@Fabcien
test: Use test_bitcoin setup in bench
b6a8418 
Summary:
```
Now that the fuzz tests can use the BasicTestingSetup [1], do the same
for bench.
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@fa8685d

This is stripped down to using the test fixture for the benchmark, as we
have no use for the utils for now.

Depends on D5484.

Test Plan:
  make
  make check
  ./src/bench/bench_bitcoin

  ninja
  ninja check
  ninja bench-bitcoin

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5493
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 19, 2020
@Fabcien
Rename test_bitcoin to test/setup_common
e289a06 
Summary:
Original replacement script:
```
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e 's/test_bitcoin\.(h|cpp)/setup_common.\1/g'
$(git grep -l test_bitcoin)
git mv ./src/test/test_bitcoin.h   ./src/test/setup_common.h
git mv ./src/test/test_bitcoin.cpp ./src/test/setup_common.cpp
sed -i -e 's/BITCOIN_TEST_TEST_BITCOIN_H/BITCOIN_TEST_SETUP_COMMON_H/g'
./src/test/setup_common.h
-END VERIFY SCRIPT-
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@fa82190#diff-cbf056cfc6c760726f2dff1391fd37d8

Depends on D5493.

This touches a lot of files, but is renaming only and unfortunately
cannot be split without breaking the build.

Test Plan:
  make
  make check
  ./src/bench/bench_bitcoin

  ninja
  ninja check
  ninja bench-bitcoin

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5494
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 19, 2020
@Fabcien
Bump copyright headers in tests (part3)
6282730 
Summary:
Original script:
```
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-
```

Completes backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@faf4000

Depends on D5494.

Subset of test files only for ease of review.

Test Plan: Check nothing but core's copyright year changed.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5498
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 27, 2020
@deadalnix
bench: Add block assemble benchmark
4de1bab 
Summary: This is a backport of Core [[bitcoin/bitcoin#13219 | PR13219]] and [[bitcoin/bitcoin#13806 | PR13806]] . Due to many backport being done out of oder or improperly, this contains elements of [[bitcoin/bitcoin#15413 | PR15413]] and [[bitcoin/bitcoin#15788 | PR15788]]

Test Plan:
  make check
  ./src/bench/bench_bitcoin

Check that the new benchmark is run.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5848
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 27, 2020
@deadalnix
Backport leftovers from 15788
622fa9f 
Summary:
test: Have segwit always active in (Basic)TestingSetup
test: Use test_bitcoin setup in bench, Add test utils

This is a partial bakport of Core [[bitcoin/bitcoin#15788 | PR15788]] . the original backport missed many bits.

I did not port the RPC like method because they don't build for us and they are dead code.

Depends on D5848

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5849
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request May 19, 2020
@ftrader
Rename test_bitcoin to test/setup_common
4d1ebac 
Summary:
Original replacement script:
```
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e 's/test_bitcoin\.(h|cpp)/setup_common.\1/g'
$(git grep -l test_bitcoin)
git mv ./src/test/test_bitcoin.h   ./src/test/setup_common.h
git mv ./src/test/test_bitcoin.cpp ./src/test/setup_common.cpp
sed -i -e 's/BITCOIN_TEST_TEST_BITCOIN_H/BITCOIN_TEST_SETUP_COMMON_H/g'
./src/test/setup_common.h
-END VERIFY SCRIPT-
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@fa82190#diff-cbf056cfc6c760726f2dff1391fd37d8

Depends on D5493.

This touches a lot of files, but is renaming only and unfortunately
cannot be split without breaking the build.

Test Plan:
  make
  make check
  ./src/bench/bench_bitcoin

  ninja
  ninja check
  ninja bench-bitcoin

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5494
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request May 23, 2020
@deadalnix
bench: Add wallet_balance benchmarks
bf444f4 
Summary:
This is a partial backport of Core [[bitcoin/bitcoin#15779 | PR15779]] : bitcoin/bitcoin@fa46ac3

Also include pieces of [[bitcoin/bitcoin#15788 | PR15788]] that previously were ignored becauseit was dead code.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D6236
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Aug 17, 2020
@ftrader
test: Use test_bitcoin setup in bench
d110c08 
Summary:
```
Now that the fuzz tests can use the BasicTestingSetup [1], do the same
for bench.
```

Partial backport of core [[bitcoin/bitcoin#15788 | PR15788]]:
bitcoin/bitcoin@fa8685d

This is stripped down to using the test fixture for the benchmark, as we
have no use for the utils for now.

Depends on D5484.

Test Plan:
  make
  make check
  ./src/bench/bench_bitcoin

  ninja
  ninja check
  ninja bench-bitcoin

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5493
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Aug 17, 2020
@ftrader
bench: Add block assemble benchmark
db6ebe7 
Summary: This is a backport of Core [[bitcoin/bitcoin#13219 | PR13219]] and [[bitcoin/bitcoin#13806 | PR13806]] . Due to many backport being done out of oder or improperly, this contains elements of [[bitcoin/bitcoin#15413 | PR15413]] and [[bitcoin/bitcoin#15788 | PR15788]]

Test Plan:
  make check
  ./src/bench/bench_bitcoin

Check that the new benchmark is run.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5848
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Aug 17, 2020
@ftrader
Backport leftovers from 15788
d7dda0e 
Summary:
test: Have segwit always active in (Basic)TestingSetup
test: Use test_bitcoin setup in bench, Add test utils

This is a partial bakport of Core [[bitcoin/bitcoin#15788 | PR15788]] . the original backport missed many bits.

I did not port the RPC like method because they don't build for us and they are dead code.

Depends on D5848

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5849

freetrader: added chain.h includes to bench.cpp and util.cpp as
            required by rebasing; thanks to Calin Culianu for the patch
@Munkybooty Munkybooty mentioned this pull request Oct 7, 2021
Merged
@kwvg kwvg mentioned this pull request Oct 25, 2021
Merged
11 tasks
kwvg added a commit to kwvg/dash that referenced this pull request Oct 25, 2021
@kwvg
merge bitcoin#15788: Unify testing setups for fuzz, bench, and unit t…
ad20301 
…ests
pravblockc pushed a commit to pravblockc/dash that referenced this pull request Nov 18, 2021
@kwvg @pravblockc
merge bitcoin#15788: Unify testing setups for fuzz, bench, and unit t…
752c013 
…ests
@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.

Reviewers

1 more reviewer

@jonatack jonatack jonatack left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Build system Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maflcko @practicalswift @DrahtBot @jonatack