fix: skip uplayable tracks when the next track is selected while in a paused state.

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

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

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)