Merge lp:~veebers/autopilot/fixing_backend_being_none into lp:autopilot/1.3

[Christopher Lee]

Fixes issue where a classes _Backend can be None causes uncaught exceptions.

Fixes issue where a classes _Backend can be None causes uncaught exceptions.
The backend is now stored as an instance variable.

[Thomi Richards (thomir-deactivatedaccount)]

Looks good to me, except that changing class methods into regular (instance) methods is likely to break compatibility - probably not with Touch apps, but more liekly with unity7 or the desktop apps. Let's make sure this is not the case before merging this.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:343
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/27/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-i386-ci/11

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/27/rebuild

review: Approve (continuous-integration)

[Andy Doan (doanac)]

I've run the smoke image testing suite on my mako at home with this branch. I'm not sure if this is a problem or not. When run under our CI logic, I get a unity8 failure:

18:29:58.118 INFO globals:50 - Starting test unity8.indicators_client.tests.test_battery.TestDisplayMenus.test_brightness_slider (with touch)
18:29:58.119 WARNING testcase:118 - No tracing available - install the python-autopilot-trace package!
18:29:58.155 WARNING testcase:155 - Process manager backend unavailable, application snapshot support disabled.
18:29:58.227 INFO __init__:156 - Launching process: ['/usr/bin/indicators-client', '-testability', '-geometry', None]
18:29:58.272 ERROR testresult:35 - ERROR: unity8.indicators_client.tests.test_battery.TestDisplayMenus.test_brightness_slider(with touch)
18:29:58.274 ERROR testresult:35 - traceback: {{{
Traceback (most recent call last):
File "/home/phablet/autopilot/unity8/indicators_client/tests/test_battery.py", line 22, in setUp
super(TestDisplayMenus, self).setUp(self.geometry, self.grid_unit)
File "/home/phablet/autopilot/unity8/indicators_client/tests/__init__.py", line 57, in setUp
self.launch_test_local(geometry)
File "/home/phablet/autopilot/unity8/indicators_client/tests/__init__.py", line 62, in launch_test_local
"-geometry", geometry, app_type='qt')
File "/home/phablet/autopilot-override/autopilot/testcase.py", line 279, in launch_test_application
process = launch_application(launcher, app_path, *arguments, **kwargs)
File "/home/phablet/autopilot-override/autopilot/introspection/__init__.py", line 128, in launch_application
cwd=cwd
File "/home/phablet/autopilot-override/autopilot/introspection/__init__.py", line 167, in launch_process
**kwargs
File "/usr/lib/python2.7/subprocess.py", line 709, in __init__
errread, errwrite)
File "/usr/lib/python2.7/subprocess.py", line 1326, in _execute_child
raise child_exception
TypeError: execv() arg 2 must contain only strings
}}}

However, phablet-test-run is working. So i think its just something environmental at my place.

[Andy Doan (doanac)]

