launchpad-reviewers team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/launchpad/bug-1003040 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1003040 in Launchpad itself: "lp.services.webapp.tests.test_error.TestDatabaseErrorViews.test_disconnectionerror_view_integration fails intermittently/rarely in parallel tests"
  https://bugs.launchpad.net/launchpad/+bug/1003040

For more details, see:
https://code.launchpad.net/~frankban/launchpad/bug-1003040/+merge/108005

= Summary =

TestDatabaseErrorViews.test_disconnectionerror_view_integration fails intermittently/rarely in parallel tests.

== Proposed fix ==

The failure is similar to the one in bug 974617, so the same fix should apply in this case.

== Implementation details ==

lib/lp/services/webapp/tests/test_error.py:
see Proposed fix, abstracting the retry code now used in two places.

== Tests ==

$ bin/test -cvvt lp.services.webapp.tests.test_error.TestDatabaseErrorViews

== Demo and Q/A ==

no qa

== lint ==

Linting changed files:
  lib/lp/services/webapp/tests/test_error.py

-- 
https://code.launchpad.net/~frankban/launchpad/bug-1003040/+merge/108005
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~frankban/launchpad/bug-1003040 into lp:launchpad.

=== modified file 'lib/lp/services/webapp/tests/test_error.py'
--- lib/lp/services/webapp/tests/test_error.py	2012-05-15 20:47:25 +0000
+++ lib/lp/services/webapp/tests/test_error.py	2012-05-30 16:08:21 +0000
@@ -66,6 +66,24 @@
         else:
             self.fail("We should have gotten an HTTP error")
 
+    def retryConnection(self, url, retries=10):
+        """Retry to connect to *url* for *retries* times.
+
+        Return the file-like object returned by *urllib2.urlopen(url)*.
+        Raise a TimeoutException if the connection can not be established.
+        """
+        for i in xrange(retries):
+            try:
+                return urllib2.urlopen(url)
+            except urllib2.HTTPError as e:
+                if e.code != httplib.SERVICE_UNAVAILABLE:
+                    raise
+            time.sleep(1)
+        else:
+            raise TimeoutException(
+                "Launchpad did not come up after {0} attempts."
+                    .format(retries))
+
     def test_disconnectionerror_view_integration(self):
         # Test setup.
         self.useFixture(Urllib2Fixture())
@@ -94,7 +112,7 @@
         self.assertEqual(503, self.getHTTPError(url).code)
         # When the database is available again, requests succeed.
         bouncer.start()
-        urllib2.urlopen(url)
+        self.retryConnection(url)
 
     def test_disconnectionerror_view(self):
         request = LaunchpadTestRequest()
@@ -118,25 +136,9 @@
         # We keep seeing the correct exception on subsequent requests.
         self.assertEqual(httplib.SERVICE_UNAVAILABLE,
                          self.getHTTPError(url).code)
-        # When the database is available again...
+        # When the database is available again, requests succeed.
         bouncer.start()
-        # ...and Launchpad has succesfully connected to it...
-        retries = 10
-        for i in xrange(retries):
-            try:
-                urllib2.urlopen(url)
-            except urllib2.HTTPError as e:
-                if e.code != httplib.SERVICE_UNAVAILABLE:
-                    raise
-            else:
-                break
-            time.sleep(1)
-        else:
-            raise TimeoutException(
-                "Launchpad did not come up after {0} attempts."
-                    .format(retries))
-        # ...requests succeed again.
-        urllib2.urlopen(url)
+        self.retryConnection(url)
 
     def test_operationalerror_view(self):
         request = LaunchpadTestRequest()