Merge lp:~vila/bzr/1451448-skip-racy-tests into lp:bzr

[Vincent Ladeuil]

Fix test failures blocking package builds.

This MP fixes the failures encountered since python 2.7.9 (see bug).

The http tests are the most trivial http tests so I'm confident the code is ok and that a *new* race is exposed. Moreover it's exposed only when the http client and server are running in the same thread.

I also skip the windows tests are this needs to be fixed and tested on windows (and I don't have access to a windows host) and that shouldn't block other platforms.

[Richard Wilbur (richard-wilbur)]

Thanks for the fixes, Vincent. Both situations seem suspect: running the http client and server in the same thread seems prone to races, running windows filesystem-specific tests on a non-windows filesystem sounds like it would be likely to fail.
+1

review: Approve

[Vincent Ladeuil (vila)]

Thanks for the review.

> Thanks for the fixes, Vincent. Both situations seem suspect:

> running the http client and server in the same thread seems prone to races,

Historically, all client/server tests has been written this way since day one. It took years to run into the races and realize the root cause was that the python library was not intended to support both the client and the server in this context.

So, after the fact, it seems obvious that this was a bad design decision but the thing is: when everything works and once in a blue moon a test fails, nobody suspects that design... ;)

> running
> windows filesystem-specific tests on a non-windows filesystem sounds like it
> would be likely to fail.

Well, history again, it was easier at one point to be able to run those tests on ubuntu to make sure the windows support wasn't regressing until tests can be run there. Nowadays... it's more complicated.

Hope this shed some light on the past ;)

[Richard Wilbur (richard-wilbur)]

On Mon, Sep 7, 2015 at 2:32 AM, Vincent Ladeuil <email address hidden> wrote:
> Thanks for the review.

I'm down to one machine that boots off a CD (won't boot from HD any
more :(, so I get on to check my E-mail but haven't bothered trying to
set up hydrazine, etc. Thus I can review and Approve, but not merge
till I finish fixing at least one of these machines.

[...]
> Hope this shed some light on the past ;)

I quite appreciate the history lesson. Thank you!

Can you send this to PQM? If not, let me know and it will be a good
excuse to concentrate on fixing at least one of my computers.

[Vincent Ladeuil (vila)]

> On Mon, Sep 7, 2015 at 2:32 AM, Vincent Ladeuil <email address hidden> wrote:
> > Thanks for the review.
>
> I'm down to one machine that boots off a CD (won't boot from HD any
> more :(,

:-/

> so I get on to check my E-mail but haven't bothered trying to
> set up hydrazine, etc. Thus I can review and Approve, but not merge
> till I finish fixing at least one of these machines.

ack.

>
> [...]
> > Hope this shed some light on the past ;)
>
> I quite appreciate the history lesson. Thank you!
>
> Can you send this to PQM? If not, let me know and it will be a good
> excuse to concentrate on fixing at least one of my computers.

Still waiting for PQM to resurrect (the related ticket priority has been bumped so crossed-fingers-hoping the fix will happen rsn).

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/tests/test_http.py'
--- bzrlib/tests/test_http.py	2012-09-17 06:58:15 +0000
+++ bzrlib/tests/test_http.py	2015-09-08 08:00:02 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2011 Canonical Ltd
+# Copyright (C) 2005-2012, 2015 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -656,13 +656,28 @@
 
     _req_handler_class = BadStatusRequestHandler
 
+    def setUp(self):
+        super(TestBadStatusServer, self).setUp()
+        # See https://bugs.launchpad.net/bzr/+bug/1451448 for details.
+        # TD;LR: Running both a TCP client and server in the same process and
+        # thread uncovers a race in python. The fix is to run the server in a
+        # different process. Trying to fix yet another race here is not worth
+        # the effort. -- vila 2015-09-06
+        if 'HTTP/1.0' in self.id():
+            raise tests.TestSkipped(
+                'Client/Server in the same process and thread can hang')
+
     def test_http_has(self):
         t = self.get_readonly_transport()
-        self.assertRaises(errors.InvalidHttpResponse, t.has, 'foo/bar')
+        self.assertRaises((errors.ConnectionError, errors.ConnectionReset,
+                           errors.InvalidHttpResponse),
+                          t.has, 'foo/bar')
 
     def test_http_get(self):
         t = self.get_readonly_transport()
