make check runs a bunch of other subtree tests that exercise code that is hardly ever changed and have a comparatively long runtime. There seems to be no target for running just the unit tests, so add one.
Alternatively the secp256k1 tests could be removed from the check-local target, reducing its runtime. This was rejected before though in #20264.
make check runs a bunch of other subtree tests that exercise code that
is hardly ever changed and have a comparatively long runtime. There
seems to be no target for running just the unit tests, so add one.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For detailed information about the code coverage, see the test coverage report.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | delta1, edilmedeiros, ryanofsky, achow101 |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
utACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf
Tested ACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf
make -C src check-unit does execute the same unit tests as make check.
I tried to include as many things as possible:
Options used to compile and link:
external signer = yes
multiprocess = no
with libs = yes
with wallet = yes
with sqlite = yes
with bdb = yes
with gui / qt = yes
with qr = yes
with zmq = yes
with test = yes
with fuzz binary = yes
with bench = yes
with upnp = yes
with natpmp = no
use asm = yes
USDT tracing = no
sanitizers =
debug enabled = no
gprof enabled = no
werror = no
Tested ACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf.
This is very convenient but the check-unit target does not seem to depend on the test_bitcoin executable, so it can run with a stale test binary.
I also noticed if I try to build and run the unit tests with make -j12 -C src test/test_bitcoin check-unit it will try to run the tests at the same time as building the test binary instead of waiting for the binary to be built.
I also wonder if this new check-unit test option is redundant with the existing bitcoin_test_check target. It's not clear what the purpose of that target it supposed to be if not to run unit tests, it would be useful to have comments in the makefile saying what these targets are intended to be used for, and maybe deleting one or making one an alias for the other (since check-unit is easier to type).
ACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf
I also wonder if this new
check-unittest option is redundant with the existingbitcoin_test_checktarget.
bitcoin_test_check just fails for me, so they are at least different in this aspect. It looks like this target is used by the check target in src/test/Makefile.
Post-merge ACK. This certainly is handy, and I've switched my workflow to use it.