[Development] QTestlib: how not to test mouseMoveEvent handling

[Volker Hilsheimer]

On 8 Jul 2019, at 17:38, Shawn Rutledge <Shawn.Rutledge at qt.io<mailto:Shawn.Rutledge at qt.io>> wrote:


On 8 Jul 2019, at 16:24, Volker Hilsheimer <volker.hilsheimer at qt.io<mailto:volker.hilsheimer at qt.io>> wrote:

Hi,

Executive summary:

* QTest::mouseMove seems to be broken
* when simulating QEvent::MouseMove events by constructing event objects, always construct them with global position


The details:

While trying to fix https://bugreports.qt.io/browse/QTBUG-76765 to not update the mouse cursor when the cursor-fied QGraphicsItem the mouse is on top of is disabled, I noticed the following pattern in the respective test:

QTest::mouseMove(localPoint, widget);
QMouseEvent event(QEvent::MouseMove, localPoint, Qt::NoButton, 0, 0);
QApplication::sendEvent(widget, &event);


which confused me a bit. Shouldn’t QTest::mouseMove already have sent the event? Apparently not. That there is no overload that takes modifiers and keys is also strange.

In the end, when running the test locally on my Mac, I never got it ot pass.

What seems to happen is this:

* for QWidget receivers, QTest::mouseMove just calls QCursor::setPos

QCursor::setPos is not guaranteed to generate mouse events. The documentation of some overloads of that function [1] in fact explicitly advises against using that function in unit tests.

Yep.  Cursor support can be disabled, I think that’s one reason at least.  Another is that some platforms (i.e. Wayland) don’t allow applications to set cursor position.  Anyway it’s heavy-handed and slow.  But tests that need to simulate mouse hover or mouse enter/exit by sending events do need to ensure that the cursor is _not_ on top of the window, to avoid getting spurious events… and that’s usually done by QCursor::setPos(); so we can’t seem to get rid of it after all.

https://codereview.qt-project.org/c/qt/qtbase/+/5290 seems to have put it into its current state… it’s old.  But before that was https://codereview.qt-project.org/c/qt/qtbase/+/4462 which comments out QCursor::setPos() and sends a platform event instead… at least in one case.  Too bad the commit message is so sparse.

I suppose there’s the usual tradeoff between the philosophy that real platform testing should involve real platform events (in this case: if you really move the system mouse cursor, then the window system will hopefully send a mouse move event to the window, and aren’t you making the test more realistic that way?) vs. the practical tendency for that kind of testing to be less reliable, with unpredictable latency, needing to use QTRY_ much more because you don’t know how long to wait until the window system reacts, etc.



Thanks for digging in the history. This explains why targeting a QWindow with mouseMove synthesises an event; or at least it shows when it started doing that...

If we need to have better tests that confirm that platform events are correctly received by Qt, translated into QEvents, and then routed to the correct receiver, then let’s have that.

But we need to be able to stand on top of this guarantee when we test the internal logic of widgets. I need to be able to rely on basic event handling being done correctly; or in fact, I shouldn’t care when I test my unit. QTestlib is primarily a unit testing framework after all, not an integration or UI testing framework. The assumption that I can only test QGraphicsView’s hit testing logic by sending an event thought the operating system and the event system has nothing philosophical about it :) We use an object oriented language to build Qt, with hopefully well encapsulated internal states that don’t magically change randomly.

If we need make sure that Qt doesn't produce any events that we don’t want, wouldn’t it be easier to disable/filter out those events (ie all input events with QEvent::spontaneous) programmatically, than to rely on QCursor::setPos?


Volker


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190708/a67fcdc1/attachment.html>