macOS minimal platform is frequently broken, and these are currently failing with Qt 5.11.1.
The tests do pass when run on the full cocoa platform (with test_bitcoin-qt -platform cocoa).
Stack trace from test crash: https://gist.github.com/ryanofsky/3401fb63c52d13d5585e7fc777361f1e
macOS Qt minimal platform is frequently broken, and these are currently failing
with Qt 5.11.1.
The tests do pass when run on the full cocoa platform
(with `test_bitcoin-qt -platform cocoa`).
146+ // framework when it tries to look up unimplmented cocoa functions, and
147+ // fails to handle returned nulls
148+ // (https://bugreports.qt.io/browse/QTBUG-49686).
149+ QWARN("Skipping AddressBookTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke "
150+ "with 'test_bitcoin-qt -platform cocoa' on mac, or else use a linux or windows build.");
151+ return;
16@@ -17,6 +17,7 @@
17 #include <key_io.h>
18 #include <wallet/wallet.h>
19
20+#include <QApplication>
QGuiApplication?
I did make check and src/qt/test/test_bitcoin-qt -platform cocoa, but it’s still running WalletTests and AddressBookTests. Maybe I need a different -platform argument?
The test pass for me on macOS 10.13.6 with QT 5.11.1 via Homebrew, with and without this change.
Updated 391cca51582269015f23bf931461160a111cc420 -> f7a533f17992e73ced6b57a9725997e040eeaa8a (pr/fuqtmac.2 -> pr/fuqtmac.3) just fixing whitespace.
From comments above I guess fanquake could reproduce the test failures, and sjors couldn’t, but I think it’s best just to skip these tests on the uncommonly used minimal mac platform, rather than try to debug.
tACK f7a533f
make check is now fixed for me.
139@@ -139,5 +140,16 @@ void TestAddAddressesToSendBook()
140
141 void AddressBookTests::addressBookTests()
142 {
143+#ifdef Q_OS_MAC
144+ if (QApplication::platformName() == "minimal") {
145+ // Disable for mac on "minimal" platform to avoid crashes inside the Qt
146+ // framework when it tries to look up unimplmented cocoa functions, and
re-tACK a3197c5
Merged on top of https://github.com/bitcoin/bitcoin/commit/816fab9ccae568612d5ed90378b4587256925a1e and checked make check -j6.
testedAck https://github.com/bitcoin/bitcoin/pull/14011/commits/a3197c5294df4711bab0ff6dcdd061ceab115c7d QWarns correctly outputted to logs
0********* Start testing of WalletTests *********
1Config: Using QtTest library 5.11.2, Qt 5.11.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 9.1.0 (clang-902.0.39.2) (Apple))
2PASS : WalletTests::initTestCase()
3WARNING: WalletTests::walletTests() Skipping WalletTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke with 'test_bitcoin-qt -platform cocoa' on mac, or else use a linux or windows build.
4 Loc: [qt/test/wallettests.cpp(253)]
5PASS : WalletTests::walletTests()
6PASS : WalletTests::cleanupTestCase()
7Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 0ms
8********* Finished testing of WalletTests *********
9********* Start testing of AddressBookTests *********
10Config: Using QtTest library 5.11.2, Qt 5.11.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 9.1.0 (clang-902.0.39.2) (Apple))
11PASS : AddressBookTests::initTestCase()
12WARNING: AddressBookTests::addressBookTests() Skipping AddressBookTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke with 'test_bitcoin-qt -platform cocoa' on mac, or else use a linux or windows build.
13 Loc: [qt/test/addressbooktests.cpp(150)]
14PASS : AddressBookTests::addressBookTests()
15PASS : AddressBookTests::cleanupTestCase()
16Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 0ms
17********* Finished testing of AddressBookTests *********
18PASS qt/test/test_bitcoin-qt (exit status: 0)
Can’t reproduce the error on macOS 10.14.
0src/qt/test/test_bitcoin-qt -platform minimal
1********* Start testing of URITests *********
2Config: Using QtTest library 5.11.1, Qt 5.11.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 9.1.0 (clang-902.0.39.2) (Apple))
3...
I think this is ready for merge. It is platform specific and has tested acks from fanquake #14011 (comment) #14011 (comment) and IlyasRidhuan #14011 (comment) and an untested ACK from jonasschnelli #14011 (comment).
Sjors and promag have reported in #14011 (comment) and #14011 (comment) that they actually see tests passing without this change, so the issue does seem be a little unpredictable. But this is a good, safe fix for #14349, and also a prerequisite for #11625, which expands qt test coverage and exposes more problems with the qt mac minimal platform.
utACK https://github.com/bitcoin/bitcoin/pull/14011/commits/a3197c5294df4711bab0ff6dcdd061ceab115c7d
Code changes are confined to tests and look good to me.