From f5b20114b42f35513853041427d4ea263efa618e Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Fri, 5 May 2023 20:34:57 +0200 Subject: [PATCH 1/7] fetchart: revert a cleanup from 4774 that could break plugins cf. @arogl's comment https://github.com/beetbox/beets/commit/254bb297c894c0323174012ff27500867416d02b#commitcomment-111922347 > Now that this has been merged, external plugins that add to the fetchart plugin now fail with: > > ```AttributeError: module 'beetsplug.fetchart' has no attribute 'SOURCES_ALL'`` --- beetsplug/fetchart.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 39b5a52d4..24368c86f 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -977,6 +977,12 @@ class Spotify(RemoteArtSource): return # Try each source in turn. +# Note that SOURCES_ALL is redundant (and presently unused). However, we keep +# it around nn order not break plugins that "register" (a.k.a. monkey-patch) +# their own fetchart sources. +SOURCES_ALL = ['filesystem', 'coverart', 'itunes', 'amazon', 'albumart', + 'wikipedia', 'google', 'fanarttv', 'lastfm', 'spotify'] + ART_SOURCES = { 'filesystem': FileSystem, 'coverart': CoverArtArchive, 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 2/7] 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 96b230c59..7d5eaf329 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 3/7] 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 80d85c3bb..c25157b85 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -171,6 +171,7 @@ class MigrationTest(unittest.TestCase): '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 @@ class MigrationTest(unittest.TestCase): 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 @@ class MigrationTest(unittest.TestCase): 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 @@ class MigrationTest(unittest.TestCase): 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 @@ class MigrationTest(unittest.TestCase): 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 4/7] 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 94396f81b..a3909cf14 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -975,7 +975,11 @@ class Database: # 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 @@ class Database: 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 5/7] 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 7de00c65e..967f5fc95 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1221,7 +1221,7 @@ class InferAlbumDataTest(_common.TestCase): 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 @@ class ImportDuplicateAlbumTest(unittest.TestCase, TestHelper, 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 a24dc6ca8..2c1284aab 100644 --- a/test/test_m3ufile.py +++ b/test/test_m3ufile.py @@ -77,12 +77,12 @@ class M3UFileTest(unittest.TestCase): 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 6/7] 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 9f9487a6a..870e45f4c 100644 --- a/test/test_ui_init.py +++ b/test/test_ui_init.py @@ -135,7 +135,8 @@ class ParentalDirCreation(_common.TestCase): 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 @@ class ParentalDirCreation(_common.TestCase): 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 7/7] 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 a3909cf14..6621b55d3 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