diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 63259f7d..e0eab403 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -346,7 +346,6 @@ class PlaybackController(object): pending = tl_track or current or self.core.tracklist.next_track(None) while pending: - # TODO: should we consume unplayable tracks in this loop? if self._change(pending, PlaybackState.PLAYING): break else: diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 2a4ec8b6..6d7ceeb7 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -621,6 +621,8 @@ class TracklistController(object): def _mark_unplayable(self, tl_track): """Internal method for :class:`mopidy.core.PlaybackController`.""" logger.warning('Track is not playable: %s', tl_track.track.uri) + if self.get_consume() and tl_track is not None: + self.remove({'tlid': [tl_track.tlid]}) if self.get_random() and tl_track in self._shuffled: self._shuffled.remove(tl_track) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 4f20830e..a43724f0 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -185,6 +185,17 @@ class TestNextHandling(BaseTest): self.assertIn(tl_track, self.core.tracklist.tl_tracks) + def test_next_skips_over_unplayable_track(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + self.audio.trigger_fake_playback_failure(tl_tracks[1].track.uri) + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.core.playback.next() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + class TestPreviousHandling(BaseTest): # TODO Test previous() more @@ -230,6 +241,17 @@ class TestPreviousHandling(BaseTest): self.assertIn(tl_tracks[1], self.core.tracklist.tl_tracks) + def test_previous_skips_over_unplayable_track(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + self.audio.trigger_fake_playback_failure(tl_tracks[1].track.uri) + self.core.playback.play(tl_tracks[2]) + self.replay_events() + + self.core.playback.previous() + self.replay_events() + + assert self.core.playback.get_current_tl_track() == tl_tracks[0] + class OnAboutToFinishTest(BaseTest): @@ -256,6 +278,20 @@ class TestConsumeHandling(BaseTest): self.assertNotIn(tl_track, self.core.tracklist.get_tl_tracks()) + def test_next_in_consume_mode_removes_unplayable_track(self): + last_playable_tl_track = self.core.tracklist.get_tl_tracks()[-2] + unplayable_tl_track = self.core.tracklist.get_tl_tracks()[-1] + self.audio.trigger_fake_playback_failure(unplayable_tl_track.track.uri) + + self.core.playback.play(last_playable_tl_track) + self.core.tracklist.set_consume(True) + + self.core.playback.next() + self.replay_events() + + self.assertNotIn( + unplayable_tl_track, self.core.tracklist.get_tl_tracks()) + def test_on_about_to_finish_in_consume_mode_removes_finished_track(self): tl_track = self.core.tracklist.get_tl_tracks()[0] @@ -964,3 +1000,30 @@ class TestBug1177Regression(unittest.TestCase): c.playback.pause() c.playback.next() b.playback.change_track.assert_called_once_with(track2) + + +class TestBug1352Regression(BaseTest): + tracks = [ + Track(uri='dummy:a', length=40000), + Track(uri='dummy:b', length=40000), + ] + + def test_next_when_paused_updates_history(self): + self.core.history._add_track = mock.Mock() + self.core.tracklist._mark_playing = mock.Mock() + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.playback.play() + self.replay_events() + + self.core.history._add_track.assert_called_once_with(self.tracks[0]) + self.core.tracklist._mark_playing.assert_called_once_with(tl_tracks[0]) + self.core.history._add_track.reset_mock() + self.core.tracklist._mark_playing.reset_mock() + + self.playback.pause() + self.playback.next() + self.replay_events() + + self.core.history._add_track.assert_called_once_with(self.tracks[1]) + self.core.tracklist._mark_playing.assert_called_once_with(tl_tracks[1])