Merge lp:~mvo/software-center/fix-more-dep8 into lp:software-center

Proposed by Michael Vogt on 2012-08-24
Status: Merged
Merged at revision: 3127
Proposed branch: lp:~mvo/software-center/fix-more-dep8
Merge into: lp:software-center
Diff against target: 262 lines (+43/-25)
4 files modified
softwarecenter/backend/installbackend_impl/aptd.py (+18/-15)
tests/test_aptd.py (+16/-10)
tests/test_logging.py (+3/-0)
tests/test_utils.py (+6/-0)
To merge this branch: bzr merge lp:~mvo/software-center/fix-more-dep8
Reviewer Review Type Date Requested Status
Gary Lasker (community) 2012-08-24 Approve on 2012-08-27
Review via email: mp+121138@code.launchpad.net

Description of the change

Some fixes related to the dep8 jenkins failures from:
https://jenkins.qa.ubuntu.com/job/quantal-adt-software-center/

This fixes the failures releated to the fact that adt runs the tests
as root so some stuff like "os.access()" will always work

To post a comment you must log in.
Gary Lasker (gary-lasker) wrote :

This is nice and clean. I really like how unittest.skipIf supports adding a description for the reason that a test is being skipped (and I like the descriptions you've used for them). Very nice, thank you, Michael!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/backend/installbackend_impl/aptd.py'
2--- softwarecenter/backend/installbackend_impl/aptd.py 2012-08-23 12:25:44 +0000
3+++ softwarecenter/backend/installbackend_impl/aptd.py 2012-08-24 09:09:38 +0000
4@@ -87,6 +87,7 @@
5
6
7 class AptdaemonTransaction(BaseTransaction):
8+
9 def __init__(self, trans):
10 self._trans = trans
11
12@@ -210,6 +211,8 @@
13 (GObject.TYPE_PYOBJECT, bool,)),
14 }
15
16+ LICENSE_KEY_SERVER = "ubuntu-production"
17+
18 def __init__(self):
19 GObject.GObject.__init__(self)
20
21@@ -258,7 +261,7 @@
22 TransactionTypes.REPAIR)
23 yield self._run_transaction(trans, None, None, None)
24 except Exception as error:
25- self._on_trans_error(error)
26+ self._on_trans_error(error, trans)
27
28 @inline_callbacks
29 def fix_incomplete_install(self):
30@@ -268,7 +271,7 @@
31 TransactionTypes.REPAIR)
32 yield self._run_transaction(trans, None, None, None)
33 except Exception as error:
34- self._on_trans_error(error)
35+ self._on_trans_error(error, trans)
36
37 # FIXME: upgrade add-ons here
38 @inline_callbacks
39@@ -285,7 +288,7 @@
40 yield self._run_transaction(trans, pkgname, appname, iconname,
41 metadata)
42 except Exception as error:
43- self._on_trans_error(error, pkgname)
44+ self._on_trans_error(error, trans, pkgname)
45
46 # broken
47 # @inline_callbacks
48@@ -327,7 +330,7 @@
49 yield self._run_transaction(trans, pkgname, appname, iconname,
50 metadata)
51 except Exception as error:
52- self._on_trans_error(error, pkgname)
53+ self._on_trans_error(error, trans, pkgname)
54
55 @inline_callbacks
56 def remove_multiple(self, apps, iconnames, addons_install=[],
57@@ -371,7 +374,7 @@
58 yield self._run_transaction(
59 trans, pkgname, appname, iconname, metadata)
60 except Exception as error:
61- self._on_trans_error(error, pkgname)
62+ self._on_trans_error(error, trans, pkgname)
63
64 @inline_callbacks
65 def install_multiple(self, apps, iconnames, addons_install=[],
66@@ -401,7 +404,7 @@
67 TransactionTypes.APPLY)
68 yield self._run_transaction(trans, pkgname, appname, iconname)
69 except Exception as error:
70- self._on_trans_error(error)
71+ self._on_trans_error(error, trans)
72
73 @inline_callbacks
74 def reload(self, sources_list=None, metadata=None):
75@@ -420,7 +423,7 @@
76 sources_list=sources_list, defer=True)
77 yield self._run_transaction(trans, None, None, None, metadata)
78 except Exception as error:
79- self._on_trans_error(error)
80+ self._on_trans_error(error, trans)
81 # note that the cache re-open will happen via the connected
82 # "transaction-finished" signal
83
84@@ -433,7 +436,7 @@
85 # signals
86 yield trans.run(defer=True)
87 except Exception as error:
88- self._on_trans_error(error, component)
89+ self._on_trans_error(error, trans, component)
90 return_value(None)
91 # now update the cache
92 yield self.reload()
93@@ -472,7 +475,7 @@
94 keyid, keyserver, defer=True)
95 yield self._run_transaction(trans, None, None, None, metadata)
96 except Exception as error:
97- self._on_trans_error(error)
98+ self._on_trans_error(error, trans)
99
100 @inline_callbacks
101 def add_sources_list_entry(self, source_entry, sourcepart=None):
102@@ -533,9 +536,8 @@
103 # system-wide keys
104 try:
105 self._logger.info("adding license key for '%s'" % pkgname)
106- server = "ubuntu-production"
107 trans = yield self.aptd_client.add_license_key(
108- pkgname, license_key_oauth, server)
109+ pkgname, license_key_oauth, self.LICENSE_KEY_SERVER)
110 yield self._run_transaction(trans, None, None, None)
111 except Exception as e:
112 self._logger.error("add_license_key: '%s'" % e)
113@@ -697,7 +699,7 @@
114 yield self._run_transaction(trans, app.pkgname, app.appname,
115 "", metadata)
116 except Exception as error:
117- self._on_trans_error(error, app.pkgname)
118+ self._on_trans_error(error, trans, app.pkgname)
119 # add license_key
120 # FIXME: aptd fails if there is a license_key_path already
121 # but I wonder if we should ease that restriction
122@@ -959,7 +961,7 @@
123 yield trans.set_meta_data(defer=True, **metadata)
124 yield trans.run(defer=True)
125 except Exception as error:
126- self._on_trans_error(error, pkgname)
127+ self._on_trans_error(error, trans, pkgname)
128 # on error we need to clean the pending purchases
129 self._clean_pending_purchases(pkgname)
130 # on success the pending purchase is cleaned when the package
131@@ -971,8 +973,9 @@
132 if pkgname and pkgname in self.pending_purchases:
133 del self.pending_purchases[pkgname]
134
135- def _on_trans_error(self, error, pkgname=None):
136- self._logger.warn("_on_trans_error: %s", error)
137+ def _on_trans_error(self, error, trans, pkgname=None):
138+ self._logger.warn("_on_trans_error: '%s' '%s' '%s'" % (
139+ error, trans, pkgname))
140 # re-enable the action button again if anything went wrong
141 result = TransactionFinishedResult(None, False)
142 result.pkgname = pkgname
143
144=== modified file 'tests/test_aptd.py'
145--- tests/test_aptd.py 2012-08-23 12:18:43 +0000
146+++ tests/test_aptd.py 2012-08-24 09:09:38 +0000
147@@ -9,6 +9,8 @@
148
149 import dbus
150
151+from gi.repository import GObject
152+
153 from tests.utils import (
154 setup_test_env,
155 )
156@@ -42,6 +44,7 @@
157 data = "some-data"
158 # test HOME
159 target = "~/.fasfasdfsdafdfsdafdsfa"
160+ self.addCleanup(lambda: os.remove(os.path.expanduser(target)))
161 pkgname = "2vcard"
162 json_auth = ""
163 yield self.aptd.add_license_key(data, target, json_auth, pkgname)
164@@ -50,26 +53,29 @@
165 data2 = "other-data"
166 yield self.aptd.add_license_key(data2, target, json_auth, pkgname)
167 self.assertEqual(open(os.path.expanduser(target)).read(), data)
168- # cleanup
169- os.remove(os.path.expanduser(target))
170
171+ @unittest.skipIf(os.getuid() != 0,
172+ "test_add_license_key_opt test needs to run as root")
173+ @unittest.skipIf(not "SC_TEST_JSON" in os.environ,
174+ "Need a SC_TEST_JSON environment with the credentials")
175 def test_add_license_key_opt(self):
176- if os.getuid() != 0:
177- logging.info("skipping add_license_key_opt test")
178- return
179 # test /opt
180- data = "some-data"
181+ license_key = "some-data"
182 pkgname = "hellox"
183 path = "/opt/hellox/conf/license-key.txt"
184+ self.addCleanup(lambda: os.remove(path))
185 json_auth = os.environ.get("SC_TEST_JSON") or "no-json-auth"
186 def _error(*args):
187 print "errror", args
188 self.aptd.ui = Mock()
189+ self.aptd.LICENSE_KEY_SERVER = "ubuntu-staging"
190 self.aptd.ui.error = _error
191 @inline_callbacks
192 def run():
193- yield self.aptd.add_license_key(data, path, json_auth, pkgname)
194- aptdaemon.loop.mainloop.quit()
195+ yield self.aptd.add_license_key(
196+ license_key, path, json_auth, pkgname)
197+ # ensure signals get delivered before quit()
198+ GObject.timeout_add(500, lambda: aptdaemon.loop.mainloop.quit())
199 # run the callback
200 run()
201 aptdaemon.loop.mainloop.run()
202@@ -111,7 +117,6 @@
203 addons_remove = ["gimp-plugin-registry"]
204 yield self.aptd.apply_changes(pkgname, appname ,iconname, addons_install, addons_remove)
205
206-
207 def test_trans_error_ui_display(self):
208 """ test if the right error ui is displayed for various dbus
209 errors
210@@ -127,7 +132,7 @@
211 error_ui_mock.reset()
212 dbus_name_mock.return_value = dbus_name
213 error.get_dbus_name = dbus_name_mock
214- self.aptd._on_trans_error(error)
215+ self.aptd._on_trans_error(error, Mock())
216 self.assertEqual(error_ui_mock.called, show_error_ui)
217
218 @inline_callbacks
219@@ -164,4 +169,5 @@
220
221
222 if __name__ == "__main__":
223+ logging.basicConfig(level=logging.INFO)
224 unittest.main()
225
226=== modified file 'tests/test_logging.py'
227--- tests/test_logging.py 2012-05-30 18:39:55 +0000
228+++ tests/test_logging.py 2012-08-24 09:09:38 +0000
229@@ -17,6 +17,9 @@
230 class TestLogging(unittest.TestCase):
231 """Tests for the sc logging facilities."""
232
233+ @unittest.skipIf(
234+ os.getuid() == 0,
235+ "fails when run as root as os.access() is always successful")
236 def test_no_write_access_for_cache_dir(self):
237 """Test for bug LP: #688682."""
238 cache_dir = './foo'
239
240=== modified file 'tests/test_utils.py'
241--- tests/test_utils.py 2012-08-21 09:05:53 +0000
242+++ tests/test_utils.py 2012-08-24 09:09:38 +0000
243@@ -189,6 +189,9 @@
244 # ensure it does not crash for empty strings, LP: #1002271
245 self.assertEqual(capitalize_first_word(""), "")
246
247+ @unittest.skipIf(
248+ os.getuid() == 0,
249+ "fails when run as root as os.access() is always successful")
250 def test_ensure_file_writable_and_delete_if_not(self):
251 # first test that a non-writable file (0400) is deleted
252 test_file_not_writeable = NamedTemporaryFile()
253@@ -203,6 +206,9 @@
254 ensure_file_writable_and_delete_if_not(test_file_writeable.name)
255 self.assertTrue(os.path.exists(test_file_writeable.name))
256
257+ @unittest.skipIf(
258+ os.getuid() == 0,
259+ "fails when run as root as os.chmod() is always successful")
260 def test_safe_makedirs(self):
261 tmp = mkdtemp()
262 # create base dir

Subscribers

People subscribed via source and target branches