[MERGE] Fix test suite regression on OSX in TestReadMergeableFromUrl.test_smart_server_connection_reset.

Andrew Bennetts andrew at canonical.com
Tue Sep 30 09:09:08 BST 2008


Vincent Ladeuil wrote:
> revno 3703.2.1 'Allow ConnectionReset to propagate from
> read_mergeable_from_url' added a test that fails on OSX.
> 
> It fails sometimes only as is often the case when sockets are
> involved (python on OSX often produces different execution orders
> when several threads are involved) which made it a bit hard to
> reproduce and may explain why it doesn't seem to occur on linux.
> 
> The attached patch fixed the issue but I'd appreciate if Andrew
> can review it since it may not be the right approach (or more
> precisely I suspect there is a better one or more complete one
> but I can't put my finger on it).

bb:resubmit

Out of interest, what does the stacktrace for the failing test look
like?  I think I can guess, but it'd be good to avoid unnecessary
guesswork :)

I don't think this is the right fix.  This seems to be the wrong layer
to be catching socket.errors.  I think we really need to make the
various medium implementations present a consistent interface as far as
possible, or we'll inevitably have to put this sort of band-aid in other
places.  It's the medium that should be translating the low-level
details of delivering and receiving bytes into a simple
read_bytes/accept_bytes interface.  socket.errors should be an
implementation detail of one particular medium. 

So I think the relevant SmartClientMedium (in this case
SmartTCPClientMedium I guess) should be catching this error and
translating it into something consistent for the caller, which at the
moment would be to return ''.  So that would mean updating
SmartTCPClientMedium._read_bytes.  (Or
SmartClientHTTPMediumRequest._read_bytes?)

I'm starting to think that the right thing to do would be to make the
medium become responsible for raising ConnectionReset, rather than the
caller of read_bytes.  This is probably a deeper change than necessary
for fixing this particular bug, so I wouldn't demand it happen
immediately.  But I think we should be moving in this direction.

-Andrew.




More information about the bazaar mailing list