Checkpoint.

Some cleanup of test_io.py and io.py.
Added seeking to buffered reader and writer, but no tests yet.

2 files changed, 111 insertions, 77 deletions

diff --git a/Lib/io.py b/Lib/io.py
index 78877f0..1c487e3 100644
--- a/Lib/io.py
+++ b/Lib/io.py
@@ -8,6 +8,7 @@ See PEP 3116.
 XXX need to default buffer size to 1 if isatty()
 XXX need to support 1 meaning line-buffered
 XXX change behavior of blocking I/O
+XXX don't use assert to validate input requirements
 """
 
 __author__ = ("Guido van Rossum <guido@python.org>, "
@@ -265,7 +266,12 @@ class _PyFileIO(RawIOBase):
         os.ftruncate(self._fd, pos)
 
     def close(self):
-        os.close(self._fd)
+        # Must be idempotent
+        # XXX But what about thread-safe?
+        fd = self._fd
+        self._fd = -1
+        if fd >= 0:
+            os.close(fd)
 
     def readable(self):
         return "r" in self._mode or "+" in self._mode
@@ -431,6 +437,9 @@ class BufferedIOBase(RawIOBase):
         """Flush the buffer to the underlying raw IO object."""
         raise IOError(".flush() unsupported")
 
+    def seekable(self):
+        return self.raw.seekable()
+
 
 class BufferedReader(BufferedIOBase):
 
@@ -457,10 +466,12 @@ class BufferedReader(BufferedIOBase):
         mode. If n is None, read until EOF or until read() would
         block.
         """
+        # XXX n == 0 should return b""? n < 0 should be the same as n is None?
         assert n is None or n > 0, '.read(): Bad read size %r' % n
         nodata_val = b""
         while n is None or len(self._read_buf) < n:
-            to_read = None if n is None else max(n, self.buffer_size)
+            to_read = max(self.buffer_size,
+                          n if n is not None else 2*len(self._read_buf))
             current = self.raw.read(to_read)
 
             if current in (b"", None):
@@ -486,6 +497,15 @@ class BufferedReader(BufferedIOBase):
         # Flush is a no-op
         pass
 
+    def tell(self):
+        return self.raw.tell() - len(self._read_buf)
+
+    def seek(self, pos, whence=0):
+        if whence == 1:
+            pos -= len(self._read_buf)
+        self.raw.seek(pos, whence)
+        self._read_buf = b""
+
     def close(self):
         self.raw.close()
 
@@ -500,7 +520,7 @@ class BufferedWriter(BufferedIOBase):
         self.raw = raw
         self.buffer_size = buffer_size
         self.max_buffer_size = max_buffer_size
-        self._write_buf = b''
+        self._write_buf = b""
 
     def write(self, b):
         # XXX we can implement some more tricks to try and avoid partial writes
@@ -511,9 +531,10 @@ class BufferedWriter(BufferedIOBase):
                 self.flush()
             except BlockingIO as e:
                 # We can't accept anything else.
+                # XXX Why not just let the exception pass through?
                 raise BlockingIO(e.errno, e.strerror, 0)
         self._write_buf += b
-        if (len(self._write_buf) > self.buffer_size):
+        if len(self._write_buf) > self.buffer_size:
             try:
                 self.flush()
             except BlockingIO as e:
@@ -528,24 +549,34 @@ class BufferedWriter(BufferedIOBase):
         return True
 
     def flush(self):
+        written = 0
         try:
-            while len(self._write_buf):
-                self._write_buf = self._write_buf[
-                                    self.raw.write(self._write_buf):]
+            while self._write_buf:
+                n = self.raw.write(self._write_buf)
+                del self._write_buf[:n]
+                written += n
         except BlockingIO as e:
-            self._write_buf[e.characters_written:]
-            raise
+            n = e.characters_written
+            del self._write_buf[:n]
+            written += n
+            raise BlockingIO(e.errno, e.strerror, written)
+
+    def tell(self):
+        return self.raw.tell() + len(self._write_buf)
+
+    def seek(self, pos, whence=0):
+        self.flush()
+        self.raw.seek(pos, whence)
 
     def fileno(self):
         return self.raw.fileno()
 
     def close(self):
