Rev 3953: Add more tests, fix pycurl double handling, revert previous tracking. in file:///net/bigmamac/Volumes/home/vila/src/bzr/experimental/pb-http/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Jan 30 00:49:44 GMT 2009


At file:///net/bigmamac/Volumes/home/vila/src/bzr/experimental/pb-http/

------------------------------------------------------------
revno: 3953
revision-id: v.ladeuil+lp at free.fr-20090130004941-820fpd2ryyo127vv
parent: v.ladeuil+lp at free.fr-20090129193101-w0008aa1ufoe7cmd
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: pb-http
timestamp: Fri 2009-01-30 01:49:41 +0100
message:
  Add more tests, fix pycurl double handling, revert previous tracking.
  
  * bzrlib/tests/test_http.py:
  (PredefinedRequestHandler): Renamed from
  PreRecordedRequestHandler.
  (PredefinedRequestHandler.handle_one_request): Get the canned
  response from the test server directly.
  (ActivityServerMixin): Make it a true object and intialize the
  attributes in the constructor. Tests can now set the
  canned_response attribute before querying the server.
  (TestActivity.setUp, TestActivity.tearDown,
  TestActivity.get_transport, TestActivity.assertActivitiesMatch):
  Extracted from test_get to be able to write other tests.
  (TestActivity.test_has, TestActivity.test_readv,
  TestActivity.test_post): New tests, all cases should be covered
  now.
  
  * bzrlib/transport/http/response.py:
  (RangeFile.__init__, RangeFile.read, handle_response): Revert
  previous tracking, both http implementations can now report
  activity from the socket.
  
  * bzrlib/transport/http/_pycurl.py:
  (PyCurlTransport._get_ranged, PyCurlTransport._post): Revert
  previous tracking.
-------------- next part --------------
=== modified file 'BRANCH.TODO'
--- a/BRANCH.TODO	2009-01-29 19:31:01 +0000
+++ b/BRANCH.TODO	2009-01-30 00:49:41 +0000
@@ -3,8 +3,3 @@
 # 
 #
 
