This PR removes massive warnings like:
0QWARN : ... QFont::setPointSizeF: Point size <= 0 (...), must be greater than 0
from test_bitcoin-qt output.
On master (e258ce792a4849927a6db51786732d71cbbb65fc):
0$ ./src/qt/test/test_bitcoin-qt | grep QFont | wc -l
1~BitcoinApplication : Stopping thread
2~BitcoinApplication : Stopped thread
357
With this PR:
0$ ./src/qt/test/test_bitcoin-qt | grep QFont | wc -l
1~BitcoinApplication : Stopping thread
2~BitcoinApplication : Stopped thread
30
Not sure about this change. What if this is eventually fixed on Qt? Are these warnings worth the change?
Alternatively you could filter the warning in DebugMessageHandler or even call qInstallMessageHandler with a specific test message handler.
Either way a comment explaining the condition would be nice.
Updated 8ab99dd9544e6fa61da1cd77922c85e2970c0416 -> d318a14cc87f672198aea7490d76950919357f7d (pr17908.01 -> pr17908.02, compare). @promag
Not sure about this change. What if this is eventually fixed on Qt? Are these warnings worth the change?
Tests should not be noisy, IMO. 57 warnings messages look spammy ;)
Either way a comment explaining the condition would be nice.
A comment has been added.
496@@ -496,7 +497,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty
497 ui->detailWidget->hide();
498 ui->peerHeading->setText(tr("Select a peer to view detailed information."));
499
500- consoleFontSize = settings.value(fontSizeSettingsKey, QFontInfo(QFont()).pointSize()).toInt();
501+ consoleFontSize = settings.value(fontSizeSettingsKey, QFont().pointSize()).toInt();
Why is this change needed? And what is the observable change?
QT_QPA_PLATFORM=minimal (i.e., without window system integration), QFontInfo(QFont()).pointSize() returns -1.
QFont{}, no?
RPCConsole::setFontSize() is called. With non-qminimal plugin Qt always could draw a correct font.
Qt Documentation -> Drawing and Filling:
Qt will use the font with the specified attributes, or if no matching font exists, Qt will use the closest matching installed font.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
396@@ -396,8 +397,9 @@ void TransactionView::contextualMenu(const QPoint &point)
397 abandonAction->setEnabled(model->wallet().transactionCanBeAbandoned(hash));
398 bumpFeeAction->setEnabled(model->wallet().transactionCanBeBumped(hash));
399
400- if(index.isValid())
401- {
402+ if (index.isValid() && QGuiApplication::platformName() != "minimal") {
403+ // The qminimal plugin does not provide window system integration.
404+ // Skip this call to suppress warnings during the tests with QT_QPA_PLATFORM=minimal.
405 contextMenu->popup(transactionView->viewport()->mapToGlobal(point));
GUIUtil::popup(contextMenu, ...) and remove the new include which is already in guiutil.cpp.
Alternatively you could filter the warning in
DebugMessageHandleror even callqInstallMessageHandlerwith a specific test message handler.@hebasto have you considered this approach?
it seems DebugMessageHandler is not used in test_bitcoin-qt
qInstallMessageHandler with a specific test message handler could suppress warnings for possible new cases in the future
Updated d318a14cc87f672198aea7490d76950919357f7d -> 50c2791e35929c53c351e82a6188e2b58f5151e8 (pr17908.02 -> pr17908.03, diff):
903@@ -904,4 +904,11 @@ void LogQtInfo()
904 }
905 }
906
907+void QMenuPopup(QMenu* menu, const QPoint& point, QAction* atAction)
popupMenu, not sure why the Q prefix.
903@@ -904,4 +904,11 @@ void LogQtInfo()
904 }
905 }
906
907+void QMenuPopup(QMenu* menu, const QPoint& point, QAction* atAction)
908+{
909+ // The qminimal plugin does not provide window system integration.
910+ if (QGuiApplication::platformName() == "minimal") return;
QApplication just to make it clear what’s the required include.
QApplication inherits platformName() from QGuiApplication :tiger2:
Updated 50c2791e35929c53c351e82a6188e2b58f5151e8 -> 2a388bdf8f6685d639713c903443458afc2458e2 (pr17908.03 -> pr17908.04, diff):
Code review ACK 1122817c194ed49abf896e68604e725c3b5c8569.
Only Qt issue I found that could be related is https://bugreports.qt.io/browse/QTBUG-72335.
