Skip to content

build: Create test utility library from src/test/util/ #17542

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 22, 2019
Merged

build: Create test utility library from src/test/util/ #17542

merged 1 commit into from
Nov 22, 2019

Conversation

brakmic
Copy link
Contributor

@brakmic brakmic commented Nov 20, 2019

This PR creates a static test utility library that replaces repetitive compilations of sources from src/test/util in unit, gui and bench tests.

The original issue is here: #17401

The changes are:

  • a new Makefile.test_util.include
  • a new entry in Makefile.am that includes Makefile.test_util.include when testing is enabled
  • removal of all src/test/util headers & sources from unit, gui and bench Makefiles
  • addition of libtest_util.a at LDADD's of every test

maflcko
maflcko approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK

Not sure why the mac builds succeed, but the linux ones fail, but maybe you need to link BITCOIN_SERVER to the test util lib?

@practicalswift
Copy link
Contributor

Concept ACK

Welcome as a contributor @brakmic! Hope to see more great contributions from you in the future!

@brakmic
Copy link
Contributor Author

brakmic commented Nov 20, 2019 •
edited
Loading

ACK

Not sure why the mac builds succeed, but the linux ones fail, but maybe you need to link BITCOIN_SERVER to the test util lib?

I must admit that I'm not sure what the best way is to link another lib to libtest_util.a.

@brakmic
Copy link
Contributor Author

brakmic commented Nov 20, 2019

Concept ACK

Welcome as a contributor @brakmic! Hope to see more great contributions from you in the future!

Many thanks! :)

@brakmic
Copy link
Contributor Author

brakmic commented Nov 20, 2019 •
edited
Loading

ACK

Not sure why the mac builds succeed, but the linux ones fail, but maybe you need to link BITCOIN_SERVER to the test util lib?

I'll try it with LIBADD.

--EDIT: the linking seems to be working now.

@maflcko
Copy link
Member

maflcko commented Nov 21, 2019

Could squash the commits into one with

git reset --soft b4a1da9ef8e4b673c290d5b882527e627ae1b43a
git commit -m "build: Create test utility library from src/test/util/"
git push ... -f

maflcko
maflcko reviewed Nov 21, 2019
View reviewed changes
@maflcko
Copy link
Member

maflcko commented Nov 21, 2019

ACK a2e581d 🍞

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK a2e581de942ece85b84c4f5a15287654b02867cb 🍞
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYEACgkQzit1aX5p
pUjDOQv+OyAcqDgHcIOr0IFlXmYp9mHraRe2d32wREQdaWZWxV/6MEZCxf98Pcl3
DZDnrkuXw7EuONBXKZj6zTTEAnL1Q87FMyZYRTWlcn5py0WlT7CbOxCP66nteCDq
wq1HkMscLAIm6ymxGRBNCe45FbZVLT1GXcvb5Ylb1W2zJMOgmkwZfkESqdaN9MgZ
g8Y7BljZdeEWJU8eXWbiJJiwipUMadWN4WU0azzfgvhVpxc86HZgMWblR3BICIYw
gJ5y7oW/tddXubmgGlApY7SvZ1oAnn77PSLM0Prz0NJoPXJGFfJYsEOoEw4yBgcj
7bgOZnQv5qsGNk8wDQJHZDAISdKLCrMZn68hJ68didlMAyqtMXQTWFiQBn4br707
VrksXVKLmovxTkcj9W5OYj9/yfYltb1wcEu72O3by22UbfKf707TpQleXGFWwHYB
xfHimAbyv0AsGqktVp2XOguPVt8Uoni6sS0MjZXLsxFLYSa6WRNMB+ZXrnJVAWMt
/C8rLkOp
=a+eq
-----END PGP SIGNATURE-----

Timestamp of file with hash 4cbf916b05be4c1f535990b2e54da8495aba2db3e8a27b1b66c3a76beb122412 -

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Nov 22, 2019
Merge bitcoin#17542: build: Create test utility library from src/test…
b983e7e 
…/util/

a2e581d build: Create test utility library from src/test/util/ (Harris)

Pull request description:

  This PR creates a static **test utility library** that replaces repetitive compilations of sources from *src/test/util* in **unit**, **gui** and **bench** **tests**.

  The original issue is here: bitcoin#17401

  The changes are:

  * a new *Makefile.test_util.include*
  * a new entry in *Makefile.am* that includes *Makefile.test_util.include* when testing is enabled
  * removal of all *src/test/util* headers & sources from unit, gui and bench Makefiles
  * addition of *libtest_util.a* at LDADD's of every test

ACKs for top commit:
  MarcoFalke:
    ACK a2e581d 🍞

Tree-SHA512: d172127a26ee70d16625e17d7d94337a65472c57bb97f910c357c52d3dc082ea478ee586ee9074d9ebfeb05b75027e5e15f5bcd2aa35962dadfd9ac6bfd55ab9
@maflcko maflcko merged commit a2e581d into bitcoin:master Nov 22, 2019
@brakmic brakmic deleted the test-util-lib branch November 22, 2019 14:26
@sipsorcery sipsorcery mentioned this pull request Nov 23, 2019
Merged
fanquake added a commit that referenced this pull request Nov 23, 2019
@fanquake
Merge #17571: Add libtest_util library to msvc build configuration
2c98e20 
b509554 Added libtest_util library to msvc build configuration. (Aaron Clauson)

Pull request description:

  libtest_util was introduced in #17542. This PR adds the msvc build configuration.

