From 08274028ebf1f3c611e61e23440df88334377455 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:01:12 +0200 Subject: [PATCH 1/4] Set limit to 50 by default, not 1 --- src/pylast/__init__.py | 2 +- tests/test_artist.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/pylast/__init__.py b/src/pylast/__init__.py index e02c764..2f527c0 100644 --- a/src/pylast/__init__.py +++ b/src/pylast/__init__.py @@ -1149,7 +1149,7 @@ class _BaseObject: def _get_things(self, method, thing, thing_type, params=None, cacheable=True): """Returns a list of the most played thing_types by this thing.""" - limit = params.get("limit", 1) + limit = params.get("limit", 50) seq = [] for node in _collect_nodes( limit, self, self.ws_prefix + "." + method, cacheable, params diff --git a/tests/test_artist.py b/tests/test_artist.py index 679d917..69dafb9 100755 --- a/tests/test_artist.py +++ b/tests/test_artist.py @@ -131,6 +131,17 @@ class TestPyLastArtist(TestPyLastWithLastFm): # Assert assert len(things) == 100 + def test_artist_top_albums_limit_default(self): + # Arrange + # Pick an artist with plenty of plays + artist = self.network.get_top_artists(limit=1)[0].item + + # Act + things = artist.get_top_albums() + + # Assert + assert len(things) == 50 + def test_artist_listener_count(self): # Arrange artist = self.network.get_artist("Test Artist") From a5034c43c043f0ff8caf9463c02b76bff30a4001 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:05:52 +0200 Subject: [PATCH 2/4] Refactor with pytest.mark.parametrize --- tests/test_artist.py | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/tests/test_artist.py b/tests/test_artist.py index 69dafb9..c99b04e 100755 --- a/tests/test_artist.py +++ b/tests/test_artist.py @@ -95,41 +95,17 @@ class TestPyLastArtist(TestPyLastWithLastFm): # Assert self.helper_two_different_things_in_top_list(things, pylast.Album) - def test_artist_top_albums_limit_1(self): + @pytest.mark.parametrize("test_limit", [1, 50, 100]) + def test_artist_top_albums_limit(self, test_limit: int) -> None: # Arrange - limit = 1 # Pick an artist with plenty of plays artist = self.network.get_top_artists(limit=1)[0].item # Act - things = artist.get_top_albums(limit=limit) + things = artist.get_top_albums(limit=test_limit) # Assert - assert len(things) == 1 - - def test_artist_top_albums_limit_50(self): - # Arrange - limit = 50 - # Pick an artist with plenty of plays - artist = self.network.get_top_artists(limit=1)[0].item - - # Act - things = artist.get_top_albums(limit=limit) - - # Assert - assert len(things) == 50 - - def test_artist_top_albums_limit_100(self): - # Arrange - limit = 100 - # Pick an artist with plenty of plays - artist = self.network.get_top_artists(limit=1)[0].item - - # Act - things = artist.get_top_albums(limit=limit) - - # Assert - assert len(things) == 100 + assert len(things) == test_limit def test_artist_top_albums_limit_default(self): # Arrange From 23503a72128c93b726324a345a47a1f9f03e6c3a Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:22:20 +0200 Subject: [PATCH 3/4] Refactor to remove unused parameter --- src/pylast/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/pylast/__init__.py b/src/pylast/__init__.py index 2f527c0..e53d9da 100644 --- a/src/pylast/__init__.py +++ b/src/pylast/__init__.py @@ -1146,8 +1146,8 @@ class _BaseObject: return first_child.wholeText.strip() - def _get_things(self, method, thing, thing_type, params=None, cacheable=True): - """Returns a list of the most played thing_types by this thing.""" + def _get_things(self, method, thing_type, params=None, cacheable=True): + """Returns a list of the most played thing_types.""" limit = params.get("limit", 50) seq = [] @@ -1812,7 +1812,7 @@ class Artist(_BaseObject, _Taggable): if limit: params["limit"] = limit - return self._get_things("getTopAlbums", "album", Album, params, cacheable) + return self._get_things("getTopAlbums", Album, params, cacheable) def get_top_tracks(self, limit=None, cacheable=True): """Returns a list of the most played Tracks by this artist.""" @@ -1820,7 +1820,7 @@ class Artist(_BaseObject, _Taggable): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_url(self, domain_name=DOMAIN_ENGLISH): """Returns the URL of the artist page on the network. @@ -1894,7 +1894,7 @@ class Country(_BaseObject): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_url(self, domain_name=DOMAIN_ENGLISH): """Returns the URL of the country page on the network. @@ -2024,7 +2024,7 @@ class Tag(_BaseObject, _Chartable): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_top_artists(self, limit=None, cacheable=True): """Returns a sequence of the most played artists.""" @@ -2498,7 +2498,7 @@ class User(_BaseObject, _Chartable): if limit: params["limit"] = limit - return self._get_things("getTopTracks", "track", Track, params, cacheable) + return self._get_things("getTopTracks", Track, params, cacheable) def get_track_scrobbles(self, artist, track, cacheable=False): """ From 7327b303371a653c71bae3a77254b2166028fe82 Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Sun, 27 Dec 2020 14:22:41 +0200 Subject: [PATCH 4/4] Refactor to avoid shadowing built-in --- tests/test_pylast.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_pylast.py b/tests/test_pylast.py index 789afad..c734d32 100755 --- a/tests/test_pylast.py +++ b/tests/test_pylast.py @@ -34,11 +34,11 @@ def load_secrets(): # pragma: no cover class PyLastTestCase: - def assert_startswith(self, str, prefix, start=None, end=None): - assert str.startswith(prefix, start, end) + def assert_startswith(self, s, prefix, start=None, end=None): + assert s.startswith(prefix, start, end) - def assert_endswith(self, str, suffix, start=None, end=None): - assert str.endswith(suffix, start, end) + def assert_endswith(self, s, suffix, start=None, end=None): + assert s.endswith(suffix, start, end) def _no_xfail_rerun_filter(err, name, test, plugin):