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

Proposed by Christopher Lee on 2013-10-02
Status: Merged
Approved by: Ɓukasz Zemczak on 2013-10-03
Approved revision: 343
Merged at revision: 343
Proposed branch: lp:~veebers/autopilot/fixing_backend_being_none
Merge into: lp:autopilot/1.3
Diff against target: 271 lines (+47/-59)
5 files modified
autopilot/introspection/__init__.py (+10/-10)
autopilot/introspection/dbus.py (+16/-37)
autopilot/introspection/qt.py (+1/-1)
autopilot/tests/unit/test_introspection_features.py (+19/-10)
autopilot/tests/unit/test_matchers.py (+1/-1)
To merge this branch: bzr merge lp:~veebers/autopilot/fixing_backend_being_none
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2013-10-03
Thomi Richards (community) 2013-10-02 Approve on 2013-10-03
Review via email: mp+188762@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Thomi Richards (thomir) wrote :

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
Andy Doan (doanac) wrote :

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) wrote :

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) :
review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'autopilot/introspection/__init__.py'
2--- autopilot/introspection/__init__.py 2013-09-26 05:31:16 +0000
3+++ autopilot/introspection/__init__.py 2013-10-02 04:15:28 +0000
4@@ -42,7 +42,6 @@
5 AP_INTROSPECTION_IFACE,
6 )
7 from autopilot.introspection.dbus import (
8- _clear_backends_for_proxy_object,
9 CustomEmulatorBase,
10 DBusIntrospectionObject,
11 get_classname_from_path,
12@@ -474,16 +473,17 @@
13 proxy_bases = proxy_bases + (emulator_base, )
14 cls_name, cls_state = _get_proxy_object_class_name_and_state(data_source)
15
16- _clear_backends_for_proxy_object(emulator_base)
17- clsobj = type(
18- # Merge the object hierarchy.
19- str("%sBase" % cls_name), proxy_bases, dict(_Backend=data_source)
20- )
21+ # Merge the object hierarchy.
22+ clsobj = type(str("%sBase" % cls_name), proxy_bases, {})
23
24 proxy_class = type(str(cls_name), (clsobj,), {})
25
26- proxy = proxy_class.get_root_instance()
27- return proxy
28+ try:
29+ dbus_tuple = data_source.introspection_iface.GetState("/")[0]
30+ path, state = dbus_tuple
31+ return proxy_class(state, path, data_source)
32+ except IndexError:
33+ raise RuntimeError("Unable to find root object of %r" % proxy_class)
34
35
36 def _get_proxy_object_base_classes(backend):
37@@ -518,8 +518,8 @@
38 class ApplicationProxyObject(DBusIntrospectionObject):
39 """A class that better supports query data from an application."""
40
41- def __init__(self, state, path):
42- super(ApplicationProxyObject, self).__init__(state, path)
43+ def __init__(self, state, path, backend):
44+ super(ApplicationProxyObject, self).__init__(state, path, backend)
45 self._process = None
46
47 def set_process(self, process):
48
49=== modified file 'autopilot/introspection/dbus.py'
50--- autopilot/introspection/dbus.py 2013-09-27 05:48:12 +0000
51+++ autopilot/introspection/dbus.py 2013-10-02 04:15:28 +0000
52@@ -111,20 +111,6 @@
53 return class_object
54
55
56-def _clear_backends_for_proxy_object(proxy_object):
57- """Iterate over the object registry and clear the dbus backend address set
58- on any class that has the same id as the specified proxy_object.
59-
60- This is required so consecutive tests do not end up re-using the same dbus
61- backend address as a previous run, which will probably be incorrect.
62-
63- """
64- global _object_registry
65- for cls in _object_registry[proxy_object._id].itervalues():
66- if type(proxy_object) is not cls:
67- cls._Backend = None
68-
69-
70 def translate_state_keys(state_dict):
71 """Translates the *state_dict* passed in so the keys are usable as python
72 attributes."""
73@@ -161,13 +147,12 @@
74
75 __metaclass__ = IntrospectableObjectMetaclass
76
77- _Backend = None
78-
79- def __init__(self, state_dict, path):
80+ def __init__(self, state_dict, path, backend):
81 self.__state = {}
82 self.__refresh_on_attribute = True
83 self._set_properties(state_dict)
84 self._path = path
85+ self._backend = backend
86
87 def _set_properties(self, state_dict):
88 """Creates and set attributes of *self* based on contents of
89@@ -462,8 +447,7 @@
90 _, new_state = self.get_new_state()
91 self._set_properties(new_state)
92
93- @classmethod
94- def get_all_instances(cls):
95+ def get_all_instances(self):
96 """Get all instances of this class that exist within the Application
97 state tree.
98
99@@ -481,23 +465,22 @@
100 :return: List (possibly empty) of class instances.
101
102 """
103- cls_name = cls.__name__
104- instances = cls.get_state_by_path("//%s" % (cls_name))
105- return [cls.make_introspection_object(i) for i in instances]
106+ cls_name = type(self).__name__
107+ instances = self.get_state_by_path("//%s" % (cls_name))
108+ return [self.make_introspection_object(i) for i in instances]
109
110- @classmethod
111- def get_root_instance(cls):
112+ def get_root_instance(self):
113 """Get the object at the root of this tree.
114
115 This will return an object that represents the root of the
116 introspection tree.
117
118 """
119- instances = cls.get_state_by_path("/")
120+ instances = self.get_state_by_path("/")
121 if len(instances) != 1:
122 logger.error("Could not retrieve root object.")
123 return None
124- return cls.make_introspection_object(instances[0])
125+ return self.make_introspection_object(instances[0])
126
127 def __getattr__(self, name):
128 # avoid recursion if for some reason we have no state set (should never
129@@ -514,8 +497,7 @@
130 "Class '%s' has no attribute '%s'." %
131 (self.__class__.__name__, name))
132
133- @classmethod
134- def get_state_by_path(cls, piece):
135+ def get_state_by_path(self, piece):
136 """Get state for a particular piece of the state tree.
137
138 You should probably never need to call this directly.
139@@ -530,7 +512,7 @@
140 "XPath query must be a string, not %r", type(piece))
141
142 with Timer("GetState %s" % piece):
143- data = cls._Backend.introspection_iface.GetState(piece)
144+ data = self._backend.introspection_iface.GetState(piece)
145 if len(data) > 15:
146 logger.warning(
147 "Your query '%s' returned a lot of data (%d items). This "
148@@ -562,8 +544,7 @@
149 else:
150 return self._path + "[id=%d]" % self.id
151
152- @classmethod
153- def make_introspection_object(cls, dbus_tuple):
154+ def make_introspection_object(self, dbus_tuple):
155 """Make an introspection object given a DBus tuple of
156 (path, state_dict).
157
158@@ -572,23 +553,21 @@
159 path, state = dbus_tuple
160 name = get_classname_from_path(path)
161 try:
162- class_type = _object_registry[cls._id][name]
163- if class_type._Backend is None:
164- class_type._Backend = cls._Backend
165+ class_type = _object_registry[self._id][name]
166 except KeyError:
167 get_debug_logger().warning(
168 "Generating introspection instance for type '%s' based on "
169 "generic class.", name)
170 # we want the object to inherit from the custom emulator base, not
171 # the object class that is doing the selecting
172- for base in cls.__bases__:
173+ for base in type(self).__bases__:
174 if issubclass(base, CustomEmulatorBase):
175 base_class = base
176 break
177 else:
178- base_class = cls
179+ base_class = type(self)
180 class_type = type(str(name), (base_class,), {})
181- return class_type(state, path)
182+ return class_type(state, path, self._backend)
183
184 @contextmanager
185 def no_automatic_refreshing(self):
186
187=== modified file 'autopilot/introspection/qt.py'
188--- autopilot/introspection/qt.py 2013-09-13 14:53:52 +0000
189+++ autopilot/introspection/qt.py 2013-10-02 04:15:28 +0000
190@@ -101,7 +101,7 @@
191
192 """
193
194- return self._Backend.qt_introspection_iface
195+ return self._backend.qt_introspection_iface
196
197 @property
198 def slots(self):
199
200=== modified file 'autopilot/tests/unit/test_introspection_features.py'
201--- autopilot/tests/unit/test_introspection_features.py 2013-09-27 05:48:12 +0000
202+++ autopilot/tests/unit/test_introspection_features.py 2013-10-02 04:15:28 +0000
203@@ -18,7 +18,7 @@
204 #
205
206
207-from mock import patch
208+from mock import patch, Mock
209 from testtools import TestCase
210 from testtools.matchers import Equals, NotEquals
211 from testscenarios import TestWithScenarios
212@@ -89,7 +89,8 @@
213 def test_can_access_path_attribute(self):
214 fake_object = DBusIntrospectionObject(
215 dict(id=123, path='/some/path'),
216- '/'
217+ '/',
218+ None
219 )
220 with fake_object.no_automatic_refreshing():
221 self.assertThat(fake_object.path, Equals('/some/path'))
222@@ -101,10 +102,14 @@
223 'large' is defined as more than 15.
224
225 """
226- with patch.object(DBusIntrospectionObject, '_Backend') as p:
227- p.introspection_iface.GetState.return_value = \
228- [('/path', {}) for i in range(16)]
229- DBusIntrospectionObject.get_state_by_path('some_query')
230+ fake_object = DBusIntrospectionObject(
231+ dict(id=123, path='/some/path'),
232+ '/',
233+ Mock()
234+ )
235+ fake_object._backend.introspection_iface.GetState.return_value = \
236+ [('/path', {}) for i in range(16)]
237+ fake_object.get_state_by_path('some_query')
238
239 mock_logger.warning.assert_called_once_with(
240 "Your query '%s' returned a lot of data (%d items). This "
241@@ -120,9 +125,13 @@
242 'small' is defined as 15 or fewer.
243
244 """
245- with patch.object(DBusIntrospectionObject, '_Backend') as p:
246- p.introspection_iface.GetState.return_value = \
247- [('/path', {}) for i in range(15)]
248- DBusIntrospectionObject.get_state_by_path('some_query')
249+ fake_object = DBusIntrospectionObject(
250+ dict(id=123, path='/some/path'),
251+ '/',
252+ Mock()
253+ )
254+ fake_object._backend.introspection_iface.GetState.return_value = \
255+ [('/path', {}) for i in range(15)]
256+ fake_object.get_state_by_path('some_query')
257
258 self.assertThat(mock_logger.warning.called, Equals(False))
259
260=== modified file 'autopilot/tests/unit/test_matchers.py'
261--- autopilot/tests/unit/test_matchers.py 2013-08-07 03:18:32 +0000
262+++ autopilot/tests/unit/test_matchers.py 2013-10-02 04:15:28 +0000
263@@ -47,7 +47,7 @@
264 """
265 class FakeObject(DBusIntrospectionObject):
266 def __init__(self, props):
267- super(FakeObject, self).__init__(props, "/FakeObject")
268+ super(FakeObject, self).__init__(props, "/FakeObject", None)
269 FakeObject._fake_props = props
270
271 @classmethod

Subscribers

People subscribed via source and target branches