-* add tests for has and readv (_get_full wasn't addressed
-   by jam's patch but _get_ranged should be reported twice currently).
-
-* complete _ReportingFileSocket implementation for write
-   operations ?

=== modified file 'NEWS'
--- a/NEWS	2009-01-29 16:54:58 +0000
+++ b/NEWS	2009-01-30 00:49:41 +0000
@@ -12,8 +12,9 @@
     * Progress bars now show the rate of activity for some sftp 
       operations, and they are drawn different.  (Martin Pool, #172741)
 
-    * Progress bars now show the rate of activity for some^H *all* hhtp ;)
-      operations for both urllib and pycurl implementations.
+    * Progress bars now show the rate of activity for urllib and pycurl based
+      http client implementations. The operations are tracked as the socket
+      level for better precision.
       (Vincent Ladeuil)
 
   BUG FIXES:

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2009-01-29 19:31:01 +0000
+++ b/bzrlib/tests/test_http.py	2009-01-30 00:49:41 +0000
@@ -1794,26 +1794,8 @@
         self.assertEquals(t._user, r._user)
 
 
-class ActivityServerMixin(object):
-
-    # Bytes read and written by the server
-    bytes_read = 0
-    bytes_written = 0
-
-
-class ActivityHTTPServer(ActivityServerMixin, http_server.HttpServer):
-    pass
-
-
-if tests.HTTPSServerFeature.available():
-    from bzrlib.tests import https_server
-    class ActivityHTTPSServer(ActivityServerMixin, https_server.HTTPSServer):
-        pass
-
-
-class PreRecordedRequestHandler(http_server.TestingHTTPRequestHandler):
-    """Pre-recorded request handler.
-
+class PredefinedRequestHandler(http_server.TestingHTTPRequestHandler):
+    """Request handler for a unique and pre-defined request.
 
     The only thing we care about here is how many bytes travel on the wire. But
     since we want to measure it for a real http client, we have to send it
@@ -1824,14 +1806,8 @@
     line.
     """
 
-    canned_response = ''
-
     def handle_one_request(self):
         tcs = self.server.test_case_server
-        if not isinstance(tcs, ActivityServerMixin):
-            raise AssertionError(
-                'PreRecoredRequestHandler assumes a %r server, not %r'
-                % (ActivityHttpServer, tcs))
         requestline = self.rfile.readline()
         headers = self.MessageClass(self.rfile, 0)
         # We just read: the request, the headers, an empty line indicating the
@@ -1840,16 +1816,44 @@
         for line in headers.headers:
             bytes_read += len(line)
         bytes_read += len('\r\n')
+        if requestline.startswith('POST'):
+            # The body should be a single line (or we don't know where it ends
+            # and we don't want to issue a blocking read)
+            body = self.rfile.readline()
+            bytes_read += len(body)
         tcs.bytes_read = bytes_read
-        # Set the bytes written *before* issuing the write, the client is
-        # supposed to consume every produced byte *before* checkingthat value.
+
+        # We set the bytes written *before* issuing the write, the client is
+        # supposed to consume every produced byte *before* checking that value.
 
         # Doing the oppposite may lead to test failure: we may be interrupted
         # after the write but before updating the value. The client can then
         # continue and read the value *before* we can update it. And yes,
         # this has been observed -- vila 20090129
-        tcs.bytes_written = len(self.canned_response)
-        self.wfile.write(self.canned_response)
+        tcs.bytes_written = len(tcs.canned_response)
+        self.wfile.write(tcs.canned_response)
+
+
+class ActivityServerMixin(object):
+
+    def __init__(self, protocol_version):
+        super(ActivityServerMixin, self).__init__(
+            request_handler=PredefinedRequestHandler,
+            protocol_version=protocol_version)
+        # Bytes read and written by the server
+        self.bytes_read = 0
+        self.bytes_written = 0
+        self.canned_response = None
+
+
+class ActivityHTTPServer(ActivityServerMixin, http_server.HttpServer):
+    pass
+
+
+if tests.HTTPSServerFeature.available():
+    from bzrlib.tests import https_server
+    class ActivityHTTPSServer(ActivityServerMixin, https_server.HTTPSServer):
+        pass
 
 
 class TestActivity(tests.TestCase):
@@ -1859,45 +1863,140 @@
     be able to predict the activity on the client socket.
     """
 
-    def test_http_get(self):
-        class MyRequestHandler(PreRecordedRequestHandler):
-
-            canned_response = '''HTTP/1.1 200 OK\r
-Date: Tue, 11 Jul 2006 04:32:56 GMT\r
-Server: Apache/2.0.54 (Fedora)\r
-Last-Modified: Sun, 23 Apr 2006 19:35:20 GMT\r
-ETag: "56691-23-38e9ae00"\r
-Accept-Ranges: bytes\r
-Content-Length: 35\r
-Connection: close\r
-Content-Type: text/plain; charset=UTF-8\r
-\r
-Bazaar-NG meta directory, format 1
-'''
-
-        server =  self._activity_server(MyRequestHandler,
-                                        self._protocol_version)
-        server.setUp()
-        self.addCleanup(server.tearDown)
-
-        activities = {}
+    def setUp(self):
+        tests.TestCase.setUp(self)
+        self.server = self._activity_server(self._protocol_version)
+        self.server.setUp()
+        self.activities = {}
         def report_activity(t, bytes, direction):
-            count = activities.get(direction, 0)
+            count = self.activities.get(direction, 0)
             count += bytes
-            activities[direction] = count
+            self.activities[direction] = count
 
         # We override at class level because constructors may propagate the
         # bound method and render instance overriding ineffective (an
         # alternative would be be to define a specific ui factory instead...)
-        orig_ra = self._transport._report_activity
-        def restore_report_activity():
-            self._transport._report_activity = orig_ra
-        self.addCleanup(restore_report_activity)
+        self.orig_report_activity = self._transport._report_activity
         self._transport._report_activity = report_activity
 
-        t = self._transport(server.get_url())
-
+    def tearDown(self):
+        self._transport._report_activity = self.orig_report_activity
+        self.server.tearDown()
+        tests.TestCase.tearDown(self)
+
+    def get_transport(self):
+        return self._transport(self.server.get_url())
+
+    def assertActivitiesMatch(self):
+        self.assertEqual(self.server.bytes_read,
+                         self.activities.get('write', 0), 'written bytes')
+        self.assertEqual(self.server.bytes_written,
+                         self.activities.get('read', 0), 'read bytes')
+
+    def test_get(self):
+        self.server.canned_response = '''HTTP/1.1 200 OK\r
+Date: Tue, 11 Jul 2006 04:32:56 GMT\r
+Server: Apache/2.0.54 (Fedora)\r
+Last-Modified: Sun, 23 Apr 2006 19:35:20 GMT\r
+ETag: "56691-23-38e9ae00"\r
+Accept-Ranges: bytes\r
+Content-Length: 35\r
+Connection: close\r
+Content-Type: text/plain; charset=UTF-8\r
+\r
+Bazaar-NG meta directory, format 1
+'''
+        t = self.get_transport()
         self.assertEqual('Bazaar-NG meta directory, format 1\n',
                          t.get('foo/bar').read())
-        self.assertEqual(server.bytes_read, activities.get('write', 0))
-        self.assertEqual(server.bytes_written, activities.get('read', 0))
+        self.assertActivitiesMatch()
+
+    def test_has(self):
+        self.server.canned_response = '''HTTP/1.1 200 OK\r
+Server: SimpleHTTP/0.6 Python/2.5.2\r
+Date: Thu, 29 Jan 2009 20:21:47 GMT\r
+Content-type: application/octet-stream\r
+Content-Length: 20\r
+Last-Modified: Thu, 29 Jan 2009 20:21:47 GMT\r
+\r
+'''
+        t = self.get_transport()
+        self.assertTrue(t.has('foo/bar'))
+        self.assertActivitiesMatch()
+
+    def test_readv(self):
+        self.server.canned_response = '''HTTP/1.1 206 Partial Content\r
+Date: Tue, 11 Jul 2006 04:49:48 GMT\r
+Server: Apache/2.0.54 (Fedora)\r
+Last-Modified: Thu, 06 Jul 2006 20:22:05 GMT\r
+ETag: "238a3c-16ec2-805c5540"\r
+Accept-Ranges: bytes\r
+Content-Length: 1534\r
+Connection: close\r
+Content-Type: multipart/byteranges; boundary=418470f848b63279b\r
+\r
+\r
+--418470f848b63279b\r
+Content-type: text/plain; charset=UTF-8\r
+Content-range: bytes 0-254/93890\r
+\r
+mbp at sourcefrog.net-20050309040815-13242001617e4a06
+mbp at sourcefrog.net-20050309040929-eee0eb3e6d1e7627
+mbp at sourcefrog.net-20050309040957-6cad07f466bb0bb8
+mbp at sourcefrog.net-20050309041501-c840e09071de3b67
+mbp at sourcefrog.net-20050309044615-c24a3250be83220a
+\r
+--418470f848b63279b\r
+Content-type: text/plain; charset=UTF-8\r
+Content-range: bytes 1000-2049/93890\r
+\r
+40-fd4ec249b6b139ab
+mbp at sourcefrog.net-20050311063625-07858525021f270b
+mbp at sourcefrog.net-20050311231934-aa3776aff5200bb9
+mbp at sourcefrog.net-20050311231953-73aeb3a131c3699a
+mbp at sourcefrog.net-20050311232353-f5e33da490872c6a
+mbp at sourcefrog.net-20050312071639-0a8f59a34a024ff0
+mbp at sourcefrog.net-20050312073432-b2c16a55e0d6e9fb
+mbp at sourcefrog.net-20050312073831-a47c3335ece1920f
+mbp at sourcefrog.net-20050312085412-13373aa129ccbad3
+mbp at sourcefrog.net-20050313052251-2bf004cb96b39933
+mbp at sourcefrog.net-20050313052856-3edd84094687cb11
+mbp at sourcefrog.net-20050313053233-e30a4f28aef48f9d
+mbp at sourcefrog.net-20050313053853-7c64085594ff3072
+mbp at sourcefrog.net-20050313054757-a86c3f5871069e22
+mbp at sourcefrog.net-20050313061422-418f1f73b94879b9
+mbp at sourcefrog.net-20050313120651-497bd231b19df600
+mbp at sourcefrog.net-20050314024931-eae0170ef25a5d1a
+mbp at sourcefrog.net-20050314025438-d52099f915fe65fc
+mbp at sourcefrog.net-20050314025539-637a636692c055cf
+mbp at sourcefrog.net-20050314025737-55eb441f430ab4ba
+mbp at sourcefrog.net-20050314025901-d74aa93bb7ee8f62
+mbp at source\r
+--418470f848b63279b--\r
+'''
+        t = self.get_transport()
+        # Remember that the request is ignored and that the ranges below
+        # doesn't have to match the canned response.
+        l = list(t.readv('/foo/bar', ((0, 255), (1000, 1050))))
+        self.assertEqual(2, len(l))
+        self.assertActivitiesMatch()
+
+    def test_post(self):
+        self.server.canned_response = '''HTTP/1.1 200 OK\r
+Date: Tue, 11 Jul 2006 04:32:56 GMT\r
+Server: Apache/2.0.54 (Fedora)\r
+Last-Modified: Sun, 23 Apr 2006 19:35:20 GMT\r
+ETag: "56691-23-38e9ae00"\r
+Accept-Ranges: bytes\r
+Content-Length: 35\r
+Connection: close\r
+Content-Type: text/plain; charset=UTF-8\r
+\r
+lalala whatever as long as itsssss
+'''
+        t = self.get_transport()
+        # We must send a single line of body bytes, see
+        # PredefinedRequestHandler.handle_one_request
+        code, f = t._post('abc def end-of-body\n')
+        self.assertEqual('lalala whatever as long as itsssss\n', f.read())
+        self.assertActivitiesMatch()

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2009-01-29 16:54:58 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2009-01-30 00:49:41 +0000
@@ -245,8 +245,7 @@
                                           'Server return code %d'
                                           % curl.getinfo(pycurl.HTTP_CODE))
         msg = self._parse_headers(header)
