Merge lp:~jelmer/brz/fix-flaky into lp:brz

[Jelmer Vernooij]

Cope with very early disconnects during TCP Server tests.

Cope with very early disconnects during TCP Server tests.

The debian packages have carried these changes as patches for a while.

[Martin Packman (gz)]

Okay.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/492/

[Vincent Ladeuil (vila)]

Err, no

Please.

Disable those tests if you wish but if you want to fix them it's either:
- the easy way: no server and client sharing a socket in the same process
- the hard way: fix races when using server and client sharing a socket in the same process

You won't get the later by peppering blindly.

[Jelmer Vernooij (jelmer)]

This test has always been broken. We can either:

1) Remove the test altogether
2) Make the test just raise KnownFailure
3) The current solution

I don't have particularly strong opinion on what we do here, but we should somehow get rid of this error.

The current approach at least means we find out about the regression that the test checks for, even if we sometimes effectively don't run the test. We take a similar approach on test_test_server.py line 195

[Vincent Ladeuil (vila)]

Yup, that approach is sound even if the price is a reduced coverage.

[Jelmer Vernooij (jelmer)]

On Tue, Sep 18, 2018 at 08:37:26AM -0000, Vincent Ladeuil wrote:
> Yup, that approach is sound even if the price is a reduced coverage.
Which of the three approaches, just getting rid of the test?

--
Jelmer Vernooij <email address hidden>
PGP Key: https://www.jelmer.uk/D729A457.asc

[Vincent Ladeuil (vila)]

The one proposed here: b'', client.read().

But I don't think the original writing of that test implied having to do that, so the coverage is reduced.

Removing the test altogether is worse.

=== modified file 'breezy/tests/test_test_server.py'
--- breezy/tests/test_test_server.py	2018-07-24 20:32:24 +0000
+++ breezy/tests/test_test_server.py	2018-09-16 22:44:45 +0000
@@ -66,7 +66,7 @@
                 self.sock.shutdown(socket.SHUT_RDWR)
                 self.sock.close()
             except socket.error as e:
-                if e.errno in (errno.EBADF, errno.ENOTCONN):
+                if e.errno in (errno.EBADF, errno.ENOTCONN, errno.ECONNRESET):
                     # Right, the socket is already down
                     pass
                 else:
@@ -291,8 +291,12 @@
         server = self.get_server()
         client = self.get_client()
         server.server.serving = False
-        client.connect((server.host, server.port))
-        self.assertEqual(b'', client.read())
+        try:
+            client.connect((server.host, server.port))
+            self.assertEqual(b'', client.read())
+        except socket.error as e:
+            if e.errno != errno.ECONNRESET:
+                raise
 
 
 class TestTestingSmartServer(tests.TestCase):