Merge lp:~veebers/unity/add_details_to_ensure_running into lp:unity

Proposed by Christopher Lee
Status: Work in progress
Proposed branch: lp:~veebers/unity/add_details_to_ensure_running
Merge into: lp:unity
Diff against target: 330 lines (+120/-45)
2 files modified
tests/autopilot/unity/emulators/__init__.py (+8/-7)
tests/autopilot/unity/tests/__init__.py (+112/-38)
To merge this branch: bzr merge lp:~veebers/unity/add_details_to_ensure_running
Reviewer Review Type Date Requested Status
Christopher Townsend Needs Information
PS Jenkins bot (community) continuous-integration Needs Fixing
Marco Trevisan (Treviño) Approve
Review via email: mp+192997@code.launchpad.net

Commit message

Add extra detail for when ensure_unity_running fails.

Description of the change

Add extra detail for when ensure_unity_running fails.
Also some pep8 cleanup for files touched during this patch.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Stephen M. Webb (bregma) wrote :

37 - raise RuntimeError("Unity debug interface is down after %d seconds of polling." % (timeout))
38 + raise RuntimeError(
39 + "Unity debug interface is down after %d seconds of polling (%s)."
40 + % (timeout)
41 + )

Is that change missing an argument to the format string?

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Veebers: in the future, please separate whitespace / PEP8 fixes from logic changes. It complicates the diff, makes diffs longer (which leads to fewer & poorer code reviews), and generally slows things down. I find it quite hard to see which changes in this diff I ought to be paying attention to, and which are harmless.

Revision history for this message
Christopher Lee (veebers) wrote :

Thomi, understood I'll make this 2 MRs now. I did separate the changes into 2 commits but that still doesn't help the diff.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

I'm moving this to WiP as I think it's probably a Good Thing thing to get in at some point. I'm not sure if you're going to do anything else with it. If not, then one of us can take what you have hear and do a new MP.

review: Needs Information

Unmerged revisions

3587. By Christopher Lee

further pep8 compliance to files touched

3586. By Christopher Lee

pep8 compliance to files touched

3585. By Christopher Lee

