test: boost unit tests don't handle assert very well

[maflcko]

When running into an assert failure, the boost unit test framework does not shut down cleanly. It does not call the BasicTestingSetup destructor. Thus, the test process will never shut down. As a result of that, the failure is never passed back to the caller.

On travis, this will result in frustrating and hard-to-debug timeouts: E.g. https://travis-ci.org/bitcoin/bitcoin/jobs/575909995#L3584

I see two ways to fix this:

• Use exceptions instead of assertions, especially in non-validation debug code
• Teach the boost unit test framework how to handle asserts

[ariard]

Seems we need to set BOOST_TEST_CATCH_SYSTEM_ERRORS somewhere and write an handler for abort signal.

https://www.boost.org/doc/libs/1_71_0/libs/test/doc/html/boost_test/utf_reference/rt_param_reference/catch_system.html

EDIT:

$> ./src/test/test_bitcoin --catch_system_errors=no -t wallet_tests 
Running 9 test cases...
Assertion failed: detected inconsistent lock order at sync.cpp:121, details in debug log.
[1]    8770 abort      ./src/test/test_bitcoin --catch_system_errors=no -t wallet_tests

In fact, abort signals are catched which explains why we see checkpoints, but boost seems to not call destructor

[ariard]

@MarcoFalke

Digging up, AFAIU the issue was solved in #18183 by setting catch_system_errors=no in the ci. That said if we're willingly to exercise asserts, have we found a way to teach the unit test framework how to handle them ?

Or otherwise do you advise to replace them by exceptions and catching them with BOOST_CHECK_THROW ?

Interested in the context of #21515's test coverage.

[maflcko]

I think if you want to exercise a logic error during a unit test, the only way is to throw something, as an assert would abort the process.

[ariard]

Thanks for the answer! Yes a lot of critical code paths with asserts aren't exercise sadly.

[maflcko]

Obviously that assumes the logic error is reachable during the unit test. If it is not reachable, an Assert is just fine. If the program may continue execution, an Assume is also fine. See ./src/util/check.h and the dev notes.