Merge pull request #4 from sheriferson/misc/improve-tests-and-misc

Improve tests and other quality of life/code changes

.gitignore

@@ -1,7 +1,8 @@
 *egg-info
+.DS_Store
 .coverage
+.idea
 __pycache__
 build
 coverage.xml
 dist
-.DS_Store

src/scrobble/app.py

@@ -19,20 +19,23 @@
 
 APP = typer.Typer()
 
+
 @APP.command()
 def musicbrainz():
     raise NotImplementedError('Scrobbling a MusicBrainz release is not implemented yet.')
 
+
 @APP.command()
 def discogs():
     raise NotImplementedError('Scrobbling a Discogs release is not implemented yet.')
 
+
 @APP.command()
 def cd(
         barcode: Annotated[str, typer.Argument(
             help='Barcode of the CD you want to scrobble. Double album releases are supported.'
         )],
-        playbackend: Annotated[Optional[str], typer.Argument(
+        playback_end: Annotated[Optional[str], typer.Argument(
             help="When did you finish listening? e.g., 'now' or '1 hour ago'."
         )] = 'now',
 
@@ -57,28 +60,28 @@ def cd(
 
     init_musicbrainz(USERAGENT)
 
-    cd = CD.find_cd(barcode, release_choice)
+    scrobble_cd: CD = CD.find_cd(barcode, release_choice)
 
     if track_choice:
-        tracks_to_scrobble = choose_tracks(cd.tracks)
+        tracks_to_scrobble = choose_tracks(scrobble_cd.tracks)
     else:
-        tracks_to_scrobble = cd.tracks
+        tracks_to_scrobble = scrobble_cd.tracks
 
-    prepped_tracks = prepare_tracks(cd, tracks_to_scrobble, playbackend)
+    prepped_tracks = prepare_tracks(scrobble_cd, tracks_to_scrobble, playback_end)
 
     if verbose:
-        print(cd)
+        print(scrobble_cd)
         for track in tracks_to_scrobble:
             print(track)
 
     if not dryrun:
-        LASTFM = get_lastfm_client()
-        LASTFM.scrobble_many(prepped_tracks)
+        lastfm_client = get_lastfm_client()
+        lastfm_client.scrobble_many(prepped_tracks)
     else:
         print('⚠️  Dry run - no tracks were scrobbled.')
 
     if notify:
-        send_notification(cd)
+        send_notification(scrobble_cd)
 
 
 def main():

src/scrobble/musicbrainz.py

@@ -73,7 +73,8 @@ def find_cd(cls, barcode: str, choice: bool = True):
                 index = 0
                 for cd in cds:
                     index += 1
-                    entry: str = f"{index}. {cd.title}, {cd.discs} {'disc' if cd.discs < 2 else ' discs'}, {len(cd.tracks)} tracks"
+                    entry: str = (f"{index}. {cd.title}, {cd.discs} {'disc' if cd.discs < 2 else ' discs'}, "
+                                  f"{len(cd.tracks)} tracks")
                     if cd.year:
                         entry += f", released in {cd.year}."
                     else:
@@ -88,11 +89,11 @@ def find_cd(cls, barcode: str, choice: bool = True):
 
     @classmethod
     def _parse_musicbrainz_result(cls, result: dict):
-        id = result['id']
-        title = result['title']
-        artist = result['artist-credit'][0]['name']
-        year = str(parser.parse(result['date']).year) if result['date'] else None
-        disc_count = len(result['medium-list'])
+        id: str = result['id']
+        title: str = result['title']
+        artist: str = result['artist-credit'][0]['name']
+        year: str = str(parser.parse(result.get('date')).year) if 'date' in result else None
+        disc_count: int = len(result['medium-list'])
 
         return CD(id, title, artist, year, disc_count)
 

src/scrobble/pushover.py

@@ -4,10 +4,10 @@
 from scrobble.musicbrainz import CD
 from scrobble.utils import Config
 
-config = Config()
 
-
-def send_notification(cd: CD):
+def send_notification(cd: CD, config=None):
+    if not config:
+        config = Config()
     conn = http.client.HTTPSConnection("api.pushover.net:443")
     query_strings = {
             "token": config.pushover_token,

src/scrobble/utils.py

@@ -9,14 +9,23 @@
 from scrobble.musicbrainz import Track
 
 
+def read_api_keys(config_path: str) -> dict:
+    if not os.path.exists(config_path):
+        raise FileNotFoundError(f'.toml config file not found in {config_path}')
+    with open(config_path, 'rb') as config_file:
+        keys = tomllib.load(config_file)
+
+    return keys
+
+
 @dataclass
 class Config:
     config_path: str = os.path.join(os.path.expanduser('~'), '.config', 'scrobble.toml')
     lastfmapi: Optional[dict[str, str]] = None
     pushoverapi: Optional[dict[str, str]] = None
 
     def __post_init__(self):
-        keys = self.read_api_keys(self.config_path)
+        keys = read_api_keys(self.config_path)
         self.lastfmapi = keys['lastfmapi']
 
         if 'pushoverapi' in keys:
@@ -40,6 +49,13 @@ def has_lastfm_username(self):
     def lastfm_username(self):
         return self.lastfmapi['username']
 
+    @lastfm_username.setter
+    def lastfm_username(self, new_lastfm_username):
+        if new_lastfm_username:
+            self.lastfmapi['username'] = new_lastfm_username
+        else:
+            raise ValueError('You cannot set the Last.fm username to an empty value.')
+
     @property
     def lastfm_api_key(self):
         return self.lastfmapi['api_key']
@@ -52,32 +68,38 @@ def lastfm_api_secret(self):
     def pushover_token(self):
         return self.pushoverapi['token']
 
+    @pushover_token.setter
+    def pushover_token(self, new_token_value: str):
+        if new_token_value:
+            self.pushoverapi['token'] = new_token_value
+        else:
+            raise ValueError('You cannot set the Pushover token to an empty value.')
+
     @property
     def pushover_user(self):
         return self.pushoverapi['user_key']
 
-    def read_api_keys(self, config_path: str) -> dict:
-        if not os.path.exists(config_path):
-            raise FileNotFoundError(f'.toml config file not found in {config_path}')
-        with open(config_path, 'rb') as config_file:
-            keys = tomllib.load(config_file)
-
-        return keys
+    @pushover_user.setter
+    def pushover_user(self, new_pushover_user: str):
+        if new_pushover_user:
+            self.pushoverapi['user_key'] = new_pushover_user
+        else:
+            raise ValueError('You cannot set the Pushover user key to an empty value.')
 
 
-def prepare_tracks(cd: CD, tracks: list[Track], playbackend: str = 'now') -> list[dict]:
+def prepare_tracks(cd: CD, tracks: list[Track], playback_end: str = 'now') -> list[dict]:
     total_run_time: int = 0
     for track in tracks:
         total_run_time += track.track_length
 
-    if playbackend != 'now':
+    if playback_end != 'now':
         import parsedatetime
         cal = parsedatetime.Calendar()
         try:
-            parsed_end, _ = cal.parse(playbackend)
+            parsed_end, _ = cal.parse(playback_end)
             stop_time = datetime(*parsed_end[:6]).timestamp()
         except:
-            raise ValueError(f"'{playbackend}' could not be parsed. Try a different input.")
+            raise ValueError(f"'{playback_end}' could not be parsed. Try a different input.")
 
     else:
         stop_time = datetime.now().timestamp()
@@ -93,7 +115,7 @@ def prepare_tracks(cd: CD, tracks: list[Track], playbackend: str = 'now') -> lis
                 'artist': cd.artist,
                 'title': track.track_title,
                 'album': cd.title,
-                'timestamp': start_time+elapsed
+                'timestamp': start_time + elapsed
             }
         )
 
@@ -129,5 +151,3 @@ def choose_tracks(tracks: list[Track]) -> list[Track]:
 
     else:
         raise NotImplementedError("Track choosing without charmbracelet/gum installation is not implemented yet.")
-
-    return [track for track in tracks if str(track) in picked_tracks]

tests/test__cd.py

@@ -1,3 +1,5 @@
+import pytest
+
 from scrobble.musicbrainz import CD, UserAgent, init_musicbrainz
 
 import importlib.metadata
@@ -8,14 +10,25 @@
                       )
 
 init_musicbrainz(USERAGENT)
-test_cd = CD.find_cd(7277017746006, choice=False)
+TEST_CD = CD.find_cd(7277017746006, choice=False)
 
 
 def test_cd_artist():
-    assert test_cd.artist == 'Lacuna Coil'
+    assert TEST_CD.artist == 'Lacuna Coil'
+
 
 def test_cd_album():
-    assert test_cd.title == 'Comalies'
+    assert TEST_CD.title == 'Comalies'
+
 
 def test_cd_track_length():
-    assert len(test_cd) == 14
+    assert len(TEST_CD) == 14
+
+
+def test_cd_string_representation():
+    assert str(TEST_CD) == "💿 Lacuna Coil - Comalies (2002)"
+
+
+def test_failed_CD_retrieval():
+    with pytest.raises(RuntimeError):
+        CD.find_cd(12345)

tests/test__pushover.py

@@ -0,0 +1,52 @@
+from scrobble.pushover import send_notification
+from scrobble.utils import Config
+from scrobble.musicbrainz import CD, UserAgent, init_musicbrainz
+import importlib.metadata
+from urllib.parse import quote_plus
+
+import unittest
+from unittest.mock import patch, Mock
+
+USERAGENT = UserAgent('scrobble (PyPI) (tests)',
+                      importlib.metadata.version('scrobble'),  # scrobble version
+                      'https://github.com/sheriferson'
+                      )
+
+init_musicbrainz(USERAGENT)
+
+TEST_CD = CD.find_cd(7277017746006, choice=False)
+
+
+class TestSendNotification(unittest.TestCase):
+
+    @patch('http.client.HTTPSConnection')
+    def test_send_notification(self, mock_https_connection):
+        # Mocking the connection
+        mock_conn_instance = Mock()
+        mock_https_connection.return_value = mock_conn_instance
+
+        # Mocking the response
+        mock_response = Mock()
+        mock_conn_instance.getresponse.return_value = mock_response
+
+        # Example CD and Config objects
+        config = Config(config_path='tests/resources/scrobble_complete_valid.toml')
+        config.pushover_token = 'test_token'
+        config.pushover_user = 'test_user'
+
+        # Call the function with the mocked connection
+        response = send_notification(TEST_CD, config)
+
+        # Assert that conn.request was called with the expected arguments
+        mock_conn_instance.request.assert_called_once_with(
+            "POST",
+            "/1/messages.json",
+            f"token={config.pushover_token}&user={config.pushover_user}&message={quote_plus(TEST_CD.title)}+%28{TEST_CD.year}%29+by+{quote_plus(TEST_CD.artist)}+scrobbled+to+your+account.&url=https%3A%2F%2Flast.fm%2Fuser%2F{config.lastfm_username}",
+            {"Content-type": "application/x-www-form-urlencoded"}
+        )
+
+        # Assert that conn.getresponse was called
+        mock_conn_instance.getresponse.assert_called_once()
+
+        # Assert that the function returned the expected response
+        self.assertEqual(response, mock_response)

tests/test__utils.py

@@ -28,6 +28,10 @@ def test_valid_config_lastfm_username():
     test_config = Config(config_path='tests/resources/scrobble_complete_valid.toml')
     assert test_config.lastfm_username == 'thespeckofme'
 
+def test_valid_config_pushover_token():
+    test_config = Config(config_path='tests/resources/scrobble_complete_valid.toml')
+    assert test_config.pushover_token == 'fakepushovertoken'
+
 def test_find_command_succeeding():
     command_check = find_command('ls')
     assert command_check is not None