Code review comment for lp://qastaging/~mikemc/ubuntuone-control-panel/launchdaemon

Revision history for this message
Manuel de la Peña (mandel) wrote :

In the following code:

354 + try:
355 + remove_fsevents_daemon(authRef)
356 + except Exception, e:
357 + logger.exception("Problem removing running daemon: %r" % e)
358 +
359 + AuthorizationFree(authRef, kAuthorizationFlagDestroyRights)

shouldn't the Free be called in a finally clause?

In the InstallDaemonTestCase you can move the following to the setup instead of calling it in each of the tests:

469 + self._patch_and_track(utils.darwin,
470 + [('get_authorization', 'Fake AuthRef'),
471 + ('remove_fsevents_daemon', None),
472 + ('install_fsevents_daemon', None),
473 + ('AuthorizationFree', None)])

review: Needs Fixing

« Back to merge proposal