+        self.flush()
         self.raw.close()
 
     def __del__(self):
-        # XXX flush buffers before dying. Is there a nicer way to do this?
-        if self._write_buf:
-            self.flush()
+        self.close()
 
 
 class BufferedRWPair(BufferedReader, BufferedWriter):
@@ -604,9 +635,6 @@ class BufferedRandom(BufferedReader, BufferedWriter):
         BufferedReader.__init__(self, raw)
         BufferedWriter.__init__(self, raw, buffer_size, max_buffer_size)
 
-    def seekable(self):
-        return self.raw.seekable()
-
     def readable(self):
         return self.raw.readable()
 
@@ -615,10 +643,15 @@ class BufferedRandom(BufferedReader, BufferedWriter):
 
     def seek(self, pos, whence=0):
         self.flush()
-        self._read_buf = b""
+        # First do the raw seek, then empty the read buffer, so that
+        # if the raw seek fails, we don't lose buffered data forever.
         self.raw.seek(pos, whence)
+        self._read_buf = b""
         # XXX I suppose we could implement some magic here to move through the
         # existing read buffer in the case of seek(<some small +ve number>, 1)
+        # XXX OTOH it might be good to *guarantee* that the buffer is
+        # empty after a seek or flush; for small relative forward
+        # seeks one might as well use small reads instead.
 
     def tell(self):
         if (self._write_buf):
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index 53419f4..5a4745f 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -1,25 +1,26 @@
 """Unit tests for io.py."""
 
 import unittest
-from test import test_support
 from itertools import chain
+from test import test_support
+
 import io
 
 
 class MockIO(io.RawIOBase):
 
-    def __init__(self, readStack=()):
-        self._readStack = list(readStack)
-        self._writeStack = []
+    def __init__(self, read_stack=()):
+        self._read_stack = list(read_stack)
+        self._write_stack = []
 
     def read(self, n=None):
         try:
-            return self._readStack.pop(0)
+            return self._read_stack.pop(0)
         except:
             return b""
 
     def write(self, b):
-        self._writeStack.append(b)
+        self._write_stack.append(b[:])
         return len(b)
 
     def writable(self):
@@ -60,7 +61,7 @@ class MockNonBlockWriterIO(io.RawIOBase):
         self._write_stack = []
 
     def write(self, b):
-        self._write_stack.append(b)
+        self._write_stack.append(b[:])
         n = self.bs.pop(0)
         if (n < 0):
             raise io.BlockingIO(0, "test blocking", -n)
@@ -159,7 +160,7 @@ class IOTest(unittest.TestCase):
         f.close()
 
 
-class MemorySeekTest(unittest.TestCase):
+class MemorySeekTestMixin:
 
     def testInit(self):
         buf = self.buftype("1234567890")
@@ -203,13 +204,13 @@ class MemorySeekTest(unittest.TestCase):
         self.assertEquals(10000, bytesIo.tell())
 
 
-class BytesIOTest(MemorySeekTest):
+class BytesIOTest(MemorySeekTestMixin, unittest.TestCase):
     buftype = bytes
     ioclass = io.BytesIO
     EOF = b""
 
 
-class StringIOTest(MemorySeekTest):
+class StringIOTest(MemorySeekTestMixin, unittest.TestCase):
     buftype = str
     ioclass = io.StringIO
     EOF = ""
@@ -218,10 +219,10 @@ class StringIOTest(MemorySeekTest):
 class BufferedReaderTest(unittest.TestCase):
 
     def testRead(self):
-        rawIo = MockIO((b"abc", b"d", b"efg"))
-        bufIo = io.BufferedReader(rawIo)
+        rawio = MockIO((b"abc", b"d", b"efg"))
+        bufio = io.BufferedReader(rawio)
 
-        self.assertEquals(b"abcdef", bufIo.read(6))
+        self.assertEquals(b"abcdef", bufio.read(6))
 
     def testBuffering(self):
         data = b"abcdefghi"
