Merge lp:~thomir/unity/autopilot-attribute-feature into lp:unity

[Thomi Richards]

Added the autopilot wait_for attribute feature.

Problem:

We often write code in autopilkot emulators that looks something like this:

for i in range(11):
    if self.some_attribute != expected_value:
        sleep(1)
        self.refresh_state()
raise RuntimeError("Attribute 'some_attribute' not set to expected_value after 10 seconds")

...since duplicated code is bad, we'd like instead to be able to write this like so:

self.some_attribute.wait_for(expected_value)

... while still using 'self.some_attribute' as an ordinary variable.

Solution:

This branch does exactly that. It also converts the dash and hud emulators to use this new feature.

[Brandon Schaefer (brandontschaefer)]

Nice clean up!

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Cool!

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Ah, just one thing: what about to increase the number of the checks and reducing the sleep time (reducing the sleep time to .25 or .5...)?
AP is already quite slow as it is now.

[Thomi Richards (thomir)]

> Ah, just one thing: what about to increase the number of the checks and
> reducing the sleep time (reducing the sleep time to .25 or .5...)?
> AP is already quite slow as it is now.

Hi Marco,

I'm happy to experiment with the timings, but right now I'd rather not hot the dbus interface too hard. Perhaps in the Q cycle we can look at making this more responsive.

[Unity Merger (unity-merger)]

No commit message specified.

[Unity Merger (unity-merger)]

No commit message specified.

=== modified file 'tests/autopilot/autopilot/emulators/unity/__init__.py'
--- tests/autopilot/autopilot/emulators/unity/__init__.py	2012-03-13 20:35:42 +0000
+++ tests/autopilot/autopilot/emulators/unity/__init__.py	2012-04-12 22:27:24 +0000
@@ -9,6 +9,7 @@
 
 from dbus import Interface
 import logging
+from time import sleep
 
 from autopilot.emulators.dbus_handler import session_bus
 
@@ -28,8 +29,9 @@
 class IntrospectableObjectMetaclass(type):
     """Metaclass to insert appropriate classes into the object registry."""
 
-    def __new__(self, classname, bases, classdict):
-        class_object = type.__new__(self, classname, bases, classdict)
+    def __new__(cls, classname, bases, classdict):
+        """Add class name to type registry."""
+        class_object = type.__new__(cls, classname, bases, classdict)
         _object_registry[classname] = class_object
         return class_object
 
@@ -134,7 +136,56 @@
             # don't store id in state dictionary -make it a proper instance attribute
             if key == 'id':
                 self.id = value
