Merge lp:~acerisara/autopilot/move-to-object into lp:autopilot

[Andrea Cerisara]

Refactored autopilot.input._X11.Mouse.move_to_object.

Refactored autopilot.input._X11.Mouse.move_to_object.

[lp:~acerisara/autopilot/move-to-object]

537. By Andrea Cerisara on 2015-01-24

Updated doc.

[Christopher Lee (veebers)]

Looking good, just a minor nit-pick re: docstring. Thanks :-)

review: Needs Fixing

[lp:~acerisara/autopilot/move-to-object]

538. By Andrea Cerisara on 2015-01-31

Updated doc.

[Andrea Cerisara (acerisara)]

The idea was to have a reference to the doc at `autopilot.input._common.get_center_point` in `autopilot.input.Mouse.move_to_object`. Not sure if I can do this, since `input._common` is private as well. Left the doc as it was for now.

[Christopher Lee (veebers)]

LGTM, thanks :-) Just waiting on CI for this. Not sure why it's not running, will ping CI now.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:538
http://jenkins.qa.ubuntu.com/job/autopilot-ci/1014/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/77
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-amd64-ci/77/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/77
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-armhf-ci/77/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/77
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-vivid-i386-ci/77/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1179
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid-autopilot/110
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1039
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1177
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1177/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17781
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid-autopilot/106
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/655
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/655/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1014/rebuild

review: Approve (continuous-integration)

[Nicholas Skaggs (nskaggs)]

LGTM.

review: Approve

[Nicholas Skaggs (nskaggs)]

Just leaving this here as commentary on the change.

<thomi> balloons: veebers: I'm disturbed by the deletion of the docstring in that MP
<balloons> is there a more elegant way to do it, while still avoiding double documentation?
<thomi> are the two methods identical?
<balloons> mm.. I suppose it should actually have a docstring.. a unique one
<balloons> meh
<thomi> yeah
<thomi> and make that function public, and give it a docstring too
<balloons> I was thinking it was a duplication, but it's not purely one
<thomi> that way the docstring can say "moves the cursor to the center point of the object. See `autopilot.input.get_center_point` for details on how the center point is calculated'

[Thomi Richards (thomir-deactivatedaccount)]

just to give a bit more context:

I don't see any problem with making the 'get_center_point' function part of the public autopilot API, and documenting it as such. Once that's done, you can then change the docstring as I suggested on IRC.

To make get_center_point public:

1) Import it in the autopilot.input package. Make sure it appears in __all__. This should cause the docstring to appear in the auto-generated docs.

2) Change any code that uses that function to import it from autopilot.input, NOT from the private module.

3) profit!

[Nicholas Skaggs (nskaggs)]

Andrea. any chance you can make the tweaks Thomi suggests?

[lp:~acerisara/autopilot/move-to-object]

539. By Andrea Cerisara on 2016-01-08

Merged trunk.

540. By Andrea Cerisara on 2016-01-08

Made get_center_point public.

541. By Andrea Cerisara on 2016-01-08

Made get_center_point public.

[Christopher Lee (veebers)]

Hi Andrea, thanks for this. It looks good on a quick once over; I'm just in the process of getting the CI infra back up and running for the MPs so we'll have to wait on that unfortunately. I'll keep you in the loop :-)

[Christopher Lee (veebers)]

Looking good to me, a single query re: the inclusion of some docstring details.

Other than the docstring, it's LGTM

review: Needs Information

[lp:~acerisara/autopilot/move-to-object]

542. By Andrea Cerisara on 2016-01-26

Added missing doc.

[platform-qa-bot (platform-qa-bot)]

review: Approve (continuous-integration)

