From ecb84788191a06b503028aa89820a3b6ae0e45da Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Tue, 31 Oct 2017 17:07:07 +0100 Subject: tests: port tst_QGraphicsProxyWidget to QTEST_QPA_MOUSE_HANDLING ... and various other cleanups. This patch re-factors tests that relied on QCursor API (directly or indirectly via QTest::mouseMove widget overload) to use the QTEST_QPA_MOUSE_HANDLING code path. Misuse of QCursor API causes tests to be flaky. Removed Qt::X11BypassWindowManagerHint as it is not really needed. Based on the comment where this flag is used, it would be needed for all test functions that use mouse events. That assumption is not valid. A window is expected to have received its final position when QTest::qWaitForWindowActive() returns. There were issues with this on Unity, but those have been fixed. Un-QSKIP-ed QTBUG_6986_sendMouseEventToAlienWidget for Q_OS_WIN. Un-QSKIP-ed hoverEnterLeaveEvent for all platforms. Un-QSKIP-ed bypassGraphicsProxyWidget for Q_OS_MAC and Q_OS_WIN as the test passes on those platforms. According to QTBUG-33067 it used to crash with 5.2.0. Removed unnecessary mouseMove() and stray mouseRelease() in mouseDoubleClickEvent(). Removed unnecessary show()/setVisible() on items that are being added to the scene - items are visible by default. Among other randomly spotted issues, one worth mentioning is that when adding items to the scene after the view is already shown, we need to ensure that the scene is updated (e.g QSignalSpy + QGraphicsScene::changed) before interacting with it. As an example, mousePressReleaseEvent() only passed because of random luck that the invalid coordinates were still within the bounding rect of the button. This patch does not attempt to cleanup all instances of this anti-pattern. Task-number: QTBUG-52546 Task-number: QTBUG-26948 Task-number: QTBUG-33067 Change-Id: I2ccbc004c1cb4f5b31c70c8568ee591c458d8446 Reviewed-by: Kari Oikarinen --- .../qgraphicsproxywidget/qgraphicsproxywidget.pro | 1 + .../tst_qgraphicsproxywidget.cpp | 128 +++++++++------------ 2 files changed, 55 insertions(+), 74 deletions(-) diff --git a/tests/auto/widgets/graphicsview/qgraphicsproxywidget/qgraphicsproxywidget.pro b/tests/auto/widgets/graphicsview/qgraphicsproxywidget/qgraphicsproxywidget.pro index e7bcccb495..c10cbe1b1a 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsproxywidget/qgraphicsproxywidget.pro +++ b/tests/auto/widgets/graphicsview/qgraphicsproxywidget/qgraphicsproxywidget.pro @@ -4,5 +4,6 @@ TARGET = tst_qgraphicsproxywidget QT += widgets widgets-private testlib QT += core-private gui-private +DEFINES += QTEST_QPA_MOUSE_HANDLING SOURCES += tst_qgraphicsproxywidget.cpp diff --git a/tests/auto/widgets/graphicsview/qgraphicsproxywidget/tst_qgraphicsproxywidget.cpp b/tests/auto/widgets/graphicsview/qgraphicsproxywidget/tst_qgraphicsproxywidget.cpp index 4cd2fef2dc..49afc5f369 100644 --- a/tests/auto/widgets/graphicsview/qgraphicsproxywidget/tst_qgraphicsproxywidget.cpp +++ b/tests/auto/widgets/graphicsview/qgraphicsproxywidget/tst_qgraphicsproxywidget.cpp @@ -33,12 +33,6 @@ #include #include // qSmartMin functions... -static void sendMouseMove(QWidget *widget, const QPoint &point, Qt::MouseButton button = Qt::NoButton) -{ - QMouseEvent event(QEvent::MouseMove, point, widget->mapToGlobal(point), button, button, 0); - QApplication::sendEvent(widget, &event); -} - /* Notes: @@ -95,10 +89,8 @@ private slots: void focusNextPrevChild(); void focusOutEvent_data(); void focusOutEvent(); -#ifndef QT_NO_CURSOR void hoverEnterLeaveEvent_data(); void hoverEnterLeaveEvent(); -#endif void hoverMoveEvent_data(); void hoverMoveEvent(); void keyPressEvent_data(); @@ -138,9 +130,7 @@ private slots: void setFocus_complexTwoWidgets(); void popup_basic(); void popup_subwidget(); -#ifndef QT_NO_CURSOR void changingCursor_basic(); -#endif void tooltip_basic(); void childPos_data(); void childPos(); @@ -685,7 +675,6 @@ void tst_QGraphicsProxyWidget::focusInEvent() SubQGraphicsProxyWidget *proxy = new SubQGraphicsProxyWidget; proxy->setEnabled(true); scene.addItem(proxy); - proxy->setVisible(true); QWidget *widget = new QWidget; widget->resize(100, 100); @@ -716,7 +705,6 @@ void tst_QGraphicsProxyWidget::focusInEventNoWidget() SubQGraphicsProxyWidget *proxy = new SubQGraphicsProxyWidget; proxy->setEnabled(true); scene.addItem(proxy); - proxy->setVisible(true); view.show(); proxy->setFlag(QGraphicsItem::ItemIsFocusable, true); // <- shouldn't need to do this @@ -931,7 +919,6 @@ protected: } }; -#ifndef QT_NO_CURSOR void tst_QGraphicsProxyWidget::hoverEnterLeaveEvent_data() { QTest::addColumn("hasWidget"); @@ -952,8 +939,6 @@ void tst_QGraphicsProxyWidget::hoverEnterLeaveEvent() QGraphicsScene scene; QGraphicsView view(&scene); - //do not let the window manager move the window while we are moving the mouse on it - view.setWindowFlags(Qt::X11BypassWindowManagerHint); view.show(); QApplication::setActiveWindow(&view); QVERIFY(QTest::qWaitForWindowActive(&view)); @@ -968,31 +953,30 @@ void tst_QGraphicsProxyWidget::hoverEnterLeaveEvent() if (hasWidget) proxy->setWidget(widgetGuard.take()); proxy->setPos(50, 0); + QSignalSpy sceneChangedSpy(&scene, &QGraphicsScene::changed); scene.addItem(proxy); - QTest::qWait(30); + QTRY_VERIFY(sceneChangedSpy.count() > 0); + + // outside graphics item QTest::mouseMove(&view, QPoint(10, 10)); - QTest::qWait(30); - // in + QCOMPARE(widget->testAttribute(Qt::WA_UnderMouse), false); + QCOMPARE(widget->enterCount, 0); + QCOMPARE(widget->hoverEnter, 0); + // over graphics item QTest::mouseMove(&view, QPoint(50, 50)); - QSKIP("QTBUG-25294"); QTRY_COMPARE(widget->testAttribute(Qt::WA_UnderMouse), hasWidget); - // ### this attribute isn't supported QCOMPARE(widget->enterCount, hasWidget ? 1 : 0); QCOMPARE(widget->hoverEnter, (hasWidget && hoverEnabled) ? 1 : 0); - // does not work on all platforms - //QCOMPARE(widget->moveCount, 0); - // out + QTRY_COMPARE(widget->leaveCount, 0); + QTRY_COMPARE(widget->hoverLeave, 0); + // outside graphics item QTest::mouseMove(&view, QPoint(10, 10)); - // QTRY_COMPARE(widget->testAttribute(Qt::WA_UnderMouse), false); - // ### this attribute isn't supported + QTRY_COMPARE(widget->testAttribute(Qt::WA_UnderMouse), false); QTRY_COMPARE(widget->leaveCount, hasWidget ? 1 : 0); QTRY_COMPARE(widget->hoverLeave, (hasWidget && hoverEnabled) ? 1 : 0); - // does not work on all platforms - //QCOMPARE(widget->moveCount, 0); } -#endif void tst_QGraphicsProxyWidget::hoverMoveEvent_data() { @@ -1174,19 +1158,20 @@ void tst_QGraphicsProxyWidget::mouseDoubleClickEvent() widget->setText("foo"); widget->resize(50, 50); view.resize(100, 100); - if (hasWidget) { + if (hasWidget) proxy->setWidget(widget); - proxy->show(); - } proxy->setPos(50, 0); + QSignalSpy sceneChangedSpy(&scene, &QGraphicsScene::changed); scene.addItem(proxy); proxy->setFocus(); - QTest::mouseMove(view.viewport(), view.mapFromScene(proxy->mapToScene(15, proxy->boundingRect().center().y()))); - QTest::mousePress(view.viewport(), Qt::LeftButton, 0, view.mapFromScene(proxy->mapToScene(15, proxy->boundingRect().center().y()))); - QTest::mouseRelease(view.viewport(), Qt::LeftButton, 0, view.mapFromScene(proxy->mapToScene(15, proxy->boundingRect().center().y()))); - QTest::mouseDClick(view.viewport(), Qt::LeftButton, 0, view.mapFromScene(proxy->mapToScene(15, proxy->boundingRect().center().y()))); - QTest::mouseRelease(view.viewport(), Qt::LeftButton, 0, view.mapFromScene(proxy->mapToScene(15, proxy->boundingRect().center().y()))); + // wait for scene to be updated before doing any coordinate mappings on it + QTRY_VERIFY(sceneChangedSpy.count() > 0); + + QPoint pointInLineEdit = view.mapFromScene(proxy->mapToScene(15, proxy->boundingRect().center().y())); + QTest::mousePress(view.viewport(), Qt::LeftButton, 0, pointInLineEdit); + QTest::mouseRelease(view.viewport(), Qt::LeftButton, 0, pointInLineEdit); + QTest::mouseDClick(view.viewport(), Qt::LeftButton, 0, pointInLineEdit); QTRY_COMPARE(widget->selectedText(), hasWidget ? QString("foo") : QString()); @@ -1217,19 +1202,20 @@ void tst_QGraphicsProxyWidget::mousePressReleaseEvent() QPushButton *widget = new QPushButton; QSignalSpy spy(widget, SIGNAL(clicked())); widget->resize(50, 50); - if (hasWidget) { + if (hasWidget) proxy->setWidget(widget); - proxy->show(); - } proxy->setPos(50, 0); + QSignalSpy sceneChangedSpy(&scene, &QGraphicsScene::changed); scene.addItem(proxy); proxy->setFocus(); - QTest::mousePress(view.viewport(), Qt::LeftButton, 0, - view.mapFromScene(proxy->mapToScene(proxy->boundingRect().center()))); + // wait for scene to be updated before doing any coordinate mappings on it + QTRY_VERIFY(sceneChangedSpy.count() > 0); + + QPoint buttonCenter = view.mapFromScene(proxy->mapToScene(proxy->boundingRect().center())); + QTest::mousePress(view.viewport(), Qt::LeftButton, 0, buttonCenter); QTRY_COMPARE(spy.count(), 0); - QTest::mouseRelease(view.viewport(), Qt::LeftButton, 0, - view.mapFromScene(proxy->mapToScene(proxy->boundingRect().center()))); + QTest::mouseRelease(view.viewport(), Qt::LeftButton, 0, buttonCenter); QTRY_COMPARE(spy.count(), (hasWidget) ? 1 : 0); if (!hasWidget) @@ -1282,16 +1268,13 @@ void tst_QGraphicsProxyWidget::paintEvent() w->show(); QVERIFY(QTest::qWaitForWindowExposed(w)); - QApplication::processEvents(); - QTest::qWait(30); proxy.setWidget(w); + QSignalSpy sceneChangedSpy(&scene, &QGraphicsScene::changed); scene.addItem(&proxy); - //make sure we flush all the paint events - QTRY_VERIFY(proxy.paintCount > 1); - QTest::qWait(30); - proxy.paintCount = 0; + QTRY_VERIFY(sceneChangedSpy.count() > 0); // make sure the scene is ready + proxy.paintCount = 0; w->update(); QTRY_VERIFY(proxy.paintCount >= 1); //the widget should have been painted now } @@ -2545,35 +2528,34 @@ void tst_QGraphicsProxyWidget::popup_subwidget() QCOMPARE(popup->size(), child->size().toSize()); } -#ifndef QT_NO_CURSOR void tst_QGraphicsProxyWidget::changingCursor_basic() { +#if !QT_CONFIG(cursor) + QSKIP("This test requires the QCursor API"); +#else // Confirm that mouse events are working properly by checking that // when moving the mouse over a line edit it will change the cursor into the I QGraphicsScene scene; QGraphicsView view(&scene); view.show(); + QVERIFY(QTest::qWaitForWindowActive(&view)); SubQGraphicsProxyWidget *proxy = new SubQGraphicsProxyWidget; QLineEdit *widget = new QLineEdit; proxy->setWidget(widget); - proxy->show(); + QSignalSpy sceneChangedSpy(&scene, &QGraphicsScene::changed); scene.addItem(proxy); - QApplication::setActiveWindow(&view); - QVERIFY(QTest::qWaitForWindowActive(&view)); - QVERIFY(view.isActiveWindow()); + QTRY_VERIFY(sceneChangedSpy.count() > 0); // make sure the scene is ready // in QTest::mouseMove(view.viewport(), view.mapFromScene(proxy->mapToScene(proxy->boundingRect().center()))); - sendMouseMove(view.viewport(), view.mapFromScene(proxy->mapToScene(proxy->boundingRect().center()))); QTRY_COMPARE(view.viewport()->cursor().shape(), Qt::IBeamCursor); // out QTest::mouseMove(view.viewport(), QPoint(1, 1)); - sendMouseMove(view.viewport(), QPoint(1, 1)); QTRY_COMPARE(view.viewport()->cursor().shape(), Qt::ArrowCursor); +#endif // !QT_CONFIG(cursor) } -#endif static bool findViewAndTipLabel(const QWidget *view) { @@ -2972,7 +2954,7 @@ void tst_QGraphicsProxyWidget::dontCrashWhenDie() MainWidget *w = new MainWidget(); w->show(); QVERIFY(QTest::qWaitForWindowExposed(w)); - QTest::qWait(100); + QTest::mouseMove(w->view->viewport(), w->view->mapFromScene(w->widget->mapToScene(w->widget->boundingRect().center()))); delete w->item; @@ -3219,9 +3201,6 @@ void tst_QGraphicsProxyWidget::bypassGraphicsProxyWidget_data() void tst_QGraphicsProxyWidget::bypassGraphicsProxyWidget() { -#if defined(Q_OS_MAC) || defined(Q_OS_WIN) - QSKIP("Test case unstable on this platform, QTBUG-33067"); -#endif QFETCH(bool, bypass); QWidget *widget = new QWidget; @@ -3230,6 +3209,7 @@ void tst_QGraphicsProxyWidget::bypassGraphicsProxyWidget() QGraphicsScene scene; QGraphicsView view(&scene); view.show(); + QVERIFY(QTest::qWaitForWindowActive(&view)); QGraphicsProxyWidget *proxy = scene.addWidget(widget); @@ -3243,6 +3223,7 @@ void tst_QGraphicsProxyWidget::bypassGraphicsProxyWidget() QFileDialog *dialog = new QFileDialog(widget, flags); dialog->setOption(QFileDialog::DontUseNativeDialog, true); dialog->show(); + QVERIFY(QTest::qWaitForWindowActive(dialog)); QCOMPARE(proxy->childItems().size(), bypass ? 0 : 1); if (!bypass) @@ -3637,25 +3618,21 @@ public: QPushButton *hideButton = new QPushButton("I'm a button with a very very long text"); hideButton->setGeometry(10, 10, 400, 50); topButton = addWidget(hideButton); - connect(hideButton, SIGNAL(clicked()), this, SLOT(hideButton())); + connect(hideButton, &QPushButton::clicked, this, [&]() { topButton->hide(); }); topButton->setFocus(); } QGraphicsProxyWidget *topButton; HoverButton *hoverButton; - -public slots: - void hideButton() { - QCursor::setPos(600,600); - topButton->hide(); - } }; void tst_QGraphicsProxyWidget::QTBUG_6986_sendMouseEventToAlienWidget() { -#if defined(Q_OS_DARWIN) || defined(Q_OS_WIN) || defined(QT_NO_CURSOR) - QSKIP("Test case unstable on this platform"); -#endif + if (QGuiApplication::platformName() == QLatin1String("cocoa")) { + // The "Second button" does not receive QEvent::HoverLeave + QSKIP("This test fails only on Cocoa. Investigate why. See QTBUG-69219"); + } + QGraphicsView view; Scene scene; view.setScene(&scene); @@ -3663,9 +3640,12 @@ void tst_QGraphicsProxyWidget::QTBUG_6986_sendMouseEventToAlienWidget() QApplication::setActiveWindow(&view); view.show(); QVERIFY(QTest::qWaitForWindowActive(&view)); - QCOMPARE(QApplication::activeWindow(), &view); - QCursor::setPos(view.mapToGlobal(view.mapFromScene(scene.topButton->boundingRect().center()))); - QTest::mouseClick(view.viewport(), Qt::LeftButton, 0, view.mapFromScene(scene.topButton->scenePos())); + + QPoint topButtonTopLeftCorner = view.mapFromScene(scene.topButton->scenePos()); + QTest::mouseClick(view.viewport(), Qt::LeftButton, 0, topButtonTopLeftCorner); + // move to the bottom right corner (buttons are placed in the top left corner) + QCOMPARE(scene.hoverButton->hoverLeaveReceived, false); + QTest::mouseMove(view.viewport(), view.viewport()->rect().bottomRight()); QTRY_COMPARE(scene.hoverButton->hoverLeaveReceived, true); } -- cgit v1.2.3