From 6e61f2ef85b01f110d3f5623bbfba6a1899258cb Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 21:53:46 +0200 Subject: [PATCH] Refactoring code to convert tl_track_at_previous() in a function, also recoded tests. --- mopidy/core/playback.py | 5 +++-- mopidy/core/tracklist.py | 28 +++++++++++++--------------- mopidy/frontends/mpris/objects.py | 5 +++-- tests/backends/base/playback.py | 23 ++++++++++++++--------- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 742864b5..8ae7b4d9 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -206,7 +206,7 @@ class PlaybackController(object): elif self.current_tl_track is None and on_error_step == 1: tl_track = self.core.tracklist.tl_track_at_next(tl_track) elif self.current_tl_track is None and on_error_step == -1: - tl_track = self.core.tracklist.tl_track_at_previous + tl_track = self.core.tracklist.tl_track_at_previous(tl_track) if tl_track is not None: self.current_tl_track = tl_track @@ -235,7 +235,8 @@ class PlaybackController(object): will continue. If it was paused, it will still be paused, etc. """ self._trigger_track_playback_ended() - self.change_track(self.core.tracklist.tl_track_at_previous, on_error_step=-1) + tl_track = self.current_tl_track + self.change_track(self.core.tracklist.tl_track_at_previous(tl_track), on_error_step=-1) def resume(self): """If paused, resume playing the current track.""" diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index aa2f18ef..7207f1a9 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -217,27 +217,25 @@ class TracklistController(object): except IndexError: return None - def get_tl_track_at_previous(self): - current_tl_track = self.core.playback.current_tl_track - if self.repeat or self.consume or self.random: - return current_tl_track + def tl_track_at_previous(self, tl_track): + """ + Returns the track that will be played if calling :meth:`previous()`. - position = self.tracklist_position(current_tl_track) + A :class:`mopidy.models.TlTrack`. + + For normal playback this is the previous track in the playlist. If + random and/or consume is enabled it should return the current track + instead. + """ + if self.repeat or self.consume or self.random: + return tl_track + + position = self.tracklist_position(tl_track) if position in (None, 0): return None return self.tl_tracks[position - 1] - tl_track_at_previous = property(get_tl_track_at_previous) - """ - The track that will be played if calling :meth:`previous()`. - - A :class:`mopidy.models.TlTrack`. - - For normal playback this is the previous track in the playlist. If - random and/or consume is enabled it should return the current track - instead. - """ def add(self, tracks=None, at_position=None, uri=None): """ diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index 5253f609..2ab4dd4a 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -414,9 +414,10 @@ class MprisObject(dbus.service.Object): def get_CanGoPrevious(self): if not self.get_CanControl(): return False + tl_track = self.core.playback.current_tl_track return ( - self.core.tracklist.tl_track_at_previous.get() != - self.core.playback.current_tl_track.get()) + self.core.tracklist.tl_track_at_previous(tl_track).get() != + tl_track) def get_CanPlay(self): if not self.get_CanControl(): diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 8f57e219..61db2d71 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -465,18 +465,21 @@ class PlaybackControllerTest(object): @populate_tracklist def test_previous_track_before_play(self): - self.assertEqual(self.tracklist.tl_track_at_previous, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), None) @populate_tracklist def test_previous_track_after_play(self): self.playback.play() - self.assertEqual(self.tracklist.tl_track_at_previous, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), None) @populate_tracklist def test_previous_track_after_next(self): self.playback.play() self.playback.next() - self.assertEqual(self.tracklist.tl_track_at_previous, self.tl_tracks[0]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), self.tl_tracks[0]) @populate_tracklist def test_previous_track_after_previous(self): @@ -484,28 +487,30 @@ class PlaybackControllerTest(object): self.playback.next() # At track 1 self.playback.next() # At track 2 self.playback.previous() # At track 1 - self.assertEqual(self.tracklist.tl_track_at_previous, self.tl_tracks[0]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), self.tl_tracks[0]) def test_previous_track_empty_playlist(self): - self.assertEqual(self.tracklist.tl_track_at_previous, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), None) @populate_tracklist def test_previous_track_with_consume(self): self.tracklist.consume = True for _ in self.tracks: self.playback.next() + tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tl_track_at_previous, - self.playback.current_tl_track) + self.tracklist.tl_track_at_previous(tl_track), tl_track) @populate_tracklist def test_previous_track_with_random(self): self.tracklist.random = True for _ in self.tracks: self.playback.next() + tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tl_track_at_previous, - self.playback.current_tl_track) + self.tracklist.tl_track_at_previous(tl_track), tl_track) @populate_tracklist def test_initial_current_track(self):