diff --git a/docs/changelog.rst b/docs/changelog.rst index bea003f5..eb389166 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,7 +21,15 @@ Bug fix release. - Core: Fix bug in playback controller. If changing to another track while the player is paused, the new track would not be added to the history or - marked as currently playing. (Fixes: :issue:`1352`) + marked as currently playing. (Fixes: :issue:`1352`) Also skips over + unplayable tracks if the user attempts to change tracks while paused. + (Fixes :issue:`1378`). + +- Main: Catch errors when loading :confval:`logging/config_file`. (Fixes: + :issue:`1320`) + +- MPD: Don't return tracks with empty URIs. (Partly fixes: :issue:`1340`, PR: + :issue:`1343`) - Main: Catch errors when loading :confval:`logging/config_file`. (Fixes: :issue:`1320`) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index eeba5106..a11b646d 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -207,8 +207,8 @@ class PlaybackController(object): if old_state == PlaybackState.PLAYING: self._play(on_error_step=on_error_step) elif old_state == PlaybackState.PAUSED: - # NOTE: this is just a quick hack to fix #1177 and #1352 as this - # code has already been killed in the gapless branch. + # NOTE: this is just a quick hack to fix #1177, #1352, and #1378 + # as this code has already been killed in the gapless branch. backend = self._get_backend() if backend: backend.playback.prepare_change() @@ -216,7 +216,16 @@ class PlaybackController(object): if success: self.core.tracklist._mark_playing(tl_track) self.core.history._add_track(tl_track.track) - self.pause() + else: + self.core.tracklist._mark_unplayable( + self.get_current_tl_track()) + if on_error_step == 1: + # TODO: can cause an endless loop for single track + # repeat. + self.next() + elif on_error_step == -1: + self.previous() + self.pause() # TODO: this is not really end of track, this is on_need_next_track def _on_end_of_track(self): diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index d176db0b..d46bdd52 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -41,19 +41,32 @@ class CorePlaybackTest(unittest.TestCase): self.backend3.uri_schemes.get.return_value = ['dummy3'] self.backend3.has_playback().get.return_value = False + # A backend for which 'change_track' fails + self.backend4 = mock.Mock() + self.backend4.uri_schemes.get.return_value = ['dummy4'] + self.playback4 = mock.Mock(spec=backend.PlaybackProvider) + self.playback4.get_time_position.return_value.get.return_value = 1000 + future_mock = mock.Mock(spec=pykka.future.Future) + future_mock.get.return_value = False + self.playback4.change_track.return_value = future_mock + self.backend4.playback = self.playback4 + self.tracks = [ Track(uri='dummy1:a', length=40000), Track(uri='dummy2:a', length=40000), - Track(uri='dummy3:a', length=40000), # Unplayable + Track(uri='dummy3:a', length=40000), # No playback provider Track(uri='dummy1:b', length=40000), Track(uri='dummy1:c', length=None), # No duration + Track(uri='dummy4:a', length=40000), # Unplayable + Track(uri='dummy1:d', length=40000), ] self.uris = [ - 'dummy1:a', 'dummy2:a', 'dummy3:a', 'dummy1:b', 'dummy1:c'] + 'dummy1:a', 'dummy2:a', 'dummy3:a', 'dummy1:b', 'dummy1:c', + 'dummy4:a', 'dummy1:d'] self.core = core.Core(config, mixer=None, backends=[ - self.backend1, self.backend2, self.backend3]) + self.backend1, self.backend2, self.backend3, self.backend4]) def lookup(uris): result = {uri: [] for uri in uris} @@ -172,16 +185,40 @@ class CorePlaybackTest(unittest.TestCase): self.playback2.change_track.return_value.get.return_value = False self.core.tracklist.clear() - self.core.tracklist.add(uris=self.uris[:2]) + self.core.tracklist.add(uris=self.uris[-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) + assert self.core.playback.get_current_tl_track() == tl_tracks[1] + + def test_pause_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(uris=self.uris[-3:]) + tl_tracks = self.core.tracklist.tl_tracks + + self.core.playback.pause() + self.core.playback._set_current_tl_track(tl_tracks[0]) + self.core.playback.next() + self.core.playback.play(self.core.playback.get_current_tl_track()) + assert self.core.playback.get_current_tl_track() == tl_tracks[2] + + def test_pause_resume_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(uris=self.uris[-3:]) + tl_tracks = self.core.tracklist.tl_tracks + + self.core.playback.pause() + self.core.playback._set_current_tl_track(tl_tracks[0]) + self.core.playback.next() + self.core.playback.resume() + assert self.core.playback.get_current_tl_track() == tl_tracks[2] @mock.patch( 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener)