Merge lp:~veebers/autopilot/1.4_fixing_backend_being_none into lp:autopilot

[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.

Forward port of: https://code.launchpad.net/~veebers/autopilot/fixing_backend_being_none/+merge/188762

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:347
http://10.97.0.26:8080/job/autopilot-ci/262/
Executed test runs:
    SUCCESS: http://10.97.0.26:8080/job/autopilot-saucy-amd64-ci/190
    SUCCESS: http://10.97.0.26:8080/job/autopilot-saucy-armhf-ci/190
    SUCCESS: http://10.97.0.26:8080/job/autopilot-saucy-i386-ci/34

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/autopilot-ci/262/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:348
http://10.97.0.26:8080/job/autopilot-ci/263/
Executed test runs:
    SUCCESS: http://10.97.0.26:8080/job/autopilot-saucy-amd64-ci/191
    SUCCESS: http://10.97.0.26:8080/job/autopilot-saucy-armhf-ci/191
    SUCCESS: http://10.97.0.26:8080/job/autopilot-saucy-i386-ci/35

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/autopilot-ci/263/rebuild

review: Approve (continuous-integration)

[Thomi Richards (thomir-deactivatedaccount)]

LGTM

review: Approve

[lp:~veebers/autopilot/1.4_fixing_backend_being_none]

349. By Christopher Lee on 2013-10-09

Added porting notes.

350. By Christopher Lee on 2013-10-09

Fixed the documentation

351. By Christopher Lee on 2013-10-09

Merge trunk

Unmerged revisions

=== modified file 'autopilot/introspection/__init__.py'
--- autopilot/introspection/__init__.py	2013-09-26 04:36:33 +0000
+++ autopilot/introspection/__init__.py	2013-10-07 03:55:25 +0000
@@ -43,7 +43,6 @@
     AP_INTROSPECTION_IFACE,
 )
 from autopilot.introspection.dbus import (
-    _clear_backends_for_proxy_object,
     CustomEmulatorBase,
     DBusIntrospectionObject,
     get_classname_from_path,
@@ -485,13 +484,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(
-        str(cls_name), proxy_bases, dict(_Backend=data_source)
-    )
-
-    proxy = clsobj.get_root_instance()
-    return proxy
+    # Merge the object hierarchy.
+    clsobj = type(str("%sBase" % cls_name), proxy_bases, {})
+
+    proxy_class = type(str(cls_name), (clsobj,), {})
+
+    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):
@@ -526,8 +529,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:55:31 +0000
+++ autopilot/introspection/dbus.py	2013-10-07 03:55:25 +0000
@@ -126,20 +126,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].values():
-        if type(proxy_object) is not cls:
-            cls._Backend = None
-
-
 def get_classname_from_path(object_path):
     return object_path.split("/")[-1]
 
@@ -174,14 +160,13 @@
 
     """
 
-    _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._poll_time = 10
+        self._backend = backend
 
     def _set_properties(self, state_dict):
         """Creates and set attributes of *self* based on contents of
@@ -484,8 +469,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.
 
@@ -503,23 +487,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
@@ -536,8 +519,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.
@@ -552,7 +534,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 "
@@ -584,8 +566,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).
 