-        return code, response.handle_response(abspath, code, msg, data,
-            report_activity=self._report_activity)
+        return code, response.handle_response(abspath, code, msg, data)
 
     def _parse_headers(self, status_and_headers):
         """Transform the headers provided by curl into an HTTPMessage"""
@@ -286,8 +285,7 @@
         data.seek(0)
         code = curl.getinfo(pycurl.HTTP_CODE)
         msg = self._parse_headers(header)
-        return code, response.handle_response(abspath, code, msg, data,
-            report_activity=self._report_activity)
+        return code, response.handle_response(abspath, code, msg, data)
 
 
     def _raise_curl_http_error(self, curl, info=None):

=== modified file 'bzrlib/transport/http/response.py'
--- a/bzrlib/transport/http/response.py	2009-01-23 21:22:39 +0000
+++ b/bzrlib/transport/http/response.py	2009-01-30 00:49:41 +0000
@@ -67,18 +67,15 @@
     # maximum size of read requests -- used to avoid MemoryError issues in recv
     _max_read_size = 512 * 1024
 
-    def __init__(self, path, infile, report_activity=None):
+    def __init__(self, path, infile):
         """Constructor.
 
         :param path: File url, for error reports.
         :param infile: File-like socket set at body start.
-        :param report_activity: A Transport._report_activity function to call
-            as bytes are read.
         """
         self._path = path
         self._file = infile
         self._boundary = None
-        self._report_activity = report_activity
         # When using multi parts response, this will be set with the headers
         # associated with the range currently read.
         self._headers = None
@@ -231,8 +228,7 @@
             limited = self._start + self._size - self._pos
             if size >= 0:
                 limited = min(limited, size)
-        osutils.pumpfile(self._file, buffer, limited, self._max_read_size,
-            report_activity=self._report_activity, direction='read')
+        osutils.pumpfile(self._file, buffer, limited, self._max_read_size)
         data = buffer.getvalue()
 
         # Update _pos respecting the data effectively read
@@ -281,7 +277,7 @@
         return self._pos
 
 
-def handle_response(url, code, msg, data, report_activity=None):
+def handle_response(url, code, msg, data):
     """Interpret the code & headers and wrap the provided data in a RangeFile.
 
     This is a factory method which returns an appropriate RangeFile based on
@@ -295,7 +291,7 @@
     :return: A file-like object that can seek()+read() the 
              ranges indicated by the headers.
     """
-    rfile = RangeFile(url, data, report_activity=report_activity)
+    rfile = RangeFile(url, data)
     if code == 200:
         # A whole file
         size = msg.getheader('content-length', None)



More information about the bazaar-commits mailing list