-            self.__state[key.replace('-', '_')] = value
+            key = key.replace('-', '_')
+            self.__state[key] = self._make_attribute(key, value)
+
+    def _make_attribute(self, name, value):
+        """Make an attribute for 'value', patched with the wait_for function."""
+
+        def wait_for(self, expected_value):
+            """Wait up to 10 seconds for our value to change to 'expected_value'.
+
+            This works by refreshing the value using repeated dbus calls.
+
+            Raises RuntimeError if the attribute was not equal to the expected value
+            after 10 seconds.
+
+            """
+            # It's guaranteed that our value is up to date, since __getattr__ calls
+            # refresh_state. This if statement stops us waiting if the value is
+            # already what we expect:
+            if self == expected_value:
+                return
+
+            for i in range(11):
+                new_state = get_state_by_name_and_id(
+                    self.parent.__class__.__name__,
+                    self.parent.id)
+                if new_state[self.name] == expected_value:
+                    self.parent.set_properties(new_state)
+                    return
+                sleep(1)
+
+            raise RuntimeError("Error: %s.%s was not equal to %r after 10 seconds."
+                % (self.parent.__class__.__name__,
+                    self.name,
+                    expected_value))
+
+        # This looks like magic, but it's really not. We're creating a new type
+        # on the fly that derives from the type of 'value' with a couple of
+        # extra attributes: wait_for is the wait_for method above. 'parent' and
+        # 'name' are needed by the wait_for method.
+        #
+        # We can't use traditional meta-classes here, since the type we're
+        # deriving from is only known at call time, not at parse time (we could
+        # override __call__ in the meta class, but that doesn't buy us anything
+        # extra).
+        #
+        # A better way to do this would be with functools.partial, which I tried
+        # initially, but doesn't work well with bound methods.
+        t = type(value)
+        attrs = {'wait_for': wait_for, 'parent':self, 'name':name}
+        return type(t.__name__, (t,), attrs)(value)
 
     def _get_child_tuples_by_type(self, desired_type):
         """Get a list of (name,dict) pairs from children of the specified type.

=== modified file 'tests/autopilot/autopilot/emulators/unity/dash.py'
--- tests/autopilot/autopilot/emulators/unity/dash.py	2012-04-10 16:47:01 +0000
+++ tests/autopilot/autopilot/emulators/unity/dash.py	2012-04-12 22:27:24 +0000
@@ -7,8 +7,6 @@
 # by the Free Software Foundation.
 #
 
-from time import sleep
-
 from autopilot.emulators.unity import (
     get_state_by_path,
     make_introspection_object,
@@ -43,9 +41,10 @@
         """
         Reveals the dash if it's currently hidden, hides it otherwise.
         """
+        old_state = self.visible
         logger.debug("Toggling dash visibility with Super key.")
         self.keybinding("dash/reveal", 0.1)
-        sleep(1)
+        self.visible.wait_for(not old_state)
 
     def ensure_visible(self, clear_search=True):
         """
@@ -53,7 +52,7 @@
         """
         if not self.visible:
             self.toggle_reveal()
-            self._wait_for_visibility(expect_visible=True)
+            self.visible.wait_for(True)
             if clear_search:
                 self.clear_search()
 
@@ -63,16 +62,7 @@
         """
         if self.visible:
             self.toggle_reveal()
-            self._wait_for_visibility(expect_visible=False)
-
-    def _wait_for_visibility(self, expect_visible):
-        for i in range(11):
-            if self.visible != expect_visible:
-                sleep(1)
-            else:
-                return
-        raise RuntimeError("Dash not %s after waiting for 10 seconds." %
-            ("Visible" if expect_visible else "Hidden"))
+            self.visible.wait_for(False)
 
     @property
     def visible(self):
@@ -260,7 +250,8 @@
                 return filter_label
         return None
 
-    def is_expanded(self):
+    @property
+    def expanded(self):
         """Return True if the filterbar on this lens is expanded, False otherwise.
         """
         searchbar = self._get_searchbar()
@@ -268,34 +259,25 @@
 
     def ensure_expanded(self):
         """Expand the filter bar, if it's not already."""
-        if not self.is_expanded():
+        if not self.expanded:
             searchbar = self._get_searchbar()
             tx = searchbar.filter_label_x + (searchbar.filter_label_width / 2)
             ty = searchbar.filter_label_y + (searchbar.filter_label_height / 2)
             m = Mouse()
             m.move(tx, ty)
             m.click()
-            self._wait_for_expansion(True)
+            self.expanded.wait_for(True)
 
     def ensure_collapsed(self):
         """Collapse the filter bar, if it's not already."""
-        if self.is_expanded():
+        if self.expanded:
             searchbar = self._get_searchbar()
             tx = searchbar.filter_label_x + (searchbar.filter_label_width / 2)
             ty = searchbar.filter_label_y + (searchbar.filter_label_height / 2)
             m = Mouse()
             m.move(tx, ty)
             m.click()