@@ -234,42 +235,42 @@ class BufferedReaderTest(unittest.TestCase):
         ]
 
         for bufsize, buf_read_sizes, raw_read_sizes in tests:
-            rawIo = MockFileIO(data)
-            bufIo = io.BufferedReader(rawIo, buffer_size=bufsize)
+            rawio = MockFileIO(data)
+            bufio = io.BufferedReader(rawio, buffer_size=bufsize)
             pos = 0
             for nbytes in buf_read_sizes:
-                self.assertEquals(bufIo.read(nbytes), data[pos:pos+nbytes])
+                self.assertEquals(bufio.read(nbytes), data[pos:pos+nbytes])
                 pos += nbytes
-            self.assertEquals(rawIo.read_history, raw_read_sizes)
+            self.assertEquals(rawio.read_history, raw_read_sizes)
 
     def testReadNonBlocking(self):
         # Inject some None's in there to simulate EWOULDBLOCK
-        rawIo = MockIO((b"abc", b"d", None, b"efg", None, None))
-        bufIo = io.BufferedReader(rawIo)
+        rawio = MockIO((b"abc", b"d", None, b"efg", None, None))
+        bufio = io.BufferedReader(rawio)
 
-        self.assertEquals(b"abcd", bufIo.read(6))
-        self.assertEquals(b"e", bufIo.read(1))
-        self.assertEquals(b"fg", bufIo.read())
-        self.assert_(None is bufIo.read())
-        self.assertEquals(b"", bufIo.read())
+        self.assertEquals(b"abcd", bufio.read(6))
+        self.assertEquals(b"e", bufio.read(1))
+        self.assertEquals(b"fg", bufio.read())
+        self.assert_(None is bufio.read())
+        self.assertEquals(b"", bufio.read())
 
     def testReadToEof(self):
-        rawIo = MockIO((b"abc", b"d", b"efg"))
-        bufIo = io.BufferedReader(rawIo)
+        rawio = MockIO((b"abc", b"d", b"efg"))
+        bufio = io.BufferedReader(rawio)
 
-        self.assertEquals(b"abcdefg", bufIo.read(9000))
+        self.assertEquals(b"abcdefg", bufio.read(9000))
 
     def testReadNoArgs(self):
-        rawIo = MockIO((b"abc", b"d", b"efg"))
-        bufIo = io.BufferedReader(rawIo)
+        rawio = MockIO((b"abc", b"d", b"efg"))
+        bufio = io.BufferedReader(rawio)
 
-        self.assertEquals(b"abcdefg", bufIo.read())
+        self.assertEquals(b"abcdefg", bufio.read())
 
     def testFileno(self):
-        rawIo = MockIO((b"abc", b"d", b"efg"))
-        bufIo = io.BufferedReader(rawIo)
+        rawio = MockIO((b"abc", b"d", b"efg"))
+        bufio = io.BufferedReader(rawio)
 
-        self.assertEquals(42, bufIo.fileno())
+        self.assertEquals(42, bufio.fileno())
 
     def testFilenoNoFileno(self):
         # XXX will we always have fileno() function? If so, kill
@@ -282,55 +283,55 @@ class BufferedWriterTest(unittest.TestCase):
     def testWrite(self):
         # Write to the buffered IO but don't overflow the buffer.
         writer = MockIO()
-        bufIo = io.BufferedWriter(writer, 8)
+        bufio = io.BufferedWriter(writer, 8)
 
-        bufIo.write(b"abc")
+        bufio.write(b"abc")
 
-        self.assertFalse(writer._writeStack)
+        self.assertFalse(writer._write_stack)
 
     def testWriteOverflow(self):
         writer = MockIO()
-        bufIo = io.BufferedWriter(writer, 8)
+        bufio = io.BufferedWriter(writer, 8)
 
-        bufIo.write(b"abc")
-        bufIo.write(b"defghijkl")
+        bufio.write(b"abc")
+        bufio.write(b"defghijkl")
 