-        self.assertRaises(errors.InvalidHttpResponse, t.get, 'foo/bar')
+        self.assertRaises((errors.ConnectionError, errors.ConnectionReset,
+                           errors.InvalidHttpResponse),
+                          t.get, 'foo/bar')
 
 
 class InvalidStatusRequestHandler(http_server.TestingHTTPRequestHandler):

=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py	2014-04-09 08:05:23 +0000
+++ bzrlib/tests/test_osutils.py	2015-09-08 08:00:02 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2011 Canonical Ltd
+# Copyright (C) 2005-2015 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -925,6 +925,7 @@
     """Test that _win32 versions of os utilities return appropriate paths."""
 
     def test_abspath(self):
+        self.requireFeature(features.win32_feature)
         self.assertEqual('C:/foo', osutils._win32_abspath('C:\\foo'))
         self.assertEqual('C:/foo', osutils._win32_abspath('C:/foo'))
         self.assertEqual('//HOST/path', osutils._win32_abspath(r'\\HOST\path'))
@@ -977,6 +978,7 @@
         self.assertEqual('C:\\foo', osutils._win32_fixdrive('c:\\foo'))
 
     def test_win98_abspath(self):
+        self.requireFeature(features.win32_feature)
         # absolute path
         self.assertEqual('C:/foo', osutils._win98_abspath('C:\\foo'))
         self.assertEqual('C:/foo', osutils._win98_abspath('C:/foo'))

=== modified file 'bzrlib/tests/test_transport.py'
--- bzrlib/tests/test_transport.py	2011-12-14 19:44:40 +0000
+++ bzrlib/tests/test_transport.py	2015-09-08 08:00:02 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2011 Canonical Ltd
+# Copyright (C) 2005-2011, 2015 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -815,6 +815,7 @@
 class TestWin32LocalTransport(tests.TestCase):
 
     def test_unc_clone_to_root(self):
+        self.requireFeature(features.win32_feature)
         # Win32 UNC path like \\HOST\path
         # clone to root should stop at least at \\HOST part
         # not on \\

=== modified file 'bzrlib/tests/test_urlutils.py'
--- bzrlib/tests/test_urlutils.py	2012-01-05 13:02:31 +0000
+++ bzrlib/tests/test_urlutils.py	2015-09-08 08:00:02 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2012 Canonical Ltd
+# Copyright (C) 2006-2012, 2015 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -26,7 +26,7 @@
     InvalidRebaseURLs,
     PathNotChild,
     )
-from bzrlib.tests import TestCaseInTempDir, TestCase, TestSkipped
+from bzrlib.tests import features, TestCaseInTempDir, TestCase, TestSkipped
 
 
 class TestUrlToPath(TestCase):
@@ -411,6 +411,7 @@
         self.assertFalse(isinstance(result, unicode))
 
     def test_win32_unc_path_to_url(self):
+        self.requireFeature(features.win32_feature)
         to_url = urlutils._win32_local_path_to_url
         self.assertEqual('file://HOST/path',
             to_url(r'\\HOST\path'))

=== modified file 'doc/en/release-notes/bzr-2.7.txt'
--- doc/en/release-notes/bzr-2.7.txt	2014-12-15 20:24:42 +0000
+++ doc/en/release-notes/bzr-2.7.txt	2015-09-08 08:00:02 +0000
@@ -76,6 +76,11 @@
    suite.  This can include new facilities for writing tests, fixes to 
    spurious test failures and changes to the way things should be tested.
 
+* Fix racy http tests (TestBadStatusServer is so simple, it exposes a race
+  in python 2.7.9. This happens only when both the http server and client
+  are run in the same process.). Only tests are affected.
+  (Vincent Ladeuil, #1451448)
+     
 * Fix warnings on stderr caused by the atexit handler triggering for the
   wrong reason: the 'config' command should explicitly save the changes when
   modifying or removing an option and not rely on the atexit
@@ -93,5 +98,9 @@
 * Restrict access to '.netrc' in tests or recent python (2.7.5-8) will
   complain. (Vincent Ladeuil, #1233413)
 
+* Skip windows-only tests that start failing with python 2.7.9, there is no
+  way to fix them without testing on windows itself.
+  (Vincent Ladeuil, #1451448)
+     
 ..
    vim: tw=74 ft=rst ff=unix