Code review comment for lp:~pedro/mago/brasero-tests

Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

Thanks for your work. I've a few comments though.

* in mago/test_suite/brasero.py
 - The cleanup code is superfluous. If you really want to close the app during cleanup a call to self.application.close() should be enough.
 - The cleanup method could also remove the brasero.iso that the test just created
* mago/application/brasero.py
 - If /tmp/brasero.iso already exist then the test fails because a dialog is displayed asking to overwrite that file. Either check the existence of this dialog or remove /tmp/brasero.iso as a precondition of the test.
* General comment about style:
I'm not a style maniac, far from it, but there are some improvements here.
 - Don't mix tabs and spaces for indentation (brasero/brasero_basics.py)
 - I know that spaces are cheap but don't use them between the function name and the args e.g
NO: brasero = ooldtp.context (self.name)
YES: brasero = ooldtp.context(self.name)

NO: brasero.getchild (self.BTN_BURN).click ()
YES: brasero.getchild(self.BTN_BURN).click()

More infos at http://www.python.org/dev/peps/pep-0008/

I'll be happy to review it again after the changes.

review: Needs Fixing

« Back to merge proposal