Skip to content
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

Add qt unit test runner summary #576

Merged
merged 3 commits into from
Apr 12, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions src/qt/test/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
#endif // ENABLE_WALLET

#include <QApplication>
#include <QDebug>
#include <QObject>
#include <QTest>

#include <functional>

#if defined(QT_STATICPLUGIN)
Expand Down Expand Up @@ -69,8 +71,6 @@ int main(int argc, char* argv[])
gArgs.ForceSetArg("-upnp", "0");
gArgs.ForceSetArg("-natpmp", "0");

bool fInvalid = false;

// Prefer the "minimal" platform for the test instead of the normal default
// platform ("xcb", "windows", or "cocoa") so tests can't unintentionally
// interfere with any background GUIs and don't require extra resources.
Expand All @@ -86,32 +86,32 @@ int main(int argc, char* argv[])
app.setApplicationName("Bitcoin-Qt-test");
app.createNode(*init);

int num_test_failures{0};

AppTests app_tests(app);
if (QTest::qExec(&app_tests) != 0) {
fInvalid = true;
}
num_test_failures += QTest::qExec(&app_tests);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@jonatack jonatack Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in my testing it returns the number of failures; if there are two failures for instance, it returns 2. This is desirable.


OptionTests options_tests(app.node());
if (QTest::qExec(&options_tests) != 0) {
fInvalid = true;
}
num_test_failures += QTest::qExec(&options_tests);

URITests test1;
if (QTest::qExec(&test1) != 0) {
fInvalid = true;
}
num_test_failures += QTest::qExec(&test1);

RPCNestedTests test3(app.node());
if (QTest::qExec(&test3) != 0) {
fInvalid = true;
}
num_test_failures += QTest::qExec(&test3);

#ifdef ENABLE_WALLET
WalletTests test5(app.node());
if (QTest::qExec(&test5) != 0) {
fInvalid = true;
}
num_test_failures += QTest::qExec(&test5);

AddressBookTests test6(app.node());
if (QTest::qExec(&test6) != 0) {
fInvalid = true;
}
num_test_failures += QTest::qExec(&test6);
#endif

return fInvalid;
if (num_test_failures) {
qWarning("\nFailed tests: %d\n", num_test_failures);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @shaavan

} else {
qDebug("\nAll tests passed.\n");
}
return num_test_failures;
}