-            self._wait_for_expansion(False)
-
-    def _wait_for_expansion(self, expect_expanded):
-        for i in range(11):
-            if self.is_expanded() != expect_expanded:
-                sleep(1)
-            else:
-                return
-        raise RuntimeError("Filters not %s after waiting for 10 seconds." %
-            ("expanded" if expect_expanded else "collapsed"))
+            self.expanded.wait_for(False)
 
     def _get_searchbar(self):
         """Get the searchbar.

=== modified file 'tests/autopilot/autopilot/emulators/unity/hud.py'
--- tests/autopilot/autopilot/emulators/unity/hud.py	2012-04-03 11:48:10 +0000
+++ tests/autopilot/autopilot/emulators/unity/hud.py	2012-04-12 22:27:24 +0000
@@ -26,15 +26,19 @@
         """Hides the hud if it's not already hidden."""
         if self.visible:
             self.toggle_reveal()
+            self.visible.wait_for(False)
 
     def ensure_visible(self):
         """Shows the hud if it's not already showing."""
         if not self.visible:
             self.toggle_reveal()
+            self.visible.wait_for(True)
 
     def toggle_reveal(self, tap_delay=0.1):
         """Tap the 'Alt' key to toggle the hud visibility."""
+        old_state = self.visible
         self.keybinding("hud/reveal", tap_delay)
+        self.visible.wait_for(not old_state)
 
     def get_embedded_icon(self):
         """Returns the HUD view embedded icon or None if is not shown."""

=== modified file 'tests/autopilot/autopilot/tests/__init__.py'
--- tests/autopilot/autopilot/tests/__init__.py	2012-04-11 06:32:50 +0000
+++ tests/autopilot/autopilot/tests/__init__.py	2012-04-12 22:27:24 +0000
@@ -292,10 +292,10 @@
         keyboard layout bits are very unweildy. This seems like the best
         solution, even a little bit brutish.
         """
-        cmd = ['gsettings', command, schema] + args
+        cmd = ['gsettings', command, schema] + list(args)
         # strip to remove the trailing \n.
-        ret = check_output(cmd, shell=True).strip()
-        time.sleep(1)
+        ret = check_output(cmd).strip()
+        time.sleep(5)
         reset_display()
         return ret
 

=== modified file 'tests/autopilot/autopilot/tests/test_dash.py'
--- tests/autopilot/autopilot/tests/test_dash.py	2012-04-11 09:32:41 +0000
+++ tests/autopilot/autopilot/tests/test_dash.py	2012-04-12 22:27:24 +0000
@@ -17,6 +17,7 @@
 class DashTestCase(AutopilotTestCase):
     def setUp(self):
         super(DashTestCase, self).setUp()
+        self.set_unity_log_level("unity", "DEBUG")
         self.set_unity_log_level("unity.shell", "DEBUG")
         self.set_unity_log_level("unity.launcher", "DEBUG")
         self.dash.ensure_hidden()
@@ -76,15 +77,12 @@
 
 class DashMultiKeyTests(DashSearchInputTests):
     def setUp(self):
-        def set_multi_key():
-            """Binds Multi_key to caps lock"""
-            old_value = "\"%s\"" % self.call_gsettings_cmd('get', 'org.gnome.libgnomekbd.keyboard', '"options"')
-            self.addCleanup(self.call_gsettings_cmd, 'set', 'org.gnome.libgnomekbd.keyboard', '"options"', old_value)
-            self.call_gsettings_cmd('set', 'org.gnome.libgnomekbd.keyboard', '"options"', "\"['Compose key\tcompose:caps']\"")
-
         # set the multi key first so that we're not getting a new _DISPLAY while keys are held down.
-        set_multi_key()
-        super(DashMultiKeyTests, self).setUp()    
+        old_value = self.call_gsettings_cmd('get', 'org.gnome.libgnomekbd.keyboard', 'options')
+        self.addCleanup(self.call_gsettings_cmd, 'set', 'org.gnome.libgnomekbd.keyboard', 'options', old_value)
+        self.call_gsettings_cmd('set', 'org.gnome.libgnomekbd.keyboard', 'options', "['Compose key\tcompose:caps']")
+
+        super(DashMultiKeyTests, self).setUp()
 
     def test_multi_key(self):
         """Pressing 'Multi_key' must not add any characters to the search."""