Code review comment for lp:~diegosarmentero/ubuntuone-windows-installer/connect-files

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

So, perhaps I was not clear before... but I still see no point in having this patched connect_files method:

    def connect_files(self, func=None):
        """Fake connect_files"""
        self.properties['connect_files_called'] = True
        self.properties['connect_files'] = func

Why do we need the func=None parameter, if the original method does not receive any paramter? Why are we storing the func value, if then we're just passing None and testing against it?

Also, the self._called = self has no much sense as well, as far as I can see.

If I understand the fake correctly, I think this is a simpler option:

class FakeBackend(object):

    """Fake ControlBackend."""

    def __init__(self, *args, **kwargs):
        self.called = []
        self.clear = lambda: None
        self.connect_files = lambda: self.called.append('connect_files')

and have the test only test for assertEqual(called, ['connect_files']).

review: Needs Fixing

« Back to merge proposal