Bugfixes and Unit Tests

ovos_plugin_common_play/ocp/player.py

@@ -234,7 +234,7 @@ def set_player_state(self, state: PlayerState):
     def set_now_playing(self, track: Union[dict, MediaEntry]):
         """
         Set `track` as the currently playing media, update the playlist, and
-        notify any GUI or MPRIS clients.
+        notify any GUI or MPRIS clients. Adds `track` to `playlist`
         @param track: MediaEntry or dict representation of a MediaEntry to play
         """
         LOG.debug(f"Playing: {track}")
@@ -455,7 +455,8 @@ def play(self):
 
     def play_shuffle(self):
         """
-        Go to a random position in the playlist and play that MediaEntry.
+        Go to a random position in the playlist and set that MediaEntry as
+        'now_playing` (does NOT call 'play').
         """
         LOG.debug("Shuffle == True")
         if len(self.playlist) > 1 and not self.playlist.is_last_track:
@@ -468,23 +469,26 @@ def play_shuffle(self):
 
     def play_next(self):
         """
-        Play the next track in the playlist.
-        If there is no next track, end playback.
+        Play the next track in the playlist or search results.
+        End playback if there is no next track, accounting for repeat and
+        shuffle settings.
         """
-        if self.active_backend in [PlaybackType.MPRIS]:
+        if self.active_backend == PlaybackType.MPRIS:
             if self.mpris:
                 self.mpris.play_next()
             return
         elif self.active_backend in [PlaybackType.SKILL,
                                      PlaybackType.UNDEFINED]:
+            LOG.debug("Defer playing next track to skill")
             self.bus.emit(Message(
                 f'ovos.common_play.{self.now_playing.skill_id}.next'))
             return
         self.pause()  # make more responsive
 
         if self.loop_state == LoopState.REPEAT_TRACK:
-            self.play()
+            LOG.debug("Repeating single track")
         elif self.shuffle:
+            LOG.debug("Shuffling")
             self.play_shuffle()
         elif not self.playlist.is_last_track:
             self.playlist.next_track()
@@ -493,13 +497,14 @@ def play_next(self):
         elif not self.media.search_playlist.is_last_track and \
                 self.settings.get("merge_search", True):
             while self.media.search_playlist.current_track in self.playlist:
+                # Don't play media already played from the playlist
                 self.media.search_playlist.next_track()
             self.set_now_playing(self.media.search_playlist.current_track)
             LOG.info(f"Next search index: "
                      f"{self.media.search_playlist.position}")
         else:
             if self.loop_state == LoopState.REPEAT and len(self.playlist):
-                LOG.debug("end of playlist, repeat == True")
+                LOG.info("end of playlist, repeat == True")
                 self.playlist.set_position(0)
             else:
                 LOG.info("requested next, but there aren't any more tracks")
@@ -716,6 +721,7 @@ def handle_playback_ended(self, message):
         LOG.debug("Playback ended")
         if self.settings.get("autoplay", True) and \
                 self.active_backend != PlaybackType.MPRIS:
+            LOG.debug("Playing next track")
             self.play_next()
             return
 

test/unittests/test_ocp.py

@@ -687,8 +687,114 @@ def test_play_shuffle(self):
         pass
 
     def test_play_next(self):
-        # TODO
-        pass
+        real_mpris = self.player.mpris.play_next
+        real_pause = self.player.pause
+        real_play = self.player.play
+        real_shuffle = self.player.play_shuffle
+        real_gui_end = self.player.gui.handle_end_of_playback
+
+        self.player.mpris.play_next = Mock()
+        self.player.pause = Mock()
+        self.player.play = Mock()
+        self.player.play_shuffle = Mock()
+        self.player.gui.handle_end_of_playback = Mock()
+
+        # MPRIS Next
+        self.player.now_playing.playback = PlaybackType.MPRIS
+        self.player.play_next()
+        self.player.mpris.play_next.assert_called_once()
+
+        # Skill Next
+        self.player.now_playing.playback = PlaybackType.SKILL
+        self.player.play_next()
+        last_message = self.emitted_msgs[-1]
+        self.assertEqual(
+            last_message.msg_type,
+            f"ovos.common_play.{self.player.now_playing.skill_id}.next")
+
+        # Repeat Track
+        self.player.now_playing.playback = PlaybackType.AUDIO
+        self.player.loop_state = LoopState.REPEAT_TRACK
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play.assert_called_once()
+
+        # Shuffle
+        self.player.pause.reset_mock()
+        self.player.play.reset_mock()
+        self.player.loop_state = LoopState.NONE
+        self.player.shuffle = True
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play_shuffle.assert_called_once()
+        self.player.play.assert_called_once()
+
+        # Playlist next track
+        self.player.pause.reset_mock()
+        self.player.play.reset_mock()
+        self.player.shuffle = False
+        self.player.playlist.replace(valid_search_results)
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play.assert_called_once()
+        self.assertEqual(self.player.now_playing, self.player.playlist[1])
+
+        # Playlist repeat
+        self.player.loop_state = LoopState.REPEAT
+        self.player.playlist.set_position(len(self.player.playlist) - 1)
+        self.player.pause.reset_mock()
+        self.player.play.reset_mock()
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play.assert_called_once()
+        self.assertTrue(self.player.playlist.is_first_track)
+
+        # Playlist no repeat
+        self.player.loop_state = LoopState.NONE
+        self.player.playlist.set_position(len(self.player.playlist) - 1)
+        self.player.pause.reset_mock()
+        self.player.play.reset_mock()
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play.assert_not_called()
+        self.player.gui.handle_end_of_playback.assert_called_once()
+
+        # Search results next track
+        self.player.gui.handle_end_of_playback.reset_mock()
+        self.player.playlist.clear()
+        self.player.media.search_playlist.replace(valid_search_results)
+        self.assertEqual(len(self.player.playlist), 0)
+        self.player.pause.reset_mock()
+        self.player.play.reset_mock()
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play.assert_called_once()
+        self.player.gui.handle_end_of_playback.assert_not_called()
+
+        # Search results end
+        self.player.pause.reset_mock()
+        self.player.play.reset_mock()
+        # TODO: should we repeat search results, or skip adding them to the playlist
+        self.player.playlist.clear()
+        self.player.media.search_playlist.set_position(
+            len(self.player.media.search_playlist) - 1)
+        self.assertTrue(self.player.media.search_playlist.is_last_track)
+        self.assertEqual(len(self.player.playlist), 0)
+        self.player.loop_state = LoopState.REPEAT
+        self.player.play_next()
+        self.player.pause.assert_called_once()
+        self.player.play.assert_not_called()
+        self.player.gui.handle_end_of_playback.assert_called_once()
+
+        # TODO: Test with `merge_search` set to False
+
+        self.player.mpris.play_next.assert_called_once()
+
+        self.player.gui.handle_end_of_playback = real_gui_end
+        self.player.play_shuffle = real_shuffle
+        self.player.play = real_play
+        self.player.pause = real_pause
+        self.player.mpris.play_next = real_mpris
 
     def test_play_prev(self):
         # TODO