Add qt unit test runner summary #576

1. 
   jonatack commented at 10:03 pm on April 6, 2022: contributor

   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.
2. gui: add test runner summary ba44aae768
3. gui: count test failures in test runner summary 2489b6fe9c
4. gui, refactor: rename fInvalid to num_test_failures in test_main.cpp d025d7f025
5. 
   jonatack commented at 10:07 pm on April 6, 2022: contributor

   A sample patch to generate 3 errors to test the runner output in this case.

    0diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp
    1index 4a943a634..9e6c8a243 100644
    2--- a/src/qt/test/optiontests.cpp
    3+++ b/src/qt/test/optiontests.cpp
    4@@ -28,6 +28,7 @@ void OptionTests::optionTests()
    5     gArgs.LockSettings([&](util::Settings& settings) {
    6         settings.rw_settings.erase("prune");
    7     });
    8+    QVERIFY(gArgs.IsArgSet("-prune"));
    9     gArgs.WriteSettingsFile();
   10 }
   11 
   12@@ -51,7 +52,7 @@ void OptionTests::parametersInteraction()
   13 
   14     OptionsModel{};
   15 
   16-    const bool expected{false};
   17+    const bool expected{true};
   18 
   19     QVERIFY(gArgs.IsArgSet("-listen"));
   20     QCOMPARE(gArgs.GetBoolArg("-listen", !expected), expected);
   21diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp
   22index 669a05fe0..4395c20cf 100644
   23--- a/src/qt/test/rpcnestedtests.cpp
   24+++ b/src/qt/test/rpcnestedtests.cpp
   25@@ -73,7 +73,7 @@ void RPCNestedTests::rpcNestedTests()
   26     QVERIFY(result.substr(0,1) == "{");
   27 
   28     RPCConsole::RPCExecuteCommandLine(m_node, result, "getblockchaininfo()[\"chain\"]"); //Quote path identifier are allowed, but look after a child containing the quotes in the key
   29-    QVERIFY(result == "null");
   30+    QVERIFY(result != "null");
   

6. in src/qt/test/test_main.cpp:112 in d025d7f025

   126+    num_test_failures += QTest::qExec(&test6);
   127 #endif
   128 
   129-    return fInvalid;
   130+    if (num_test_failures) {
   131+        qWarning("\nFailed tests: %d\n", num_test_failures);
   

   ---

   ---

   

   shaavan commented at 6:16 pm on April 7, 2022:

   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.

   ---

   

   jonatack commented at 6:30 pm on April 7, 2022:

   Thanks @shaavan! I like the line breaks to set off the summary, but if reviewers agree with you I’m ok to drop them.

   ---

   

   cheetooooo commented at 8:34 pm on April 8, 2022:

   thanks @shaavan
7. shaavan approved
8. 
   shaavan commented at 6:16 pm on April 7, 2022: contributor

   ACK d025d7f0251e26b7ab5cf48c236b6b5e46fafe26

   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

9. jarolrod approved
10. 
    jarolrod commented at 1:39 am on April 10, 2022: member

    tACK https://github.com/bitcoin-core/gui/commit/d025d7f0251e26b7ab5cf48c236b6b5e46fafe26

    I have tested and reviewed the code, I agree this can be merged.

11. in src/qt/test/test_main.cpp:92 in d025d7f025

    90+
    91     AppTests app_tests(app);
    92-    if (QTest::qExec(&app_tests) != 0) {
    93-        fInvalid = true;
    94-    }
    95+    num_test_failures += QTest::qExec(&app_tests);
    

    ---

    ---

    

    hebasto commented at 9:41 pm on April 12, 2022:

    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.

    ---

    

    jonatack commented at 10:06 pm on April 12, 2022:

    Yes, in my testing it returns the number of failures; if there are two failures for instance, it returns 2. This is desirable.
12. hebasto added the label Tests on Apr 12, 2022
13. hebasto merged this on Apr 12, 2022
14. hebasto closed this on Apr 12, 2022

    ---

15. jonatack deleted the branch on Apr 13, 2022
16. sidhujag referenced this in commit c1b62eb579 on Apr 13, 2022
17. bitcoin-core locked this on Apr 13, 2023

Contributors
jonatack shaavan cheetooooo jarolrod hebasto

Labels
Tests