Don't assume window is MockWindow when dealing with pointer position #232
Conversation
| @@ -256,7 +256,9 @@ def mouse_move(self, interactor, x, y, window=None, | |||
| alt_down=alt_down, | |||
| control_down=control_down, | |||
| shift_down=shift_down) | |||
| window.get_pointer_position.return_value = (x, y) | |||
| if isinstance(window, _MockWindow): | |||
There was a problem hiding this comment.
If one passes a real window how does the mouse_move actually happen? When a real window get_pointer_position is called it will not have the right (x, y) value. In short the call will not fail but the desired affect will probably not happen either.
There was a problem hiding this comment.
Please also note that it is get_pointer_position that should be a mock not the window. Thus the check needs to be on get_pointer_position.
There was a problem hiding this comment.
The get_pointer_position function isn't used by most consumers of the mouse_move event - in most cases the event position is sufficient. But you are right, we should check to see if the method is mocked, not the window.
There was a problem hiding this comment.
And needless to say, if you pass a real window in a test, you'll need to mock the get_pointer_position yourself if you need that handled.
There was a problem hiding this comment.
The get_pointer_position function isn't used by most consumers of the mouse_move
This is True but it is not always clear when this is the case (e.g. using a subclass of a Tool). So the user will end up with a very weird error down the line.
And needless to say, if you pass a real window in a test, you'll need to mock the
get_pointer_positionyourself if you need that handled.
Ideally we should be able to fake the mouse move in any Window (e.g maybe sent the mouse move event to the Qt widget) . However, for the moment, I would suggest to raise a warning if the get_pointer_position is not mocked and update the docstring to mention that for the mouse_move to properly fake the operation get_pointer_position needs to be a mock instance
There was a problem hiding this comment.
It would also be nice to create a test using a real Window so that users can see how it is done.
There was a problem hiding this comment.
opened feature request #233 for supporting a more native way for faking mouse moves in real windows
Current coverage is 51.98%@@ master #232 diff @@
==========================================
Files 126 126
Lines 13184 13185 +1
Methods 0 0
Messages 0 0
Branches 1943 1944 +1
==========================================
+ Hits 6844 6854 +10
+ Misses 5897 5887 -10
- Partials 443 444 +1
|
|
👍 as soon as we have a test |
|
@itziakos We have a test :) |
|
👍 |
Fixes #211.