[platform-qa-bot (platform-qa-bot)]

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
https://platform-qa-jenkins.ubuntu.com/job/autopilot-autoland/1/
Executed test runs:
    SUCCESS: https://platform-qa-jenkins.ubuntu.com/job/build-wily-amd64-package/5
        deb: https://platform-qa-jenkins.ubuntu.com/job/build-wily-amd64-package/5/artifact/work/output/*zip*/output.zip
    SUCCESS: https://platform-qa-jenkins.ubuntu.com/job/build-wily-i386-package/6
        deb: https://platform-qa-jenkins.ubuntu.com/job/build-wily-i386-package/6/artifact/work/output/*zip*/output.zip
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-land-mp/1/console

review: Needs Fixing (continuous-integration)

[platform-qa-bot (platform-qa-bot)]

review: Approve (continuous-integration)

=== modified file 'autopilot/input/_X11.py'
--- autopilot/input/_X11.py	2014-10-30 23:42:46 +0000
+++ autopilot/input/_X11.py	2016-01-26 08:08:41 +0000
@@ -26,6 +26,7 @@
 
 import logging
 
+from autopilot.input import get_center_point
 from autopilot.display import is_point_on_any_screen, move_mouse_to_screen
 from autopilot.utilities import (
     EventDelay,
@@ -395,57 +396,12 @@
     def move_to_object(self, object_proxy):
         """Attempts to move the mouse to 'object_proxy's centre point.
 
-        It does this by looking for several attributes, in order. The first
-        attribute found will be used. The attributes used are (in order):
-
-         * globalRect (x,y,w,h)
-         * center_x, center_y
-         * x, y, w, h
-
-        :raises: **ValueError** if none of these attributes are found, or if an
-         attribute is of an incorrect type.
+        See :py:meth:`~autopilot.input.get_center_point` for details on how
+        the center point is calculated.
 
         """
-        try:
-            x, y, w, h = object_proxy.globalRect
-            _logger.debug("Moving to object's globalRect coordinates.")
-            self.move(x+w/2, y+h/2)
-            return
-        except AttributeError:
-            pass
-        except (TypeError, ValueError):
-            raise ValueError(
-                "Object '%r' has globalRect attribute, but it is not of the "
-                "correct type" % object_proxy)
-
-        try:
-            x, y = object_proxy.center_x, object_proxy.center_y
-            _logger.debug("Moving to object's center_x, center_y coordinates.")
-            self.move(x, y)
-            return
-        except AttributeError:
-            pass
-        except (TypeError, ValueError):
-            raise ValueError(
-                "Object '%r' has center_x, center_y attributes, but they are "
-                "not of the correct type" % object_proxy)
-
-        try:
-            x, y, w, h = (
-                object_proxy.x, object_proxy.y, object_proxy.w, object_proxy.h)
-            _logger.debug(
-                "Moving to object's center point calculated from x,y,w,h "
-                "attributes.")
-            self.move(x+w/2, y+h/2)
-            return
-        except AttributeError:
-            raise ValueError(
-                "Object '%r' does not have any recognised position "
-                "attributes" % object_proxy)
-        except (TypeError, ValueError):
-            raise ValueError(
-                "Object '%r' has x,y attribute, but they are not of the "
-                "correct type" % object_proxy)
+        x, y = get_center_point(object_proxy)
+        self.move(x, y)
 
     def position(self):
         """

=== modified file 'autopilot/input/__init__.py'
--- autopilot/input/__init__.py	2015-08-19 00:25:00 +0000
+++ autopilot/input/__init__.py	2016-01-26 08:08:41 +0000
@@ -57,14 +57,17 @@
 
 from collections import OrderedDict
 from contextlib import contextmanager
+from autopilot.input._common import get_center_point
 from autopilot.utilities import _pick_backend, CleanupRegistered
-from autopilot.input._common import get_center_point
 
 import logging
 
 _logger = logging.getLogger(__name__)
 
 
+__all__ = ['get_center_point']
+
+
 class Keyboard(CleanupRegistered):
 
     """A simple keyboard device class.
@@ -653,23 +656,19 @@
             press_duration=0.10,
             time_between_events=0.1):
         """
-        Attempts to move the pointer to 'object_proxy's centre point.
-        and click a button
-
-        It does this by looking for several attributes, in order. The first
-        attribute found will be used. The attributes used are (in order):
-
-         * globalRect (x,y,w,h)
-         * center_x, center_y
-         * x, y, w, h
+        Attempts to move the pointer to 'object_proxy's centre point
+        and click a button.
+
+        See :py:meth:`~autopilot.input.get_center_point` for details on how
+        the center point is calculated.
 
         If the wrapped device is a mouse, the button specification is used. If
         it is a touch device, passing anything other than 1 will raise a
         ValueError exception.
 
         :param time_between_events: takes floating point to represent the
-          delay time between subsequent clicks/taps. Default value 0.1
-          represents tenth of a second.
+            delay time between subsequent clicks/taps. Default value 0.1
+            represents tenth of a second.
 
         """
 