I can reproduce the error now. It happens when running the test by itself. ie:

 phablet-test-run -n -v unity8.indicators_client.tests.test_battery.TestDisplayMenus.test_auto_bright_switch

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-autolanding/23/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-autolanding/24/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-autolanding/24
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-autolanding/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-i386-autolanding/15
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-i386-autolanding/15/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins:8080/job/generic-cleanup-mbs/2636

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'autopilot/introspection/__init__.py'
--- autopilot/introspection/__init__.py	2013-09-26 05:31:16 +0000
+++ autopilot/introspection/__init__.py	2013-10-02 04:15:28 +0000
@@ -42,7 +42,6 @@
     AP_INTROSPECTION_IFACE,
 )
 from autopilot.introspection.dbus import (
-    _clear_backends_for_proxy_object,
     CustomEmulatorBase,
     DBusIntrospectionObject,
     get_classname_from_path,
@@ -474,16 +473,17 @@
     proxy_bases = proxy_bases + (emulator_base, )
     cls_name, cls_state = _get_proxy_object_class_name_and_state(data_source)
 
-    _clear_backends_for_proxy_object(emulator_base)
-    clsobj = type(
-        # Merge the object hierarchy.
-        str("%sBase" % cls_name), proxy_bases, dict(_Backend=data_source)
-    )
+    # Merge the object hierarchy.
+    clsobj = type(str("%sBase" % cls_name), proxy_bases, {})
 
     proxy_class = type(str(cls_name), (clsobj,), {})
 
-    proxy = proxy_class.get_root_instance()
-    return proxy
+    try:
+        dbus_tuple = data_source.introspection_iface.GetState("/")[0]
+        path, state = dbus_tuple
+        return proxy_class(state, path, data_source)
+    except IndexError:
+        raise RuntimeError("Unable to find root object of %r" % proxy_class)
 
 
 def _get_proxy_object_base_classes(backend):
@@ -518,8 +518,8 @@
 class ApplicationProxyObject(DBusIntrospectionObject):
     """A class that better supports query data from an application."""
 
-    def __init__(self, state, path):
-        super(ApplicationProxyObject, self).__init__(state, path)
+    def __init__(self, state, path, backend):
+        super(ApplicationProxyObject, self).__init__(state, path, backend)
         self._process = None
 
     def set_process(self, process):
 
=== modified file 'autopilot/introspection/dbus.py'
--- autopilot/introspection/dbus.py	2013-09-27 05:48:12 +0000
+++ autopilot/introspection/dbus.py	2013-10-02 04:15:28 +0000
@@ -111,20 +111,6 @@
         return class_object
 
 
-def _clear_backends_for_proxy_object(proxy_object):
-    """Iterate over the object registry and clear the dbus backend address set
-    on any class that has the same id as the specified proxy_object.
-
-    This is required so consecutive tests do not end up re-using the same dbus
-    backend address as a previous run, which will probably be incorrect.
-
-    """
-    global _object_registry
-    for cls in _object_registry[proxy_object._id].itervalues():
-        if type(proxy_object) is not cls:
-            cls._Backend = None
-
-
 def translate_state_keys(state_dict):
     """Translates the *state_dict* passed in so the keys are usable as python
     attributes."""
@@ -161,13 +147,12 @@
 
     __metaclass__ = IntrospectableObjectMetaclass
 
-    _Backend = None
-
-    def __init__(self, state_dict, path):
+    def __init__(self, state_dict, path, backend):
         self.__state = {}
         self.__refresh_on_attribute = True
         self._set_properties(state_dict)
         self._path = path
+        self._backend = backend
 
     def _set_properties(self, state_dict):
         """Creates and set attributes of *self* based on contents of
@@ -462,8 +447,7 @@
         _, new_state = self.get_new_state()
         self._set_properties(new_state)
 
-    @classmethod
-    def get_all_instances(cls):
+    def get_all_instances(self):
         """Get all instances of this class that exist within the Application
         state tree.
 
@@ -481,23 +465,22 @@
         :return: List (possibly empty) of class instances.
 
         """
-        cls_name = cls.__name__
-        instances = cls.get_state_by_path("//%s" % (cls_name))
-        return [cls.make_introspection_object(i) for i in instances]
+        cls_name = type(self).__name__
+        instances = self.get_state_by_path("//%s" % (cls_name))
+        return [self.make_introspection_object(i) for i in instances]
 
-    @classmethod
-    def get_root_instance(cls):
+    def get_root_instance(self):
         """Get the object at the root of this tree.
 
         This will return an object that represents the root of the
         introspection tree.
 
         """
-        instances = cls.get_state_by_path("/")
+        instances = self.get_state_by_path("/")
         if len(instances) != 1:
             logger.error("Could not retrieve root object.")
             return None
-        return cls.make_introspection_object(instances[0])
+        return self.make_introspection_object(instances[0])
 
     def __getattr__(self, name):
         # avoid recursion if for some reason we have no state set (should never
@@ -514,8 +497,7 @@
             "Class '%s' has no attribute '%s'." %
             (self.__class__.__name__, name))
 
-    @classmethod
-    def get_state_by_path(cls, piece):
+    def get_state_by_path(self, piece):
         """Get state for a particular piece of the state tree.
 
         You should probably never need to call this directly.
@@ -530,7 +512,7 @@
                 "XPath query must be a string, not %r", type(piece))
 
         with Timer("GetState %s" % piece):