@@ -594,17 +575,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)
-            # override the _id attr from cls, since we don't want generated
-            # types to end up in the object registry.
-            class_type = type(str(name), (cls,), {'_id': None})
-        return class_type(state, path)
+            # we want the object to inherit from the custom emulator base, not
+            # the object class that is doing the selecting
+            for base in type(self).__bases__:
+                if issubclass(base, CustomEmulatorBase):
+                    base_class = base
+                    break
+            else:
+                base_class = type(self)
+            class_type = type(str(name), (base_class,), {})
+        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-18 23:14:02 +0000
+++ autopilot/introspection/qt.py	2013-10-07 03:55:25 +0000
@@ -100,7 +100,7 @@
 
         """
 
-        return self._Backend.qt_introspection_iface
+        return self._backend.qt_introspection_iface
 
     @property
     def slots(self):

=== modified file 'autopilot/tests/functional/test_introspection_features.py'
--- autopilot/tests/functional/test_introspection_features.py	2013-09-25 02:23:02 +0000
+++ autopilot/tests/functional/test_introspection_features.py	2013-10-07 03:55:25 +0000
@@ -24,7 +24,7 @@
 import subprocess
 import tempfile
 from tempfile import mktemp
-from testtools.matchers import Equals
+from testtools.matchers import Equals, IsInstance, Not
 from textwrap import dedent
 
 from autopilot.matchers import Eventually
@@ -86,6 +86,39 @@
 
         self.assertThat(test_widget.visible, Eventually(Equals(True)))
 
+    def test_selecting_generic_from_custom_is_not_inherited_from_custom(self):
+        """Selecting a generic proxy object from a custom proxy object must not
+        return an object derived of the custom object type.
+
+        """
+        class MouseTestWidget(EmulatorBase):
+            pass
+
+        app = self.start_mock_app(EmulatorBase)
+        mouse_widget = app.select_single(MouseTestWidget)
+
+        child_label = mouse_widget.select_many("QLabel")[0]
+
+        self.assertThat(child_label, Not(IsInstance(MouseTestWidget)))
+
+    def test_selecting_custom_from_generic_is_not_inherited_from_generic(self):
+        """Selecting a custom proxy object from a generic proxy object must
+        return an object that is of the custom type.
+
+        """
+        class MouseTestWidget(EmulatorBase):
+            pass
+
+        app = self.start_mock_app(EmulatorBase)
+        generic_window = app.select_single("QMainWindow")
+
+        mouse_widget = generic_window.select_single(MouseTestWidget)
+
+        self.assertThat(
+            mouse_widget,
+            Not(IsInstance(type(generic_window)))
+        )
+
 
 class QMLCustomEmulatorTestCase(AutopilotTestCase):
     """Test the introspection of a QML application with a custom emulator."""

=== modified file 'autopilot/tests/unit/test_introspection_features.py'
--- autopilot/tests/unit/test_introspection_features.py	2013-09-27 05:55:31 +0000
+++ autopilot/tests/unit/test_introspection_features.py	2013-10-07 03:55:25 +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
@@ -120,7 +120,8 @@
     def test_can_access_path_attribute(self):
         fake_object = DBusIntrospectionObject(
             dict(id=[0, 123], path=[0, '/some/path']),
-            '/'
+            '/',
+            Mock()
         )
         with fake_object.no_automatic_refreshing():
             self.assertThat(fake_object.path, Equals('/some/path'))
@@ -132,10 +133,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=[0, 123], path=[0, '/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 "
@@ -151,9 +156,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=[0, 123], path=[0, '/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-09-16 17:17:35 +0000
+++ autopilot/tests/unit/test_matchers.py	2013-10-07 03:55:25 +0000
@@ -66,7 +66,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

=== modified file 'autopilot/tests/unit/test_types.py'
--- autopilot/tests/unit/test_types.py	2013-09-26 23:34:52 +0000
+++ autopilot/tests/unit/test_types.py	2013-10-07 03:55:25 +0000
@@ -20,7 +20,7 @@
 from __future__ import absolute_import
 
 from datetime import datetime, time
-from mock import patch
+from mock import patch, Mock
 from testscenarios import TestWithScenarios
 from testtools import TestCase
 from testtools.matchers import Equals, IsInstance, NotEquals, raises
@@ -636,7 +636,8 @@
         """
         DBusIntrospectionObject(
             dict(foo=[0]),
-            '/some/dummy/path'
+            '/some/dummy/path',
+            Mock()
         )
         error_logger.assert_called_once_with(
             "While constructing attribute '%s.%s': %s",