* instead of duplicating the -i "test_windows.py" in run-tests, please define a new bash variable and use that
* we're trying to get rid of the authors section of the source headers, so instead of adding yourself, please remove all the authors in the files you modify :-)
* remember to always use %r for logging stuff
* why are we receiving the logger by parameter? that makes the api of the methods weird... let's grab the logger just like in the rest of the app (and then we can remove the fake logger altogether):
from ubuntuone.controlpanel import set_up_logger
logger = set_up_logger('utils')
* the logging itself could use some improvement, so when seen in a log file our of the code context, we can understand what information is giving us. So below I propose a new version of are_updates_present:
@defer.inlineCallbacks
def are_updates_present():
"""Return if there are updates for Ubuntu One."""
result = False
retcode = None
update_path = _get_update_path()
if update_path is not None:
# If there is an update present we will get 0, non-zero otherwise
retcode = yield getProcessValue(update_path, args=('--mode', 'unattended'), path=os.path.dirname(update_path))
result = retcode == 0
logger.debug('are_updates_present: update path: %r, return code: %r, result: %r', update_path, retcode, result)
defer.returnValue(result)
* Remember to have cleanup code to be run ALWAYS, even if the test fails. In your tests, the cleanup code is only run if none of the asserts failed. So, please use self.addCleanup calls as soon as you modify the env for the test.
A few minor tweaks:
* instead of duplicating the -i "test_windows.py" in run-tests, please define a new bash variable and use that
* we're trying to get rid of the authors section of the source headers, so instead of adding yourself, please remove all the authors in the files you modify :-)
* remember to always use %r for logging stuff
* why are we receiving the logger by parameter? that makes the api of the methods weird... let's grab the logger just like in the rest of the app (and then we can remove the fake logger altogether):
from ubuntuone. controlpanel import set_up_logger
logger = set_up_ logger( 'utils' )
* the logging itself could use some improvement, so when seen in a log file our of the code context, we can understand what information is giving us. So below I propose a new version of are_updates_ present:
@defer. inlineCallbacks present( ): (update_ path, args=('--mode',
'unattende d'), path=os. path.dirname( update_ path)) debug(' are_updates_ present: update path: %r, return code: %r, result: %r',
update_ path, retcode, result) returnValue( result)
def are_updates_
"""Return if there are updates for Ubuntu One."""
result = False
retcode = None
update_path = _get_update_path()
if update_path is not None:
# If there is an update present we will get 0, non-zero otherwise
retcode = yield getProcessValue
result = retcode == 0
logger.
defer.
* Remember to have cleanup code to be run ALWAYS, even if the test fails. In your tests, the cleanup code is only run if none of the asserts failed. So, please use self.addCleanup calls as soon as you modify the env for the test.