Merge pull request #1444 from jodal/fix/1418-unmerged-fixes-and-tests-from-1.1

core: Unmerged fixes and tests from 1.1 regarding unplayable tracks

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:

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)
 

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])