#17616: Improve context manager tests, fix bugs in close method and mode docs.
'mode' docs fix: the file must always be opened in binary in Python3. Bug in Wave_write.close: when the close method calls the check that the header exists and it raises an error, the _file attribute never gets set to None, so the next close tries to close the file again and we get an ignored traceback in the __del__ method. The fix is to set _file to None in a finally clause. This represents a behavior change...in theory a program could be checking for the error on close and then doing a recovery action on the still open file and closing it again. But this change will only go into 3.4, so I think that behavior change is acceptable given that it would be pretty weird and unlikely logic to begin with.
This commit is contained in:
parent
abe639f115
commit
536ffe161c
|
@ -19,21 +19,20 @@ The :mod:`wave` module defines the following function and exception:
|
||||||
.. function:: open(file, mode=None)
|
.. function:: open(file, mode=None)
|
||||||
|
|
||||||
If *file* is a string, open the file by that name, otherwise treat it as a
|
If *file* is a string, open the file by that name, otherwise treat it as a
|
||||||
seekable file-like object. *mode* can be any of
|
seekable file-like object. *mode* can be:
|
||||||
|
|
||||||
``'r'``, ``'rb'``
|
``'rb'``
|
||||||
Read only mode.
|
Read only mode.
|
||||||
|
|
||||||
``'w'``, ``'wb'``
|
``'wb'``
|
||||||
Write only mode.
|
Write only mode.
|
||||||
|
|
||||||
Note that it does not allow read/write WAV files.
|
Note that it does not allow read/write WAV files.
|
||||||
|
|
||||||
A *mode* of ``'r'`` or ``'rb'`` returns a :class:`Wave_read` object, while a
|
A *mode* of ``'rb'`` returns a :class:`Wave_read` object, while a *mode* of
|
||||||
*mode* of ``'w'`` or ``'wb'`` returns a :class:`Wave_write` object. If
|
``'wb'`` returns a :class:`Wave_write` object. If *mode* is omitted and a
|
||||||
*mode* is omitted and a file-like object is passed as *file*, ``file.mode``
|
file-like object is passed as *file*, ``file.mode`` is used as the default
|
||||||
is used as the default value for *mode* (the ``'b'`` flag is still added if
|
value for *mode*.
|
||||||
necessary).
|
|
||||||
|
|
||||||
If you pass in a file-like object, the wave object will not close it when its
|
If you pass in a file-like object, the wave object will not close it when its
|
||||||
:meth:`close` method is called; it is the caller's responsibility to close
|
:meth:`close` method is called; it is the caller's responsibility to close
|
||||||
|
|
|
@ -69,22 +69,49 @@ class TestWave(unittest.TestCase):
|
||||||
self.assertEqual(params.comptype, self.f.getcomptype())
|
self.assertEqual(params.comptype, self.f.getcomptype())
|
||||||
self.assertEqual(params.compname, self.f.getcompname())
|
self.assertEqual(params.compname, self.f.getcompname())
|
||||||
|
|
||||||
def test_context_manager(self):
|
def test_wave_write_context_manager_calls_close(self):
|
||||||
self.f = wave.open(TESTFN, 'wb')
|
# Close checks for a minimum header and will raise an error
|
||||||
self.f.setnchannels(nchannels)
|
# if it is not set, so this proves that close is called.
|
||||||
self.f.setsampwidth(sampwidth)
|
with self.assertRaises(wave.Error):
|
||||||
self.f.setframerate(framerate)
|
with wave.open(TESTFN, 'wb') as f:
|
||||||
self.f.close()
|
pass
|
||||||
|
print('in test:', f._file)
|
||||||
|
with self.assertRaises(wave.Error):
|
||||||
|
with open(TESTFN, 'wb') as testfile:
|
||||||
|
with wave.open(testfile):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def test_context_manager_with_open_file(self):
|
||||||
|
with open(TESTFN, 'wb') as testfile:
|
||||||
|
with wave.open(testfile) as f:
|
||||||
|
f.setnchannels(nchannels)
|
||||||
|
f.setsampwidth(sampwidth)
|
||||||
|
f.setframerate(framerate)
|
||||||
|
self.assertFalse(testfile.closed)
|
||||||
|
with open(TESTFN, 'rb') as testfile:
|
||||||
|
with wave.open(testfile) as f:
|
||||||
|
self.assertFalse(f.getfp().closed)
|
||||||
|
params = f.getparams()
|
||||||
|
self.assertEqual(params.nchannels, nchannels)
|
||||||
|
self.assertEqual(params.sampwidth, sampwidth)
|
||||||
|
self.assertEqual(params.framerate, framerate)
|
||||||
|
self.assertIsNone(f.getfp())
|
||||||
|
self.assertFalse(testfile.closed)
|
||||||
|
|
||||||
|
def test_context_manager_with_filename(self):
|
||||||
|
# If the file doesn't get closed, this test won't fail, but it will
|
||||||
|
# produce a resource leak warning.
|
||||||
|
with wave.open(TESTFN, 'wb') as f:
|
||||||
|
f.setnchannels(nchannels)
|
||||||
|
f.setsampwidth(sampwidth)
|
||||||
|
f.setframerate(framerate)
|
||||||
with wave.open(TESTFN) as f:
|
with wave.open(TESTFN) as f:
|
||||||
self.assertFalse(f.getfp().closed)
|
self.assertFalse(f.getfp().closed)
|
||||||
self.assertIs(f.getfp(), None)
|
params = f.getparams()
|
||||||
|
self.assertEqual(params.nchannels, nchannels)
|
||||||
with open(TESTFN, 'wb') as testfile:
|
self.assertEqual(params.sampwidth, sampwidth)
|
||||||
with self.assertRaises(wave.Error):
|
self.assertEqual(params.framerate, framerate)
|
||||||
with wave.open(testfile, 'wb'):
|
self.assertIsNone(f.getfp())
|
||||||
pass
|
|
||||||
self.assertEqual(testfile.closed, False)
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
|
|
12
Lib/wave.py
12
Lib/wave.py
|
@ -448,11 +448,13 @@ class Wave_write:
|
||||||
|
|
||||||
def close(self):
|
def close(self):
|
||||||
if self._file:
|
if self._file:
|
||||||
self._ensure_header_written(0)
|
try:
|
||||||
if self._datalength != self._datawritten:
|
self._ensure_header_written(0)
|
||||||
self._patchheader()
|
if self._datalength != self._datawritten:
|
||||||
self._file.flush()
|
self._patchheader()
|
||||||
self._file = None
|
self._file.flush()
|
||||||
|
finally:
|
||||||
|
self._file = None
|
||||||
if self._i_opened_the_file:
|
if self._i_opened_the_file:
|
||||||
self._i_opened_the_file.close()
|
self._i_opened_the_file.close()
|
||||||
self._i_opened_the_file = None
|
self._i_opened_the_file = None
|
||||||
|
|
Loading…
Reference in New Issue