Merge branch 'develop' into fix/bug-496-handle-single

Conflicts:
	mopidy/core/tracklist.py

mopidy/core/tracklist.py

@@ -21,7 +21,6 @@ class TracklistController(object):
         self._version = 0
 
         self._shuffled = []
-        self._first_shuffle = True
 
     ### Properties
 
@@ -89,6 +88,9 @@ class TracklistController(object):
     def set_random(self, value):
         if self.get_random() != value:
             self._trigger_options_changed()
+        if value:
+            self._shuffled = self.tl_tracks
+            random.shuffle(self._shuffled)
         return setattr(self, '_random', value)
 
     random = property(get_random, set_random)
@@ -187,14 +189,16 @@ class TracklistController(object):
             return None
 
         if self.random and not self._shuffled:
-            if self.repeat or self._first_shuffle:
+            if self.repeat or not tl_track:
                 logger.debug('Shuffling tracks')
                 self._shuffled = self.tl_tracks
                 random.shuffle(self._shuffled)
-                self._first_shuffle = False
 
-        if self.random and self._shuffled:
+        if self.random:
-            return self._shuffled[0]
+            try:
+                return self._shuffled[0]
+            except IndexError:
+                return None
 
         if tl_track is None:
             return self.tl_tracks[0]
@@ -435,8 +439,11 @@ class TracklistController(object):
         return True
 
     def _trigger_tracklist_changed(self):
-        self._first_shuffle = True
+        if self.random:
-        self._shuffled = []
+            self._shuffled = self.tl_tracks
+            random.shuffle(self._shuffled)
+        else:
+            self._shuffled = []
 
         logger.debug('Triggering event: tracklist_changed()')
         listener.CoreListener.send('tracklist_changed')

tests/backends/local/playback_test.py

@@ -1,7 +1,6 @@
 from __future__ import unicode_literals
 
 import mock
-import random
 import time
 import unittest
 
@@ -27,8 +26,12 @@ class LocalPlaybackProviderTest(unittest.TestCase):
             'tag_cache_file': path_to_data_dir('empty_tag_cache'),
         }
     }
+
+    # We need four tracks so that our shuffled track tests behave nicely with
+    # reversed as a fake shuffle. Ensuring that shuffled order is [4,3,2,1] and
+    # normal order [1,2,3,4] which means next_track != next_track_with_random
     tracks = [
-        Track(uri=generate_song(i), length=4464) for i in range(1, 4)]
+        Track(uri=generate_song(i), length=4464) for i in (1, 2, 3, 4)]
 
     def add_track(self, uri):
         track = Track(uri=uri, length=4464)
@@ -320,12 +323,14 @@ class LocalPlaybackProviderTest(unittest.TestCase):
             self.tracklist.next_track(tl_track), self.tl_tracks[0])
 
     @populate_tracklist
-    def test_next_track_with_random(self):
+    @mock.patch('random.shuffle')
-        random.seed(1)
+    def test_next_track_with_random(self, shuffle_mock):
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
-        tl_track = self.playback.current_tl_track
+        current_tl_track = self.playback.current_tl_track
-        self.assertEqual(
+        next_tl_track = self.tracklist.next_track(current_tl_track)
-            self.tracklist.next_track(tl_track), self.tl_tracks[2])
+        self.assertEqual(next_tl_track, self.tl_tracks[-1])
 
     @populate_tracklist
     def test_next_with_consume(self):
@@ -344,25 +349,41 @@ class LocalPlaybackProviderTest(unittest.TestCase):
         self.assertEqual(self.playback.current_track, self.tracks[1])
 
     @populate_tracklist
-    def test_next_with_random(self):
+    @mock.patch('random.shuffle')
-        # FIXME feels very fragile
+    def test_next_with_random(self, shuffle_mock):
-        random.seed(1)
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
         self.playback.play()
+        self.assertEqual(self.playback.current_track, self.tracks[-1])
         self.playback.next()
-        self.assertEqual(self.playback.current_track, self.tracks[1])
+        self.assertEqual(self.playback.current_track, self.tracks[-2])
 
     @populate_tracklist