Add extra details for reason of unity not running check

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/autopilot/unity/emulators/__init__.py'
2--- tests/autopilot/unity/emulators/__init__.py 2013-10-03 02:47:42 +0000
3+++ tests/autopilot/unity/emulators/__init__.py 2013-10-29 03:03:05 +0000
4@@ -18,8 +18,6 @@
5 from autopilot.introspection.dbus import CustomEmulatorBase
6 from autopilot.introspection.backends import DBusAddress
7
8-from dbus import DBusException
9-
10
11 class UnityIntrospectionObject(CustomEmulatorBase):
12
13@@ -32,15 +30,15 @@
14 def ensure_unity_is_running(timeout=300):
15 """Poll the unity debug interface, and return when it's ready for use.
16
17- The default timeout is 300 seconds (5 minutes) to account for the case where
18- Unity has crashed and is taking a while to get restarted (on a slow VM for
19- example).
20+ The default timeout is 300 seconds (5 minutes) to account for the case
21+ where Unity has crashed and is taking a while to get restarted (on a slow
22+ VM for example).
23
24 If, after the timeout period, unity is still not up, this function raises a
25 RuntimeError exception.
26
27 """
28- sleep_period=10
29+ sleep_period = 10
30 for i in range(0, timeout, sleep_period):
31 try:
32 get_proxy_object_for_existing_process(
33@@ -50,4 +48,7 @@
34 return True
35 except ProcessSearchError:
36 sleep(sleep_period)
37- raise RuntimeError("Unity debug interface is down after %d seconds of polling." % (timeout))
38+ raise RuntimeError(
39+ "Unity debug interface is down after %d seconds of polling (%s)."
40+ % (timeout)
41+ )
42
43=== modified file 'tests/autopilot/unity/tests/__init__.py'
44--- tests/autopilot/unity/tests/__init__.py 2013-10-03 02:47:42 +0000
45+++ tests/autopilot/unity/tests/__init__.py 2013-10-29 03:03:05 +0000
46@@ -24,9 +24,9 @@
47 try:
48 import windowmocker
49 import json
50- HAVE_WINDOWMOCKER=True
51+ HAVE_WINDOWMOCKER = True
52 except ImportError:
53- HAVE_WINDOWMOCKER=False
54+ HAVE_WINDOWMOCKER = False
55 from subprocess import check_output
56 import time
57 import tempfile
58@@ -46,7 +46,6 @@
59 from unity.emulators.X11 import reset_display
60
61 from Xlib import display
62-from Xlib import Xutil
63
64 from gi.repository import Gio
65
66@@ -60,8 +59,11 @@
67 super(UnityTestCase, self).setUp()
68 try:
69 ensure_unity_is_running()
70- except RuntimeError:
71- log.error("Unity doesn't appear to be running, exiting.")
72+ except RuntimeError as e:
73+ log.error(
74+ "Unity doesn't appear to be running, exiting (%s)."
75+ % str(e)
76+ )
77 sys.exit(1)
78
79 self._unity = get_proxy_object_for_existing_process(
80@@ -75,14 +77,19 @@
81 self.addCleanup(self.check_test_behavior)
82 #
83 # Setting this here since the show desktop feature seems to be a bit
84- # ropey. Once it's been proven to work reliably we can remove this line:
85+ # ropey. Once it's been proven to work reliably we can remove this
86+ # line:
87 self.set_unity_log_level("unity.wm.compiz", "DEBUG")
88
89 # For the length of the test, disable screen locking
90 self._desktop_settings = Gio.Settings.new("org.gnome.desktop.lockdown")
91 lock_state = self._desktop_settings.get_boolean("disable-lock-screen")
92 self._desktop_settings.set_boolean("disable-lock-screen", True)
93- self.addCleanup(self._desktop_settings.set_boolean, "disable-lock-screen", lock_state)
94+ self.addCleanup(
95+ self._desktop_settings.set_boolean,
96+ "disable-lock-screen",
97+ lock_state
98+ )
99
100 def check_test_behavior(self):
101 """Fail the test if it did something naughty.
102@@ -96,11 +103,20 @@
103 log.info("Checking system state for badly behaving test...")
104
105 # Have we switched workspace?
106- if not self.well_behaved(self.workspace, current_workspace=self._initial_workspace_num):
107+ if not self.well_behaved(
108+ self.workspace,
109+ current_workspace=self._initial_workspace_num
110+ ):
111 well_behaved = False
112- reasons.append("The test changed the active workspace from %d to %d." \
113- % (self._initial_workspace_num, self.workspace.current_workspace))
114- log.warning("Test changed the active workspace, changing it back...")
115+ reasons.append(
116+ "The test changed the active workspace from %d to %d." % (
117+ self._initial_workspace_num,
118+ self.workspace.current_workspace
119+ )
120+ )
121+ log.warning(
122+ "Test changed the active workspace, changing it back..."
123+ )
124 self.workspace.switch_to(self._initial_workspace_num)
125 # Have we left the dash open?
126 if not self.well_behaved(self.unity.dash, visible=False):
127@@ -115,10 +131,15 @@
128 log.warning("Test left the hud open, closing it...")
129 self.unity.hud.ensure_hidden()
130 # Are we in show desktop mode?
131- if not self.well_behaved(self.unity.window_manager, showdesktop_active=False):
132+ if not self.well_behaved(
133+ self.unity.window_manager,
134+ showdesktop_active=False
135+ ):
136 well_behaved = False
137 reasons.append("The test left the system in show_desktop mode.")
138- log.warning("Test left the system in show desktop mode, exiting it...")
139+ log.warning(
140+ "Test left the system in show desktop mode, exiting it..."
141+ )
142 # It is not possible to leave show desktop mode if there are no
143 # app windows. So, just open a window and perform the show
144 # desktop action until the desired state is acheived, then close
145@@ -126,12 +147,15 @@
146 #
147 # In the event that this doesn't work, wait_for will throw an
148 # exception.
149- win = self.process_manager.start_app_window('Calculator', locale='C')
150+ win = self.process_manager.start_app_window(
151+ 'Calculator',
152+ locale='C'
153+ )
154 count = 1
155 while self.unity.window_manager.showdesktop_active:
156 self.keybinding("window/show_desktop")
157 sleep(count)
158- count+=1
159+ count += 1
160 if count > 10:
161 break
162 win.close()
163@@ -139,13 +163,19 @@
164 for launcher in self.unity.launcher.get_launchers():
165 if not self.well_behaved(launcher, in_keynav_mode=False):
166 well_behaved = False
167- reasons.append("The test left the launcher keynav mode enabled.")
168- log.warning("Test left the launcher in keynav mode, exiting it...")
169+ reasons.append(
170+ "The test left the launcher keynav mode enabled."
171+ )
172+ log.warning(
173+ "Test left the launcher in keynav mode, exiting it..."
174+ )
175 launcher.key_nav_cancel()
176 if not self.well_behaved(launcher, in_switcher_mode=False):
177 well_behaved = False
178 reasons.append("The test left the launcher in switcher mode.")
179- log.warning("Test left the launcher in switcher mode, exiting it...")
180+ log.warning(
181+ "Test left the launcher in switcher mode, exiting it..."
182+ )
183 launcher.switcher_cancel()
184 if not self.well_behaved(launcher, quicklist_open=False):
185 well_behaved = False
186@@ -181,8 +211,8 @@
187 self.addCleanup(self._tearDownUnityLogging)
188
189 def _tearDownUnityLogging(self):
190- # If unity dies, our dbus interface has gone, and reset_logging will fail
191- # but we still want our log, so we ignore any errors.
192+ # If unity dies, our dbus interface has gone, and reset_logging will
193+ # fail but we still want our log, so we ignore any errors.
194 try:
195 reset_logging()
196 except DBusException:
197@@ -202,29 +232,39 @@
198 """
199 valid_levels = ('TRACE', 'DEBUG', 'INFO', 'WARN', 'WARNING', 'ERROR')
200 if level not in valid_levels:
201- raise ValueError("Log level '%s' must be one of: %r" % (level, valid_levels))
202+ raise ValueError(
203+ "Log level '%s' must be one of: %r" % (level, valid_levels)
204+ )
205 set_log_severity(component, level)
206
207 def assertNumberWinsIsEventually(self, app, num):
208- """Asserts that 'app' eventually has 'num' wins. Waits up to 10 seconds."""
209-
210- self.assertThat(lambda: len(app.get_windows()), Eventually(Equals(num)))
211+ """Asserts that 'app' eventually has 'num' wins. Waits up to 10
212+ seconds.
213+
214+ """
215+
216+ self.assertThat(
217+ lambda: len(app.get_windows()),
218+ Eventually(Equals(num))
219+ )
220
221 def launch_test_window(self, window_spec={}):
222 """Launch a test window, for the duration of this test only.
223
224 This uses the 'window-mocker' application, which is not part of the
225 python-autopilot or unity-autopilot packages. To use this method, you
226- must have python-windowmocker installed. If the package is not installed,
227- this method will raise a SkipTest exception, causing the calling test
228- to be silently skipped.
229+ must have python-windowmocker installed. If the package is not
230+ installed, this method will raise a SkipTest exception, causing the
231+ calling test to be silently skipped.
232
233 window_spec is a list or dictionary that conforms to the window-mocker
234 specification.
235
236 """
237 if not HAVE_WINDOWMOCKER:
238- raise SkipTest("The python-windowmocker package is required to run this test.")
239+ raise SkipTest(
240+ "The python-windowmocker package is required to run this test."
241+ )
242
243 if 'Window Mocker' not in self.process_manager.KNOWN_APPS:
244 self.process_manager.register_known_application(
245@@ -236,24 +276,47 @@
246 file_path = tempfile.mktemp()
247 json.dump(window_spec, open(file_path, 'w'))
248 self.addCleanup(os.remove, file_path)
249- return self.process_manager.start_app_window('Window Mocker', [file_path])
250+ return self.process_manager.start_app_window(
251+ 'Window Mocker',
252+ [file_path]
253+ )
254 else:
255 return self.process_manager.start_app_window('Window Mocker')
256
257 def close_all_windows(self, application_name):
258- for w in self.process_manager.get_open_windows_by_application(application_name):
259+ for w in self.process_manager.get_open_windows_by_application(
260+ application_name
261+ ):
262 w.close()
263
264- self.assertThat(lambda: len(self.process_manager.get_open_windows_by_application(application_name)), Eventually(Equals(0)))
265+ def close_win_fn():
266+ return len(
267+ self.process_manager.get_open_windows_by_application(
268+ application_name
269+ )
270+ )
271+ self.assertThat(close_win_fn, Eventually(Equals(0)))
272
273 def register_nautilus(self):
274- self.addCleanup(self.process_manager.unregister_known_application, "Nautilus")
275- self.process_manager.register_known_application("Nautilus", "nautilus.desktop", "nautilus")
276+ self.addCleanup(
277+ self.process_manager.unregister_known_application,
278+ "Nautilus"
279+ )
280+ self.process_manager.register_known_application(
281+ "Nautilus",
282+ "nautilus.desktop",
283+ "nautilus"
284+ )
285
286 def get_startup_notification_timestamp(self, bamf_window):
287 atom = display.Display().intern_atom('_NET_WM_USER_TIME')
288 atom_type = display.Display().intern_atom('CARDINAL')
289- return bamf_window.x_win.get_property(atom, atom_type, 0, 1024).value[0]
290+ return bamf_window.x_win.get_property(
291+ atom,
292+ atom_type,
293+ 0,
294+ 1024
295+ ).value[0]
296
297 def call_gsettings_cmd(self, command, schema, *args):
298 """Set a desktop wide gsettings option
299@@ -297,16 +360,27 @@
300 :raises: **KeyError** if the option named does not exist.
301
302 """
303- old_value = self._set_compiz_option(plugin_name, option_name, option_value)
304+ old_value = self._set_compiz_option(
305+ plugin_name,
306+ option_name,
307+ option_value
308+ )
309 # Cleanup is LIFO, during clean-up also allow unity to respond
310 self.addCleanup(time.sleep, 0.5)
311- self.addCleanup(self._set_compiz_option, plugin_name, option_name, old_value)
312+ self.addCleanup(
313+ self._set_compiz_option,
314+ plugin_name,
315+ option_name,
316+ old_value
317+ )
318 # Allow unity time to respond to the new setting.
319 time.sleep(0.5)
320
321 def _set_compiz_option(self, plugin_name, option_name, option_value):
322- log.info("Setting compiz option '%s' in plugin '%s' to %r",
323- option_name, plugin_name, option_value)
324+ log.info(
325+ "Setting compiz option '%s' in plugin '%s' to %r",
326+ option_name, plugin_name, option_value
327+ )
328 setting = get_compiz_setting(plugin_name, option_name)
329 old_value = setting.Value
330 setting.Value = option_value