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
=== modified file 'softwarecenter/backend/installbackend_impl/aptd.py'
--- softwarecenter/backend/installbackend_impl/aptd.py 2012-08-23 12:25:44 +0000
+++ softwarecenter/backend/installbackend_impl/aptd.py 2012-08-24 09:09:38 +0000
@@ -87,6 +87,7 @@
8787
8888
89class AptdaemonTransaction(BaseTransaction):89class AptdaemonTransaction(BaseTransaction):
90
90 def __init__(self, trans):91 def __init__(self, trans):
91 self._trans = trans92 self._trans = trans
9293
@@ -210,6 +211,8 @@
210 (GObject.TYPE_PYOBJECT, bool,)),211 (GObject.TYPE_PYOBJECT, bool,)),
211 }212 }
212213
214 LICENSE_KEY_SERVER = "ubuntu-production"
215
213 def __init__(self):216 def __init__(self):
214 GObject.GObject.__init__(self)217 GObject.GObject.__init__(self)
215218
@@ -258,7 +261,7 @@
258 TransactionTypes.REPAIR)261 TransactionTypes.REPAIR)
259 yield self._run_transaction(trans, None, None, None)262 yield self._run_transaction(trans, None, None, None)
260 except Exception as error:263 except Exception as error:
261 self._on_trans_error(error)264 self._on_trans_error(error, trans)
262265
263 @inline_callbacks266 @inline_callbacks
264 def fix_incomplete_install(self):267 def fix_incomplete_install(self):
@@ -268,7 +271,7 @@
268 TransactionTypes.REPAIR)271 TransactionTypes.REPAIR)
269 yield self._run_transaction(trans, None, None, None)272 yield self._run_transaction(trans, None, None, None)
270 except Exception as error:273 except Exception as error:
271 self._on_trans_error(error)274 self._on_trans_error(error, trans)
272275
273 # FIXME: upgrade add-ons here276 # FIXME: upgrade add-ons here
274 @inline_callbacks277 @inline_callbacks
@@ -285,7 +288,7 @@
285 yield self._run_transaction(trans, pkgname, appname, iconname,288 yield self._run_transaction(trans, pkgname, appname, iconname,
286 metadata)289 metadata)
287 except Exception as error:290 except Exception as error:
288 self._on_trans_error(error, pkgname)291 self._on_trans_error(error, trans, pkgname)
289292
290# broken293# broken
291# @inline_callbacks294# @inline_callbacks
@@ -327,7 +330,7 @@
327 yield self._run_transaction(trans, pkgname, appname, iconname,330 yield self._run_transaction(trans, pkgname, appname, iconname,
328 metadata)331 metadata)
329 except Exception as error:332 except Exception as error:
330 self._on_trans_error(error, pkgname)333 self._on_trans_error(error, trans, pkgname)
331334
332 @inline_callbacks335 @inline_callbacks
333 def remove_multiple(self, apps, iconnames, addons_install=[],336 def remove_multiple(self, apps, iconnames, addons_install=[],
@@ -371,7 +374,7 @@
371 yield self._run_transaction(374 yield self._run_transaction(
372 trans, pkgname, appname, iconname, metadata)375 trans, pkgname, appname, iconname, metadata)
373 except Exception as error:376 except Exception as error:
374 self._on_trans_error(error, pkgname)377 self._on_trans_error(error, trans, pkgname)
375378
376 @inline_callbacks379 @inline_callbacks
377 def install_multiple(self, apps, iconnames, addons_install=[],380 def install_multiple(self, apps, iconnames, addons_install=[],
@@ -401,7 +404,7 @@
401 TransactionTypes.APPLY)404 TransactionTypes.APPLY)
402 yield self._run_transaction(trans, pkgname, appname, iconname)405 yield self._run_transaction(trans, pkgname, appname, iconname)
403 except Exception as error:406 except Exception as error:
404 self._on_trans_error(error)407 self._on_trans_error(error, trans)
405408
406 @inline_callbacks409 @inline_callbacks
407 def reload(self, sources_list=None, metadata=None):410 def reload(self, sources_list=None, metadata=None):
@@ -420,7 +423,7 @@
420 sources_list=sources_list, defer=True)423 sources_list=sources_list, defer=True)
421 yield self._run_transaction(trans, None, None, None, metadata)424 yield self._run_transaction(trans, None, None, None, metadata)
422 except Exception as error:425 except Exception as error:
423 self._on_trans_error(error)426 self._on_trans_error(error, trans)
424 # note that the cache re-open will happen via the connected427 # note that the cache re-open will happen via the connected
425 # "transaction-finished" signal428 # "transaction-finished" signal
426429
@@ -433,7 +436,7 @@
433 # signals436 # signals
434 yield trans.run(defer=True)437 yield trans.run(defer=True)
435 except Exception as error:438 except Exception as error:
436 self._on_trans_error(error, component)439 self._on_trans_error(error, trans, component)
437 return_value(None)440 return_value(None)
438 # now update the cache441 # now update the cache
439 yield self.reload()442 yield self.reload()
@@ -472,7 +475,7 @@
472 keyid, keyserver, defer=True)475 keyid, keyserver, defer=True)
473 yield self._run_transaction(trans, None, None, None, metadata)476 yield self._run_transaction(trans, None, None, None, metadata)
474 except Exception as error:477 except Exception as error:
475 self._on_trans_error(error)478 self._on_trans_error(error, trans)
476479
477 @inline_callbacks480 @inline_callbacks
478 def add_sources_list_entry(self, source_entry, sourcepart=None):481 def add_sources_list_entry(self, source_entry, sourcepart=None):
@@ -533,9 +536,8 @@
533 # system-wide keys536 # system-wide keys
534 try:537 try:
535 self._logger.info("adding license key for '%s'" % pkgname)538 self._logger.info("adding license key for '%s'" % pkgname)
536 server = "ubuntu-production"
537 trans = yield self.aptd_client.add_license_key(539 trans = yield self.aptd_client.add_license_key(
538 pkgname, license_key_oauth, server)540 pkgname, license_key_oauth, self.LICENSE_KEY_SERVER)
539 yield self._run_transaction(trans, None, None, None)541 yield self._run_transaction(trans, None, None, None)
540 except Exception as e:542 except Exception as e:
541 self._logger.error("add_license_key: '%s'" % e)543 self._logger.error("add_license_key: '%s'" % e)
@@ -697,7 +699,7 @@
697 yield self._run_transaction(trans, app.pkgname, app.appname,699 yield self._run_transaction(trans, app.pkgname, app.appname,
698 "", metadata)700 "", metadata)
699 except Exception as error:701 except Exception as error:
700 self._on_trans_error(error, app.pkgname)702 self._on_trans_error(error, trans, app.pkgname)
701 # add license_key703 # add license_key
702 # FIXME: aptd fails if there is a license_key_path already704 # FIXME: aptd fails if there is a license_key_path already
703 # but I wonder if we should ease that restriction705 # but I wonder if we should ease that restriction
@@ -959,7 +961,7 @@
959 yield trans.set_meta_data(defer=True, **metadata)961 yield trans.set_meta_data(defer=True, **metadata)
960 yield trans.run(defer=True)962 yield trans.run(defer=True)
961 except Exception as error:963 except Exception as error:
962 self._on_trans_error(error, pkgname)964 self._on_trans_error(error, trans, pkgname)
963 # on error we need to clean the pending purchases965 # on error we need to clean the pending purchases
964 self._clean_pending_purchases(pkgname)966 self._clean_pending_purchases(pkgname)
965 # on success the pending purchase is cleaned when the package967 # on success the pending purchase is cleaned when the package
@@ -971,8 +973,9 @@
971 if pkgname and pkgname in self.pending_purchases:973 if pkgname and pkgname in self.pending_purchases:
972 del self.pending_purchases[pkgname]974 del self.pending_purchases[pkgname]
973975
974 def _on_trans_error(self, error, pkgname=None):976 def _on_trans_error(self, error, trans, pkgname=None):
975 self._logger.warn("_on_trans_error: %s", error)977 self._logger.warn("_on_trans_error: '%s' '%s' '%s'" % (
978 error, trans, pkgname))
976 # re-enable the action button again if anything went wrong979 # re-enable the action button again if anything went wrong
977 result = TransactionFinishedResult(None, False)980 result = TransactionFinishedResult(None, False)
978 result.pkgname = pkgname981 result.pkgname = pkgname
979982
=== modified file 'tests/test_aptd.py'
--- tests/test_aptd.py 2012-08-23 12:18:43 +0000
+++ tests/test_aptd.py 2012-08-24 09:09:38 +0000
@@ -9,6 +9,8 @@
99
10import dbus10import dbus
1111
12from gi.repository import GObject
13
12from tests.utils import (14from tests.utils import (
13 setup_test_env,15 setup_test_env,
14)16)
@@ -42,6 +44,7 @@
42 data = "some-data"44 data = "some-data"
43 # test HOME45 # test HOME
44 target = "~/.fasfasdfsdafdfsdafdsfa"46 target = "~/.fasfasdfsdafdfsdafdsfa"
47 self.addCleanup(lambda: os.remove(os.path.expanduser(target)))
45 pkgname = "2vcard"48 pkgname = "2vcard"
46 json_auth = ""49 json_auth = ""
47 yield self.aptd.add_license_key(data, target, json_auth, pkgname)50 yield self.aptd.add_license_key(data, target, json_auth, pkgname)
@@ -50,26 +53,29 @@
50 data2 = "other-data"53 data2 = "other-data"
51 yield self.aptd.add_license_key(data2, target, json_auth, pkgname)54 yield self.aptd.add_license_key(data2, target, json_auth, pkgname)
52 self.assertEqual(open(os.path.expanduser(target)).read(), data)55 self.assertEqual(open(os.path.expanduser(target)).read(), data)
53 # cleanup
54 os.remove(os.path.expanduser(target))
5556
57 @unittest.skipIf(os.getuid() != 0,
58 "test_add_license_key_opt test needs to run as root")
59 @unittest.skipIf(not "SC_TEST_JSON" in os.environ,
60 "Need a SC_TEST_JSON environment with the credentials")
56 def test_add_license_key_opt(self):61 def test_add_license_key_opt(self):
57 if os.getuid() != 0:
58 logging.info("skipping add_license_key_opt test")
59 return
60 # test /opt62 # test /opt
61 data = "some-data"63 license_key = "some-data"
62 pkgname = "hellox"64 pkgname = "hellox"
63 path = "/opt/hellox/conf/license-key.txt"65 path = "/opt/hellox/conf/license-key.txt"
66 self.addCleanup(lambda: os.remove(path))
64 json_auth = os.environ.get("SC_TEST_JSON") or "no-json-auth"67 json_auth = os.environ.get("SC_TEST_JSON") or "no-json-auth"
65 def _error(*args):68 def _error(*args):
66 print "errror", args69 print "errror", args
67 self.aptd.ui = Mock()70 self.aptd.ui = Mock()
71 self.aptd.LICENSE_KEY_SERVER = "ubuntu-staging"
68 self.aptd.ui.error = _error72 self.aptd.ui.error = _error
69 @inline_callbacks73 @inline_callbacks
70 def run():74 def run():
71 yield self.aptd.add_license_key(data, path, json_auth, pkgname)75 yield self.aptd.add_license_key(
72 aptdaemon.loop.mainloop.quit()76 license_key, path, json_auth, pkgname)
77 # ensure signals get delivered before quit()
78 GObject.timeout_add(500, lambda: aptdaemon.loop.mainloop.quit())
73 # run the callback79 # run the callback
74 run()80 run()
75 aptdaemon.loop.mainloop.run()81 aptdaemon.loop.mainloop.run()
@@ -111,7 +117,6 @@
111 addons_remove = ["gimp-plugin-registry"]117 addons_remove = ["gimp-plugin-registry"]
112 yield self.aptd.apply_changes(pkgname, appname ,iconname, addons_install, addons_remove)118 yield self.aptd.apply_changes(pkgname, appname ,iconname, addons_install, addons_remove)
113119
114
115 def test_trans_error_ui_display(self):120 def test_trans_error_ui_display(self):
116 """ test if the right error ui is displayed for various dbus 121 """ test if the right error ui is displayed for various dbus
117 errors122 errors
@@ -127,7 +132,7 @@
127 error_ui_mock.reset()132 error_ui_mock.reset()
128 dbus_name_mock.return_value = dbus_name133 dbus_name_mock.return_value = dbus_name
129 error.get_dbus_name = dbus_name_mock134 error.get_dbus_name = dbus_name_mock
130 self.aptd._on_trans_error(error)135 self.aptd._on_trans_error(error, Mock())
131 self.assertEqual(error_ui_mock.called, show_error_ui)136 self.assertEqual(error_ui_mock.called, show_error_ui)
132137
133 @inline_callbacks138 @inline_callbacks
@@ -164,4 +169,5 @@
164169
165170
166if __name__ == "__main__":171if __name__ == "__main__":
172 logging.basicConfig(level=logging.INFO)
167 unittest.main()173 unittest.main()
168174
=== modified file 'tests/test_logging.py'
--- tests/test_logging.py 2012-05-30 18:39:55 +0000
+++ tests/test_logging.py 2012-08-24 09:09:38 +0000
@@ -17,6 +17,9 @@
17class TestLogging(unittest.TestCase):17class TestLogging(unittest.TestCase):
18 """Tests for the sc logging facilities."""18 """Tests for the sc logging facilities."""
1919
20 @unittest.skipIf(
21 os.getuid() == 0,
22 "fails when run as root as os.access() is always successful")
20 def test_no_write_access_for_cache_dir(self):23 def test_no_write_access_for_cache_dir(self):
21 """Test for bug LP: #688682."""24 """Test for bug LP: #688682."""
22 cache_dir = './foo'25 cache_dir = './foo'
2326
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py 2012-08-21 09:05:53 +0000
+++ tests/test_utils.py 2012-08-24 09:09:38 +0000
@@ -189,6 +189,9 @@
189 # ensure it does not crash for empty strings, LP: #1002271189 # ensure it does not crash for empty strings, LP: #1002271
190 self.assertEqual(capitalize_first_word(""), "")190 self.assertEqual(capitalize_first_word(""), "")
191191
192 @unittest.skipIf(
193 os.getuid() == 0,
194 "fails when run as root as os.access() is always successful")
192 def test_ensure_file_writable_and_delete_if_not(self):195 def test_ensure_file_writable_and_delete_if_not(self):
193 # first test that a non-writable file (0400) is deleted196 # first test that a non-writable file (0400) is deleted
194 test_file_not_writeable = NamedTemporaryFile()197 test_file_not_writeable = NamedTemporaryFile()
@@ -203,6 +206,9 @@
203 ensure_file_writable_and_delete_if_not(test_file_writeable.name)206 ensure_file_writable_and_delete_if_not(test_file_writeable.name)
204 self.assertTrue(os.path.exists(test_file_writeable.name))207 self.assertTrue(os.path.exists(test_file_writeable.name))
205208
209 @unittest.skipIf(
210 os.getuid() == 0,
211 "fails when run as root as os.chmod() is always successful")
206 def test_safe_makedirs(self):212 def test_safe_makedirs(self):
207 tmp = mkdtemp()213 tmp = mkdtemp()
208 # create base dir214 # create base dir

Subscribers

People subscribed via source and target branches