transport implementation test scenarios and rabbit-holing

Michael Hudson michael.hudson at canonical.com
Mon Sep 1 07:14:13 BST 2008


In bzrlib.tests.test_transport.TestTransportImplementation.get_transport 
we find this chunk of code:

        # try getting the transport via the regular interface:
        t = get_transport(url)
        # vila--20070607 if the following are commented out the test suite
        # still pass. Is this really still needed or was it a forgotten
        # temporary fix ?
        if not isinstance(t, self.transport_class):
            # we did not get the correct transport class type. Override the
            # regular connection behaviour by direct construction.
            t = self.transport_class(url)
        return t

As this code caused me minor inconvenience when arranging to test a transport
internal to Launchpad with the bzrlib implementation tests, I was inspired to
check whether vila's comment is still correct, and it is: you can comment the
check out or even make the test explode if 't' is not an instance of 
self.transport_class and all the tests still pass.

So, it seemed to me that one might as well delete the check, which I did.  
Then I found that this was the only use of transport_class in the tests, so I
killed it and chased upwards, removing it from the scenario multiplcation code
and in the end, the various get_test_permutations functions.  Then I found this,
in bzrlib.transport.ftp._gssapi:

def get_test_permutations():
    """Return the permutations to be used in testing."""
    from bzrlib import tests
    if tests.FTPServerFeature.available():
        from bzrlib.tests import ftp_server
        return [(GSSAPIFtpTransport, ftp_server.FTPServer)]
    else:
        return []

If you know or check how the pieces fit together, you'll see that this looks
like a case that depends on the check I talked about deleting above.  But how
can the tests pass even if I put a "1/0" in the 'then' part of the now-deleted
if statement?  Well, because this function is never called, it seems.  And now
I've run out of time for today, and possibly energy for this problem, but I've
attached the bundle of as far as I've gotten so far.  But it seems like a bug
that the _gssapi stuff is not tested, does it not?

Cheers,
mwh


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: remove-test_transport-cruft-progress.bundle
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20080901/5757577c/attachment-0001.diff 


More information about the bazaar mailing list