From 74e1acf6e38c00e42f00636066f00d6a5a6b73fc Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sun, 30 Apr 2023 10:50:09 +0200 Subject: [PATCH 1/6] ci: update python versions (try to) use semver x-ranges in order to avoid needing manual updates of this in the future --- .github/workflows/ci.yaml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 96b230c596..7d5eaf329f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,7 +12,7 @@ jobs: strategy: matrix: platform: [ubuntu-latest, windows-latest] - python-version: ['3.7', '3.8', '3.9', '3.10', '3.11-dev'] + python-version: ['3.7', '3.8', '3.9', '3.x', '3.12-dev'] env: PY_COLORS: 1 @@ -45,17 +45,17 @@ jobs: sudo apt install ffmpeg # For replaygain - name: Test older Python versions with tox - if: matrix.python-version != '3.10' && matrix.python-version != '3.11-dev' + if: matrix.python-version != '3.x' && matrix.python-version != '3.12-dev' run: | tox -e py-test - name: Test latest Python version with tox and get coverage - if: matrix.python-version == '3.10' + if: matrix.python-version == '3.x' run: | tox -vv -e py-cov - name: Test latest Python version with tox and mypy - if: matrix.python-version == '3.10' + if: matrix.python-version == '3.x' # continue-on-error is not ideal since it doesn't give a visible # warning, but there doesn't seem to be anything better: # https://github.com/actions/toolkit/issues/399 @@ -64,7 +64,7 @@ jobs: tox -vv -e py-mypy - name: Test nightly Python version with tox - if: matrix.python-version == '3.11-dev' + if: matrix.python-version == '3.12-dev' # continue-on-error is not ideal since it doesn't give a visible # warning, but there doesn't seem to be anything better: # https://github.com/actions/toolkit/issues/399 @@ -73,7 +73,7 @@ jobs: tox -e py-test - name: Upload code coverage - if: matrix.python-version == '3.10' + if: matrix.python-version == '3.x' run: | pip install codecov || true codecov || true @@ -87,10 +87,10 @@ jobs: steps: - uses: actions/checkout@v2 - - name: Set up Python 3.10 + - name: Set up Python 3.x uses: actions/setup-python@v2 with: - python-version: '3.10' + python-version: '3.x' - name: Install base dependencies run: | @@ -109,10 +109,10 @@ jobs: steps: - uses: actions/checkout@v2 - - name: Set up Python 3.10 + - name: Set up Python 3.x uses: actions/setup-python@v2 with: - python-version: '3.10' + python-version: '3.x' - name: Install base dependencies run: | From 0d1fa172de4d5ba0350f5329430bbb60b1bb54e2 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Thu, 4 May 2023 08:13:45 +0200 Subject: [PATCH 2/6] tests: explicitly close sqlite3 connections on Python 3.11, the Windows CI started crashing due to the database file remainig open unexpectly in test shutdown; this attemps to fix that --- test/test_dbcore.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 80d85c3bb4..c25157b85a 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -171,6 +171,7 @@ def setUpClass(cls): 'insert into test (field_one, field_two) values (4, 2)' ) old_lib._connection().commit() + old_lib._connection().close() del old_lib @classmethod @@ -190,6 +191,7 @@ def test_open_with_same_fields_leaves_untouched(self): c = new_lib._connection().cursor() c.execute("select * from test") row = c.fetchone() + c.connection.close() self.assertEqual(len(row.keys()), len(ModelFixture2._fields)) def test_open_with_new_field_adds_column(self): @@ -197,6 +199,7 @@ def test_open_with_new_field_adds_column(self): c = new_lib._connection().cursor() c.execute("select * from test") row = c.fetchone() + c.connection.close() self.assertEqual(len(row.keys()), len(ModelFixture3._fields)) def test_open_with_fewer_fields_leaves_untouched(self): @@ -204,6 +207,7 @@ def test_open_with_fewer_fields_leaves_untouched(self): c = new_lib._connection().cursor() c.execute("select * from test") row = c.fetchone() + c.connection.close() self.assertEqual(len(row.keys()), len(ModelFixture2._fields)) def test_open_with_multiple_new_fields(self): @@ -211,12 +215,15 @@ def test_open_with_multiple_new_fields(self): c = new_lib._connection().cursor() c.execute("select * from test") row = c.fetchone() + c.connection.close() self.assertEqual(len(row.keys()), len(ModelFixture4._fields)) def test_extra_model_adds_table(self): new_lib = DatabaseFixtureTwoModels(self.libfile) try: - new_lib._connection().execute("select * from another") + c = new_lib._connection() + c.execute("select * from another") + c.close() except sqlite3.OperationalError: self.fail("select failed") From 6f0d305489a4add2935df4567a4a129f49269a2b Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Thu, 4 May 2023 08:48:25 +0200 Subject: [PATCH 3/6] tests: more explicit sqlite3 connection close()ing --- beets/dbcore/db.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 94396f81b0..a3909cf141 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -975,7 +975,11 @@ def _create_connection(self): # bytestring paths here on Python 3, so we need to # provide a `str` using `py3_path`. conn = sqlite3.connect( - py3_path(self.path), timeout=self.timeout + py3_path(self.path), + timeout=self.timeout, + # We have our own same-thread checks in _connection(), but need to + # call conn.close() in _close() + check_same_thread=False, ) self.add_functions(conn) @@ -1005,7 +1009,9 @@ def _close(self): unusable; new connections can still be opened on demand. """ with self._shared_map_lock: - self._connections.clear() + while self._connections: + _thread_id, conn = self._connections.popitem() + conn.close() @contextlib.contextmanager def _tx_stack(self): From 5de1d1261021af16540da64ca24f92bae6407da9 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Thu, 4 May 2023 09:12:41 +0200 Subject: [PATCH 4/6] tests: address some warnings - some helper functions were incorrectly detected as being tests due to their name - use of the deprecated assertEquals alias --- test/test_importer.py | 8 ++++---- test/test_m3ufile.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index 7de00c65ea..967f5fc957 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1221,7 +1221,7 @@ def test_small_single_artist_album(self): self.assertFalse(self.items[0].comp) -def test_album_info(*args, **kwargs): +def match_album_mock(*args, **kwargs): """Create an AlbumInfo object for testing. """ track_info = TrackInfo( @@ -1240,7 +1240,7 @@ def test_album_info(*args, **kwargs): return iter([album_info]) -@patch('beets.autotag.mb.match_album', Mock(side_effect=test_album_info)) +@patch('beets.autotag.mb.match_album', Mock(side_effect=match_album_mock)) class ImportDuplicateAlbumTest(unittest.TestCase, TestHelper, _common.Assertions): @@ -1349,13 +1349,13 @@ def add_album_fixture(self, **kwargs): return album -def test_track_info(*args, **kwargs): +def match_track_mock(*args, **kwargs): return iter([TrackInfo( artist='artist', title='title', track_id='new trackid', index=0,)]) -@patch('beets.autotag.mb.match_track', Mock(side_effect=test_track_info)) +@patch('beets.autotag.mb.match_track', Mock(side_effect=match_track_mock)) class ImportDuplicateSingletonTest(unittest.TestCase, TestHelper, _common.Assertions): diff --git a/test/test_m3ufile.py b/test/test_m3ufile.py index a24dc6ca88..2c1284aabb 100644 --- a/test/test_m3ufile.py +++ b/test/test_m3ufile.py @@ -77,12 +77,12 @@ def test_playlist_write_and_read_unicode_windows(self): self.assertTrue(path.exists(the_playlist_file)) m3ufile_read = M3UFile(the_playlist_file) m3ufile_read.load() - self.assertEquals( + self.assertEqual( m3ufile.media_list[0], bytestring_path( path.join('x:\\', 'This', 'is', 'å', 'path', 'to_a_file.mp3')) ) - self.assertEquals( + self.assertEqual( m3ufile.media_list[1], bytestring_path(r"x:\This\is\another\path\tö_a_file.mp3"), bytestring_path(path.join( From 7169ac81f5754dbc3d6265077554d1383973d6b0 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Thu, 4 May 2023 09:18:19 +0200 Subject: [PATCH 5/6] tests: close library in ParentalDirCreation test --- test/test_ui_init.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/test_ui_init.py b/test/test_ui_init.py index 9f9487a6a7..870e45f4ca 100644 --- a/test/test_ui_init.py +++ b/test/test_ui_init.py @@ -135,7 +135,8 @@ def test_create_yes(self): test_config = deepcopy(config) test_config['library'] = non_exist_path with control_stdin('y'): - ui._open_library(test_config) + lib = ui._open_library(test_config) + lib._close() def test_create_no(self): non_exist_path_parent = _common.util.py3_path( @@ -147,12 +148,14 @@ def test_create_no(self): with control_stdin('n'): try: - ui._open_library(test_config) + lib = ui._open_library(test_config) except ui.UserError: if os.path.exists(non_exist_path_parent): shutil.rmtree(non_exist_path_parent) raise OSError("Parent directories should not be created.") else: + if lib: + lib._close() raise OSError("Parent directories should not be created.") From 89b7e1d8648c000fed54e70e046edb9929a359ae Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Thu, 4 May 2023 09:40:32 +0200 Subject: [PATCH 6/6] dbcore: bail out if the database does't support multi-threading which we rely on. This test doesn't actually work for Python < 3.11, since Python used to hardcode the threadsafety value to 1 --- beets/dbcore/db.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index a3909cf141..6621b55d36 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -923,6 +923,11 @@ class Database: """ def __init__(self, path, timeout=5.0): + if sqlite3.threadsafety == 0: + raise RuntimeError( + "sqlite3 must be compiled with multi-threading support" + ) + self.path = path self.timeout = timeout