From 4712b6313a4e4ee137303b9bb8df86e577069fc6 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sat, 18 Jul 2026 16:09:34 +0300 Subject: [PATCH 01/10] gh-153953: Increase test coverage for the wave module Add tests for previously-uncovered paths in Lib/wave.py, all reachable through the public API: * Wave_write parameter validation: rejecting bad channel counts, sample widths, compression types and formats; the "not set" errors from the getters; the "cannot change parameters after starting to write" guards on every setter; and tell(). * Wave_read error handling: rejecting an unknown WAVE_FORMAT_EXTENSIBLE subformat, raising EOFError on a truncated fmt chunk, skipping unknown chunks, getfp(), and closing the file when opening a malformed path fails. * wave.open() rejecting an invalid mode. This raises line coverage of Lib/wave.py under test_wave from 317 to 345 of 449 executable lines. Test-only change; no behavior change. --- Lib/test/test_wave.py | 167 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index d3723c04820d9d..fb04bdae528715 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -475,5 +475,172 @@ def test_open_pathlike(self): pass +class WaveReadErrorTest(unittest.TestCase): + """Cover error and edge paths of Wave_read, _Chunk and wave.open().""" + + FMT_PCM = struct.pack(' Date: Sun, 19 Jul 2026 02:32:06 +0300 Subject: [PATCH 02/10] Update Lib/test/test_wave.py Co-authored-by: Victor Stinner --- Lib/test/test_wave.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index fb04bdae528715..0c7078c7dc2310 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -531,10 +531,14 @@ def test_read_skips_unknown_chunk(self): self.assertEqual(r.getnframes(), 4) self.assertEqual(r.readframes(4), data) - def test_getfp_returns_underlying_file(self): + def test_getfp(self): b = self._wave_file((b'fmt ', self.FMT_PCM), (b'data', b'')) - with wave.open(io.BytesIO(b)) as r: - self.assertIsNotNone(r.getfp()) + fp = io.BytesIO(b) + with wave.open(fp) as r: + chunk = r.getfp() + self.assertIsNotNone(chunk) + self.assertIs(chunk.file, fp) + self.assertEqual(chunk.chunkname, b'RIFF') def test_open_invalid_mode(self): with self.assertRaisesRegex(wave.Error, "mode must be"): From e2019ff770fa8216e514cbf4d68eda287a917bb6 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 19 Jul 2026 02:36:17 +0300 Subject: [PATCH 03/10] changed tests based on comments by vstinner --- Lib/test/test_wave.py | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index 0c7078c7dc2310..9934c09272ca3b 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -172,6 +172,15 @@ def test__all__(self): not_exported = {'KSDATAFORMAT_SUBTYPE_PCM'} support.check__all__(self, wave, not_exported=not_exported) + def test_getfp(self): + b = self._wave_file((b'fmt ', self.FMT_PCM), (b'data', b'')) + fp = io.BytesIO(b) + with wave.open(fp) as r: + chunk = r.getfp() + self.assertIsNotNone(chunk) + self.assertIs(chunk.file, fp) + self.assertEqual(chunk.chunkname, b'RIFF') + class WaveLowLevelTest(unittest.TestCase): @@ -473,6 +482,10 @@ def test_open_pathlike(self): with wave.open(fake_path, 'rb') as f: pass + + def test_open_invalid_mode(self): + with self.assertRaisesRegex(wave.Error, "mode must be"): + wave.open(io.BytesIO(), 'xb') class WaveReadErrorTest(unittest.TestCase): @@ -531,29 +544,6 @@ def test_read_skips_unknown_chunk(self): self.assertEqual(r.getnframes(), 4) self.assertEqual(r.readframes(4), data) - def test_getfp(self): - b = self._wave_file((b'fmt ', self.FMT_PCM), (b'data', b'')) - fp = io.BytesIO(b) - with wave.open(fp) as r: - chunk = r.getfp() - self.assertIsNotNone(chunk) - self.assertIs(chunk.file, fp) - self.assertEqual(chunk.chunkname, b'RIFF') - - def test_open_invalid_mode(self): - with self.assertRaisesRegex(wave.Error, "mode must be"): - wave.open(io.BytesIO(), 'xb') - - def test_open_path_closes_file_on_error(self): - # When opening by path fails to parse, the file we opened is closed - # and the parse error is propagated. - with tempfile.NamedTemporaryFile(delete=False) as fp: - fp.write(b'not a wave file') - filename = fp.name - self.addCleanup(unlink, filename) - with self.assertRaises(wave.Error): - wave.open(filename, 'rb') - class WaveWriteValidationTest(unittest.TestCase): """Cover parameter-validation paths of Wave_write.""" From 28c5f9c2fcf6600075e13db6e78b6b67d7553c3d Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 19 Jul 2026 13:34:46 +0300 Subject: [PATCH 04/10] gh-153953: Fix test_getfp after moving it to MiscTestCase The test relied on the _wave_file helper and FMT_PCM constant, which only exist on WaveReadErrorTest. Build the WAVE file with the wave module so the test is self-contained. --- Lib/test/test_wave.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index 9934c09272ca3b..1efa8e3b485b4f 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -173,8 +173,12 @@ def test__all__(self): support.check__all__(self, wave, not_exported=not_exported) def test_getfp(self): - b = self._wave_file((b'fmt ', self.FMT_PCM), (b'data', b'')) - fp = io.BytesIO(b) + fp = io.BytesIO() + with wave.open(fp, 'wb') as w: + w.setnchannels(1) + w.setsampwidth(1) + w.setframerate(11025) + fp.seek(0) with wave.open(fp) as r: chunk = r.getfp() self.assertIsNotNone(chunk) From 2ce387f52caa7b00e798981e2cf011809115cc5a Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 19 Jul 2026 13:42:00 +0300 Subject: [PATCH 05/10] removes trailing whitespace --- Lib/test/test_wave.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index 1efa8e3b485b4f..0832e2a2db5d25 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -486,7 +486,7 @@ def test_open_pathlike(self): with wave.open(fake_path, 'rb') as f: pass - + def test_open_invalid_mode(self): with self.assertRaisesRegex(wave.Error, "mode must be"): wave.open(io.BytesIO(), 'xb') From 83a14bbb977830c0e2174f3c4934c3691dd4a914 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 26 Jul 2026 18:12:35 +0300 Subject: [PATCH 06/10] apply reviewer's comments --- Lib/test/test_wave.py | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index 0832e2a2db5d25..784c56e490565b 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -552,20 +552,24 @@ def test_read_skips_unknown_chunk(self): class WaveWriteValidationTest(unittest.TestCase): """Cover parameter-validation paths of Wave_write.""" - def open_writer(self): - w = wave.open(io.BytesIO(), 'wb') - self.addCleanup(self._close_quietly, w) + @staticmethod + def _close(w): + try: + # Make sure that all parameters are set + w.setnchannels(1) + w.setsampwidth(2) + w.setframerate(44100) + except wave.Error: + # Ignore "cannot change parameters after starting to write" error + pass + + w.close() + + def open_writer(self): + w = wave.open(io.BytesIO(), 'wb') + self.addCleanup(self._close, w) return w - @staticmethod - def _close_quietly(w): - # A writer whose required parameters are never set raises on close; - # swallow that so it does not mask the behaviour under test. - try: - w.close() - except wave.Error: - pass - def test_setnchannels_rejects_nonpositive(self): w = self.open_writer() with self.assertRaisesRegex(wave.Error, 'bad # of channels'): @@ -609,14 +613,23 @@ def test_getparams_incomplete(self): with self.assertRaisesRegex(wave.Error, 'not all parameters set'): w.getparams() - def test_tell_reports_frames_written(self): - w = self.open_writer() - w.setnchannels(1) - w.setsampwidth(2) - w.setframerate(44100) - self.assertEqual(w.tell(), 0) - w.writeframes(b'\x00\x00' * 5) - self.assertEqual(w.tell(), 5) + def test_tell(self): + def check_nframes(nframes): + self.assertEqual(w.tell(), nframes) + self.assertEqual(w.getnframes(), nframes) + + w = self.open_writer() + w.setnchannels(1) + w.setsampwidth(2) + w.setframerate(44100) + check_nframes(0) + + frame = b'\x00\x00' + w.writeframes(frame * 5) + check_nframes(5) + + w.writeframes(frame * 3) + check_nframes(8) def test_cannot_change_params_after_write(self): setters = ( @@ -639,6 +652,5 @@ def test_cannot_change_params_after_write(self): 'cannot change parameters'): getattr(w, name)(*args) - if __name__ == '__main__': unittest.main() From f55830c29e7081200aba06d3867c0d7660ddf0a2 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 26 Jul 2026 18:13:35 +0300 Subject: [PATCH 07/10] Update Lib/test/test_wave.py Co-authored-by: Victor Stinner --- Lib/test/test_wave.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index 784c56e490565b..ee7b094005f575 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -632,6 +632,12 @@ def check_nframes(nframes): check_nframes(8) def test_cannot_change_params_after_write(self): + w = self.open_writer() + w.setnchannels(1) + w.setsampwidth(2) + w.setframerate(44100) + w.writeframes(b'\x00\x00') + setters = ( ('setnchannels', (1,)), ('setsampwidth', (2,)), @@ -643,11 +649,6 @@ def test_cannot_change_params_after_write(self): ) for name, args in setters: with self.subTest(setter=name): - w = self.open_writer() - w.setnchannels(1) - w.setsampwidth(2) - w.setframerate(44100) - w.writeframes(b'\x00\x00') with self.assertRaisesRegex(wave.Error, 'cannot change parameters'): getattr(w, name)(*args) From 837790ee992aa44287e8a60b45f55f81fa42119a Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Mon, 27 Jul 2026 15:37:25 +0300 Subject: [PATCH 08/10] fix linting --- Lib/test/test_wave.py | 60 +++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index ee7b094005f575..daaa8f7cb7d953 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -552,22 +552,22 @@ def test_read_skips_unknown_chunk(self): class WaveWriteValidationTest(unittest.TestCase): """Cover parameter-validation paths of Wave_write.""" - @staticmethod - def _close(w): - try: - # Make sure that all parameters are set - w.setnchannels(1) - w.setsampwidth(2) - w.setframerate(44100) - except wave.Error: - # Ignore "cannot change parameters after starting to write" error - pass - - w.close() - - def open_writer(self): - w = wave.open(io.BytesIO(), 'wb') - self.addCleanup(self._close, w) + @staticmethod + def _close(w): + try: + # Make sure that all parameters are set + w.setnchannels(1) + w.setsampwidth(2) + w.setframerate(44100) + except wave.Error: + # Ignore "cannot change parameters after starting to write" error + pass + + w.close() + + def open_writer(self): + w = wave.open(io.BytesIO(), 'wb') + self.addCleanup(self._close, w) return w def test_setnchannels_rejects_nonpositive(self): @@ -613,23 +613,23 @@ def test_getparams_incomplete(self): with self.assertRaisesRegex(wave.Error, 'not all parameters set'): w.getparams() - def test_tell(self): - def check_nframes(nframes): - self.assertEqual(w.tell(), nframes) - self.assertEqual(w.getnframes(), nframes) + def test_tell(self): + def check_nframes(nframes): + self.assertEqual(w.tell(), nframes) + self.assertEqual(w.getnframes(), nframes) - w = self.open_writer() - w.setnchannels(1) - w.setsampwidth(2) - w.setframerate(44100) - check_nframes(0) + w = self.open_writer() + w.setnchannels(1) + w.setsampwidth(2) + w.setframerate(44100) + check_nframes(0) - frame = b'\x00\x00' - w.writeframes(frame * 5) - check_nframes(5) + frame = b'\x00\x00' + w.writeframes(frame * 5) + check_nframes(5) - w.writeframes(frame * 3) - check_nframes(8) + w.writeframes(frame * 3) + check_nframes(8) def test_cannot_change_params_after_write(self): w = self.open_writer() From 46b668d54b454088f23fe7f94b62d3595a4d9eb7 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Fri, 7 Aug 2026 20:19:11 +0300 Subject: [PATCH 09/10] Group the Wave_write get and set tests Replaces the eight one-assertion tests with the two grouped ones from the review, which also cover bad frame rate, setparams validation and the comptype round trip. --- Lib/test/test_wave.py | 61 ++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index daaa8f7cb7d953..4142ee1f53688a 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -570,48 +570,61 @@ def open_writer(self): self.addCleanup(self._close, w) return w - def test_setnchannels_rejects_nonpositive(self): + def test_get(self): w = self.open_writer() - with self.assertRaisesRegex(wave.Error, 'bad # of channels'): - w.setnchannels(0) + self.assertEqual(w.getformat(), wave.WAVE_FORMAT_PCM) + self.assertEqual(w.getnframes(), 0) + # getcomptype() and getcompname() raise AttributeError + # until setcomptype() is called - def test_getnchannels_not_set(self): - w = self.open_writer() with self.assertRaisesRegex(wave.Error, 'number of channels not set'): w.getnchannels() + with self.assertRaisesRegex(wave.Error, 'sample width not set'): + w.getsampwidth() + with self.assertRaisesRegex(wave.Error, 'frame rate not set'): + w.getframerate() + with self.assertRaisesRegex(wave.Error, 'not all parameters set'): + w.getparams() + + def test_set(self): + w = self.open_writer() + + w.setnchannels(1) + self.assertEqual(w.getnchannels(), 1) + with self.assertRaisesRegex(wave.Error, 'bad # of channels'): + w.setnchannels(0) - def test_setsampwidth_rejects_out_of_range(self): + w.setsampwidth(2) + self.assertEqual(w.getsampwidth(), 2) for width in (0, 5): with self.subTest(width=width): - w = self.open_writer() with self.assertRaisesRegex(wave.Error, 'bad sample width'): w.setsampwidth(width) - def test_getsampwidth_not_set(self): - w = self.open_writer() - with self.assertRaisesRegex(wave.Error, 'sample width not set'): - w.getsampwidth() + w.setframerate(44100) + self.assertEqual(w.getframerate(), 44100) + with self.assertRaisesRegex(wave.Error, 'bad frame rate'): + w.setframerate(0) - def test_getframerate_not_set(self): - w = self.open_writer() - with self.assertRaisesRegex(wave.Error, 'frame rate not set'): - w.getframerate() + w.setnframes(10) + self.assertEqual(w.getnframes(), 0) - def test_setcomptype_rejects_unknown(self): - w = self.open_writer() + w.setcomptype('NONE', 'not compressed') + self.assertEqual(w.getcomptype(), 'NONE') + self.assertEqual(w.getcompname(), 'not compressed') with self.assertRaisesRegex(wave.Error, 'unsupported compression type'): w.setcomptype('ADPCM', 'unsupported') - def test_setformat_rejects_unknown(self): - w = self.open_writer() + w.setformat(wave.WAVE_FORMAT_PCM) + self.assertEqual(w.getformat(), wave.WAVE_FORMAT_PCM) with self.assertRaisesRegex(wave.Error, 'unsupported wave format'): w.setformat(0x1234) - def test_getparams_incomplete(self): - w = self.open_writer() - w.setnchannels(1) - with self.assertRaisesRegex(wave.Error, 'not all parameters set'): - w.getparams() + w.setparams((1, 2, 44100, 0, 'NONE', 'not compressed')) + self.assertEqual(w.getparams(), + (1, 2, 44100, 0, 'NONE', 'not compressed')) + with self.assertRaisesRegex(wave.Error, 'bad # of channels'): + w.setparams((0, 2, 44100, 0, 'NONE', 'not compressed')) def test_tell(self): def check_nframes(nframes): From da56a64b7399a7719e8b83b60dc1dd56b71e92b3 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 10 Aug 2026 16:04:22 +0200 Subject: [PATCH 10/10] Apply suggestion from @vstinner --- Lib/test/test_wave.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py index 4142ee1f53688a..c482de12f7829b 100644 --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -493,7 +493,7 @@ def test_open_invalid_mode(self): class WaveReadErrorTest(unittest.TestCase): - """Cover error and edge paths of Wave_read, _Chunk and wave.open().""" + """Cover error and edge paths of Wave_read, and wave.open().""" FMT_PCM = struct.pack('