Code review comment for lp:~diegosarmentero/ubuntuone-windows-installer/ui-form

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

* Can you please add a docstring for the FakeValidationDict class?

* This code:

1103 + temp_function = self.ui.validation_functions
1104 + self.ui.validation_functions = FakeValidationDict()
1105 + self.ui.validate_data_on_focus_changed(1)
1106 + self.assertTrue(getattr(FakeValidationDict, "valid0"))
1107 + self.assertFalse(getattr(FakeValidationDict, "valid1"))
1108 + self.assertFalse(getattr(FakeValidationDict, "valid2"))
1109 + self.assertFalse(getattr(FakeValidationDict, "valid3"))
1110 + self.ui.validation_functions = temp_function

does not guarantee that the validation_functions are restored. If any of the assertion fails, the code will be never retored. So, always, always call self.patch, like this:

1104 + self.patch(self.ui, 'validation_functions', FakeValidationDict())

if for some reason you can't use this, add a cleanup statement right after the assignment:

1103 + temp_function = self.ui.validation_functions
1104 + self.ui.validation_functions = FakeValidationDict()
1105 + self.addCleanup(setattr, self.ui, 'validation_functions', temp_function)

* No need to change the following, but you should notice that all the calls to self.ui.hide will not be executed if the test fails. So, ideally, the hide() call should be right next to the show() call, as a cleanup call:

+ self.ui.show()
+ self.addCleanup(ui.hide)

review: Needs Fixing

« Back to merge proposal