From ef9238c99c25fc7e2d65b58eca2f2cb7ccf1c44d Mon Sep 17 00:00:00 2001 From: Oliver Eftevaag Date: Tue, 11 Jun 2024 00:38:43 +0200 Subject: [PATCH] Stabilize tst_QQuickPopup::popupWindowChangingParent I had accidentally added a `visible: true` binding on the Popup, in the QML document used by the test. Which caused the main window to not always be initialized, before the popup window. The main window's geometry is used by the QQuickItem::mapToGlobal() function, which is called when comparing the global position of the popup window's parent item, to verify that the popup window is positioned relative to it. Removing the `visible: true` binding, seems to fix the issue. But further improvements to the test are also added to make it more robust. The following improvements are being made to the macros VERIFY_GLOBAL_POS and VERIFY_LOCAL_POS, for the sake of making future debugging easier: - A better failure message. - They no longer need to store any local variables on the stack. - Redundant compare statements are removed. Since the the test is fixed, it's now safe to unblacklist. Fixes: QTBUG-126175 Pick-to: 6.8 Change-Id: I1032494818bf85bf8aaf3d2d60b56c0d991855b4 Reviewed-by: Richard Moe Gustavsen --- .../auto/quickcontrols/qquickpopup/BLACKLIST | 3 -- .../qquickpopup/data/reparentingPopup.qml | 1 - .../qquickpopup/tst_qquickpopup.cpp | 39 +++++++++---------- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/tests/auto/quickcontrols/qquickpopup/BLACKLIST b/tests/auto/quickcontrols/qquickpopup/BLACKLIST index 67d99026aa..3beb1d6cc7 100644 --- a/tests/auto/quickcontrols/qquickpopup/BLACKLIST +++ b/tests/auto/quickcontrols/qquickpopup/BLACKLIST @@ -27,6 +27,3 @@ opensuse-leap [popupWindowFocus] * # QTBUG-121363 -# QTBUG-126175 -[popupWindowChangingParent] -linux diff --git a/tests/auto/quickcontrols/qquickpopup/data/reparentingPopup.qml b/tests/auto/quickcontrols/qquickpopup/data/reparentingPopup.qml index e747704e4b..bf622b7713 100644 --- a/tests/auto/quickcontrols/qquickpopup/data/reparentingPopup.qml +++ b/tests/auto/quickcontrols/qquickpopup/data/reparentingPopup.qml @@ -15,7 +15,6 @@ Window { Popup { id: simplepopup - visible: true popupType: Popup.Window x: 10 y: 10 diff --git a/tests/auto/quickcontrols/qquickpopup/tst_qquickpopup.cpp b/tests/auto/quickcontrols/qquickpopup/tst_qquickpopup.cpp index 6777fac721..650d168c65 100644 --- a/tests/auto/quickcontrols/qquickpopup/tst_qquickpopup.cpp +++ b/tests/auto/quickcontrols/qquickpopup/tst_qquickpopup.cpp @@ -2385,19 +2385,16 @@ void tst_QQuickPopup::noDimmer() QTRY_VERIFY(!drawer->isModal()); } -#define VERIFY_LOCAL_POS(POPUP, EXPECTED) \ - QTRY_COMPARE_LE(qAbs(POPUP->x() - qreal(EXPECTED.x())), 1); \ - QCOMPARE_LE(qAbs(POPUP->position().x() - qreal(EXPECTED.x())), 1); \ - QCOMPARE_LE(qAbs(POPUP->y() - qreal(EXPECTED.y())), 1); \ - QCOMPARE_LE(qAbs(POPUP->position().y() - qreal(EXPECTED.y())), 1) +#define VERIFY_LOCAL_POS(POPUP, EXPECTED) \ + QTRY_VERIFY2(qAbs(POPUP->x() - qreal(EXPECTED.x())) <= 1, \ + qPrintable(QStringLiteral("QQuickPopup::x() = %1, expected = %2").arg(POPUP->x()).arg(EXPECTED.x())));\ + QVERIFY2(qAbs(POPUP->y() - qreal(EXPECTED.y())) <= 1, \ + qPrintable(QStringLiteral("QQuickPopup::y() = %1, expected = %2").arg(POPUP->y()).arg(EXPECTED.y()))) -#define VERIFY_GLOBAL_POS(FROM, POPUPWINDOW, EXPECTED) \ - do { \ - const auto expectedGlobalPos = FROM->mapToGlobal(EXPECTED.x(), EXPECTED.y()); \ - const auto actualGlobalPos = POPUPWINDOW->position(); \ - QTRY_COMPARE_LE(qAbs(actualGlobalPos.x() - qFloor(expectedGlobalPos.x())), 1); \ - QCOMPARE_LE(qAbs(actualGlobalPos.y() - qFloor(expectedGlobalPos.y())), 1); \ - } while (false) +#define VERIFY_GLOBAL_POS(FROM, POPUPWINDOW, EXPECTED) \ + QTRY_VERIFY2((POPUPWINDOW->position() - FROM->mapToGlobal(EXPECTED.x(), EXPECTED.y())).manhattanLength() <= 2, \ + qPrintable(QStringLiteral("PopupWindow pos = (%1, %2), expected (%3, %4)") \ + .arg(POPUPWINDOW->x()).arg(POPUPWINDOW->y()).arg(EXPECTED.x()).arg(EXPECTED.y()))) void tst_QQuickPopup::popupWindowPositioning() { @@ -2645,16 +2642,16 @@ void tst_QQuickPopup::popupWindowChangingParent() QQuickApplicationHelper helper(this, "reparentingPopup.qml"); QVERIFY2(helper.ready, helper.failureMessage()); - QQuickWindow *window = helper.window; - window->show(); - QVERIFY(QTest::qWaitForWindowExposed(window)); - auto *popup = window->contentItem()->findChild(); QVERIFY(popup); auto *popupPrivate = QQuickPopupPrivate::get(popup); QVERIFY(popupPrivate); + window->show(); + QVERIFY(QTest::qWaitForWindowExposed(window)); + QVERIFY(!popup->isVisible()); + QQuickItem *item1 = window->property("rectangle1").value(); QVERIFY(item1); @@ -2665,11 +2662,13 @@ void tst_QQuickPopup::popupWindowChangingParent() QVERIFY(item3); popup->open(); - QTRY_VERIFY(popup->isVisible()); + QTRY_VERIFY(popup->isOpened()); - auto *popupWindow = popupPrivate->popupWindow; - QVERIFY(popupWindow); - QVERIFY(popupWindow->isVisible()); + QTRY_VERIFY(popupPrivate->popupWindow); + QWindow *popupWindow = popupPrivate->popupWindow; + + QTRY_VERIFY(popupWindow->isVisible()); + QVERIFY(QTest::qWaitForWindowExposed(popupWindow)); const QPoint initialPos(10, 10);