@@ -679,15 +678,8 @@
     def move_to_object(self, object_proxy):
         """Attempts to move the pointer to 'object_proxy's centre point.
 
-        It does this by looking for several attributes, in order. The first
-        attribute found will be used. The attributes used are (in order):
-
-         * globalRect (x,y,w,h)
-         * center_x, center_y
-         * x, y, w, h
-
-        :raises: **ValueError** if none of these attributes are found, or if an
-         attribute is of an incorrect type.
+        See :py:meth:`~autopilot.input.get_center_point` for details on how
+        the center point is calculated.
 
         """
         x, y = get_center_point(object_proxy)

=== modified file 'autopilot/input/_common.py'
--- autopilot/input/_common.py	2014-03-25 14:42:25 +0000
+++ autopilot/input/_common.py	2016-01-26 08:08:41 +0000
@@ -29,7 +29,11 @@
     """Get the center point of an object.
 
     It searches for several different ways of determining exactly where the
-    center is.
+    center is. The attributes used are (in order):
+
+     * globalRect (x,y,w,h)
+     * center_x, center_y
+     * x, y, w, h
 
     :raises ValueError: if `object_proxy` has the globalRect attribute but it
         is not of the correct type.

=== modified file 'autopilot/input/_uinput.py'
--- autopilot/input/_uinput.py	2015-07-07 22:12:40 +0000
+++ autopilot/input/_uinput.py	2016-01-26 08:08:41 +0000
@@ -25,7 +25,7 @@
 
 from autopilot.input import Keyboard as KeyboardBase
 from autopilot.input import Touch as TouchBase
-from autopilot.input._common import get_center_point
+from autopilot.input import get_center_point
 from autopilot.utilities import deprecated, EventDelay, sleep
 
 

=== modified file 'autopilot/tests/functional/test_input_stack.py'
--- autopilot/tests/functional/test_input_stack.py	2015-08-19 00:25:00 +0000
+++ autopilot/tests/functional/test_input_stack.py	2016-01-26 08:08:41 +0000
@@ -43,8 +43,7 @@
 )
 from autopilot.display import Display
 from autopilot.gestures import pinch
-from autopilot.input import Keyboard, Mouse, Pointer, Touch
-from autopilot.input._common import get_center_point
+from autopilot.input import Keyboard, Mouse, Pointer, Touch, get_center_point
 from autopilot.matchers import Eventually
 from autopilot.testcase import AutopilotTestCase, multiply_scenarios
 from autopilot.tests.functional import QmlScriptRunnerMixin

=== modified file 'autopilot/tests/unit/test_input.py'
--- autopilot/tests/unit/test_input.py	2015-07-07 22:50:24 +0000
+++ autopilot/tests/unit/test_input.py	2016-01-26 08:08:41 +0000
@@ -30,8 +30,7 @@
     tests,
     utilities
 )
-from autopilot.input import _uinput
-from autopilot.input._common import get_center_point
+from autopilot.input import _uinput, get_center_point
 
 
 class Empty(object):