From 443f10e088909b61eb162eb49fe5c510cfe9fb72 Mon Sep 17 00:00:00 2001 From: Edwin Lee Date: Tue, 17 Dec 2024 16:37:02 -0600 Subject: [PATCH 1/2] First pass at attempting to include unit testing --- .github/workflows/build.yml | 6 +- requirements.txt | 2 + spotifycli/spotifycli.py | 62 ++++---- spotifycli/test/__init__.py | 0 spotifycli/test/dbus_metadata_response.py | 14 ++ spotifycli/test/test_spotifycli.py | 163 ++++++++++++++++++++++ 6 files changed, 220 insertions(+), 27 deletions(-) create mode 100644 spotifycli/test/__init__.py create mode 100644 spotifycli/test/dbus_metadata_response.py create mode 100644 spotifycli/test/test_spotifycli.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2e72970..d3976ee 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -9,10 +9,10 @@ on: jobs: build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-20.04 # TODO: This will be deprecated soon, should probably move to ubuntu-24.04 strategy: matrix: - python-version: ["3.6", "3.7"] + python-version: ["3.6", "3.7"] # TODO: Both of these have reached end of life, should probably move to 3.12+ steps: - name: checkout uses: actions/checkout@v3 @@ -25,3 +25,5 @@ jobs: run: pip install -r requirements.txt - name: check-format run: python ./check_format.py + - name: run-tests + run: pytest diff --git a/requirements.txt b/requirements.txt index dd6f550..b11a5b0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,9 @@ autopep8 +python-dbusmock jeepney lyricwikia pycodestyle pylint +pytest setuptools twine diff --git a/spotifycli/spotifycli.py b/spotifycli/spotifycli.py index 5ab435f..57d0e40 100755 --- a/spotifycli/spotifycli.py +++ b/spotifycli/spotifycli.py @@ -118,27 +118,28 @@ def add_arguments(): def get_arguments(): return [ - ("--version", "shows version number"), - ("--status", "shows song name and artist"), + ("--version", "shows version number", False), + ("--status", "shows song name and artist", False), ( "--statusposition", - "shows song name and artist, with current playback position" + "shows song name and artist, with current playback position", + False ), - ("--statusshort", "shows status in a short way"), - ("--song", "shows the song name"), - ("--songshort", "shows the song name in a short way"), - ("--artist", "shows artist name"), - ("--artistshort", "shows artist name in a short way"), - ("--album", "shows album name"), - ("--position", "shows song position"), - ("--arturl", "shows album image url"), - ("--playbackstatus", "shows playback status"), - ("--play", "plays the song"), - ("--pause", "pauses the song"), - ("--playpause", "plays or pauses the song (toggles a state)"), - ("--lyrics", "shows the lyrics for the song"), - ("--next", "plays the next song"), - ("--prev", "plays the previous song"), + ("--statusshort", "shows status in a short way", False), + ("--song", "shows the song name", False), + ("--songshort", "shows the song name in a short way", False), + ("--artist", "shows artist name", False), + ("--artistshort", "shows artist name in a short way", False), + ("--album", "shows album name", False), + ("--position", "shows song position", False), + ("--arturl", "shows album image url", False), + ("--playbackstatus", "shows playback status", False), + ("--play", "plays the song", False), + ("--pause", "pauses the song", False), + ("--playpause", "plays or pauses the song (toggles a state)", False), + ("--lyrics", "shows the lyrics for the song", False), + ("--next", "plays the next song", False), + ("--prev", "plays the previous song", False), ("--songuri", "plays the track at the provided Uri", True), ("--listuri", "plays the playlist at the provided Uri", True), ] @@ -150,7 +151,9 @@ def show_version(): def get_song(): metadata = get_spotify_property("Metadata") - artist = ", ".join(metadata['xesam:artist'][1]) + artist = metadata['xesam:artist'][1] + if isinstance(artist, list): + artist = ", ".join(artist) title = metadata['xesam:title'][1] return artist, title @@ -175,9 +178,9 @@ def show_status_position(): artist, title = get_song() # Both values are in microseconds - position = datetime.timedelta(milliseconds=position_raw / 1000) + position = datetime.timedelta(milliseconds=int(position_raw) / 1000) length = datetime.timedelta( - milliseconds=metadata['mpris:length'][1] / 1000 + milliseconds=int(metadata['mpris:length'][1]) / 1000 ) p_hours, p_minutes, p_seconds = convert_timedelta(position) @@ -310,8 +313,17 @@ def get_spotify_property(spotify_property): f"Could not connect to Spotify instance, full response:\n{err}" ) - body = reply.body[0] - return body[1] + # the reply body holds a tuple of possible response objects + reply_body = reply.body + + # grab the first (only) response object, a tuple of signature, data + content = reply_body[0] + + # get the data member off of the content, since we don't need the signature + response = content[1] + + # the return type is expected to be a dictionary + return response def perform_spotify_action(spotify_command, extra_arg=None): @@ -334,9 +346,9 @@ def show_position(): metadata = get_spotify_property("Metadata") position_raw = get_spotify_property("Position") # Both values are in microseconds - position = datetime.timedelta(milliseconds=position_raw / 1000) + position = datetime.timedelta(milliseconds=int(position_raw) / 1000) length = datetime.timedelta( - milliseconds=metadata['mpris:length'][1] / 1000 + milliseconds=int(metadata['mpris:length'][1]) / 1000 ) p_hours, p_minutes, p_seconds = convert_timedelta(position) diff --git a/spotifycli/test/__init__.py b/spotifycli/test/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/spotifycli/test/dbus_metadata_response.py b/spotifycli/test/dbus_metadata_response.py new file mode 100644 index 0000000..25ac971 --- /dev/null +++ b/spotifycli/test/dbus_metadata_response.py @@ -0,0 +1,14 @@ +if args[1] == 'Metadata': + ret = { + 'xesam:artist': ['', 'The Beatles'], + 'xesam:title': ['', 'Yesterday'], + 'xesam:album': ['', 'Help!'], + 'mpris:length': ['', '10000000'], + 'mpris:artUrl': ['', 'https://github.com/pwittchen/spotify-cli-linux'], + } +elif args[1] == 'PlaybackStatus': + ret = 'Playing' +elif args[1] == 'Position': + ret = 1000000 +else: + ret = args diff --git a/spotifycli/test/test_spotifycli.py b/spotifycli/test/test_spotifycli.py new file mode 100644 index 0000000..d84d0a0 --- /dev/null +++ b/spotifycli/test/test_spotifycli.py @@ -0,0 +1,163 @@ +from io import StringIO +from pathlib import Path +from subprocess import PIPE +from unittest import skip +from unittest.mock import patch + +import dbus +import dbusmock + +from spotifycli.spotifycli import main +from spotifycli.version import __version__ + + +class TestSpotifyCLI(dbusmock.DBusTestCase): + @classmethod + def setUpClass(cls): + cls.start_session_bus() + cls.dbus_con = cls.get_dbus(system_bus=False) + + def setUp(self): + self.dbus_spotify_server_mock = self.spawn_server( + "org.mpris.MediaPlayer2.spotify", + "/org/mpris/MediaPlayer2", + "org.freedesktop.DBus.Properties", + system_bus=False, + stdout=PIPE + ) + self.dbus_spotify_interface_mock = dbus.Interface( + self.dbus_con.get_object( + 'org.mpris.MediaPlayer2.spotify', '/org/mpris/MediaPlayer2' + ), + dbusmock.MOCK_IFACE + ) + metadata_file = Path(__file__).resolve().parent / 'dbus_metadata_response.py' + self.dbus_spotify_interface_mock.AddMethod( + '', 'Get', 'ss', 'v', + metadata_file.read_text(), + ) + # TODO: Mock up the controls bus/interface, since I think it is slightly different + # self.dbus_spotify_interface_mock.AddMethod( + # '', 'Play', '', '', '' + # ) + + def tearDown(self): + self.dbus_spotify_server_mock.stdout.close() + self.dbus_spotify_server_mock.terminate() + self.dbus_spotify_server_mock.wait() + + def test_cli_usage(self): + with patch('sys.argv', ["spotifycli", "--help"]), patch("sys.exit") as mock_exit, patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("usage", buffer.getvalue()) + mock_exit.assert_called_with(0) + + def test_cli_version(self): + with patch('sys.argv', ["spotifycli", "--version"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn(__version__, buffer.getvalue()) + + def test_cli_status(self): + with patch('sys.argv', ["spotifycli", "--status"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("The Beatles - Yesterday", buffer.getvalue()) + + def test_cli_status_position(self): + with patch('sys.argv', ["spotifycli", "--statusposition"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + output = buffer.getvalue() + self.assertIn('00:01', output) + self.assertIn('00:10', output) + + def test_cli_status_short(self): + with patch('sys.argv', ["spotifycli", "--statusshort"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + output = buffer.getvalue() + self.assertIn('Beatles', output) + self.assertIn('Yesterday', output) + + def test_cli_song(self): + with patch('sys.argv', ["spotifycli", "--song"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("Yesterday", buffer.getvalue()) + + def test_cli_song_short(self): + # TODO: Change to a long song title to check the trimming + with patch('sys.argv', ["spotifycli", "--songshort"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("Yesterday", buffer.getvalue()) + + def test_cli_artist(self): + with patch('sys.argv', ["spotifycli", "--artist"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("Beatles", buffer.getvalue()) + + def test_cli_artist_short(self): + # TODO: Change to a long artist name to check the trimming + with patch('sys.argv', ["spotifycli", "--artistshort"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("Beatles", buffer.getvalue()) + + def test_cli_album(self): + with patch('sys.argv', ["spotifycli", "--album"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn("Help!", buffer.getvalue()) + + def test_cli_position(self): + with patch('sys.argv', ["spotifycli", "--position"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + output = buffer.getvalue() + self.assertIn('00:01', output) + self.assertIn('00:10', output) + + def test_cli_art_url(self): + with patch('sys.argv', ["spotifycli", "--arturl"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + self.assertIn('http', buffer.getvalue()) + + def test_cli_playback_status(self): + with patch('sys.argv', ["spotifycli", "--playbackstatus"]), patch('sys.stdout', new_callable=StringIO) as buffer: + main() + playback_symbols = ['▶', '▮▮', '■'] + output = buffer.getvalue() + self.assertTrue(any([x in output for x in playback_symbols])) + + @skip('Lyrics dont seem to be working right now...is it still valid?') + def test_cli_lyrics(self): + with patch('sys.argv', ["spotifycli", "--lyrics"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_play(self): + with patch('sys.argv', ["spotifycli", "--play"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_pause(self): + with patch('sys.argv', ["spotifycli", "--pause"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_play_pause(self): + with patch('sys.argv', ["spotifycli", "--playpause"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_next(self): + with patch('sys.argv', ["spotifycli", "--next"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_prev(self): + with patch('sys.argv', ["spotifycli", "--prev"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_songuri(self): + with patch('sys.argv', ["spotifycli", "--songuri"]): + main() + + @skip('TODO: Need to mock up the spotify_action dbus interface') + def test_cli_listuri(self): + with patch('sys.argv', ["spotifycli", "--listuri"]): + main() From 0b2d654454d5dc695005ff6fae2bee7b8c9e0adb Mon Sep 17 00:00:00 2001 From: Edwin Lee Date: Tue, 17 Dec 2024 16:39:37 -0600 Subject: [PATCH 2/2] Revert minor tweak --- spotifycli/spotifycli.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/spotifycli/spotifycli.py b/spotifycli/spotifycli.py index 57d0e40..5ac6e12 100755 --- a/spotifycli/spotifycli.py +++ b/spotifycli/spotifycli.py @@ -313,17 +313,8 @@ def get_spotify_property(spotify_property): f"Could not connect to Spotify instance, full response:\n{err}" ) - # the reply body holds a tuple of possible response objects - reply_body = reply.body - - # grab the first (only) response object, a tuple of signature, data - content = reply_body[0] - - # get the data member off of the content, since we don't need the signature - response = content[1] - - # the return type is expected to be a dictionary - return response + body = reply.body[0] + return body[1] def perform_spotify_action(spotify_command, extra_arg=None):