[pypy-dev] [pypy-commit] pypy default: fix bz2. tests didn't find this.

[Justin Peel]

On Mon, Sep 5, 2011 at 12:54 AM, Maciej Fijalkowski <fijall at gmail.com> wrote:
> On Wed, Aug 31, 2011 at 8:18 AM, justinpeel <noreply at buildbot.pypy.org> wrote:
>> Author: Justin Peel <notmuchtotell at gmail.com>
>> Branch:
>> Changeset: r46937:b4d8eb5fdf6c
>> Date: 2011-08-31 00:17 -0600
>> http://bitbucket.org/pypy/pypy/changeset/b4d8eb5fdf6c/
>>
>> Log:    fix bz2. tests didn't find this.
>>
>> diff --git a/pypy/module/bz2/interp_bz2.py b/pypy/module/bz2/interp_bz2.py
>> --- a/pypy/module/bz2/interp_bz2.py
>> +++ b/pypy/module/bz2/interp_bz2.py
>> @@ -446,7 +446,9 @@
>>             result = self.buffer[pos:pos + n]
>>             self.pos += n
>>         else:
>> -            result = self.buffer
>> +            pos = self.pos
>> +            assert pos >= 0
>> +            result = self.buffer[pos:]
>>             self.pos = 0
>>             self.buffer = ""
>>         self.readlength += len(result)
>> _______________________________________________
>> pypy-commit mailing list
>> pypy-commit at python.org
>> http://mail.python.org/mailman/listinfo/pypy-commit
>>
>
> This should come with a test
> _______________________________________________
> pypy-dev mailing list
> pypy-dev at python.org
> http://mail.python.org/mailman/listinfo/pypy-dev
>

I thought that I already wrote back to you about this, but here is more info.

I didn't put in a separate test for this, but I changed a test as per
the following to make it catch this bug. I tested it with the code
before the bug was fixed and after and it does find this bug. The
reason that the test didn't find the bug before is because the test
data has a length of 770 and the test was reading chunks of 10
characters. Since 10 divides evenly into 770, this other path in the
code was never taken. However, if we read 9 character chunks, 9
doesn't divide evenly into 770, this other path is taken and we end up
with the wrong result in the test.

The following is commit 13c94c0591c3 which I did later on in the day
(my time) after I put in the fix. It didn't occur to me right away
that this was the easy way to get coverage for this or I would have
done it right away. Perhaps it was because it was around 2 am my time
when I discovered the bug.

# HG changeset patch
# User Justin Peel <notmuchtotell at gmail.com>
# Date 1314819164 21600
# Node ID 13c94c0591c34b5c0f10978871e33880cdbb5ce7
# Parent  0d75ab342438fc401e1a44fdf7d2822bedc5e392
change bz2 test so that it reads chunks which don't divide evenly into
test data's length

diff -r 0d75ab342438fc401e1a44fdf7d2822bedc5e392 -r
13c94c0591c34b5c0f10978871e33880cdbb5ce7
pypy/module/bz2/test/test_bz2_file.py
--- a/pypy/module/bz2/test/test_bz2_file.py	Wed Aug 31 20:01:39 2011 +0200
+++ b/pypy/module/bz2/test/test_bz2_file.py	Wed Aug 31 13:32:44 2011 -0600
@@ -274,14 +274,14 @@
             pass
         del bz2f   # delete from this frame, which is captured in the traceback

-    def test_read_chunk10(self):
+    def test_read_chunk9(self):
         from bz2 import BZ2File
         self.create_temp_file()

         bz2f = BZ2File(self.temppath)
         text_read = ""
         while True:
-            data = bz2f.read(10)
+            data = bz2f.read(9) # 9 doesn't divide evenly into data length
             if not data:
                 break
             text_read = "%s%s" % (text_read, data)