fix(mousepress): replace QTest mouse actions

[adam-grant-hendry]

QTest mouse actions mouseClick and mousePress do not release
the mouse, causing qtbot to fail tests. Replace with QtGui mouse
events.

Fixes Issue #428

[nicoddemus]

Thanks @adam-grant-hendry!

Please take a look at my comments. In addition:

1. We should update the documentation to clarify that the new/changed methods are custom implementations (and no longer just forwarding to the QTest API), also pointing out to the relevant discussions if appropriate.
2. We need tests for the custom methods: we might not have tests for all of them before because they just forwarded the calls to QTest (which we assumed "worked"), but now we must sure they are tested and covered, to ensure we don't regress.
3. Add a new CHANGELOG entry detailing the new methods and the rationale for not using the QTest API anymore.

Also I would like to get @The-Compiler's take on this.

[nicoddemus]

This doesn't seem right: if I issue a mouseClick, I don't expect a mouseMove to be issued.

[adam-grant-hendry]

That's fair: mousePress and mouseRelease each take pos as an argument, so they should click the right spot without moving the cursor.

[nicoddemus]

This is new so we should have a docstring and a test.

[nicoddemus]

This if is specific to pyqtgraph and should be removed.

Suggested change

	if isinstance(widget, qt_api.QtWidgets.QGraphicsView):
	widget = widget.viewport()

[nicoddemus]

Same here.

Suggested change

	if isinstance(widget, qt_api.QtWidgets.QGraphicsView):
	widget = widget.viewport()

[nicoddemus]

Just to make sure: these changes to the tests are not required, are they?

[nicoddemus]

Same.

Suggested change

	if isinstance(widget, qt_api.QtWidgets.QGraphicsView):
	widget = widget.viewport()

[The-Compiler]

I'm -1 on this in its entirety. I don't think it's our job to reimplement the QTest methods where we can avoid it, and there is a big chance we're introducing subtle issues and incompatibilities here with very questionable benefit.

I don't quite follow on #428 (comment) either. Yes, they are in QTest, why is that a problem? What exactly wasn't bound in C++? What makes you think that mouseClick wouldn't work?

Above, you also say "mousePress do not release the mouse" - well yes, that's why it's named mousePress?

Finally, you say:

as it is unlikely they will continue to work properly and/or be provided in python bindings of Qt in the future

...again, where are you getting that from?

Sorry, but I'm really confused at this point at what the goal of those changes are. As-is, I believe they will introduce more issues than they would fix.

[nicoddemus]

Thanks @The-Compiler, I did the overall code review but I wasn't certain about the changes too.

[adam-grant-hendry]

I'm -1 on this in its entirety. I don't think it's our job to reimplement the QTest methods where we can avoid it, and there is a big chance we're introducing subtle issues and incompatibilities here with very questionable benefit.

Currently, you cannot avoid it because their are QTest methods that do not work. At a minimum, your documentation should list those features that do not work properly due to Qt bugs, namely:

1. mouseClick not working in PyQt5, but fixed in PyQt6: QTBUG-5232
2. Clicking away from activated menu does not close it: QTBUG-104075

I don't quite follow on #428 (comment) either. Yes, they are in QTest, why is that a problem? What exactly wasn't bound in C++?

What makes you think that mouseClick wouldn't work?

The PySide2 documentation states:

All macros in the C++ version of QtTest were not binded in PySide, this module is useful only for GUI testing and benchmarking, for ordinary unit testing you should use the unittest Python module.

The PySide6 documentation states:

Not all macros in the C++ version of QtTest were exposed in PySide. This module is useful only for GUI testing and benchmarking; for ordinary unit testing you should use the pytest Python module.

The API documentation itself only lists mouse and key actions for the C++ Qt libraries, not the python PyQt/PySide libraries. This leads me to believe (based on the notes) that not all those features work in the python bindings.

Consequently,

1. mouseClick does not work on PyQt5/PySide5. See QTBUG-5232
2. Clicking away from activated menu does not close it: QTBUG-104075

Your documentation does not state what features are known not to work in pytest-qt due to Qt bugs. This makes it hard for developers to decipher if the issue is with their code, pytest-qt, or Qt itself.

Above, you also say "mousePress do not release the mouse" - well yes, that's why it's named mousePress?

That should read mouseClick.

...again, where are you getting that from?

The PySide2 /PySide6 documentation notes not all bindings were transferred and it does not list mouse and key actions. Perhaps this is a documentation error on the side of Qt?

However, at a minimum, you should add to your documentation those features that will not work (are known not to work) due to Qt bugs:

1. mouseClick does not work on PyQt5/PySide5. See QTBUG-5232
2. Clicking away from activated menu does not close it: QTBUG-104075

Developers will appreciate you for this and it will spare you bug reports that are to do with Qt and not your library.

[adam-grant-hendry]

I'm -1 on this in its entirety.

