test: boost unit tests don’t handle assert very well #16700

1. 
   MarcoFalke commented at 6:06 pm on August 23, 2019: member

   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
2. MarcoFalke added the label Tests on Aug 23, 2019
3. 
   ariard commented at 7:04 pm on August 23, 2019: member

   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:

   0$> ./src/test/test_bitcoin --catch_system_errors=no -t wallet_tests 
   1Running 9 test cases...
   2Assertion failed: detected inconsistent lock order at sync.cpp:121, details in debug log.
   3[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

4. MarcoFalke referenced this in commit 36e507227e on Feb 21, 2020
5. MarcoFalke closed this on Feb 21, 2020

   ---

6. sidhujag referenced this in commit b69fd1e704 on Feb 22, 2020
7. sidhujag referenced this in commit c2a4db207c on Nov 10, 2020
8. 
   ariard commented at 6:34 pm on April 29, 2021: member

   @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.

9. 
   MarcoFalke commented at 6:39 pm on April 29, 2021: member

   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.
10. 
    ariard commented at 3:10 pm on April 30, 2021: member

    Thanks for the answer! Yes a lot of critical code paths with asserts aren’t exercise sadly.
11. 
    MarcoFalke commented at 3:23 pm on April 30, 2021: member

    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.
12. PastaPastaPasta referenced this in commit a5ff578062 on Jun 27, 2021
13. PastaPastaPasta referenced this in commit ea0e6550b1 on Jun 28, 2021
14. PastaPastaPasta referenced this in commit 7801e7b965 on Jun 29, 2021
15. PastaPastaPasta referenced this in commit 44ee0fd3e9 on Jul 1, 2021
16. PastaPastaPasta referenced this in commit b1968646ab on Jul 1, 2021
17. PastaPastaPasta referenced this in commit d74366e54b on Jul 14, 2021
18. DrahtBot locked this on Aug 18, 2022

Contributors
MarcoFalke ariard

Labels
Tests