-    def test_next_track_with_random_after_append_playlist(self):
+    @mock.patch('random.shuffle')
-        random.seed(1)
+    def test_next_track_with_random_after_append_playlist(self, shuffle_mock):
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
-        tl_track = self.playback.current_tl_track
+        current_tl_track = self.playback.current_tl_track
-        self.assertEqual(self.tracklist.next_track(tl_track),
+
-                         self.tl_tracks[2])
+        expected_tl_track = self.tracklist.tl_tracks[-1]
+        next_tl_track = self.tracklist.next_track(current_tl_track)
+
+        # Baseline checking that first next_track is last tl track per our fake
+        # shuffle.
+        self.assertEqual(next_tl_track, expected_tl_track)
+
         self.tracklist.add(self.tracks[:1])
-        tl_track = self.playback.current_tl_track
+
-        self.assertEqual(
+        old_next_tl_track = next_tl_track
-            self.tracklist.next_track(tl_track), self.tl_tracks[1])
+        expected_tl_track = self.tracklist.tl_tracks[-1]
+        next_tl_track = self.tracklist.next_track(current_tl_track)
+
+        # Verify that first next track has changed since we added to the
+        # playlist.
+        self.assertEqual(next_tl_track, expected_tl_track)
+        self.assertNotEqual(next_tl_track, old_next_tl_track)
 
     @populate_tracklist
     def test_end_of_track(self):
@@ -476,12 +497,14 @@ class LocalPlaybackProviderTest(unittest.TestCase):
             self.tracklist.next_track(tl_track), self.tl_tracks[0])
 
     @populate_tracklist
-    def test_end_of_track_track_with_random(self):
+    @mock.patch('random.shuffle')
-        random.seed(1)
+    def test_end_of_track_track_with_random(self, shuffle_mock):
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
         tl_track = self.playback.current_tl_track
         self.assertEqual(
-            self.tracklist.next_track(tl_track), self.tl_tracks[2])
+            self.tracklist.next_track(tl_track), self.tl_tracks[-1])
 
     @populate_tracklist
     def test_end_of_track_with_consume(self):
@@ -491,25 +514,42 @@ class LocalPlaybackProviderTest(unittest.TestCase):
         self.assertNotIn(self.tracks[0], self.tracklist.tracks)
 
     @populate_tracklist
-    def test_end_of_track_with_random(self):
+    @mock.patch('random.shuffle')
-        # FIXME feels very fragile
+    def test_end_of_track_with_random(self, shuffle_mock):
-        random.seed(1)
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
         self.playback.play()
+        self.assertEqual(self.playback.current_track, self.tracks[-1])
         self.playback.on_end_of_track()
-        self.assertEqual(self.playback.current_track, self.tracks[1])
+        self.assertEqual(self.playback.current_track, self.tracks[-2])
 
     @populate_tracklist
-    def test_end_of_track_track_with_random_after_append_playlist(self):
+    @mock.patch('random.shuffle')
-        random.seed(1)
+    def test_end_of_track_track_with_random_after_append_playlist(
+            self, shuffle_mock):
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
-        tl_track = self.playback.current_tl_track
+        current_tl_track = self.playback.current_tl_track
-        self.assertEqual(
+
-            self.tracklist.next_track(tl_track), self.tl_tracks[2])
+        expected_tl_track = self.tracklist.tl_tracks[-1]
+        eot_tl_track = self.tracklist.eot_track(current_tl_track)
+
+        # Baseline checking that first eot_track is last tl track per our fake
+        # shuffle.
+        self.assertEqual(eot_tl_track, expected_tl_track)
+
         self.tracklist.add(self.tracks[:1])
-        tl_track = self.playback.current_tl_track
+
-        self.assertEqual(
+        old_eot_tl_track = eot_tl_track
-            self.tracklist.next_track(tl_track), self.tl_tracks[1])
+        expected_tl_track = self.tracklist.tl_tracks[-1]
+        eot_tl_track = self.tracklist.eot_track(current_tl_track)
+
+        # Verify that first next track has changed since we added to the
+        # playlist.
+        self.assertEqual(eot_tl_track, expected_tl_track)
+        self.assertNotEqual(eot_tl_track, old_eot_tl_track)
 
     @populate_tracklist
     def test_previous_track_before_play(self):
