Bugfix: make check: Only run tests that were compiled (& print correct log on failure) #16228

pull luke-jr wants to merge 2 commits into bitcoin:master from luke-jr:bugfix_raii_check_fail changing 1 files +20 −11

luke-jr commented at 2:26 am on June 18, 2019: member

raii_event_tests is not built if EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined. This would previously cause make check to fail.

Side effect: Verbose mode will no longer show the test_bitcoin command line.

Edit: Also added a fix for a regression introduced in #19385

(This is a clean merge to 0.20, 0.21, 22.x, 23.x, and master branches)
fanquake added the label Build system on Jun 18, 2019
laanwj commented at 8:09 am on June 18, 2019: member

Concept ACK, though the ever-complicating knot of shell script is not something I feel confident to review.
fanquake requested review from theuni on Jun 18, 2019
fanquake requested review from dongcarl on Jun 18, 2019
MarcoFalke commented at 1:52 pm on June 18, 2019: member

I guess the alternative would be to replace those tests with BOOST_CHECK(true);. But anything seems like a hack. Why is the file compiled at all when none of the tests are compiled?
luke-jr commented at 4:45 pm on June 18, 2019: member

My guess is to avoid checking for the boost define in the build system.
luke-jr commented at 7:11 pm on September 20, 2019: member

This doesn’t work with Boost 1.47 and makes gitian hang because its boost doesn’t support the –list_content option.

In search for a better solution, I ended up falling back to basically #16564

Theoretically, we could have configure check the define for us and remove the file entirely, but I’m not sure it’s worth the trouble (and doesn’t give the user any indication the test was skipped!).
luke-jr closed this on Sep 20, 2019
DrahtBot locked this on Dec 16, 2021
hebasto commented at 2:45 pm on December 25, 2021: member

This doesn’t work with Boost 1.47 and makes gitian hang because its boost doesn’t support the –list_content option.

Now we have minimum Boost 1.64.0.

Also this PR makes make check compatible with https://www.boost.org/doc/libs/1_64_0/libs/test/doc/html/boost_test/tests_organization/enabling.html.

Maybe reopen?
MarcoFalke unlocked this on Dec 25, 2021
luke-jr reopened this on Dec 26, 2021
DrahtBot added the label Needs rebase on Dec 26, 2021
hebasto commented at 9:08 am on December 26, 2021: member

Approach ACK 97c6bde34e5da77ff5b5f2deb8be0366cc66b2ba.

For now there are unresolved conflicts with #18183 and #18472.
fanquake commented at 3:52 pm on March 24, 2022: member

What is the status of this? Needs conflicts fixed. Needs the CI to run. Also needs the PR description updated.
luke-jr force-pushed on Mar 24, 2022
luke-jr closed this on Mar 24, 2022
luke-jr reopened this on Mar 24, 2022
luke-jr commented at 10:51 pm on March 24, 2022: member

Rebased
DrahtBot removed the label Needs rebase on Mar 25, 2022
DrahtBot added the label Needs rebase on Mar 25, 2022
DrahtBot commented at 1:06 pm on March 25, 2022: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

Bugfix: make check: Only run tests that were compiled

raii_event_tests is not built if EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined.
This would previously cause `make check` to fail.

Side effect: Verbose mode will no longer show the test_bitcoin command line.

7201865b04

Bugfix: make check: If a test fails, print the log from the correct file 3032cf7a36
luke-jr force-pushed on Mar 25, 2022
luke-jr requested review from hebasto on Mar 25, 2022
luke-jr commented at 2:18 pm on March 25, 2022: member

Rebased again, and added a fix for a regression introduced by #19385
luke-jr renamed this:
~~Bugfix: make check: Only run tests that were compiled~~
Bugfix: make check: Only run tests that were compiled (& print correct log on failure)
on Mar 25, 2022

MarcoFalke commented at 2:32 pm on March 25, 2022: member

This would previously cause make check to fail.

I tried to test this, but failed. make check passes. See also:

0./src/test/test_bitcoin -t raii_event_tests -l message && echo $?
1Running 1 test case...
2Skipping raii_event_tess: libevent doesn't support event_set_mem_functions
3Test case raii_event_tests/raii_event_tests_SKIPPED did not check any assertions
4
5*** No errors detected
60

MarcoFalke commented at 2:33 pm on March 25, 2022: member

This was already fixed in 091cc4b94e009f7140493cd37798d447c6881e5e
luke-jr closed this on Mar 25, 2022
luke-jr commented at 2:36 pm on March 25, 2022: member

Good point.
DrahtBot locked this on Mar 25, 2023

Contributors
luke-jr laanwj MarcoFalke hebasto fanquake DrahtBot

Review Requested
theuni dongcarl hebasto

Labels
Build system Needs rebase