ACKs for top commit:
  fanquake:
    ACK b509554 - Appveyor looks good.

Tree-SHA512: abd9f8427375ae0e75e8227d853cccc666fd9e906038d97b787d9185dec1024232a6c64301e26e66fcaf86492183328abe4a7abd7af3321f062cd8722eb83b4c
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 23, 2019
@fanquake
Merge bitcoin#17571: Add libtest_util library to msvc build configura…
c7cea23 
…tion

b509554 Added libtest_util library to msvc build configuration. (Aaron Clauson)

Pull request description:

  libtest_util was introduced in bitcoin#17542. This PR adds the msvc build configuration.

ACKs for top commit:
  fanquake:
    ACK b509554 - Appveyor looks good.

Tree-SHA512: abd9f8427375ae0e75e8227d853cccc666fd9e906038d97b787d9185dec1024232a6c64301e26e66fcaf86492183328abe4a7abd7af3321f062cd8722eb83b4c
@mzumsande mzumsande mentioned this pull request Nov 25, 2019
Merged
maflcko pushed a commit that referenced this pull request Dec 16, 2019
Merge #17593: test: move more utility functions into test utility lib…
94c6f2b 
…rary

78e283e [test] move wallet helper functions into test library (Martin Zumsande)
f613e5d [test] move mining helper functions into test library (Martin Zumsande)
2cb4e8b [test] move string helper functions into test library (Martin Zumsande)

Pull request description:

  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`

ACKs for top commit:
  MarcoFalke:
    ACK 78e283e 🔧

Tree-SHA512: f182a61e86e76c32bcb84e37f44904d3a4a9c5a321f7a8efdda5368a6623cb8b5a5384ec4f96e67f0357b0c22099f6e3ecd0ac4cb467e3fa3f3128f8d36edfb8
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 16, 2019
Merge bitcoin#17593: test: move more utility functions into test util…
d566bad 
…ity library

78e283e [test] move wallet helper functions into test library (Martin Zumsande)
f613e5d [test] move mining helper functions into test library (Martin Zumsande)
2cb4e8b [test] move string helper functions into test library (Martin Zumsande)

Pull request description:

  This disbands `test/util.h` and `test/util.cpp` and moves the content into the test utility library recently created in bitcoin#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`

ACKs for top commit:
  MarcoFalke:
    ACK 78e283e 🔧

Tree-SHA512: f182a61e86e76c32bcb84e37f44904d3a4a9c5a321f7a8efdda5368a6623cb8b5a5384ec4f96e67f0357b0c22099f6e3ecd0ac4cb467e3fa3f3128f8d36edfb8
@MarkLTZ MarkLTZ mentioned this pull request Apr 4, 2020
Closed
@maflcko maflcko mentioned this pull request Apr 4, 2020
Closed
@brakmic brakmic mentioned this pull request Apr 4, 2020
Merged
maflcko pushed a commit that referenced this pull request Apr 5, 2020
Merge #18528: build: create test_fuzz library from src/test/fuzz/fuzz…
96a30b9 
….cpp

691e2a7 build: create test_fuzz library from src/test/fuzz/fuzz.cpp (Harris)

Pull request description:

  This PR creates a static library **libtest_fuzz.a** to speed up the compilation of fuzz tests. It is functionally similar to #17542

  Fixes #18527

ACKs for top commit:
  MarcoFalke:
    ACK 691e2a7 🦁

Tree-SHA512: 39d7d2731ca4370db518dbb969eb17ddbf9c030c3fe0dec0d04ff6578f24a128563fe5aced78300c92ce296623a7079fea5aea70619819a20c56fb34191f00ef
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jun 28, 2020
@brakmic @deadalnix
build: Create test utility library from src/test/util/
c4788ee 
Summary: Thsi is a backport fo Core [[bitcoin/bitcoin#17542 | PR17542]]

Test Plan:
  ninja all check bitcoin-fuzzers

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D6759
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
@fanquake
Merge bitcoin#17571: Add libtest_util library to msvc build configura…
54bf552 
…tion

b509554 Added libtest_util library to msvc build configuration. (Aaron Clauson)

Pull request description:

  libtest_util was introduced in bitcoin#17542. This PR adds the msvc build configuration.

ACKs for top commit:
  fanquake:
    ACK b509554 - Appveyor looks good.

Tree-SHA512: abd9f8427375ae0e75e8227d853cccc666fd9e906038d97b787d9185dec1024232a6c64301e26e66fcaf86492183328abe4a7abd7af3321f062cd8722eb83b4c
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
Merge bitcoin#17593: test: move more utility functions into test util…
d346a17 
…ity library

78e283e [test] move wallet helper functions into test library (Martin Zumsande)
f613e5d [test] move mining helper functions into test library (Martin Zumsande)
2cb4e8b [test] move string helper functions into test library (Martin Zumsande)

Pull request description:

  This disbands `test/util.h` and `test/util.cpp` and moves the content into the test utility library recently created in bitcoin#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`

ACKs for top commit:
  MarcoFalke:
    ACK 78e283e 🔧

Tree-SHA512: f182a61e86e76c32bcb84e37f44904d3a4a9c5a321f7a8efdda5368a6623cb8b5a5384ec4f96e67f0357b0c22099f6e3ecd0ac4cb467e3fa3f3128f8d36edfb8
@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

@maflcko maflcko maflcko approved these changes

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
@brakmic @practicalswift @maflcko @fanquake