http://hg.python.org/cpython/rev/4f3b6eff2ede changeset: 84944:4f3b6eff2ede user: R David Murray <rdmurray@bitdance.com> date: Wed Jul 31 20:48:26 2013 -0400 summary: #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. files: Doc/library/wave.rst | 15 ++++---- Lib/test/test_wave.py | 53 +++++++++++++++++++++++------- Lib/wave.py | 12 ++++-- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/Doc/library/wave.rst b/Doc/library/wave.rst --- a/Doc/library/wave.rst +++ b/Doc/library/wave.rst @@ -19,21 +19,20 @@ .. function:: open(file, mode=None) 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. - ``'w'``, ``'wb'`` + ``'wb'`` Write only mode. Note that it does not allow read/write WAV files. - A *mode* of ``'r'`` or ``'rb'`` returns a :class:`Wave_read` object, while a - *mode* of ``'w'`` or ``'wb'`` returns a :class:`Wave_write` object. If - *mode* is omitted and a file-like object is passed as *file*, ``file.mode`` - is used as the default value for *mode* (the ``'b'`` flag is still added if - necessary). + A *mode* of ``'rb'`` returns a :class:`Wave_read` object, while a *mode* of + ``'wb'`` returns a :class:`Wave_write` object. If *mode* is omitted and a + file-like object is passed as *file*, ``file.mode`` is used as the default + value for *mode*. 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 diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -69,22 +69,49 @@ self.assertEqual(params.comptype, self.f.getcomptype()) self.assertEqual(params.compname, self.f.getcompname()) - def test_context_manager(self): - self.f = wave.open(TESTFN, 'wb') - self.f.setnchannels(nchannels) - self.f.setsampwidth(sampwidth) - self.f.setframerate(framerate) - self.f.close() + def test_wave_write_context_manager_calls_close(self): + # Close checks for a minimum header and will raise an error + # if it is not set, so this proves that close is called. + with self.assertRaises(wave.Error): + with wave.open(TESTFN, 'wb') as f: + 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: self.assertFalse(f.getfp().closed) - self.assertIs(f.getfp(), None) - - with open(TESTFN, 'wb') as testfile: - with self.assertRaises(wave.Error): - with wave.open(testfile, 'wb'): - pass - self.assertEqual(testfile.closed, False) + params = f.getparams() + self.assertEqual(params.nchannels, nchannels) + self.assertEqual(params.sampwidth, sampwidth) + self.assertEqual(params.framerate, framerate) + self.assertIsNone(f.getfp()) if __name__ == '__main__': diff --git a/Lib/wave.py b/Lib/wave.py --- a/Lib/wave.py +++ b/Lib/wave.py @@ -448,11 +448,13 @@ def close(self): if self._file: - self._ensure_header_written(0) - if self._datalength != self._datawritten: - self._patchheader() - self._file.flush() - self._file = None + try: + self._ensure_header_written(0) + if self._datalength != self._datawritten: + self._patchheader() + self._file.flush() + finally: + self._file = None if self._i_opened_the_file: self._i_opened_the_file.close() self._i_opened_the_file = None -- Repository URL: http://hg.python.org/cpython