fix bugs in remote tests

main.py

@@ -776,8 +776,8 @@ def get_parser() -> argparse.ArgumentParser:
 
 
 ## SELECTED FOR DEVELOPMENT
-# TODO: fix bug in spotify api test: artist albums have unexpected keys
 # TODO: separate all CLI concerns to other repo
+# TODO: check loaded numbers on linux again
 # TODO: expand readme + check all example functions work
 # TODO: expand docstrings everywhere
 # TODO: release to pypi + implement CI/CD structure on GitHub

pyproject.toml

@@ -38,10 +38,12 @@ test = [
     "pytest~=7.4",
     "pytest-lazy-fixture~=0.6",
     "pytest-mock~=3.12",
+    "pytest-repeat~=0.9",
     "pycountry~=23.12",
     "requests-mock~=1.11",
 ]
 dev = [
+    "syncify[test]",
     "grip~=4.6",
 ]
 docs = [

src/syncify/shared/remote/library.py

@@ -302,9 +302,7 @@ def load_saved_artists(self) -> None:
             artist = self._object_cls.artist(response=response, api=self.api, skip_checks=True)
 
             current = next((item for item in self._artists if item == artist), None)
-            artist_uris = {artist.uri for artist in self.artists}
             if current is None:
-                assert artist.uri not in artist_uris
                 self._artists.append(artist)
             else:
                 current._response = artist.response

src/syncify/shared/remote/object.py

@@ -198,12 +198,8 @@ def writeable(self) -> bool:
         try:
             self._check_for_api()
         except APIError:
-            print("NO API")
             return False
-        writeable = self.api.user_id == self.owner_id
-        if not writeable:  # TODO: check these values when test_sync next fails
-            print(self.api.user_id, self.owner_id, self.response["owner"])
-        return writeable
+        return self.api.user_id == self.owner_id
 
     @classmethod
     def create(cls, api: RemoteAPI, name: str, public: bool = True, collaborative: bool = False) -> Self:

tests/local/library/testers.py

@@ -9,6 +9,6 @@
 
 class LocalLibraryTester(LibraryTester, LocalCollectionTester, metaclass=ABCMeta):
 
-    @pytest.mark.skip(reason="# TODO: write merge_playlists tests")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_merge_playlists(self, library: LocalLibrary):
         pass

tests/local/track/utils.py

@@ -16,14 +16,13 @@
 class MutagenMock(mutagen.FileType):
     class MutagenInfoMock(mutagen.StreamInfo):
         def __init__(self):
-            self.length = 0
+            self.length = randrange(int(10e4), int(6*10e5))  # 1 second to 10 minutes range
             self.channels = randrange(1, 5)
             self.bitrate = randrange(96, 1400) * 1000
             self.sample_rate = choice([44.1, 48, 88.2, 96]) * 1000
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.mock = True
+    # noinspection PyMissingConstructor
+    def __init__(self):
         self.info = self.MutagenInfoMock()
         self.pictures = []
 

tests/processors/test_compare.py

@@ -221,6 +221,6 @@ def test_from_xml_2(self):
         assert comparer.condition == "contains"
         assert comparer._processor_method == comparer._contains
 
-    @pytest.mark.skip(reason="# TODO: add test for to_xml")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_to_xml(self):
         pass

tests/processors/test_filter.py

@@ -257,6 +257,6 @@ def test_from_xml_2(self, path_mapper: PathStemMapper):
         assert len(filter_.include) == 0
         assert len(filter_.exclude) == 0
 
-    @pytest.mark.skip(reason="# TODO: add test for to_xml")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_to_xml(self):
         pass

tests/processors/test_limit.py

@@ -137,6 +137,6 @@ def test_from_xml_2(self):
         assert limiter.allowance == 1.25
         assert limiter._processor_method == limiter._most_recently_added
 
-    @pytest.mark.skip(reason="# TODO: add test for to_xml")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_to_xml(self):
         pass

tests/processors/test_sort.py

@@ -124,6 +124,6 @@ def test_from_xml_2(self):
         assert sorter.shuffle_by == ShuffleBy.TRACK
         assert sorter.shuffle_weight == 0
 
-    @pytest.mark.skip(reason="# TODO: add test for to_xml")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_to_xml(self):
         pass

tests/shared/api/test_authorise.py

@@ -10,6 +10,7 @@
 from pytest_mock import MockerFixture
 from requests_mock import Mocker
 
+from syncify import MODULE_ROOT
 from syncify.shared.api.authorise import APIAuthoriser
 from syncify.shared.api.exception import APIError
 from tests.shared.api.utils import path_token
@@ -109,7 +110,7 @@ def check_url(url: str):
         socket_listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
 
         requests_mock.post(user_url)
-        mocker.patch("syncify.shared.api.authorise.webopen", new=check_url)
+        mocker.patch(f"{MODULE_ROOT}.shared.api.authorise.webopen", new=check_url)
         mocker.patch.object(socket.socket, attribute="accept", return_value=(socket_listener, None))
         mocker.patch.object(socket.socket, attribute="send")
         mocker.patch.object(socket.socket, attribute="recv", return_value=response.encode("utf-8"))

tests/shared/core/collection.py

@@ -201,12 +201,11 @@ def playlist(self, *args, **kwargs) -> Playlist:
     def collection(self, playlist: Playlist) -> ItemCollection:
         return playlist
 
-    @pytest.mark.skip(reason="# TODO: write merge tests")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_merge(self, playlist: Playlist):
-        # TODO: write merge tests
         pass
 
-    @pytest.mark.skip(reason="# TODO: write merge tests")
+    @pytest.mark.skip(reason="not implemented yet")
     def test_merge_dunder_methods(self, playlist: Playlist):
         pass
 

tests/shared/remote/library.py

@@ -1,6 +1,7 @@
 from abc import ABCMeta, abstractmethod
 from collections.abc import Collection, Mapping
 from copy import copy, deepcopy
+from random import choice
 from typing import Any
 
 from syncify.shared.core.base import Item
@@ -143,7 +144,6 @@ def assert_restore(library: RemoteLibrary, backup: Any):
         library_test = deepcopy(library)
         library_test.restore_playlists(playlists=backup, dry_run=False)
         assert len(library_test.playlists[name_actual]) == len(backup_check[name_actual])
-        # TODO: figure out why this occasionally fails
         assert len(library_test.playlists[name_actual]) != len(library.playlists[name_actual])
 
         assert name_new in library_test.playlists
@@ -152,7 +152,7 @@ def assert_restore(library: RemoteLibrary, backup: Any):
         assert library.api.handler.get(pl_new.url)  # new playlist was created and is callable
 
     def test_restore(self, library: RemoteLibrary, collection_merge_items: list[RemoteTrack]):
-        name_actual, pl_actual = next((name, pl) for name, pl in library.playlists.items() if len(pl) > 10)
+        name_actual, pl_actual = choice([(name, pl) for name, pl in library.playlists.items() if len(pl) > 10])
         name_new = "new playlist"
 
         # check test parameters are valid
@@ -183,6 +183,7 @@ def test_restore(self, library: RemoteLibrary, collection_merge_items: list[Remo
 
         # Library
         backup_library = deepcopy(library)
+        backup_library._playlists = {name_actual: backup_library.playlists[name_actual]}
         backup_library.restore_playlists(playlists=backup_uri, dry_run=False)
         self.assert_restore(library=library, backup=backup_library)
 
@@ -229,12 +230,8 @@ def assert_sync(library: RemoteLibrary, playlists: Any, api_mock: RemoteMock):
         requests = [req for req in api_mock.get_requests(method="POST") if req.url.startswith(url)]
         assert len(requests) > 0
 
-    # TODO: figure out why the 'actual' playlist in this test is sometimes un-writeable
-    def test_sync(
-            self, library: RemoteLibrary, collection_merge_items: list[RemoteTrack], api_mock: RemoteMock
-    ):
-
-        name_actual, pl_actual = next((name, pl) for name, pl in library.playlists.items() if len(pl) > 10)
+    def test_sync(self, library: RemoteLibrary, collection_merge_items: list[RemoteTrack], api_mock: RemoteMock):
+        name_actual, pl_actual = choice([(name, pl) for name, pl in library.playlists.items() if len(pl) > 10])
         name_new = "new playlist"
 
         # check test parameters are valid

tests/shared/remote/processors/search.py

@@ -183,7 +183,6 @@ def search_album(search_albums: list[LocalAlbum]):
             item.uri = item.remote_wrangler.unavailable_uri_dummy
             assert item.has_uri is False
         assert skip > 0  # check test input is valid
-        print("SKIP", skip)
 
         return collection
 
@@ -228,28 +227,17 @@ def test_search_result_combined(
             searcher: RemoteItemSearcher,
             search_items: list[LocalTrack],
             search_album: LocalAlbum,
-            unmatchable_items: list[LocalTrack]
+            unmatchable_items: list[LocalTrack],
     ):
-        skip = len([item for item in search_album if item.has_uri is not None])
-        skip += len([item for item in search_items if item.has_uri is not None])
-        skip += len([item for item in unmatchable_items if item.has_uri is not None])
-
         matchable = len(search_album) + len(search_items)
         search_album.items.extend(search_items)
         search_album.items.extend(unmatchable_items)
+        skip = len([item for item in search_album if item.has_uri is not None])
 
         result = searcher._search_collection(search_album)
         assert len(result.matched) + len(result.unmatched) + len(result.skipped) == len(search_album)
-        print(
-            len(result.matched),
-            len(result.unmatched),
-            len(result.skipped),
-            len(unmatchable_items),
-            len(search_album),
-            len(search_items),
-            skip
-        )
-        assert len(result.matched) == matchable - skip  # TODO: figure out why this occasionally fails
+
+        assert len(result.matched) == matchable - skip
         assert len(result.unmatched) == len(unmatchable_items)
         assert len(result.skipped) == skip
 

tests/spotify/api/mock.py

@@ -147,6 +147,11 @@ def get_duration(track: dict[str, Any]) -> int:
         self.setup_specific_conditions_user()
         self.setup_requests_mock()
 
+        track_uris = {track["uri"] for track in self.tracks}
+        for album in self.albums:
+            for track in album["tracks"]["items"]:
+                assert track["uri"] in track_uris
+
     ###########################################################################
     ## Setup
     ###########################################################################
@@ -190,7 +195,8 @@ def setup_specific_conditions_user(self):
         # ensure a certain minimum number of small user playlists
         count = max(10 - len([pl for pl in self.user_playlists if self.limit_lower < pl["tracks"]["total"] <= 60]), 0)
         for _ in range(count):
-            self.user_playlists.append(self.generate_playlist(item_count=randrange(self.limit_lower + 1, 60)))
+            pl = self.generate_playlist(owner=self.user, item_count=randrange(self.limit_lower + 1, 60))
+            self.user_playlists.append(pl)
 
     def setup_valid_references(self):
         """Sets up cross-referenced valid responses needed for RemoteObject tests"""
@@ -307,23 +313,22 @@ def response_getter(req: Request, _: Context) -> dict[str, Any]:
             for kind in kinds:
                 kind_enum = ObjectType.from_name(kind)[0]
                 values = self.item_type_map[kind_enum]
-                matches = [v for v in values if v["name"] == query]  # simple match on name for given query
+                # simple match on name for given query
+                results[kind + "s"] = [v for v in values if v["name"].casefold() == query.casefold()]
                 total += len(values)
 
                 # ensure minimal items response for collections to improve speed on some tests
                 if kind_enum in SpotifyAPI.collection_item_map:
                     key = SpotifyAPI.collection_item_map[kind_enum].name.casefold() + "s"
                     values = [v for v in values if 2 < v[key]["total"] <= self.limit_lower]
 
-                results[kind + "s"] = matches[:count]
-
-                available = len(values) - len(matches)
-                if len(matches) < limit and available:
-                    offset_min = offset + len(matches)
-                    offset_max = min(available, offset + limit) - len(matches) - count
-                    results[kind + "s"] += values[offset_min:offset_max]
+                available = len(values) - len(results[kind + "s"])
+                if len(results[kind + "s"]) < limit and available:
+                    offset_max = min(available, offset + limit) - len(results[kind + "s"]) - count
+                    results[kind + "s"] += values[offset:max(offset, offset_max)]
 
                 shuffle(results[kind + "s"])
+                count += len(results[kind + "s"])
 
             return {
                 kind: self.format_items_block(url=url, items=items, offset=offset, limit=limit, total=total)

tests/spotify/api/test_spotify_api_item.py

@@ -47,17 +47,9 @@ def assert_calls(
         initial_calls = max(len(list(batched(expected, limit))) if limit else len(expected), 1)
         extend_calls = 0
         if key:
-            # minus 1 for initial call to get the collection
-            extend_calls += sum(api_mock.calculate_pages_from_response(expect) - 1 for expect in expected)
+            # minus 1 for initial call to get the collection unless all items were present in the initial call
+            extend_calls += sum(max(api_mock.calculate_pages_from_response(expect) - 1, 0) for expect in expected)
 
-        # TODO: figure out why test_get_items_many sometimes fails on PLAYLIST
-        print(len(requests), initial_calls, extend_calls)
-        if key:
-            for expect in expected:
-                kind = ObjectType.from_name(expect["type"])[0]
-                key = SpotifyAPI.collection_item_map[kind].name.casefold() + "s"
-
-                print(expect[key]["limit"], expect[key]["total"])
         assert len(requests) == initial_calls + extend_calls
 
     ###########################################################################
@@ -498,8 +490,8 @@ def test_get_tracks_extra_many(self, api: SpotifyAPI, api_mock: SpotifyMock):
         api_mock.reset_mock()  # test checks the number of requests made
 
         source = api_mock.tracks
-        source = sample(source, api_mock.limit_lower) if len(source) > api_mock.limit_lower else source
-        source_map = {item["id"]: deepcopy(item) for item in source}
+        source = deepcopy(sample(source, api_mock.limit_lower) if len(source) > api_mock.limit_lower else source)
+        source_map = {item["id"]: item for item in source}
         source_features = {item["id"]: api_mock.audio_features[item["id"]] for item in source}
         source_analysis = {item["id"]: api_mock.audio_analysis[item["id"]] for item in source}
         test = random_id_types(id_list=source_map, wrangler=api, kind=ObjectType.TRACK)
@@ -610,7 +602,7 @@ def test_get_artist_albums_single(self, api: SpotifyAPI, api_mock: SpotifyMock):
             for artist in api_mock.artists
         }
         id_, expected = next((id_, albums) for id_, albums in expected_map.items() if len(albums) >= 10)
-        source = deepcopy(next(artist for artist in api_mock.artists if artist["id"] == id_))
+        source = next(deepcopy(artist) for artist in api_mock.artists if artist["id"] == id_)
         test = random_id_type(id_=id_, wrangler=api, kind=ObjectType.ARTIST)
 
         # force pagination
@@ -643,8 +635,8 @@ def test_get_artist_albums_single(self, api: SpotifyAPI, api_mock: SpotifyMock):
 
     def test_get_artist_albums_many(self, api: SpotifyAPI, api_mock: SpotifyMock):
         source = api_mock.artists
-        source = sample(source, api_mock.limit_lower) if len(source) > api_mock.limit_lower else source
-        source_map = {item["id"]: deepcopy(item) for item in source}
+        source = deepcopy(sample(source, api_mock.limit_lower) if len(source) > api_mock.limit_lower else source)
+        source_map = {item["id"]: item for item in source}
         expected_map = {
             id_: [
                 deepcopy(album) for album in api_mock.artist_albums

tests/spotify/api/test_spotify_api_playlist.py

@@ -21,15 +21,15 @@ class TestSpotifyAPIPlaylists:
     @pytest.fixture
     def playlist(api_mock: SpotifyMock) -> dict[str, Any]:
         """Yields a response representing a user playlist on Spotify"""
-        return deepcopy(next(pl for pl in api_mock.user_playlists if pl["tracks"]["total"] > api_mock.limit_lower))
+        return next(deepcopy(pl) for pl in api_mock.user_playlists if pl["tracks"]["total"] > api_mock.limit_lower)
 
     @staticmethod
     @pytest.fixture
     def playlist_unique(api_mock: SpotifyMock) -> dict[str, Any]:
         """Yields a response representing a uniquely named user playlist on Spotify"""
         names = [pl["name"] for pl in api_mock.user_playlists]
         return next(
-            pl for pl in api_mock.user_playlists
+            deepcopy(pl) for pl in api_mock.user_playlists
             if names.count(pl["name"]) == 1 and len(pl["name"]) != RemoteIDType.ID.value
         )
 

tests/spotify/conftest.py

@@ -19,5 +19,5 @@ def api(spotify_api: SpotifyAPI, api_mock: SpotifyMock) -> SpotifyAPI:
 
 @pytest.fixture(scope="module")
 def api_mock(spotify_mock: SpotifyMock) -> SpotifyMock:
-    """Yield an authorised :py:class:`SpotifyMock` object"""
+    """Yield a :py:class:`SpotifyMock` object with valid mock data ready to be called via HTTP requests"""
     return spotify_mock

tests/spotify/test_spotify_collection.py

@@ -43,11 +43,10 @@ def response_random(self, api_mock: SpotifyMock) -> dict[str, Any]:
 
     @pytest.fixture(scope="class")
     def _response_valid(self, api: SpotifyAPI, api_mock: SpotifyMock) -> dict[str, Any]:
-        response = deepcopy(next(
-            album for album in api_mock.albums
-            if album["tracks"]["total"] > len(album["tracks"]["items"]) > 5
-            and album["genres"]
-        ))
+        response = next(
+            deepcopy(album) for album in api_mock.albums
+            if album["tracks"]["total"] > len(album["tracks"]["items"]) > 5 and album["genres"]
+        )
         api.extend_items(items_block=response, key=RemoteObjectType.TRACK)
 
         api_mock.reset_mock()
@@ -276,12 +275,13 @@ def response_valid(self, api_mock: SpotifyMock) -> dict[str, Any]:
         """Yield a valid enriched response from the Spotify API for an artist item type."""
         artist_album_map = {
             artist["id"]: [
-                album for album in api_mock.artist_albums if any(art["id"] == artist["id"] for art in album["artists"])
+                deepcopy(album) for album in api_mock.artist_albums
+                if any(art["id"] == artist["id"] for art in album["artists"])
             ]
             for artist in api_mock.artists
         }
         id_, albums = next((id_, albums) for id_, albums in artist_album_map.items() if len(albums) >= 10)
-        artist = deepcopy(next(artist for artist in api_mock.artists if artist["id"] == id_))
+        artist = next(deepcopy(artist) for artist in api_mock.artists if artist["id"] == id_)
 
         for album in albums:
             tracks = [deepcopy(track) for track in api_mock.tracks if track["album"]["id"] == album["id"]]