Rev 3054: Add tests and fix trivial bugs and other typos. in file:///v/home/vila/src/bzr/bugs/172701/

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Nov 29 23:22:05 GMT 2007


At file:///v/home/vila/src/bzr/bugs/172701/

------------------------------------------------------------
revno: 3054
revision-id:v.ladeuil+lp at free.fr-20071129232201-thjmmlgo5ucbrwfn
parent: v.ladeuil+lp at free.fr-20071129215945-qwzjfzz6kygljh4r
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 172701
timestamp: Fri 2007-11-30 00:22:01 +0100
message:
  Add tests and fix trivial bugs and other typos.
  
  * bzrlib/transport/http/response.py:
  (handle_response): Delete obsolete TODO.
  
  * bzrlib/transport/http/__init__.py:
  (HttpTransportBase._readv): Fix some trivial bugs revealed by the
  test suite.
  (HttpTransportBase._coalesce_readv): Fix list splice access, first
  seen by jam, but found by the test suite too.
  
  * bzrlib/tests/test_http.py:
  Fix some imports.
  (TestRangeRequestServer.test_readv_multiple_get_requests):
  Exercise the rewritten readv.
  
  * bzrlib/tests/HttpServer.py:
  (TestingHTTPRequestHandler.do_GET): Count the requests (more tests
  use that info).
  
  * bzrlib/tests/HTTPTestUtil.py:
  (LimitedRangeRequestHandler.do_GET): The statistics are collected
  in the base request handler now.
  (NoRangeRequestHandler.do_GET): But since we bypass the base
  handler, re-enable the statistics collection here.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/HTTPTestUtil.py   HTTPTestUtil.py-20050914180604-247d3aafb7a43343
  bzrlib/tests/HttpServer.py     httpserver.py-20061012142527-m1yxdj1xazsf8d7s-1
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
  bzrlib/transport/http/response.py _response.py-20060613154423-a2ci7hd4iw5c7fnt-1
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2007-11-29 18:41:01 +0000
+++ b/NEWS	2007-11-29 23:22:01 +0000
@@ -84,6 +84,9 @@
    * Catch connection errors for ftp.
      (Vincent Ladeuil, #164567)
 
+   * Catch ShortReadvErrors while using pycurl.
+     (Vincent Ladeuil, 172701)
+
    * ``check`` no longer reports spurious unreferenced text versions.
      (Robert Collins, John A Meinel, #162931, #165071)
 

=== modified file 'bzrlib/tests/HTTPTestUtil.py'
--- a/bzrlib/tests/HTTPTestUtil.py	2007-10-17 15:36:20 +0000
+++ b/bzrlib/tests/HTTPTestUtil.py	2007-11-29 23:22:01 +0000
@@ -164,11 +164,6 @@
         return TestingHTTPRequestHandler.get_multiple_ranges(self, file,
                                                              file_size, ranges)
 
-    def do_GET(self):
-        tcs = self.server.test_case_server
-        tcs.GET_request_nb += 1
-        return TestingHTTPRequestHandler.do_GET(self)
-
 
 class LimitedRangeHTTPServer(HttpServer):
     """An HttpServer erroring out on requests with too much range specifiers"""
@@ -177,7 +172,6 @@
                  range_limit=None):
         HttpServer.__init__(self, request_handler)
         self.range_limit = range_limit
-        self.GET_request_nb = 0
 
 
 class SingleRangeRequestHandler(TestingHTTPRequestHandler):
@@ -208,8 +202,11 @@
 class NoRangeRequestHandler(TestingHTTPRequestHandler):
     """Ignore range requests without notice"""
 
-    # Just bypass the range handling done by TestingHTTPRequestHandler
-    do_GET = SimpleHTTPRequestHandler.do_GET
+    def do_GET(self):
+        # Update the statistics
+        self.server.test_case_server.GET_request_nb += 1
+        # Just bypass the range handling done by TestingHTTPRequestHandler
+        return SimpleHTTPRequestHandler.do_GET(self)
 
 
 class TestCaseWithWebserver(TestCaseWithTransport):

=== modified file 'bzrlib/tests/HttpServer.py'
--- a/bzrlib/tests/HttpServer.py	2007-10-31 12:42:42 +0000
+++ b/bzrlib/tests/HttpServer.py	2007-11-29 23:22:01 +0000
@@ -149,6 +149,8 @@
 
         Handles the Range header.
         """
+        # Update statistics
+        self.server.test_case_server.GET_request_nb += 1
 
         path = self.translate_path(self.path)
         ranges_header_value = self.headers.get('Range')
@@ -304,6 +306,8 @@
         self.host = 'localhost'
         self.port = 0
         self._httpd = None
+        # Allows tests to verify number of GET requests issued
+        self.GET_request_nb = 0
 
     def _get_httpd(self):
         if self._httpd is None:

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2007-11-04 15:29:17 +0000
+++ b/bzrlib/tests/test_http.py	2007-11-29 23:22:01 +0000
@@ -32,15 +32,10 @@
     config,
     errors,
     osutils,
+    tests,
     ui,
     urlutils,
     )
-from bzrlib.tests import (
-    TestCase,
-    TestUIFactory,
-    TestSkipped,
-    StringIOWrapper,
-    )
 from bzrlib.tests.HttpServer import (
     HttpServer,
     HttpServer_PyCurl,
@@ -158,12 +153,12 @@
             from bzrlib.transport.http._pycurl import PyCurlTransport
             return PyCurlTransport
         except errors.DependencyNotPresent:
-            raise TestSkipped('pycurl not present')
+            raise tests.TestSkipped('pycurl not present')
 
     _transport = property(_get_pycurl_maybe)
 
 
-class TestHttpUrls(TestCase):
+class TestHttpUrls(tests.TestCase):
 
     # TODO: This should be moved to authorization tests once they
     # are written.
@@ -226,7 +221,7 @@
             server.tearDown()
 
 
-class TestHttpUrls_urllib(TestHttpTransportUrls, TestCase):
+class TestHttpUrls_urllib(TestHttpTransportUrls, tests.TestCase):
     """Test http urls with urllib"""
 
     _transport = HttpTransport_urllib
@@ -235,7 +230,7 @@
 
 
 class TestHttpUrls_pycurl(TestWithTransport_pycurl, TestHttpTransportUrls,
-                          TestCase):
+                          tests.TestCase):
     """Test http urls with pycurl"""
 
     _server = HttpServer_PyCurl
@@ -254,7 +249,7 @@
         try:
             import pycurl
         except ImportError:
-            raise TestSkipped('pycurl not present')
+            raise tests.TestSkipped('pycurl not present')
         # Now that we have pycurl imported, we can fake its version_info
         # This was taken from a windows pycurl without SSL
         # (thanks to bialix)
@@ -352,7 +347,7 @@
     """Test http connections with pycurl"""
 
 
-class TestHttpTransportRegistration(TestCase):
+class TestHttpTransportRegistration(tests.TestCase):
     """Test registrations of various http implementations"""
 
     def test_http_registered(self):
@@ -372,7 +367,7 @@
         try:
             http_transport = get_transport(url)
         except errors.UnsupportedProtocol:
-            raise TestSkipped('%s not available' % scheme)
+            raise tests.TestSkipped('%s not available' % scheme)
         code, response = http_transport._post('abc def end-of-body')
         self.assertTrue(
             server.received_bytes.startswith('POST /.bzr/smart HTTP/1.'))
@@ -384,7 +379,7 @@
             server.received_bytes.endswith('\r\n\r\nabc def end-of-body'))
 
 
-class TestPost_urllib(TestCase, TestPost):
+class TestPost_urllib(tests.TestCase, TestPost):
     """TestPost for urllib implementation"""
 
     _transport = HttpTransport_urllib
@@ -393,14 +388,14 @@
         self._test_post_body_is_received('http+urllib')
 
 
-class TestPost_pycurl(TestWithTransport_pycurl, TestCase, TestPost):
+class TestPost_pycurl(TestWithTransport_pycurl, tests.TestCase, TestPost):
     """TestPost for pycurl implementation"""
 
     def test_post_body_is_received_pycurl(self):
         self._test_post_body_is_received('http+pycurl')
 
 
-class TestRangeHeader(TestCase):
+class TestRangeHeader(tests.TestCase):
     """Test range_header method"""
 
     def check_header(self, value, ranges=[], tail=0):
@@ -575,7 +570,7 @@
     """Tests forbidden server for pycurl implementation"""
 
 
-class TestRecordingServer(TestCase):
+class TestRecordingServer(tests.TestCase):
 
     def test_create(self):
         server = RecordingServer(expect_body_tail=None)
@@ -653,6 +648,20 @@
         self.assertListRaises((errors.InvalidRange, errors.ShortReadvError,),
                               t.readv, 'a', [(12,2)])
 
+    def test_readv_multiple_get_requests(self):
+        server = self.get_readonly_server()
+        t = self._transport(server.get_url())
+        # force transport to issue multiple requests
+        t._max_readv_combine = 1
+        t._max_get_ranges = 1
+        l = list(t.readv('a', ((0, 1), (1, 1), (3, 2), (9, 1))))
+        self.assertEqual(l[0], (0, '0'))
+        self.assertEqual(l[1], (1, '1'))
+        self.assertEqual(l[2], (3, '34'))
+        self.assertEqual(l[3], (9, '9'))
+        # The server should have issued 4 requests
+        self.assertEqual(4, self.get_readonly_server().GET_request_nb)
+
 
 class TestSingleRangeRequestServer(TestRangeRequestServer):
     """Test readv against a server which accept only single range requests"""
@@ -709,8 +718,8 @@
 
 
 class TestNoRangeRequestServer_pycurl(TestWithTransport_pycurl,
-                               TestNoRangeRequestServer,
-                               TestCaseWithWebserver):
+                                      TestNoRangeRequestServer,
+                                      TestCaseWithWebserver):
     """Tests range requests refusing server for pycurl implementation"""
 
 
@@ -738,7 +747,7 @@
         TestCaseWithWebserver.setUp(self)
         # We need to manipulate ranges that correspond to real chunks in the
         # response, so we build a content appropriately.
-        filler = ''.join(['abcdefghij' for _ in range(102)])
+        filler = ''.join(['abcdefghij' for x in range(102)])
         content = ''.join(['%04d' % v + filler for v in range(16)])
         self.build_tree_contents([('a', content)],)
 
@@ -749,7 +758,7 @@
         self.assertEqual(l[1], (1024, '0001'))
         self.assertEqual(1, self.get_readonly_server().GET_request_nb)
 
-    def test_a_lot_of_ranges(self):
+    def test_more_ranges(self):
         t = self.get_transport()
         l = list(t.readv('a', ((0, 4), (1024, 4), (4096, 4), (8192, 4))))
         self.assertEqual(l[0], (0, '0000'))
@@ -775,14 +784,14 @@
 
 
 
-class TestHttpProxyWhiteBox(TestCase):
+class TestHttpProxyWhiteBox(tests.TestCase):
     """Whitebox test proxy http authorization.
 
     Only the urllib implementation is tested here.
     """
 
     def setUp(self):
-        TestCase.setUp(self)
+        tests.TestCase.setUp(self)
         self._old_env = {}
 
     def tearDown(self):
@@ -933,12 +942,12 @@
         # pycurl does not check HTTP_PROXY for security reasons
         # (for use in a CGI context that we do not care
         # about. Should we ?)
-        raise TestSkipped('pycurl does not check HTTP_PROXY '
-            'for security reasons')
+        raise tests.TestSkipped('pycurl does not check HTTP_PROXY '
+                                'for security reasons')
 
     def test_HTTP_PROXY_with_NO_PROXY(self):
-        raise TestSkipped('pycurl does not check HTTP_PROXY '
-            'for security reasons')
+        raise tests.TestSkipped('pycurl does not check HTTP_PROXY '
+                                'for security reasons')
 
     def test_http_proxy_without_scheme(self):
         # pycurl *ignores* invalid proxy env variables. If that
@@ -1272,8 +1281,8 @@
     def test_prompt_for_password(self):
         self.server.add_user('joe', 'foo')
         t = self.get_user_transport('joe', None)
-        stdout = StringIOWrapper()
-        ui.ui_factory = TestUIFactory(stdin='foo\n', stdout=stdout)
+        stdout = tests.StringIOWrapper()
+        ui.ui_factory = tests.TestUIFactory(stdin='foo\n', stdout=stdout)
         self.assertEqual('contents of a\n',t.get('a').read())
         # stdin should be empty
         self.assertEqual('', ui.ui_factory.stdin.readline())
@@ -1302,8 +1311,8 @@
         stdin_content = 'bar\n'  # Not the right password
         self.server.add_user(user, password)
         t = self.get_user_transport(user, None)
-        ui.ui_factory = TestUIFactory(stdin=stdin_content,
-                                      stdout=StringIOWrapper())
+        ui.ui_factory = tests.TestUIFactory(stdin=stdin_content,
+                                            stdout=tests.StringIOWrapper())
         # Create a minimal config file with the right password
         conf = config.AuthenticationConfig()
         conf._get_config().update(

=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2007-11-29 15:43:33 +0000
+++ b/bzrlib/transport/http/__init__.py	2007-11-29 23:22:01 +0000
@@ -293,14 +293,14 @@
                 for cur_coal, file in self._coalesce_readv(relpath, coalesced):
                     # Split the received chunk
                     for offset, size in cur_coal.ranges:
-                        key = (cur_coal.start + offset, size)
-                        file.seek(cur_coal.start + offset, 0)
+                        start = cur_coal.start + offset
+                        file.seek(start, 0)
                         data = file.read(size)
                         data_len = len(data)
                         if data_len != size:
                             raise errors.ShortReadvError(relpath, start, size,
                                                          actual=data_len)
-                        data_map[key] = data
+                        data_map[(start, size)] = data
 
                     # Yield everything we can
                     while cur_offset_and_size in data_map:
@@ -315,7 +315,8 @@
                 self._degrade_range_hint(relpath, coalesced, sys.exc_info())
                 # Some offsets may have been already processed, so we retry
                 # only the unsuccessful ones.
-                offsets = [cur_offset_and_size] + [o for o in offset_stack]
+                offsets = [cur_offset_and_size] + [o for o in iter_offsets]
+                try_again = True
 
     def _coalesce_readv(self, relpath, coalesced):
         """Issue several GET requests to satisfy the coalesced offsets"""
@@ -328,11 +329,11 @@
             # The whole file will be downloaded anyway
             max_ranges = total
         for group in xrange(0, len(coalesced), max_ranges):
-            ranges = coalesced[group * max_ranges:group+1 * max_ranges]
+            ranges = coalesced[group:group+max_ranges]
             # Note that the following may raise errors.InvalidRange. It's the
             # caller responsability to decide how to retry since it may provide
             # different coalesced offsets.
-            file = self._get(relpath, ranges)
+            code, file = self._get(relpath, ranges)
             for range in ranges:
                 yield range, file
 

=== modified file 'bzrlib/transport/http/response.py'
--- a/bzrlib/transport/http/response.py	2007-06-20 13:56:21 +0000
+++ b/bzrlib/transport/http/response.py	2007-11-29 23:22:01 +0000
@@ -287,9 +287,6 @@
         # magic value.
         raise errors.InvalidRange(url,0)
 
-    # TODO: jam 20060713 Properly handle redirects (302 Found, etc)
-    #       The '_get' code says to follow redirects, we probably 
-    #       should actually handle the return values
     else:
         raise errors.InvalidHttpResponse(url, "Unknown response code %s" 
                                               % (code,))



More information about the bazaar-commits mailing list