-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmake, doc: Document log output for failed tests #329
Conversation
Describe log output for failed tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 784a87e
Do we really need the --log_level=test_suite option for these logs?
No, it is too verbose in an useless way. Correct me if I am wrong, but it is not used now by ctest --test-dir build
, right?
Yes, it is not used now. My question implies a need to update this line: bitcoin/src/test/CMakeLists.txt Line 199 in 7c23041
|
C++ does not print the backtrace on a segmentation fault (or other issue), like other languages like Rust ( So this is not useless. In any case, changing this seems unrelated to the migration. The previous behavior should be matched here. If there was reason to change it, it could be done in a follow-up. |
Hmm, not for me: --- i/src/test/net_tests.cpp
+++ w/src/test/net_tests.cpp
@@ -45,4 +45,5 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port)
uint16_t altPort = 12345;
BOOST_CHECK(gArgs.SoftSetArg("-port", ToString(altPort)));
+ assert(0);
port = GetListenPort();
BOOST_CHECK(port == altPort); Without
With
|
You will have to use the other options as well. For example |
To clarify, the catch_system_error comes from #329 (comment) |
Again, the only difference between:
and
is an extra clutter in the second case. The checkpoint is not printed in either case (when Can you show the exact commands and their output that demonstrate that adding |
Again, you'll have to use the exact same options as the ctest command in #329 (comment) and compare it to the automake command. Using something else will give another result. For example, you are specifying the test case, so there is only one test running. However, ctest specifies a test suite. Also, Thus, the commands to compare would be:
and
|
The last checkpoint is not printed for neither of the above commands 👀 |
Ah right, the log level for that would be |
Sure, see below, where I test this with a segfault. Omitting the log level doesn't give any hint where the issue happened. However, with the log level, one can at least see the unit test.
|
On the master branch,
make check
generates afoo_tests.log
file for eachfoo_tests.cpp
. These log files are only printed when a test fails. Since no one has raised concerns about the log file renaming in bitcoin#19385, these files appear to serve no other purpose.CTest provides its own mechanisms for logging failed tests, making the creation of
foo_tests.log
files unnecessary.This PR documents these CTest logging mechanisms.
An example of output with a mocked test error:
A question for reviewers: Do we really need the
--log_level=test_suite
option for these logs?