Run bitcoin_test-qt under minimal QPA platform #10142
Conversation
Fixes broken "make check" reported by Matt Corallo <[email protected]> in bitcoin#10110 Fix was suggested and initially implemented by Cory Fields <[email protected]> in bitcoin#10117 (comment)
|
Concept ACK |
|
Concept ACK |
|
@TheBlueMatt can you check if this solves your issue #10110? |
theuni
left a comment
theuni
left a comment
There was a problem hiding this comment.
Concept ACK and ACK after fixing up the configure check.
| //! Simple qt wallet tests. | ||
| // | ||
| // Test widgets can be debugged interactively calling show() on them and | ||
| // manually running the event loop, e.g.: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| 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]) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
|
@laanwj I can confirm that it does, I'm seeing the issue locally as well. |
|
Tested ACK on debian 8 with Qt5.3 headless: |
|
Thanks for testing. Then I'm going to merge this to prevent test failures for affected devs; the build system integration can be improved later. |
bf10264 Run bitcoin_test-qt under minimal QPA platform (Russell Yanofsky) Tree-SHA512: 35782f0d7e4dcdc27d991d5a10fcffbd2d201139293fe7917ef6f7cd7ae4d3a162ebc21f83266d821ae3bad86f62d947b047bb317f6c5899df4d6bcb4c957157
Should no longer be needed after bitcoin#10142: bf10264 "Run bitcoin_test-qt under minimal QPA platform"
bf10264 Run bitcoin_test-qt under minimal QPA platform (Russell Yanofsky) Tree-SHA512: 35782f0d7e4dcdc27d991d5a10fcffbd2d201139293fe7917ef6f7cd7ae4d3a162ebc21f83266d821ae3bad86f62d947b047bb317f6c5899df4d6bcb4c957157
6 participants
Fixes broken "make check" reported by @TheBlueMatt in #10110
Fix was suggested and initially implemented by @theuni in #10117 (comment)