[Development] QTestlib: how not to test mouseMoveEvent handling

[Jason McDonald]

On Tue, Jul 9, 2019 at 1:39 AM Shawn Rutledge <Shawn.Rutledge at qt.io> wrote:
> > On 8 Jul 2019, at 16:24, Volker Hilsheimer <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

That's good advice for now.  I think it would be worth filing a bug to
see if this part of testlib can be fixed/improved in the future.  At
the very least, it will make it clear that we're aware of the issue.

I also recall that there used to be a wiki page that listed some
best-practices for writing unit tests. If that list still exists
somewhere, this advice should be added there.

>>That there is no overload that takes modifiers and keys is also strange.

Most likely this omission is simply because nobody ever asked for such
an overload.  I'm fairly sure that that part of testlib existed before
modifiers and keys were part of the Qt API and testlib never caught up
when those were added elsewhere.

> 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.

I think it got that way a bit earlier than that patch.  The patch
appears to just move code around without modifying any functionality.

> 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.

Unfortunately, our approval process wasn't followed very well for
either of those patches.  Both patches were pushed through very
quickly outside the working hours of the testlib maintainer (me, in
GMT+10 timezone, where the patches were each pushed through in my
evening).  If I had had an opportunity to review them I would
certainly have objected to the uninformative commit message.  (Those
who were around in the Trolltech and Nokia days may remember my vocal
campaigns for meaningful commit messages, motivated by the fact that
for some years I was the poor sucker who had to read thousands of
commit messages every few months and turn them into release notes.)

> 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.

I have always been a little uncomfortable with the part of testlib
that handles mouse and keyboard events because it feels like some of
it crosses the line from unit testing into integration and system
testing, and thus really belongs in a separate system-level test
framework rather than in testlib. The system-level tests that
masqueraded as unit tests were always more likely to be flaky than
'pure' unit tests.

In the period before Qt 5, I had been hopeful that there would be an
opportunity to tidy this up and cleanly separate the true unit tests
from the others.  At that time there was a team working on a separate
system-level testing framework for Qt, but that team was in Brisbane
and evaporated when the decision was made to eject Qt from Nokia and
the new system-test framework was never completed.

Cheers,
--
Jason