Skip to content
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: Unify testing setups for fuzz, bench, and unit tests #15788

Merged
merged 4 commits into from Apr 15, 2019

Conversation

Projects
None yet
4 participants
@MarcoFalke
Copy link
Member

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

MarcoFalke added some commits Apr 10, 2019

@practicalswift

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

Concept ACK

Nice!

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1904-testUnify branch from a1310c8 to fa25cb0 Apr 11, 2019

scripted-diff: Rename test_bitcoin to test/setup_common
-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-

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1904-testUnify branch from fa25cb0 to fa0e2b8 Apr 11, 2019

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 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:

  • #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
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1904-testUnify branch from fa0e2b8 to faf4000 Apr 11, 2019

@jonatack
Copy link
Contributor

left a comment

ACK faf4000

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

@jonatack

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

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.

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

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

@MarcoFalke MarcoFalke merged commit faf4000 into bitcoin:master Apr 15, 2019

2 checks passed

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

MarcoFalke added a commit that referenced this pull request Apr 15, 2019

Merge #15788: test: Unify testing setups for fuzz, bench, and 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) #15504

ACKs for commit faf400:
  jonatack:
    ACK faf4000

Tree-SHA512: 8ac5692e72cf50e460958f291643ae6b8bb04d5c1331ed50dce9eb4e9457e5a925144c532c42b360a26707e11eeece74aab27db8c76ab9a429b9dd7167e7cdc4

@MarcoFalke MarcoFalke deleted the MarcoFalke:1904-testUnify branch Apr 15, 2019

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Apr 18, 2019

Merge bitcoin#15843: tests: fix outdated include in blockfilter_index…
…_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

LitecoinZ added a commit to litecoinz-core/wip that referenced this pull request May 31, 2019

Merge bitcoin#15843: tests: fix outdated include in blockfilter_index…
…_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

@LitecoinZ LitecoinZ referenced this pull request May 31, 2019

Open

Backports from upstream #1

44 of 244 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.