cmake: Adjust test/README.md instructions
#328
Conversation
|
cc @paplorinc @maflcko |
hebasto
force-pushed
the
240814-cmake-IY
branch
from
18a8736
to
64ee198
Compare
There was a problem hiding this comment.
We might want to update https://github.com/hebasto/bitcoin/pull/328/files#diff-4d2a64ce14cb8b971dbba9455421b04ae7ed0c489c66d983664be5632b0de4a3R102 as well, i.e. the Makefile.test.include mention, right?
And probably https://github.com/hebasto/bitcoin/pull/328/files#diff-4d2a64ce14cb8b971dbba9455421b04ae7ed0c489c66d983664be5632b0de4a3R111 also.
| test_bitcoin --log_level=all --run_test=getarg_tests | ||
| build/src/test/test_bitcoin --log_level=all --run_test=getarg_tests |
There was a problem hiding this comment.
The previous variant assumed . is in PATH!
There was a problem hiding this comment.
I couldn't find that in the docs, do most people have that set?
I've put that into a separated PR. |
Adjust test/README.md instructions for cmake
hebasto
force-pushed
the
240814-cmake-IY
branch
from
64ee198
to
36117af
Compare
Reworked. |
| after a test file was modified, run `cmake --build build` and then run the test again. If you | ||
| modify a non-test file, use `cmake --build build --target test_bitcoin` to recompile only what's needed |
There was a problem hiding this comment.
Maybe I'm missing context, but whether test file was modified or production code, both cmake --build build --target test_bitcoin and cmake --build build will build all dependencies automagically, so maybe we can simply recommend cmake --build build --target test_bitcoin in all cases - and if we're worried about speed, we could even mention the -j N param here as well.
There was a problem hiding this comment.
Yes, cmake --build build --target test_bitcoin always works.
| To add a new unit test file to our test suite you need | ||
| to add the file to `src/Makefile.test.include`. The pattern is to create | ||
| To add a new unit test file to our test suite, you need | ||
| to add the file to either `src/test/CMakeLists.txt` or |
There was a problem hiding this comment.
I see you've mentioned the wallet as well, should we maybe mention that test utility functions could be extracted to src/test/util/CMakeLists.txt (or is that too advanced for an intro readme)?
There was a problem hiding this comment.
or is that too advanced for an intro readme
It seems so.
| test_bitcoin --log_level=all --run_test=getarg_tests | ||
| build/src/test/test_bitcoin --log_level=all --run_test=getarg_tests |
There was a problem hiding this comment.
I couldn't find that in the docs, do most people have that set?
|
re-ACK 36117af |
Based on #322 with the minimized diff.