[MERGE] [Bug 215426] Re: MemoryError - recv for HTTPthroughProxy
Aaron Bentley
aaron at aaronbentley.com
Fri May 2 16:51:19 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Eric Holmberg wrote:
Overall, this looks good. But the lack of tests for os.pumpfile was a
dealbreaker. There are also some minor style issues.
bb:resubmit
> === modified file 'bzrlib/osutils.py'
> --- bzrlib/osutils.py 2008-04-07 07:34:32 +0000
> +++ bzrlib/osutils.py 2008-04-30 23:56:08 +0000
> @@ -530,15 +530,15 @@
> return False
>
>
> -def pumpfile(fromfile, tofile):
> - """Copy contents of one file to another.
> +def pumpfile(fromfile, tofile, buffsize=32768):
> + """Copy contents of one file to another using the specified buffer
> + size.
>
> :return: The number of bytes copied.
> """
> - BUFSIZE = 32768
> length = 0
> while True:
> - b = fromfile.read(BUFSIZE)
> + b = fromfile.read(buffsize)
> if not b:
> break
> tofile.write(b)
Actually, I would prefer if you added a size parameter to pumpfile.
This would allow you you pull all the logic out of read_limited, and
would make pumpfile more generally useful.
But you have changed pumpfile's functionality, so you must add some
tests for that new functionality.
I would recommend:
1. add a size parameter to os.pumpfile
2. call os.pumpfile directly from read and remove _read_limited
3. change your tests to test pumpfile. You can test one edge case of
read to ensure pumpfile is being used, but you don't need to test it all
over again.
> +class ReadFile(object):
> + """A file-like object that can be given predefined content and read
> + like a file. The maximum size of the reads is recorded."""
> +
> + def __init__(self, data):
> + """Initialize the mock file object with the provided data."""
> + self.data = StringIO(data)
> + self.max_read_size = None
> + self.read_count = 0
> +
> + def read(self,size=-1):
Please put a space after all commas.
> +class TestRangeFileSizeReadLimited(tests.TestCase):
> + """Test RangeFile._read_limited method which limits the size of read
> + blocks to prevent MemoryError messages in socket.recv.
> + """
> +
> + def setUp(self):
> + # create a test datablock larger than _read_limited_buf_size.
> + chunk_size = response.RangeFile._read_limited_buf_size
> + test_pattern = '0123456789ABCDEF'
> + self.test_data = test_pattern * (3 * chunk_size / len(test_pattern))
> + self.test_data_len = len(self.test_data)
> +
> + def test_read_not_limited(self):
> + """Read data in blocks smaller than the maximum block size and verify
> + that the read size was not limited.
> + """
> + mock_read_file = ReadFile(self.test_data)
> + range_file = response.RangeFile('test_read_not_limited',
> + mock_read_file
> + )
^^^^ Please use one of:
RangeFile('test_read_not_limited',
mock_read_file)
RangeFile('test_read_not_limited',
mock_read_file)
RangeFile('test_read_not_limited',
mock_read_file,
)
^^^ for lists only.
> + # read (max / 2) bytes and verify read size wasn't affected
> + num_bytes_to_read = response.RangeFile._read_limited_buf_size / 2
> + response_buffer.write( range_file.read(num_bytes_to_read) )
Please do not put spaces immediately after opening parentheses or
immediately before closing parentheses.
> + # read (max) bytes and verify read size wasn't affected
> + num_bytes_to_read = response.RangeFile._read_limited_buf_size
> + mock_read_file.reset_read_count()
> + response_buffer.write( range_file.read(num_bytes_to_read) )
^^^ parentheses again
> + # read (max + 1) bytes and verify read size was limited
> + num_bytes_to_read = response.RangeFile._read_limited_buf_size + 1
> + mock_read_file.reset_read_count()
> + response_buffer.write( range_file.read(num_bytes_to_read) )
> + self.assertEqual(mock_read_file.get_max_read_size(),
> + response.RangeFile._read_limited_buf_size
> + )
line-continuation style again
> + self.assertEqual(mock_read_file.get_read_count(), 2)
> +
> + # finish reading the rest of the data
> + num_bytes_to_read = self.test_data_len - response_buffer.tell()
> + response_buffer.write( range_file.read(num_bytes_to_read) )
parentheses again
> + # report error if the data wasn't equal (we only report the size due
> + # to the length of the data)
> + response_data = response_buffer.getvalue()
> + if response_data != self.test_data:
> + message = "Data not equal. Expected %d bytes, received %d." % (
> + len(response_data), self.test_data_len
> + )
line-continuation again
> + self.fail(message)
> +
> + def test_read_limited(self):
> + """Read data in blocks and verify that the reads are not larger than
> + the maximum read size.
> + """
> +
> + # retrieve data in large blocks from response.RangeFile object
> + mock_read_file = ReadFile(self.test_data)
> + range_file = response.RangeFile('test_read_limited',
> + mock_read_file
> + )
line-continuation again
> +
> + response_data = range_file.read(self.test_data_len)
> +
> + # verify read size was equal to the maximum read size
> + self.assertTrue(mock_read_file.get_max_read_size() > 0)
> + self.assertEqual(mock_read_file.get_max_read_size(),
> + response.RangeFile._read_limited_buf_size
> + )
line-continuation again
> + self.assertEqual(mock_read_file.get_read_count(), 3)
> +
> + # report error if the data wasn't equal (we only report the size due
> + # to the length of the data)
> + if response_data != self.test_data:
> + message = "Data not equal. Expected %d bytes, received %d." % (
> + len(response_data), self.test_data_len
> + )
line-continuation again
> + self.fail(message)
> +
> + def test_read_limited_eof(self):
> + """Read to end-of-file and verify that the reads are not larger than
> + the maximum read size.
> + """
> + # make sure test data is larger than max read size
> + self.assertTrue(self.test_data_len >
> + response.RangeFile._read_limited_buf_size
> + )
line-continuation again
> + # retrieve data to EOF from response.RangeFile object
> + mock_read_file = ReadFile(self.test_data)
> + range_file = response.RangeFile('test_read_limited_eof',
> + mock_read_file
> + )
line-continuation again
> + response_data = range_file.read(-1)
> +
> + # verify read size was equal to the maximum read size
> + self.assertEqual(mock_read_file.get_max_read_size(),
> + response.RangeFile._read_limited_buf_size
> + )
line-continuation again
> + self.assertEqual(mock_read_file.get_read_count(), 4)
> +
> + # report error if the data wasn't equal (we only report the size due
> + # to the length of the data)
> + if response_data != self.test_data:
> + message = "Data not equal. Expected %d bytes, received %d." % (
> + len(response_data), self.test_data_len
> + )
line-continuation again
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIGzh20F+nu1YWqI0RAjPsAJ4uLS5CU52DVQzePIt/ZyDUumEFZgCePCsg
t/FePn5t4wxfcD27ExNb2Fc=
=r0T5
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list