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.

Sign up for GitHub

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

Jump to bottom

Run bitcoin_test-qt under minimal QPA platform #10142

Merged
merged 1 commit into from
Apr 10, 2017
Merged

Run bitcoin_test-qt under minimal QPA platform #10142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions build-aux/m4/bitcoin_qt.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ AC_DEFUN([BITCOIN_QT_CONFIGURE],[
if test "x$bitcoin_cv_need_acc_widget" = "xyes"; then
_BITCOIN_QT_CHECK_STATIC_PLUGINS([Q_IMPORT_PLUGIN(AccessibleFactory)], [-lqtaccessiblewidgets])
fi
_BITCOIN_QT_CHECK_STATIC_PLUGINS([Q_IMPORT_PLUGIN(QMinimalIntegrationPlugin)],[-lqminimal])
Copy link
Member

Choose a reason for hiding this comment

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

We need to make it so that this doesn't signal failed qt detection if libqminimal isn't found. It'd be fine if it were just test_bitcoin-qt disabled, but this would also disable the bitcoin-qt build which has no dependency on the lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to make it so that this doesn't signal failed qt detection if libqminimal isn't found.

What should happen if minimal isn't found, and why we can't just make it a requirement? I looked into trying making it make it optional, but it seemed like a mess, see my comment #10117 (comment)

AC_DEFINE(QT_QPA_PLATFORM_MINIMAL, 1, [Define this symbol if the minimal qt platform exists])
if test x$TARGET_OS = xwindows; then
_BITCOIN_QT_CHECK_STATIC_PLUGINS([Q_IMPORT_PLUGIN(QWindowsIntegrationPlugin)],[-lqwindows])
AC_DEFINE(QT_QPA_PLATFORM_WINDOWS, 1, [Define this symbol if the qt platform is windows])
Expand Down
8 changes: 8 additions & 0 deletions src/qt/test/test_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ Q_IMPORT_PLUGIN(qjpcodecs)
Q_IMPORT_PLUGIN(qtwcodecs)
Q_IMPORT_PLUGIN(qkrcodecs)
#else
#if defined(QT_QPA_PLATFORM_MINIMAL)
Q_IMPORT_PLUGIN(QMinimalIntegrationPlugin);
#endif
#if defined(QT_QPA_PLATFORM_XCB)
Q_IMPORT_PLUGIN(QXcbIntegrationPlugin);
#elif defined(QT_QPA_PLATFORM_WINDOWS)
Expand All @@ -53,6 +56,11 @@ int main(int argc, char *argv[])

bool fInvalid = false;

// Prefer the "minimal" platform for the test instead of the normal default
// platform ("xcb", "windows", or "cocoa") so tests can't unintentially
// interfere with any background GUIs and don't require extra resources.
setenv("QT_QPA_PLATFORM", "minimal", 0);

// Don't remove this, it's needed to access
// QApplication:: and QCoreApplication:: in the tests
QApplication app(argc, argv);
Expand Down
12 changes: 12 additions & 0 deletions src/qt/test/wallettests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
}

//! Simple qt wallet tests.
//
// Test widgets can be debugged interactively calling show() on them and
// manually running the event loop, e.g.:
Copy link
Member

Choose a reason for hiding this comment

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

Can this be done automatically based on the platform selected? Or does it require manual placement of the .show()s? If not, that would at least make it so that you can avoid a recompile for debug cycles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can this be done automatically

It would probably be possible to test code that would show all widgets. So far I've found it useful just to show selected widgets when debugging.

Copy link
Member

Choose a reason for hiding this comment

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

As I understand it the issues troubleshooted here are rare enough that it's fine to have the developer insert his own .show()s at appropriate places. It's just useful to have this comment in that case.

//
// sendCoinsDialog.show();
// QEventLoop().exec();
//
// This also requires overriding the default minimal Qt platform:
//
// src/qt/test/test_bitcoin-qt -platform xcb # Linux
// src/qt/test/test_bitcoin-qt -platform windows # Windows
// src/qt/test/test_bitcoin-qt -platform cocoa # macOS
void WalletTests::walletTests()
{
// Set up wallet and chain with 101 blocks (1 mature block for spending).
Expand Down