@@ -873,15 +913,19 @@ class LocalPlaybackProviderTest(unittest.TestCase):
         self.assertEqual(len(self.tracklist.tracks), 0)
 
     @populate_tracklist
-    def test_play_with_random(self):
+    @mock.patch('random.shuffle')
-        random.seed(1)
+    def test_play_with_random(self, shuffle_mock):
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
         self.playback.play()
-        self.assertEqual(self.playback.current_track, self.tracks[2])
+        self.assertEqual(self.playback.current_track, self.tracks[-1])
 
     @populate_tracklist
-    def test_previous_with_random(self):
+    @mock.patch('random.shuffle')
-        random.seed(1)
+    def test_previous_with_random(self, shuffle_mock):
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
         self.tracklist.random = True
         self.playback.play()
         self.playback.next()
@@ -956,9 +1000,19 @@ class LocalPlaybackProviderTest(unittest.TestCase):
         tl_track = self.playback.current_tl_track
         self.assertEqual(self.tracklist.next_track(tl_track), None)
 
+    @populate_tracklist
+    def test_random_with_eot_until_end_of_playlist(self):
+        self.tracklist.random = True
+        self.playback.play()
+        for _ in self.tracks[1:]:
+            self.playback.on_end_of_track()
+        tl_track = self.playback.current_tl_track
+        self.assertEqual(self.tracklist.eot_track(tl_track), None)
+
     @populate_tracklist
     def test_random_until_end_of_playlist_and_play_from_start(self):
-        self.tracklist.repeat = True
+        self.tracklist.random = True
+        self.playback.play()
         for _ in self.tracks:
             self.playback.next()
         tl_track = self.playback.current_tl_track
@@ -967,12 +1021,24 @@ class LocalPlaybackProviderTest(unittest.TestCase):
         self.playback.play()
         self.assertEqual(self.playback.state, PlaybackState.PLAYING)
 
+    @populate_tracklist
+    def test_random_with_eot_until_end_of_playlist_and_play_from_start(self):
+        self.tracklist.random = True
+        self.playback.play()
+        for _ in self.tracks:
+            self.playback.on_end_of_track()
+        tl_track = self.playback.current_tl_track
+        self.assertNotEqual(self.tracklist.eot_track(tl_track), None)
+        self.assertEqual(self.playback.state, PlaybackState.STOPPED)
+        self.playback.play()
+        self.assertEqual(self.playback.state, PlaybackState.PLAYING)
+
     @populate_tracklist
     def test_random_until_end_of_playlist_with_repeat(self):
         self.tracklist.repeat = True
         self.tracklist.random = True
         self.playback.play()
-        for _ in self.tracks:
+        for _ in self.tracks[1:]:
             self.playback.next()
         tl_track = self.playback.current_tl_track
         self.assertNotEqual(self.tracklist.next_track(tl_track), None)
@@ -987,6 +1053,22 @@ class LocalPlaybackProviderTest(unittest.TestCase):
             played.append(self.playback.current_track)
             self.playback.next()
 
+    @populate_tracklist
+    @mock.patch('random.shuffle')
+    def test_play_track_then_enable_random(self, shuffle_mock):
+        # Covers underlying issue IssueGH17RegressionTest tests for.
+        shuffle_mock.side_effect = lambda tracks: tracks.reverse()
+
+        expected = self.tl_tracks[::-1] + [None]
+        actual = []
+
+        self.playback.play()
+        self.tracklist.random = True
+        while self.playback.state != PlaybackState.STOPPED:
+            self.playback.next()
+            actual.append(self.playback.current_tl_track)
+        self.assertEqual(actual, expected)
+
     @populate_tracklist
     def test_playing_track_that_isnt_in_playlist(self):
         test = lambda: self.playback.play((17, Track()))