-        self.assertEquals(b"abcdefghijkl", writer._writeStack[0])
+        self.assertEquals(b"abcdefghijkl", writer._write_stack[0])
 
     def testWriteNonBlocking(self):
         raw = MockNonBlockWriterIO((9, 2, 22, -6, 10, 12, 12))
-        bufIo = io.BufferedWriter(raw, 8, 16)
+        bufio = io.BufferedWriter(raw, 8, 16)
 
-        bufIo.write(b"asdf")
-        bufIo.write(b"asdfa")
+        bufio.write(b"asdf")
+        bufio.write(b"asdfa")
         self.assertEquals(b"asdfasdfa", raw._write_stack[0])
 
-        bufIo.write(b"asdfasdfasdf")
+        bufio.write(b"asdfasdfasdf")
         self.assertEquals(b"asdfasdfasdf", raw._write_stack[1])
-        bufIo.write(b"asdfasdfasdf")
+        bufio.write(b"asdfasdfasdf")
         self.assertEquals(b"dfasdfasdf", raw._write_stack[2])
         self.assertEquals(b"asdfasdfasdf", raw._write_stack[3])
 
-        bufIo.write(b"asdfasdfasdf")
+        bufio.write(b"asdfasdfasdf")
 
         # XXX I don't like this test. It relies too heavily on how the
         # algorithm actually works, which we might change. Refactor
         # later.
 
     def testFileno(self):
-        rawIo = MockIO((b"abc", b"d", b"efg"))
-        bufIo = io.BufferedWriter(rawIo)
+        rawio = MockIO((b"abc", b"d", b"efg"))
+        bufio = io.BufferedWriter(rawio)
 
-        self.assertEquals(42, bufIo.fileno())
+        self.assertEquals(42, bufio.fileno())
 
     def testFlush(self):
         writer = MockIO()
-        bufIo = io.BufferedWriter(writer, 8)
+        bufio = io.BufferedWriter(writer, 8)
 
-        bufIo.write(b"abc")
-        bufIo.flush()
+        bufio.write(b"abc")
+        bufio.flush()
 
-        self.assertEquals(b"abc", writer._writeStack[0])
+        self.assertEquals(b"abc", writer._write_stack[0])
 
 
 class BufferedRWPairTest(unittest.TestCase):
@@ -352,9 +353,9 @@ class BufferedRandomTest(unittest.TestCase):
         self.assertEqual(b"as", rw.read(2))
         rw.write(b"ddd")
         rw.write(b"eee")
-        self.assertFalse(raw._writeStack) # Buffer writes
+        self.assertFalse(raw._write_stack) # Buffer writes
         self.assertEqual(b"ghjk", rw.read()) # This read forces write flush
-        self.assertEquals(b"dddeee", raw._writeStack[0])
+        self.assertEquals(b"dddeee", raw._write_stack[0])
 
     def testSeekAndTell(self):
         raw = io.BytesIO(b"asdfghjkl")
@@ -400,19 +401,19 @@ class TextIOWrapperTest(unittest.TestCase):
 
                     for newline, exp_line_ends in tests:
                         exp_lines = [ pad + line for line in exp_line_ends ]
-                        bufIo = io.BufferedReader(io.BytesIO(data))
-                        textIo = io.TextIOWrapper(bufIo, newline=newline,
+                        bufio = io.BufferedReader(io.BytesIO(data))
+                        textio = io.TextIOWrapper(bufio, newline=newline,
                                                   encoding=encoding)
                         if do_reads:
                             got_lines = []
                             while True:
-                                c2 = textIo.read(2)
+                                c2 = textio.read(2)
                                 if c2 == '':
                                     break
                                 self.assertEquals(len(c2), 2)
-                                got_lines.append(c2 + textIo.readline())
+                                got_lines.append(c2 + textio.readline())
                         else:
-                            got_lines = list(textIo)
+                            got_lines = list(textio)
 
                         for got_line, exp_line in zip(got_lines, exp_lines):
                             self.assertEquals(got_line, exp_line)
@@ -427,4 +428,4 @@ def test_main():
                               BufferedRandomTest, TextIOWrapperTest)
 
 if __name__ == "__main__":
-    test_main()
+    unittest.main()