It would be nice to add an example to the documentation of overriding certain qtbot methods (see SilentQtBot in https://stackoverflow.com/questions/68696865/pytest-qt-function-mousemove-not-working).

For instance,

1. Here I add mouseDrag, which isn't available in your package
2. These mouse actions seem to behave better in QGraphicsView. QGraphicsView is a QWidget, but the viewport QWidget should be passed to receive mouse events.

[The-Compiler]

I'm -1 on this in its entirety. I don't think it's our job to reimplement the QTest methods where we can avoid it, and there is a big chance we're introducing subtle issues and incompatibilities here with very questionable benefit.

Currently, you cannot avoid it because their are QTest methods that do not work. At a minimum, your documentation should list those features that do not work properly due to Qt bugs, namely:

1. mouseClick not working in PyQt5, but fixed in PyQt6: QTBUG-5232
2. Clicking away from activated menu does not close it: QTBUG-104075

I'd be fine with adding a workaround for the former - though only on Qt 5, and with a test. What I'm not fine with is reimplementing all of them without having more context why.

As for the latter, do your changes fix that as well presently?

I'd also be fine with adding a list of such bugs to the troubleshooting page or something - though we can't possibly forsee all issues someone could run into.

I don't quite follow on #428 (comment) either. Yes, they are in QTest, why is that a problem? What exactly wasn't bound in C++?

What makes you think that mouseClick wouldn't work?

The PySide2 documentation states:

All macros in the C++ version of QtTest were not binded in PySide, this module is useful only for GUI testing and benchmarking, for ordinary unit testing you should use the unittest Python module.

The PySide6 documentation states:

Not all macros in the C++ version of QtTest were exposed in PySide. This module is useful only for GUI testing and benchmarking; for ordinary unit testing you should use the pytest Python module.

Yes, the emphasis being on macros. That is, things from this list like QVERIFY. Those make no sense when writing Python, because you get them from your test framework. As the text points out, GUI testing methods are exposed: "this module is useful only for GUI testing".

Consequently,

1. mouseClick does not work on PyQt5/PySide5. See QTBUG-5232

...you mean mouseMove, yes?

...again, where are you getting that from?

The PySide2 /PySide6 documentation notes not all bindings were transferred and it does not list mouse and key actions. Perhaps this is a documentation error on the side of Qt?

That seems like a documentation bug for PySide2/PySide6 indeed. They exist, have for a long time, and they are in the PyQt API docs too.

It would be nice to add an example to the documentation of overriding certain qtbot methods (see SilentQtBot in https://stackoverflow.com/questions/68696865/pytest-qt-function-mousemove-not-working).

I don't think subclassing QtBot and replacing the qtbot fixture is an indended/supported usage of the plugin (and thus also don't believe that kind of thing should be documented). If you need something different, consider writing your own fixture instead of replacing qtbot.

[adam-grant-hendry]

I'd be fine with adding a workaround for the former - though only on Qt 5, and with a test. What I'm not fine with is reimplementing all of them without having more context why.

That's entirely fair.

As for the latter, do your changes fix that as well presently?

On further investigation, unfortunately no: I found these do not actually solve the problem at hand. From my understanding, QMenu.showMenu() calls exec(), which creates a blocking event loop (i.e. menu windows are modal). From QTBUG-104075:

In general, the top level mouse dispatcher is the window system, and it normally only sends events that occur inside the window to that window... with a couple of exceptions: a window could have a mouse grab, and modal popups (like menus) are also special. qtestlib calls qt_handleMouseEvent() which calls QWindowSystemInterface::handleMouseEvent(), passing a QPA mouse event: it does not actually generate a window system mouse event. So I suspect some window system functionality is getting bypassed when you do it like that, and if so, it's outside the scope of qtestlib to do that kind of testing. (I will need to refresh my memory about the details of closing modal popups by clicking outside, some time this year, I suspect.) I think there are other test frameworks that do send more-native events.

Hence, the menus have to be closed manually (using an Escape key press or clicking outside from QTest will not work).

To implement such things, one would have to do what pyautogui and pywinauto do, which is use call the lower-level OS API functions to generate those events:

Windows: ctypes, ctypes.wintypes
Mac: pyobjc-core, pyobjc, pyobjc-framework-Quartz
Linux: python-xlib

all which seems very out of scope for this project, as you rightly mentioned.

Considering the above, I think this PR should be closed and instead a new PR opened to add a little bit to the documentation.

I don't think subclassing QtBot and replacing the qtbot fixture is an indended/supported usage of the plugin

Ya, we can ignore that.

I'd also be fine with adding a list of such bugs to the troubleshooting page or something - though we can't possibly forsee all issues someone could run into.

No, I wouldn't want you to do that. I don't think that's reasonable. I only meant to suggest track things as users find them on a troubleshooting page. For now, I will create a new PR for documentation that simply adds some troubleshooting tips for the bugs I mentioned.