From efe9430c7af630dd36df52677ea6834100c11af0 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 22 Mar 2015 22:12:51 +0100 Subject: [PATCH] core: Update playback code to take change track into account. This change has us checking the return value of change_track when deciding if the play call was a success or if the track is unplayable. Which ensures that the following can no longer happen: 1) play stream 2) play stream that fails change_track 3) stream 1) continues playing. Correct behavior being the next stream playing instead. --- mopidy/core/playback.py | 4 ++-- tests/core/test_playback.py | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 4f51f328..d5736808 100644 --- a/mopidy/core/playback.py +++ b/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) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index e84e7301..5f305c4e 100644 --- a/tests/core/test_playback.py +++ b/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 change track failing.""" + self.playback2.change_track().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):