Code review comment for lp:~mandel/ubuntuone-control-panel/auto-update-looping-call

This style of coding tests is very ugly and error prone:

534 + def _clean_fake_window_class(self):
535 + """Clean the fake window class."""
536 + FakeMainWindow.called = []
537 + FakeMainWindow.close_callback = None
538 +
539 + def _clean_fake_icon_class(self):
540 + """Clean the fake icon class."""
541 + FakeTrayIcon.window = None
542 + FakeTrayIcon.auto_update = None
543 +
544 + def _clean_fake_looping_call_class(self):
545 + """Clean the fake looping call class."""
546 + FakeLoopingCall.function = None
547 + FakeLoopingCall.args = None
548 + FakeLoopingCall.kwargs = None
549 + FakeLoopingCall.called = []

Instead of patching with a class that gets its attributes overwritten, please patch with an instance that gets thrown away, so there's no need to reset the class attributes like in the code above.

For instance:
"self.patch(gui, 'LoopingCall', FakeLoopingCall)" ->
"self.patch(gui, 'LoopingCall', FakeLoopingCallClass())"

This is present a few times in this branch, so make sure all occurrences end up fixed.

---

Also, to test code that uses LoopingCalls (and reactor.callLaters, too) remember that you can use twisted.internet.task.Clock, by patching the LoopingCall.clock attribute. Then you will be able to fake the reactor moving forward in time by calling clock.advance.

This gives you a lot of flexibility to test this kind of code, and the tests end up a lot faster and reliable than if they were using a small value for the LoopingCall or callLater time.