test: Add ASSERT_DEBUG_LOG to unit test framework

[maflcko]

Similar to assert_debug_log in the functional test framework

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #16224 (gui: Bilingual GUI error messages by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[practicalswift]

Concept ACK

[maflcko]

re-run ci

[ajtowns]

Concept ACK. I think this might be better if implemented with a RAII approach, so you write:

{
    ASSERT_DEBUG_LOG("xyzzy");
    code();
}

and an error appears if the string wasn't found by the end of the block. See https://github.com/ajtowns/bitcoin/commits/201910-assertdebuglog for a patch. This way avoids the globals and lets you just do two asserts in the same block rather than having to always create a vector. I also changed it so it only captures the log if you say ASSERT_DEBUG_LOG_CAPTURE("blah");.

[maflcko]

Thanks, taken some of the patch and rebased

[ajtowns]

If I change ASSERT_DEBUG_LOG("is not a directory") to "isnt a directory" to trigger an error, it seems to hang after the failure rather than finishing shutdown?

[maflcko]

@ajtowns This is an issue every time an assert is hit. The tests in that case would sleep forever, due to some boost internals, see #16700.

The correct error is still printed and the tests fail. I am not sure how to fix this minor usability issue, given that the logger is global (shared state) and the TestingSetup is not shut down between test cases. (make check shuts down the TestingSetup between test suites, so that is something at least)

[maflcko]

Rebased

[ajtowns]

ACK fafa1b9

[maflcko]

@laanwj Any objections? Otherwise I will merge tomorrow

[ryanofsky]

Haven't looked closely, but I'm kind of surprised you can get away with throwing an exception in a destructor (check_found is called by ~DebugLogHelper).

If this works, it seems fine. But in case there are issues, an alternative could be to use a callback instead of a scope, so instead of:

{
    ASSERT_DEBUG_LOG("Please check that your computer's date and time are correct!");
    MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5
}

Something like:

AssertDebugLog("Please check that your computer's date and time are correct!", [&] {
    MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5
});

[ajtowns]

I believe C++11 makes destructors implicitly noexcept, which means that an uncaught exception from results in std::terminate() being called, which is what we want from throwing a runtime exception anyway

[maflcko]

I forgot about this, but indeed the test seem to fail either way, which is what we want.