Add qt unit test runner summary#576
Conversation
|
A sample patch to generate 3 errors to test the runner output in this case. patch
diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp
index 4a943a634..9e6c8a243 100644
--- a/src/qt/test/optiontests.cpp
+++ b/src/qt/test/optiontests.cpp
@@ -28,6 +28,7 @@ void OptionTests::optionTests()
gArgs.LockSettings([&](util::Settings& settings) {
settings.rw_settings.erase("prune");
});
+ QVERIFY(gArgs.IsArgSet("-prune"));
gArgs.WriteSettingsFile();
}
@@ -51,7 +52,7 @@ void OptionTests::parametersInteraction()
OptionsModel{};
- const bool expected{false};
+ const bool expected{true};
QVERIFY(gArgs.IsArgSet("-listen"));
QCOMPARE(gArgs.GetBoolArg("-listen", !expected), expected);
diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp
index 669a05fe0..4395c20cf 100644
--- a/src/qt/test/rpcnestedtests.cpp
+++ b/src/qt/test/rpcnestedtests.cpp
@@ -73,7 +73,7 @@ void RPCNestedTests::rpcNestedTests()
QVERIFY(result.substr(0,1) == "{");
RPCConsole::RPCExecuteCommandLine(m_node, result, "getblockchaininfo()[\"chain\"]"); //Quote path identifier are allowed, but look after a child containing the quotes in the key
- QVERIFY(result == "null");
+ QVERIFY(result != "null"); |
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
ACK d025d7f
It makes sense to give the user an overview of how many tests were failed. Though it won’t wholly communicate the information of which tests failed to the user, it still would make him alert to the situation and allow him to save his efforts of eyeballing in case the “Test failed” message is missing.
I was able to compile the PR and run the test_bitcoin-qt file successfully. I have attached the screenshot of the difference in output between the Master and PR branches.
| Master | PR |
|---|---|
 |
 |
|
|
||
| return fInvalid; | ||
| if (num_test_failures) { | ||
| qWarning("\nFailed tests: %d\n", num_test_failures); |
There was a problem hiding this comment.
nit:
This is less of a suggestion and more of a preference. I don't think this line needs separate line breaks to make it stand out from other lines. I think we can forgo the "\n"s.
There was a problem hiding this comment.
Thanks @shaavan! I like the line breaks to set off the summary, but if reviewers agree with you I'm ok to drop them.
| if (QTest::qExec(&app_tests) != 0) { | ||
| fInvalid = true; | ||
| } | ||
| num_test_failures += QTest::qExec(&app_tests); |
There was a problem hiding this comment.
QTest::qExec does not guarantee return value 1 in case of a test failure, but:
a value other than 0 if one or more tests failed or in case of unhandled exceptions.
There was a problem hiding this comment.
Yes, in my testing it returns the number of failures; if there are two failures for instance, it returns 2. This is desirable.
d025d7f gui, refactor: rename fInvalid to num_test_failures in test_main.cpp (Jon Atack) 2489b6f gui: count test failures in test runner summary (Jon Atack) ba44aae gui: add test runner summary (Jon Atack) Pull request description: Append a one-line summary to the output of running `./src/qt/test/test_bitcoin-qt` indicating that all tests passed or showing the number of failing tests. It's currently a bit inconvenient to see this result by eyeballing all of the output. ACKs for top commit: shaavan: ACK d025d7f jarolrod: tACK bitcoin-core/gui@d025d7f Tree-SHA512: 981c5daa13db127d38167bcf78b296b1a7e5b2d12e65f364ec6382b24f1008a223521d3b6c56e920bcd037479da5414e43758794688019d09e9aa696f3964746
5 participants
Append a one-line summary to the output of running
./src/qt/test/test_bitcoin-qtindicating that all tests passed or showing the number of failing tests. It's currently a bit inconvenient to see this result by eyeballing all of the output.