Merge pull request #1071 from adamcik/fix/change-track-failure-should-fail-playback

core: Update playback code to take change track into account.

docs/changelog.rst

@@ -69,6 +69,10 @@ v1.0.0 (UNRELEASED)
   - :meth:`mopidy.core.TracklistController.mark_playing`
   - :meth:`mopidy.core.TracklistController.mark_unplayable`
 
+- Updated :meth:`mopidy.core.PlaybackController.play` to take
+  :meth:`mopidy.backend.PlaybackProvider.change_track` into account when
+  determining success. (PR: :issue:`1071`)
+
 **Backend API**
 
 - Remove default implementation of

mopidy/core/playback.py

@@ -312,8 +312,8 @@ class PlaybackController(object):
 
         if backend:
             backend.playback.prepare_change()
-            backend.playback.change_track(tl_track.track)
-            success = backend.playback.play().get()
+            success = (backend.playback.change_track(tl_track.track).get() and
+                       backend.playback.play().get())
 
         if success:
             self.core.tracklist._mark_playing(tl_track)

tests/core/test_playback.py

@@ -12,6 +12,7 @@ from mopidy.models import Track
 from tests import dummy_audio as audio
 
 
+# TODO: split into smaller easier to follow tests. setup is way to complex.
 class CorePlaybackTest(unittest.TestCase):
     def setUp(self):  # noqa: N802
         self.backend1 = mock.Mock()
@@ -113,7 +114,7 @@ class CorePlaybackTest(unittest.TestCase):
         self.playback2.change_track.assert_called_once_with(self.tracks[1])
         self.playback2.play.assert_called_once_with()
 
-    def test_play_skips_to_next_on_unplayable_track(self):
+    def test_play_skips_to_next_on_track_without_playback_backend(self):
         self.core.playback.play(self.unplayable_tl_track)
 
         self.playback1.prepare_change.assert_called_once_with()
@@ -124,6 +125,22 @@ class CorePlaybackTest(unittest.TestCase):
         self.assertEqual(
             self.core.playback.current_tl_track, self.tl_tracks[3])
 
+    def test_play_skips_to_next_on_unplayable_track(self):
+        """Checks that we handle backend.change_track failing."""
+        self.playback2.change_track.return_value.get.return_value = False
+
+        self.core.tracklist.clear()
+        self.core.tracklist.add(self.tracks[:2])
+        tl_tracks = self.core.tracklist.tl_tracks
+
+        self.core.playback.play(tl_tracks[0])
+        self.core.playback.play(tl_tracks[1])
+
+        # TODO: we really want to check that the track was marked unplayable
+        # and that next was called. This is just an indirect way of checking
+        # this :(
+        self.assertEqual(self.core.playback.state, core.PlaybackState.STOPPED)
+
     @mock.patch(
         'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener)
     def test_play_when_stopped_emits_events(self, listener_mock):