-            data = cls._Backend.introspection_iface.GetState(piece)
+            data = self._backend.introspection_iface.GetState(piece)
             if len(data) > 15:
                 logger.warning(
                     "Your query '%s' returned a lot of data (%d items). This "
@@ -562,8 +544,7 @@
         else:
             return self._path + "[id=%d]" % self.id
 
-    @classmethod
-    def make_introspection_object(cls, dbus_tuple):
+    def make_introspection_object(self, dbus_tuple):
         """Make an introspection object given a DBus tuple of
         (path, state_dict).
 
@@ -572,23 +553,21 @@
         path, state = dbus_tuple
         name = get_classname_from_path(path)
         try:
-            class_type = _object_registry[cls._id][name]
-            if class_type._Backend is None:
-                class_type._Backend = cls._Backend
+            class_type = _object_registry[self._id][name]
         except KeyError:
             get_debug_logger().warning(
                 "Generating introspection instance for type '%s' based on "
                 "generic class.", name)
             # we want the object to inherit from the custom emulator base, not
             # the object class that is doing the selecting
-            for base in cls.__bases__:
+            for base in type(self).__bases__:
                 if issubclass(base, CustomEmulatorBase):
                     base_class = base
                     break
             else:
-                base_class = cls
+                base_class = type(self)
             class_type = type(str(name), (base_class,), {})
-        return class_type(state, path)
+        return class_type(state, path, self._backend)
 
     @contextmanager
     def no_automatic_refreshing(self):
 
=== modified file 'autopilot/introspection/qt.py'
--- autopilot/introspection/qt.py	2013-09-13 14:53:52 +0000
+++ autopilot/introspection/qt.py	2013-10-02 04:15:28 +0000
@@ -101,7 +101,7 @@
 
         """
 
-        return self._Backend.qt_introspection_iface
+        return self._backend.qt_introspection_iface
 
     @property
     def slots(self):
 
=== modified file 'autopilot/tests/unit/test_introspection_features.py'
--- autopilot/tests/unit/test_introspection_features.py	2013-09-27 05:48:12 +0000
+++ autopilot/tests/unit/test_introspection_features.py	2013-10-02 04:15:28 +0000
@@ -18,7 +18,7 @@
 #
 
 
-from mock import patch
+from mock import patch, Mock
 from testtools import TestCase
 from testtools.matchers import Equals, NotEquals
 from testscenarios import TestWithScenarios
@@ -89,7 +89,8 @@
     def test_can_access_path_attribute(self):
         fake_object = DBusIntrospectionObject(
             dict(id=123, path='/some/path'),
-            '/'
+            '/',
+            None
         )
         with fake_object.no_automatic_refreshing():
             self.assertThat(fake_object.path, Equals('/some/path'))
@@ -101,10 +102,14 @@
         'large' is defined as more than 15.
 
         """
-        with patch.object(DBusIntrospectionObject, '_Backend') as p:
-            p.introspection_iface.GetState.return_value = \
-                [('/path', {}) for i in range(16)]
-            DBusIntrospectionObject.get_state_by_path('some_query')
+        fake_object = DBusIntrospectionObject(
+            dict(id=123, path='/some/path'),
+            '/',
+            Mock()
+        )
+        fake_object._backend.introspection_iface.GetState.return_value = \
+            [('/path', {}) for i in range(16)]
+        fake_object.get_state_by_path('some_query')
 
         mock_logger.warning.assert_called_once_with(
             "Your query '%s' returned a lot of data (%d items). This "
@@ -120,9 +125,13 @@
         'small' is defined as 15 or fewer.
 
         """
-        with patch.object(DBusIntrospectionObject, '_Backend') as p:
-            p.introspection_iface.GetState.return_value = \
-                [('/path', {}) for i in range(15)]
-            DBusIntrospectionObject.get_state_by_path('some_query')
+        fake_object = DBusIntrospectionObject(
+            dict(id=123, path='/some/path'),
+            '/',
+            Mock()
+        )
+        fake_object._backend.introspection_iface.GetState.return_value = \
+            [('/path', {}) for i in range(15)]
+        fake_object.get_state_by_path('some_query')
 
         self.assertThat(mock_logger.warning.called, Equals(False))
 
=== modified file 'autopilot/tests/unit/test_matchers.py'
--- autopilot/tests/unit/test_matchers.py	2013-08-07 03:18:32 +0000
+++ autopilot/tests/unit/test_matchers.py	2013-10-02 04:15:28 +0000
@@ -47,7 +47,7 @@
     """
     class FakeObject(DBusIntrospectionObject):
         def __init__(self, props):
-            super(FakeObject, self).__init__(props, "/FakeObject")
+            super(FakeObject, self).__init__(props, "/FakeObject", None)
             FakeObject._fake_props = props
 
         @classmethod