From 4db3999371934bb1b2f54a7e5505595718e83415 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 10:44:22 +0200 Subject: [PATCH 01/42] Cleanup of PlaybackController to be more specific on the current track and moved those functions to TracklistController, which may control all related to the tracks. Updated tests too. --- mopidy/core/playback.py | 210 +----------------- mopidy/core/tracklist.py | 195 +++++++++++++++- mopidy/frontends/mpd/protocol/playback.py | 18 +- mopidy/frontends/mpd/protocol/status.py | 24 +- mopidy/frontends/mpris/objects.py | 28 +-- tests/backends/base/playback.py | 132 +++++------ tests/frontends/mpd/protocol/playback_test.py | 32 +-- tests/frontends/mpd/status_test.py | 6 +- .../frontends/mpris/player_interface_test.py | 56 ++--- tests/utils/jsonrpc_test.py | 30 +-- 10 files changed, 367 insertions(+), 364 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 2e79827a..8b33140a 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -24,8 +24,6 @@ class PlaybackController(object): self.core = core self._state = PlaybackState.STOPPED - self._shuffled = [] - self._first_shuffle = True self._volume = None def _get_backend(self): @@ -37,22 +35,6 @@ class PlaybackController(object): ### Properties - def get_consume(self): - return getattr(self, '_consume', False) - - def set_consume(self, value): - if self.get_consume() != value: - self._trigger_options_changed() - return setattr(self, '_consume', value) - - consume = property(get_consume, set_consume) - """ - :class:`True` - Tracks are removed from the playlist when they have been played. - :class:`False` - Tracks are not removed from the playlist. - """ - def get_current_tl_track(self): return self.current_tl_track @@ -72,56 +54,6 @@ class PlaybackController(object): Read-only. Extracted from :attr:`current_tl_track` for convenience. """ - def get_random(self): - return getattr(self, '_random', False) - - def set_random(self, value): - if self.get_random() != value: - self._trigger_options_changed() - return setattr(self, '_random', value) - - random = property(get_random, set_random) - """ - :class:`True` - Tracks are selected at random from the playlist. - :class:`False` - Tracks are played in the order of the playlist. - """ - - def get_repeat(self): - return getattr(self, '_repeat', False) - - def set_repeat(self, value): - if self.get_repeat() != value: - self._trigger_options_changed() - return setattr(self, '_repeat', value) - - repeat = property(get_repeat, set_repeat) - """ - :class:`True` - The current playlist is played repeatedly. To repeat a single track, - select both :attr:`repeat` and :attr:`single`. - :class:`False` - The current playlist is played once. - """ - - def get_single(self): - return getattr(self, '_single', False) - - def set_single(self, value): - if self.get_single() != value: - self._trigger_options_changed() - return setattr(self, '_single', value) - - single = property(get_single, set_single) - """ - :class:`True` - Playback is stopped after current song, unless in :attr:`repeat` - mode. - :class:`False` - Playback continues after current song. - """ - def get_state(self): return self._state @@ -159,122 +91,6 @@ class PlaybackController(object): time_position = property(get_time_position) """Time position in milliseconds.""" - def get_tracklist_position(self): - if self.current_tl_track is None: - return None - try: - return self.core.tracklist.tl_tracks.index(self.current_tl_track) - except ValueError: - return None - - tracklist_position = property(get_tracklist_position) - """ - The position of the current track in the tracklist. - - Read-only. - """ - - def get_tl_track_at_eot(self): - # pylint: disable = R0911 - # Too many return statements - - tl_tracks = self.core.tracklist.tl_tracks - - if not tl_tracks: - return None - - if self.random and not self._shuffled: - if self.repeat or self._first_shuffle: - logger.debug('Shuffling tracks') - self._shuffled = tl_tracks - random.shuffle(self._shuffled) - self._first_shuffle = False - - if self.random and self._shuffled: - return self._shuffled[0] - - if self.current_tl_track is None: - return tl_tracks[0] - - if self.repeat and self.single: - return tl_tracks[self.tracklist_position] - - if self.repeat and not self.single: - return tl_tracks[(self.tracklist_position + 1) % len(tl_tracks)] - - try: - return tl_tracks[self.tracklist_position + 1] - except IndexError: - return None - - tl_track_at_eot = property(get_tl_track_at_eot) - """ - The track that will be played at the end of the current track. - - Read-only. A :class:`mopidy.models.TlTrack`. - - Not necessarily the same track as :attr:`tl_track_at_next`. - """ - - def get_tl_track_at_next(self): - tl_tracks = self.core.tracklist.tl_tracks - - if not tl_tracks: - return None - - if self.random and not self._shuffled: - if self.repeat or self._first_shuffle: - logger.debug('Shuffling tracks') - self._shuffled = tl_tracks - random.shuffle(self._shuffled) - self._first_shuffle = False - - if self.random and self._shuffled: - return self._shuffled[0] - - if self.current_tl_track is None: - return tl_tracks[0] - - if self.repeat: - return tl_tracks[(self.tracklist_position + 1) % len(tl_tracks)] - - try: - return tl_tracks[self.tracklist_position + 1] - except IndexError: - return None - - tl_track_at_next = property(get_tl_track_at_next) - """ - The track that will be played if calling :meth:`next()`. - - Read-only. A :class:`mopidy.models.TlTrack`. - - For normal playback this is the next track in the playlist. If repeat - is enabled the next track can loop around the playlist. When random is - enabled this should be a random track, all tracks should be played once - before the list repeats. - """ - - def get_tl_track_at_previous(self): - if self.repeat or self.consume or self.random: - return self.current_tl_track - - if self.tracklist_position in (None, 0): - return None - - return self.core.tracklist.tl_tracks[self.tracklist_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 get_volume(self): if self.audio: return self.audio.get_volume().get() @@ -325,13 +141,13 @@ class PlaybackController(object): original_tl_track = self.current_tl_track - if self.tl_track_at_eot: + if self.core.tracklist.tl_track_at_eot: self._trigger_track_playback_ended() - self.play(self.tl_track_at_eot) + self.play(self.core.tracklist.tl_track_at_eot) else: self.stop(clear_current_track=True) - if self.consume: + if self.core.tracklist.consume: self.core.tracklist.remove(tlid=original_tl_track.tlid) def on_tracklist_change(self): @@ -340,8 +156,6 @@ class PlaybackController(object): Used by :class:`mopidy.core.TracklistController`. """ - self._first_shuffle = True - self._shuffled = [] if (not self.core.tracklist.tl_tracks or self.current_tl_track not in @@ -355,9 +169,9 @@ class PlaybackController(object): The current playback state will be kept. If it was playing, playing will continue. If it was paused, it will still be paused, etc. """ - if self.tl_track_at_next: + if self.core.tracklist.tl_track_at_next: self._trigger_track_playback_ended() - self.change_track(self.tl_track_at_next) + self.change_track(self.core.tracklist.tl_track_at_next) else: self.stop(clear_current_track=True) @@ -388,9 +202,9 @@ class PlaybackController(object): elif self.current_tl_track is not None: tl_track = self.current_tl_track elif self.current_tl_track is None and on_error_step == 1: - tl_track = self.tl_track_at_next + tl_track = self.core.tracklist.tl_track_at_next elif self.current_tl_track is None and on_error_step == -1: - tl_track = self.tl_track_at_previous + tl_track = self.core.tracklist.tl_track_at_previous if tl_track is not None: self.current_tl_track = tl_track @@ -398,16 +212,16 @@ class PlaybackController(object): backend = self._get_backend() if not backend or not backend.playback.play(tl_track.track).get(): logger.warning('Track is not playable: %s', tl_track.track.uri) - if self.random and self._shuffled: - self._shuffled.remove(tl_track) + if self.core.tracklist.random and self.core.tracklist._shuffled: + self.core.tracklist._shuffled.remove(tl_track) if on_error_step == 1: self.next() elif on_error_step == -1: self.previous() return - if self.random and self.current_tl_track in self._shuffled: - self._shuffled.remove(self.current_tl_track) + if self.core.tracklist.random and self.current_tl_track in self.core.tracklist._shuffled: + self.core.tracklist._shuffled.remove(self.current_tl_track) self._trigger_track_playback_started() @@ -419,7 +233,7 @@ class PlaybackController(object): will continue. If it was paused, it will still be paused, etc. """ self._trigger_track_playback_ended() - self.change_track(self.tl_track_at_previous, on_error_step=-1) + self.change_track(self.core.tracklist.tl_track_at_previous, 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 1c8f437f..33bebdee 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -15,11 +15,16 @@ class TracklistController(object): pykka_traversable = True def __init__(self, core): - self._core = core + self.core = core self._next_tlid = 0 self._tl_tracks = [] self._version = 0 + self._shuffled = [] + self._first_shuffle = True + + ### Properties + def get_tl_tracks(self): return self._tl_tracks[:] @@ -51,7 +56,7 @@ class TracklistController(object): def _increase_version(self): self._version += 1 - self._core.playback.on_tracklist_change() + self.core.playback.on_tracklist_change() self._trigger_tracklist_changed() version = property(get_version) @@ -62,6 +67,188 @@ class TracklistController(object): Is not reset before Mopidy is restarted. """ + def get_consume(self): + return getattr(self, '_consume', False) + + def set_consume(self, value): + if self.get_consume() != value: + self.core.playback._trigger_options_changed() + return setattr(self, '_consume', value) + + consume = property(get_consume, set_consume) + """ + :class:`True` + Tracks are removed from the playlist when they have been played. + :class:`False` + Tracks are not removed from the playlist. + """ + + def get_random(self): + return getattr(self, '_random', False) + + def set_random(self, value): + if self.get_random() != value: + self.core.playback._trigger_options_changed() + return setattr(self, '_random', value) + + random = property(get_random, set_random) + """ + :class:`True` + Tracks are selected at random from the playlist. + :class:`False` + Tracks are played in the order of the playlist. + """ + + def get_repeat(self): + return getattr(self, '_repeat', False) + + def set_repeat(self, value): + if self.get_repeat() != value: + self.core.playback._trigger_options_changed() + return setattr(self, '_repeat', value) + + repeat = property(get_repeat, set_repeat) + """ + :class:`True` + The current playlist is played repeatedly. To repeat a single track, + select both :attr:`repeat` and :attr:`single`. + :class:`False` + The current playlist is played once. + """ + + def get_single(self): + return getattr(self, '_single', False) + + def set_single(self, value): + if self.get_single() != value: + self.core.playback._trigger_options_changed() + return setattr(self, '_single', value) + + single = property(get_single, set_single) + """ + :class:`True` + Playback is stopped after current song, unless in :attr:`repeat` + mode. + :class:`False` + Playback continues after current song. + """ + + def get_tracklist_position(self): + if self.core.playback.current_tl_track is None: + return None + try: + return self.core.tracklist.tl_tracks.index(self.core.playback.current_tl_track) + except ValueError: + return None + + tracklist_position = property(get_tracklist_position) + """ + The position of the current track in the tracklist. + + Read-only. + """ + + def get_tl_track_at_eot(self): + # pylint: disable = R0911 + # Too many return statements + + tl_tracks = self.core.tracklist.tl_tracks + + if not tl_tracks: + return None + + if self.random and not self._shuffled: + if self.repeat or self._first_shuffle: + logger.debug('Shuffling tracks') + self._shuffled = tl_tracks + random.shuffle(self._shuffled) + self._first_shuffle = False + + if self.random and self._shuffled: + return self._shuffled[0] + + if self.core.playback.current_tl_track is None: + return tl_tracks[0] + + if self.repeat and self.single: + return tl_tracks[self.tracklist_position] + + if self.repeat and not self.single: + return tl_tracks[(self.tracklist_position + 1) % len(tl_tracks)] + + try: + return tl_tracks[self.tracklist_position + 1] + except IndexError: + return None + + tl_track_at_eot = property(get_tl_track_at_eot) + """ + The track that will be played at the end of the current track. + + Read-only. A :class:`mopidy.models.TlTrack`. + + Not necessarily the same track as :attr:`tl_track_at_next`. + """ + + def get_tl_track_at_next(self): + tl_tracks = self.core.tracklist.tl_tracks + + if not tl_tracks: + return None + + if self.random and not self._shuffled: + if self.repeat or self._first_shuffle: + logger.debug('Shuffling tracks') + self._shuffled = tl_tracks + random.shuffle(self._shuffled) + self._first_shuffle = False + + if self.random and self._shuffled: + return self._shuffled[0] + + if self.core.playback.current_tl_track is None: + return tl_tracks[0] + + if self.repeat: + return tl_tracks[(self.tracklist_position + 1) % len(tl_tracks)] + + try: + return tl_tracks[self.tracklist_position + 1] + except IndexError: + return None + + tl_track_at_next = property(get_tl_track_at_next) + """ + The track that will be played if calling :meth:`next()`. + + Read-only. A :class:`mopidy.models.TlTrack`. + + For normal playback this is the next track in the playlist. If repeat + is enabled the next track can loop around the playlist. When random is + enabled this should be a random track, all tracks should be played once + before the list repeats. + """ + + def get_tl_track_at_previous(self): + if self.repeat or self.core.tracklist.consume or self.random: + return self.core.playback.current_tl_track + + if self.tracklist_position in (None, 0): + return None + + return self.core.tracklist.tl_tracks[self.tracklist_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): """ Add the track or list of tracks to the tracklist. @@ -87,7 +274,7 @@ class TracklistController(object): 'tracks or uri must be provided' if tracks is None and uri is not None: - tracks = self._core.library.lookup(uri) + tracks = self.core.library.lookup(uri) tl_tracks = [] @@ -260,5 +447,7 @@ class TracklistController(object): return self._tl_tracks[start:end] def _trigger_tracklist_changed(self): + self._first_shuffle = True + self._shuffled = [] logger.debug('Triggering event: tracklist_changed()') listener.CoreListener.send('tracklist_changed') diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index 8e08585f..fcc465aa 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -19,9 +19,9 @@ def consume(context, state): playlist. """ if int(state): - context.core.playback.consume = True + context.core.tracklist.consume = True else: - context.core.playback.consume = False + context.core.tracklist.consume = False @handle_request(r'^crossfade "(?P\d+)"$') @@ -263,9 +263,9 @@ def random(context, state): Sets random state to ``STATE``, ``STATE`` should be 0 or 1. """ if int(state): - context.core.playback.random = True + context.core.tracklist.random = True else: - context.core.playback.random = False + context.core.tracklist.random = False @handle_request(r'^repeat (?P[01])$') @@ -279,9 +279,9 @@ def repeat(context, state): Sets repeat state to ``STATE``, ``STATE`` should be 0 or 1. """ if int(state): - context.core.playback.repeat = True + context.core.tracklist.repeat = True else: - context.core.playback.repeat = False + context.core.tracklist.repeat = False @handle_request(r'^replay_gain_mode "(?P(off|track|album))"$') @@ -329,7 +329,7 @@ def seek(context, songpos, seconds): - issues ``seek 1 120`` without quotes around the arguments. """ - if context.core.playback.tracklist_position.get() != int(songpos): + if context.core.tracklist.tracklist_position.get() != int(songpos): playpos(context, songpos) context.core.playback.seek(int(seconds) * 1000).get() @@ -404,9 +404,9 @@ def single(context, state): song is repeated if the ``repeat`` mode is enabled. """ if int(state): - context.core.playback.single = True + context.core.tracklist.single = True else: - context.core.playback.single = False + context.core.tracklist.single = False @handle_request(r'^stop$') diff --git a/mopidy/frontends/mpd/protocol/status.py b/mopidy/frontends/mpd/protocol/status.py index 34e2fa64..f42b3531 100644 --- a/mopidy/frontends/mpd/protocol/status.py +++ b/mopidy/frontends/mpd/protocol/status.py @@ -38,7 +38,7 @@ def currentsong(context): """ current_tl_track = context.core.playback.current_tl_track.get() if current_tl_track is not None: - position = context.core.playback.tracklist_position.get() + position = context.core.tracklist.tracklist_position.get() return track_to_mpd_format(current_tl_track, position=position) @@ -178,14 +178,14 @@ def status(context): 'tracklist.length': context.core.tracklist.length, 'tracklist.version': context.core.tracklist.version, 'playback.volume': context.core.playback.volume, - 'playback.consume': context.core.playback.consume, - 'playback.random': context.core.playback.random, - 'playback.repeat': context.core.playback.repeat, - 'playback.single': context.core.playback.single, + 'tracklist.consume': context.core.tracklist.consume, + 'tracklist.random': context.core.tracklist.random, + 'tracklist.repeat': context.core.tracklist.repeat, + 'tracklist.single': context.core.tracklist.single, 'playback.state': context.core.playback.state, 'playback.current_tl_track': context.core.playback.current_tl_track, - 'playback.tracklist_position': ( - context.core.playback.tracklist_position), + 'tracklist.tracklist_position': ( + context.core.tracklist.tracklist_position), 'playback.time_position': context.core.playback.time_position, } pykka.get_all(futures.values()) @@ -218,7 +218,7 @@ def _status_bitrate(futures): def _status_consume(futures): - if futures['playback.consume'].get(): + if futures['tracklist.consume'].get(): return 1 else: return 0 @@ -233,15 +233,15 @@ def _status_playlist_version(futures): def _status_random(futures): - return int(futures['playback.random'].get()) + return int(futures['tracklist.random'].get()) def _status_repeat(futures): - return int(futures['playback.repeat'].get()) + return int(futures['tracklist.repeat'].get()) def _status_single(futures): - return int(futures['playback.single'].get()) + return int(futures['tracklist.single'].get()) def _status_songid(futures): @@ -253,7 +253,7 @@ def _status_songid(futures): def _status_songpos(futures): - return futures['playback.tracklist_position'].get() + return futures['tracklist.tracklist_position'].get() def _status_state(futures): diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index 15be1eea..d4f28a83 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -301,8 +301,8 @@ class MprisObject(dbus.service.Object): return 'Stopped' def get_LoopStatus(self): - repeat = self.core.playback.repeat.get() - single = self.core.playback.single.get() + repeat = self.core.tracklist.repeat.get() + single = self.core.tracklist.single.get() if not repeat: return 'None' else: @@ -316,14 +316,14 @@ class MprisObject(dbus.service.Object): logger.debug('Setting %s.LoopStatus not allowed', PLAYER_IFACE) return if value == 'None': - self.core.playback.repeat = False - self.core.playback.single = False + self.core.tracklist.repeat = False + self.core.tracklist.single = False elif value == 'Track': - self.core.playback.repeat = True - self.core.playback.single = True + self.core.tracklist.repeat = True + self.core.tracklist.single = True elif value == 'Playlist': - self.core.playback.repeat = True - self.core.playback.single = False + self.core.tracklist.repeat = True + self.core.tracklist.single = False def set_Rate(self, value): if not self.get_CanControl(): @@ -335,16 +335,16 @@ class MprisObject(dbus.service.Object): self.Pause() def get_Shuffle(self): - return self.core.playback.random.get() + return self.core.tracklist.random.get() def set_Shuffle(self, value): if not self.get_CanControl(): logger.debug('Setting %s.Shuffle not allowed', PLAYER_IFACE) return if value: - self.core.playback.random = True + self.core.tracklist.random = True else: - self.core.playback.random = False + self.core.tracklist.random = False def get_Metadata(self): current_tl_track = self.core.playback.current_tl_track.get() @@ -407,14 +407,14 @@ class MprisObject(dbus.service.Object): if not self.get_CanControl(): return False return ( - self.core.playback.tl_track_at_next.get() != + self.core.tracklist.tl_track_at_next.get() != self.core.playback.current_tl_track.get()) def get_CanGoPrevious(self): if not self.get_CanControl(): return False return ( - self.core.playback.tl_track_at_previous.get() != + self.core.tracklist.tl_track_at_previous.get() != self.core.playback.current_tl_track.get()) def get_CanPlay(self): @@ -422,7 +422,7 @@ class MprisObject(dbus.service.Object): return False return ( self.core.playback.current_tl_track.get() is not None or - self.core.playback.tl_track_at_next.get() is not None) + self.core.tracklist.tl_track_at_next.get() is not None) def get_CanPause(self): if not self.get_CanControl(): diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 44ae40f9..11e21628 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -179,13 +179,13 @@ class PlaybackControllerTest(object): def test_next(self): self.playback.play() - old_position = self.playback.tracklist_position + old_position = self.tracklist.tracklist_position old_uri = self.playback.current_track.uri self.playback.next() self.assertEqual( - self.playback.tracklist_position, old_position + 1) + self.tracklist.tracklist_position, old_position + 1) self.assertNotEqual(self.playback.current_track.uri, old_uri) @populate_tracklist @@ -205,7 +205,7 @@ class PlaybackControllerTest(object): for i, track in enumerate(self.tracks): self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) - self.assertEqual(self.playback.tracklist_position, i) + self.assertEqual(self.tracklist.tracklist_position, i) self.playback.next() @@ -241,55 +241,55 @@ class PlaybackControllerTest(object): @populate_tracklist def test_next_track_before_play(self): - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[0]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) @populate_tracklist def test_next_track_during_play(self): self.playback.play() - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[1]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) @populate_tracklist def test_next_track_after_previous(self): self.playback.play() self.playback.next() self.playback.previous() - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[1]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) def test_next_track_empty_playlist(self): - self.assertEqual(self.playback.tl_track_at_next, None) + self.assertEqual(self.tracklist.tl_track_at_next, None) @populate_tracklist def test_next_track_at_end_of_playlist(self): self.playback.play() for _ in self.tracklist.tl_tracks[1:]: self.playback.next() - self.assertEqual(self.playback.tl_track_at_next, None) + self.assertEqual(self.tracklist.tl_track_at_next, None) @populate_tracklist def test_next_track_at_end_of_playlist_with_repeat(self): - self.playback.repeat = True + self.tracklist.repeat = True self.playback.play() for _ in self.tracks[1:]: self.playback.next() - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[0]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) @populate_tracklist def test_next_track_with_random(self): random.seed(1) - self.playback.random = True - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[2]) + self.tracklist.random = True + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) @populate_tracklist def test_next_with_consume(self): - self.playback.consume = True + self.tracklist.consume = True self.playback.play() self.playback.next() self.assertIn(self.tracks[0], self.tracklist.tracks) @populate_tracklist def test_next_with_single_and_repeat(self): - self.playback.single = True - self.playback.repeat = True + self.tracklist.single = True + self.tracklist.repeat = True self.playback.play() self.playback.next() self.assertEqual(self.playback.current_track, self.tracks[1]) @@ -298,7 +298,7 @@ class PlaybackControllerTest(object): def test_next_with_random(self): # FIXME feels very fragile random.seed(1) - self.playback.random = True + self.tracklist.random = True self.playback.play() self.playback.next() self.assertEqual(self.playback.current_track, self.tracks[1]) @@ -306,22 +306,22 @@ class PlaybackControllerTest(object): @populate_tracklist def test_next_track_with_random_after_append_playlist(self): random.seed(1) - self.playback.random = True - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[2]) + self.tracklist.random = True + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[1]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) @populate_tracklist def test_end_of_track(self): self.playback.play() - old_position = self.playback.tracklist_position + old_position = self.tracklist.tracklist_position old_uri = self.playback.current_track.uri self.playback.on_end_of_track() self.assertEqual( - self.playback.tracklist_position, old_position + 1) + self.tracklist.tracklist_position, old_position + 1) self.assertNotEqual(self.playback.current_track.uri, old_uri) @populate_tracklist @@ -341,7 +341,7 @@ class PlaybackControllerTest(object): for i, track in enumerate(self.tracks): self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) - self.assertEqual(self.playback.tracklist_position, i) + self.assertEqual(self.tracklist.tracklist_position, i) self.playback.on_end_of_track() @@ -377,47 +377,47 @@ class PlaybackControllerTest(object): @populate_tracklist def test_end_of_track_track_before_play(self): - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[0]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_during_play(self): self.playback.play() - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[1]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) @populate_tracklist def test_end_of_track_track_after_previous(self): self.playback.play() self.playback.on_end_of_track() self.playback.previous() - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[1]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) def test_end_of_track_track_empty_playlist(self): - self.assertEqual(self.playback.tl_track_at_next, None) + self.assertEqual(self.tracklist.tl_track_at_next, None) @populate_tracklist def test_end_of_track_track_at_end_of_playlist(self): self.playback.play() for _ in self.tracklist.tl_tracks[1:]: self.playback.on_end_of_track() - self.assertEqual(self.playback.tl_track_at_next, None) + self.assertEqual(self.tracklist.tl_track_at_next, None) @populate_tracklist def test_end_of_track_track_at_end_of_playlist_with_repeat(self): - self.playback.repeat = True + self.tracklist.repeat = True self.playback.play() for _ in self.tracks[1:]: self.playback.on_end_of_track() - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[0]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_with_random(self): random.seed(1) - self.playback.random = True - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[2]) + self.tracklist.random = True + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) @populate_tracklist def test_end_of_track_with_consume(self): - self.playback.consume = True + self.tracklist.consume = True self.playback.play() self.playback.on_end_of_track() self.assertNotIn(self.tracks[0], self.tracklist.tracks) @@ -426,7 +426,7 @@ class PlaybackControllerTest(object): def test_end_of_track_with_random(self): # FIXME feels very fragile random.seed(1) - self.playback.random = True + self.tracklist.random = True self.playback.play() self.playback.on_end_of_track() self.assertEqual(self.playback.current_track, self.tracks[1]) @@ -434,25 +434,25 @@ class PlaybackControllerTest(object): @populate_tracklist def test_end_of_track_track_with_random_after_append_playlist(self): random.seed(1) - self.playback.random = True - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[2]) + self.tracklist.random = True + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) - self.assertEqual(self.playback.tl_track_at_next, self.tl_tracks[1]) + self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) @populate_tracklist def test_previous_track_before_play(self): - self.assertEqual(self.playback.tl_track_at_previous, None) + self.assertEqual(self.tracklist.tl_track_at_previous, None) @populate_tracklist def test_previous_track_after_play(self): self.playback.play() - self.assertEqual(self.playback.tl_track_at_previous, None) + self.assertEqual(self.tracklist.tl_track_at_previous, None) @populate_tracklist def test_previous_track_after_next(self): self.playback.play() self.playback.next() - self.assertEqual(self.playback.tl_track_at_previous, self.tl_tracks[0]) + self.assertEqual(self.tracklist.tl_track_at_previous, self.tl_tracks[0]) @populate_tracklist def test_previous_track_after_previous(self): @@ -460,27 +460,27 @@ class PlaybackControllerTest(object): self.playback.next() # At track 1 self.playback.next() # At track 2 self.playback.previous() # At track 1 - self.assertEqual(self.playback.tl_track_at_previous, self.tl_tracks[0]) + self.assertEqual(self.tracklist.tl_track_at_previous, self.tl_tracks[0]) def test_previous_track_empty_playlist(self): - self.assertEqual(self.playback.tl_track_at_previous, None) + self.assertEqual(self.tracklist.tl_track_at_previous, None) @populate_tracklist def test_previous_track_with_consume(self): - self.playback.consume = True + self.tracklist.consume = True for _ in self.tracks: self.playback.next() self.assertEqual( - self.playback.tl_track_at_previous, + self.tracklist.tl_track_at_previous, self.playback.current_tl_track) @populate_tracklist def test_previous_track_with_random(self): - self.playback.random = True + self.tracklist.random = True for _ in self.tracks: self.playback.next() self.assertEqual( - self.playback.tl_track_at_previous, + self.tracklist.tl_track_at_previous, self.playback.current_tl_track) @populate_tracklist @@ -500,24 +500,24 @@ class PlaybackControllerTest(object): @populate_tracklist def test_initial_tracklist_position(self): - self.assertEqual(self.playback.tracklist_position, None) + self.assertEqual(self.tracklist.tracklist_position, None) @populate_tracklist def test_tracklist_position_during_play(self): self.playback.play() - self.assertEqual(self.playback.tracklist_position, 0) + self.assertEqual(self.tracklist.tracklist_position, 0) @populate_tracklist def test_tracklist_position_after_next(self): self.playback.play() self.playback.next() - self.assertEqual(self.playback.tracklist_position, 1) + self.assertEqual(self.tracklist.tracklist_position, 1) @populate_tracklist def test_tracklist_position_at_end_of_playlist(self): self.playback.play(self.tracklist.tl_tracks[-1]) self.playback.on_end_of_track() - self.assertEqual(self.playback.tracklist_position, None) + self.assertEqual(self.tracklist.tracklist_position, None) def test_on_tracklist_change_gets_called(self): callback = self.playback.on_tracklist_change @@ -775,13 +775,13 @@ class PlaybackControllerTest(object): @populate_tracklist def test_play_with_consume(self): - self.playback.consume = True + self.tracklist.consume = True self.playback.play() self.assertEqual(self.playback.current_track, self.tracks[0]) @populate_tracklist def test_playlist_is_empty_after_all_tracks_are_played_with_consume(self): - self.playback.consume = True + self.tracklist.consume = True self.playback.play() for _ in range(len(self.tracklist.tracks)): self.playback.on_end_of_track() @@ -790,14 +790,14 @@ class PlaybackControllerTest(object): @populate_tracklist def test_play_with_random(self): random.seed(1) - self.playback.random = True + self.tracklist.random = True self.playback.play() self.assertEqual(self.playback.current_track, self.tracks[2]) @populate_tracklist def test_previous_with_random(self): random.seed(1) - self.playback.random = True + self.tracklist.random = True self.playback.play() self.playback.next() current_track = self.playback.current_track @@ -812,8 +812,8 @@ class PlaybackControllerTest(object): @populate_tracklist def test_end_of_song_with_single_and_repeat_starts_same(self): - self.playback.single = True - self.playback.repeat = True + self.tracklist.single = True + self.tracklist.repeat = True self.playback.play() self.playback.on_end_of_track() self.assertEqual(self.playback.current_track, self.tracks[0]) @@ -825,44 +825,44 @@ class PlaybackControllerTest(object): self.assertEqual(self.playback.state, PlaybackState.STOPPED) def test_repeat_off_by_default(self): - self.assertEqual(self.playback.repeat, False) + self.assertEqual(self.tracklist.repeat, False) def test_random_off_by_default(self): - self.assertEqual(self.playback.random, False) + self.assertEqual(self.tracklist.random, False) def test_consume_off_by_default(self): - self.assertEqual(self.playback.consume, False) + self.assertEqual(self.tracklist.consume, False) @populate_tracklist def test_random_until_end_of_playlist(self): - self.playback.random = True + self.tracklist.random = True self.playback.play() for _ in self.tracks[1:]: self.playback.next() - self.assertEqual(self.playback.tl_track_at_next, None) + self.assertEqual(self.tracklist.tl_track_at_next, None) @populate_tracklist def test_random_until_end_of_playlist_and_play_from_start(self): - self.playback.repeat = True + self.tracklist.repeat = True for _ in self.tracks: self.playback.next() - self.assertNotEqual(self.playback.tl_track_at_next, None) + self.assertNotEqual(self.tracklist.tl_track_at_next, 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.playback.repeat = True - self.playback.random = True + self.tracklist.repeat = True + self.tracklist.random = True self.playback.play() for _ in self.tracks: self.playback.next() - self.assertNotEqual(self.playback.tl_track_at_next, None) + self.assertNotEqual(self.tracklist.tl_track_at_next, None) @populate_tracklist def test_played_track_during_random_not_played_again(self): - self.playback.random = True + self.tracklist.random = True self.playback.play() played = [] for _ in self.tracks: diff --git a/tests/frontends/mpd/protocol/playback_test.py b/tests/frontends/mpd/protocol/playback_test.py index 2cfc1b98..fc91c09c 100644 --- a/tests/frontends/mpd/protocol/playback_test.py +++ b/tests/frontends/mpd/protocol/playback_test.py @@ -16,22 +16,22 @@ STOPPED = PlaybackState.STOPPED class PlaybackOptionsHandlerTest(protocol.BaseTestCase): def test_consume_off(self): self.sendRequest('consume "0"') - self.assertFalse(self.core.playback.consume.get()) + self.assertFalse(self.core.tracklist.consume.get()) self.assertInResponse('OK') def test_consume_off_without_quotes(self): self.sendRequest('consume 0') - self.assertFalse(self.core.playback.consume.get()) + self.assertFalse(self.core.tracklist.consume.get()) self.assertInResponse('OK') def test_consume_on(self): self.sendRequest('consume "1"') - self.assertTrue(self.core.playback.consume.get()) + self.assertTrue(self.core.tracklist.consume.get()) self.assertInResponse('OK') def test_consume_on_without_quotes(self): self.sendRequest('consume 1') - self.assertTrue(self.core.playback.consume.get()) + self.assertTrue(self.core.tracklist.consume.get()) self.assertInResponse('OK') def test_crossfade(self): @@ -40,42 +40,42 @@ class PlaybackOptionsHandlerTest(protocol.BaseTestCase): def test_random_off(self): self.sendRequest('random "0"') - self.assertFalse(self.core.playback.random.get()) + self.assertFalse(self.core.tracklist.random.get()) self.assertInResponse('OK') def test_random_off_without_quotes(self): self.sendRequest('random 0') - self.assertFalse(self.core.playback.random.get()) + self.assertFalse(self.core.tracklist.random.get()) self.assertInResponse('OK') def test_random_on(self): self.sendRequest('random "1"') - self.assertTrue(self.core.playback.random.get()) + self.assertTrue(self.core.tracklist.random.get()) self.assertInResponse('OK') def test_random_on_without_quotes(self): self.sendRequest('random 1') - self.assertTrue(self.core.playback.random.get()) + self.assertTrue(self.core.tracklist.random.get()) self.assertInResponse('OK') def test_repeat_off(self): self.sendRequest('repeat "0"') - self.assertFalse(self.core.playback.repeat.get()) + self.assertFalse(self.core.tracklist.repeat.get()) self.assertInResponse('OK') def test_repeat_off_without_quotes(self): self.sendRequest('repeat 0') - self.assertFalse(self.core.playback.repeat.get()) + self.assertFalse(self.core.tracklist.repeat.get()) self.assertInResponse('OK') def test_repeat_on(self): self.sendRequest('repeat "1"') - self.assertTrue(self.core.playback.repeat.get()) + self.assertTrue(self.core.tracklist.repeat.get()) self.assertInResponse('OK') def test_repeat_on_without_quotes(self): self.sendRequest('repeat 1') - self.assertTrue(self.core.playback.repeat.get()) + self.assertTrue(self.core.tracklist.repeat.get()) self.assertInResponse('OK') def test_setvol_below_min(self): @@ -115,22 +115,22 @@ class PlaybackOptionsHandlerTest(protocol.BaseTestCase): def test_single_off(self): self.sendRequest('single "0"') - self.assertFalse(self.core.playback.single.get()) + self.assertFalse(self.core.tracklist.single.get()) self.assertInResponse('OK') def test_single_off_without_quotes(self): self.sendRequest('single 0') - self.assertFalse(self.core.playback.single.get()) + self.assertFalse(self.core.tracklist.single.get()) self.assertInResponse('OK') def test_single_on(self): self.sendRequest('single "1"') - self.assertTrue(self.core.playback.single.get()) + self.assertTrue(self.core.tracklist.single.get()) self.assertInResponse('OK') def test_single_on_without_quotes(self): self.sendRequest('single 1') - self.assertTrue(self.core.playback.single.get()) + self.assertTrue(self.core.tracklist.single.get()) self.assertInResponse('OK') def test_replay_gain_mode_off(self): diff --git a/tests/frontends/mpd/status_test.py b/tests/frontends/mpd/status_test.py index ded0c3b2..d86f7dcd 100644 --- a/tests/frontends/mpd/status_test.py +++ b/tests/frontends/mpd/status_test.py @@ -64,7 +64,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(int(result['repeat']), 0) def test_status_method_contains_repeat_is_1(self): - self.core.playback.repeat = 1 + self.core.tracklist.repeat = 1 result = dict(status.status(self.context)) self.assertIn('repeat', result) self.assertEqual(int(result['repeat']), 1) @@ -75,7 +75,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(int(result['random']), 0) def test_status_method_contains_random_is_1(self): - self.core.playback.random = 1 + self.core.tracklist.random = 1 result = dict(status.status(self.context)) self.assertIn('random', result) self.assertEqual(int(result['random']), 1) @@ -91,7 +91,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(int(result['consume']), 0) def test_status_method_contains_consume_is_1(self): - self.core.playback.consume = 1 + self.core.tracklist.consume = 1 result = dict(status.status(self.context)) self.assertIn('consume', result) self.assertEqual(int(result['consume']), 1) diff --git a/tests/frontends/mpris/player_interface_test.py b/tests/frontends/mpris/player_interface_test.py index 52cd964b..4cd903e6 100644 --- a/tests/frontends/mpris/player_interface_test.py +++ b/tests/frontends/mpris/player_interface_test.py @@ -50,45 +50,45 @@ class PlayerInterfaceTest(unittest.TestCase): self.assertEqual('Stopped', result) def test_get_loop_status_is_none_when_not_looping(self): - self.core.playback.repeat = False - self.core.playback.single = False + self.core.tracklist.repeat = False + self.core.tracklist.single = False result = self.mpris.Get(objects.PLAYER_IFACE, 'LoopStatus') self.assertEqual('None', result) def test_get_loop_status_is_track_when_looping_a_single_track(self): - self.core.playback.repeat = True - self.core.playback.single = True + self.core.tracklist.repeat = True + self.core.tracklist.single = True result = self.mpris.Get(objects.PLAYER_IFACE, 'LoopStatus') self.assertEqual('Track', result) def test_get_loop_status_is_playlist_when_looping_tracklist(self): - self.core.playback.repeat = True - self.core.playback.single = False + self.core.tracklist.repeat = True + self.core.tracklist.single = False result = self.mpris.Get(objects.PLAYER_IFACE, 'LoopStatus') self.assertEqual('Playlist', result) def test_set_loop_status_is_ignored_if_can_control_is_false(self): self.mpris.get_CanControl = lambda *_: False - self.core.playback.repeat = True - self.core.playback.single = True + self.core.tracklist.repeat = True + self.core.tracklist.single = True self.mpris.Set(objects.PLAYER_IFACE, 'LoopStatus', 'None') - self.assertEqual(self.core.playback.repeat.get(), True) - self.assertEqual(self.core.playback.single.get(), True) + self.assertEqual(self.core.tracklist.repeat.get(), True) + self.assertEqual(self.core.tracklist.single.get(), True) def test_set_loop_status_to_none_unsets_repeat_and_single(self): self.mpris.Set(objects.PLAYER_IFACE, 'LoopStatus', 'None') - self.assertEqual(self.core.playback.repeat.get(), False) - self.assertEqual(self.core.playback.single.get(), False) + self.assertEqual(self.core.tracklist.repeat.get(), False) + self.assertEqual(self.core.tracklist.single.get(), False) def test_set_loop_status_to_track_sets_repeat_and_single(self): self.mpris.Set(objects.PLAYER_IFACE, 'LoopStatus', 'Track') - self.assertEqual(self.core.playback.repeat.get(), True) - self.assertEqual(self.core.playback.single.get(), True) + self.assertEqual(self.core.tracklist.repeat.get(), True) + self.assertEqual(self.core.tracklist.single.get(), True) def test_set_loop_status_to_playlists_sets_repeat_and_not_single(self): self.mpris.Set(objects.PLAYER_IFACE, 'LoopStatus', 'Playlist') - self.assertEqual(self.core.playback.repeat.get(), True) - self.assertEqual(self.core.playback.single.get(), False) + self.assertEqual(self.core.tracklist.repeat.get(), True) + self.assertEqual(self.core.tracklist.single.get(), False) def test_get_rate_is_greater_or_equal_than_minimum_rate(self): rate = self.mpris.Get(objects.PLAYER_IFACE, 'Rate') @@ -116,32 +116,32 @@ class PlayerInterfaceTest(unittest.TestCase): self.assertEqual(self.core.playback.state.get(), PAUSED) def test_get_shuffle_returns_true_if_random_is_active(self): - self.core.playback.random = True + self.core.tracklist.random = True result = self.mpris.Get(objects.PLAYER_IFACE, 'Shuffle') self.assertTrue(result) def test_get_shuffle_returns_false_if_random_is_inactive(self): - self.core.playback.random = False + self.core.tracklist.random = False result = self.mpris.Get(objects.PLAYER_IFACE, 'Shuffle') self.assertFalse(result) def test_set_shuffle_is_ignored_if_can_control_is_false(self): self.mpris.get_CanControl = lambda *_: False - self.core.playback.random = False + self.core.tracklist.random = False self.mpris.Set(objects.PLAYER_IFACE, 'Shuffle', True) - self.assertFalse(self.core.playback.random.get()) + self.assertFalse(self.core.tracklist.random.get()) def test_set_shuffle_to_true_activates_random_mode(self): - self.core.playback.random = False - self.assertFalse(self.core.playback.random.get()) + self.core.tracklist.random = False + self.assertFalse(self.core.tracklist.random.get()) self.mpris.Set(objects.PLAYER_IFACE, 'Shuffle', True) - self.assertTrue(self.core.playback.random.get()) + self.assertTrue(self.core.tracklist.random.get()) def test_set_shuffle_to_false_deactivates_random_mode(self): - self.core.playback.random = True - self.assertTrue(self.core.playback.random.get()) + self.core.tracklist.random = True + self.assertTrue(self.core.tracklist.random.get()) self.mpris.Set(objects.PLAYER_IFACE, 'Shuffle', False) - self.assertFalse(self.core.playback.random.get()) + self.assertFalse(self.core.tracklist.random.get()) def test_get_metadata_has_trackid_even_when_no_current_track(self): result = self.mpris.Get(objects.PLAYER_IFACE, 'Metadata') @@ -308,7 +308,7 @@ class PlayerInterfaceTest(unittest.TestCase): def test_can_go_next_is_false_if_next_track_is_the_same(self): self.mpris.get_CanControl = lambda *_: True self.core.tracklist.add([Track(uri='dummy:a')]) - self.core.playback.repeat = True + self.core.tracklist.repeat = True self.core.playback.play() result = self.mpris.Get(objects.PLAYER_IFACE, 'CanGoNext') self.assertFalse(result) @@ -331,7 +331,7 @@ class PlayerInterfaceTest(unittest.TestCase): def test_can_go_previous_is_false_if_previous_track_is_the_same(self): self.mpris.get_CanControl = lambda *_: True self.core.tracklist.add([Track(uri='dummy:a')]) - self.core.playback.repeat = True + self.core.tracklist.repeat = True self.core.playback.play() result = self.mpris.Get(objects.PLAYER_IFACE, 'CanGoPrevious') self.assertFalse(result) diff --git a/tests/utils/jsonrpc_test.py b/tests/utils/jsonrpc_test.py index 5dccbe05..a0709ebc 100644 --- a/tests/utils/jsonrpc_test.py +++ b/tests/utils/jsonrpc_test.py @@ -266,12 +266,12 @@ class JsonRpcSingleNotificationTest(JsonRpcTestBase): class JsonRpcBatchTest(JsonRpcTestBase): def test_batch_of_only_commands_returns_all(self): - self.core.playback.set_random(True).get() + self.core.tracklist.set_random(True).get() request = [ - {'jsonrpc': '2.0', 'method': 'core.playback.get_repeat', 'id': 1}, - {'jsonrpc': '2.0', 'method': 'core.playback.get_random', 'id': 2}, - {'jsonrpc': '2.0', 'method': 'core.playback.get_single', 'id': 3}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_repeat', 'id': 1}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_random', 'id': 2}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_single', 'id': 3}, ] response = self.jrw.handle_data(request) @@ -283,12 +283,12 @@ class JsonRpcBatchTest(JsonRpcTestBase): self.assertEqual(response[3]['result'], False) def test_batch_of_commands_and_notifications_returns_some(self): - self.core.playback.set_random(True).get() + self.core.tracklist.set_random(True).get() request = [ - {'jsonrpc': '2.0', 'method': 'core.playback.get_repeat'}, - {'jsonrpc': '2.0', 'method': 'core.playback.get_random', 'id': 2}, - {'jsonrpc': '2.0', 'method': 'core.playback.get_single', 'id': 3}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_repeat'}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_random', 'id': 2}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_single', 'id': 3}, ] response = self.jrw.handle_data(request) @@ -300,12 +300,12 @@ class JsonRpcBatchTest(JsonRpcTestBase): self.assertEqual(response[3]['result'], False) def test_batch_of_only_notifications_returns_nothing(self): - self.core.playback.set_random(True).get() + self.core.tracklist.set_random(True).get() request = [ - {'jsonrpc': '2.0', 'method': 'core.playback.get_repeat'}, - {'jsonrpc': '2.0', 'method': 'core.playback.get_random'}, - {'jsonrpc': '2.0', 'method': 'core.playback.get_single'}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_repeat'}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_random'}, + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_single'}, ] response = self.jrw.handle_data(request) @@ -522,10 +522,10 @@ class JsonRpcBatchErrorTest(JsonRpcTestBase): {'jsonrpc': '2.0', 'method': 'core.playback.set_volume', 'params': [47], 'id': '1'}, # Notification - {'jsonrpc': '2.0', 'method': 'core.playback.set_consume', + {'jsonrpc': '2.0', 'method': 'core.tracklist.set_consume', 'params': [True]}, # Call with positional params - {'jsonrpc': '2.0', 'method': 'core.playback.set_repeat', + {'jsonrpc': '2.0', 'method': 'core.tracklist.set_repeat', 'params': [False], 'id': '2'}, # Invalid request {'foo': 'boo'}, @@ -533,7 +533,7 @@ class JsonRpcBatchErrorTest(JsonRpcTestBase): {'jsonrpc': '2.0', 'method': 'foo.get', 'params': {'name': 'myself'}, 'id': '5'}, # Call without params - {'jsonrpc': '2.0', 'method': 'core.playback.get_random', + {'jsonrpc': '2.0', 'method': 'core.tracklist.get_random', 'id': '9'}, ] response = self.jrw.handle_data(request) From 68ea483c7ba2dcf8e8254a8a2f9fb40f0536136a Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 17:30:46 +0200 Subject: [PATCH 02/42] Converted tracklist_position() to be called with the track one wants to have information of, /mopidy/frontends/mpd/protocol/status.py@189 should be checked for gathering a value before the rest. --- mopidy/core/tracklist.py | 43 +++++++++++++---------- mopidy/frontends/mpd/protocol/playback.py | 3 +- mopidy/frontends/mpd/protocol/status.py | 11 +++--- tests/backends/base/playback.py | 34 +++++++++++------- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 33bebdee..ded8a3c4 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -133,25 +133,25 @@ class TracklistController(object): Playback continues after current song. """ - def get_tracklist_position(self): - if self.core.playback.current_tl_track is None: + def tracklist_position(self, tl_track): + """ + The position of the current track in the tracklist. + + Read-only. + """ + if tl_track is None: return None try: - return self.core.tracklist.tl_tracks.index(self.core.playback.current_tl_track) + return self.core.tracklist.tl_tracks.index(tl_track) except ValueError: return None - tracklist_position = property(get_tracklist_position) - """ - The position of the current track in the tracklist. - - Read-only. - """ def get_tl_track_at_eot(self): # pylint: disable = R0911 # Too many return statements + current_tl_track = self.core.playback.current_tl_track tl_tracks = self.core.tracklist.tl_tracks if not tl_tracks: @@ -167,17 +167,18 @@ class TracklistController(object): if self.random and self._shuffled: return self._shuffled[0] - if self.core.playback.current_tl_track is None: + if current_tl_track is None: return tl_tracks[0] + position = self.tracklist_position(current_tl_track) if self.repeat and self.single: - return tl_tracks[self.tracklist_position] + return tl_tracks[position] if self.repeat and not self.single: - return tl_tracks[(self.tracklist_position + 1) % len(tl_tracks)] + return tl_tracks[(position + 1) % len(tl_tracks)] try: - return tl_tracks[self.tracklist_position + 1] + return tl_tracks[position + 1] except IndexError: return None @@ -192,6 +193,7 @@ class TracklistController(object): def get_tl_track_at_next(self): tl_tracks = self.core.tracklist.tl_tracks + current_tl_track = self.core.playback.current_tl_track if not tl_tracks: return None @@ -206,14 +208,15 @@ class TracklistController(object): if self.random and self._shuffled: return self._shuffled[0] - if self.core.playback.current_tl_track is None: + if current_tl_track is None: return tl_tracks[0] + position = self.tracklist_position(current_tl_track) if self.repeat: - return tl_tracks[(self.tracklist_position + 1) % len(tl_tracks)] + return tl_tracks[(position + 1) % len(tl_tracks)] try: - return tl_tracks[self.tracklist_position + 1] + return tl_tracks[position + 1] except IndexError: return None @@ -230,13 +233,15 @@ class TracklistController(object): """ def get_tl_track_at_previous(self): + current_tl_track = self.core.playback.current_tl_track if self.repeat or self.core.tracklist.consume or self.random: - return self.core.playback.current_tl_track + return current_tl_track - if self.tracklist_position in (None, 0): + position = self.tracklist_position(current_tl_track) + if position in (None, 0): return None - return self.core.tracklist.tl_tracks[self.tracklist_position - 1] + return self.core.tracklist.tl_tracks[position - 1] tl_track_at_previous = property(get_tl_track_at_previous) """ diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index fcc465aa..06a456d9 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -329,7 +329,8 @@ def seek(context, songpos, seconds): - issues ``seek 1 120`` without quotes around the arguments. """ - if context.core.tracklist.tracklist_position.get() != int(songpos): + tl_track = context.core.playback.current_tl_track.get() + if context.core.tracklist.tracklist_position(tl_track).get() != int(songpos): playpos(context, songpos) context.core.playback.seek(int(seconds) * 1000).get() diff --git a/mopidy/frontends/mpd/protocol/status.py b/mopidy/frontends/mpd/protocol/status.py index f42b3531..8f2207f6 100644 --- a/mopidy/frontends/mpd/protocol/status.py +++ b/mopidy/frontends/mpd/protocol/status.py @@ -36,10 +36,10 @@ def currentsong(context): Displays the song info of the current song (same song that is identified in status). """ - current_tl_track = context.core.playback.current_tl_track.get() - if current_tl_track is not None: - position = context.core.tracklist.tracklist_position.get() - return track_to_mpd_format(current_tl_track, position=position) + tl_track = context.core.playback.current_tl_track.get() + if tl_track is not None: + position = context.core.tracklist.tracklist_position(tl_track).get() + return track_to_mpd_format(tl_track, position=position) @handle_request(r'^idle$') @@ -185,7 +185,8 @@ def status(context): 'playback.state': context.core.playback.state, 'playback.current_tl_track': context.core.playback.current_tl_track, 'tracklist.tracklist_position': ( - context.core.tracklist.tracklist_position), + context.core.tracklist.tracklist_position( + context.core.playback.current_tl_track.get())), 'playback.time_position': context.core.playback.time_position, } pykka.get_all(futures.values()) diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 11e21628..32c0abf7 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -179,13 +179,15 @@ class PlaybackControllerTest(object): def test_next(self): self.playback.play() - old_position = self.tracklist.tracklist_position - old_uri = self.playback.current_track.uri + tl_track = self.playback.current_tl_track + old_position = self.tracklist.tracklist_position(tl_track) + old_uri = tl_track.track.uri self.playback.next() + tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tracklist_position, old_position + 1) + self.tracklist.tracklist_position(tl_track), old_position + 1) self.assertNotEqual(self.playback.current_track.uri, old_uri) @populate_tracklist @@ -205,7 +207,8 @@ class PlaybackControllerTest(object): for i, track in enumerate(self.tracks): self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) - self.assertEqual(self.tracklist.tracklist_position, i) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tracklist_position(tl_track), i) self.playback.next() @@ -315,13 +318,15 @@ class PlaybackControllerTest(object): def test_end_of_track(self): self.playback.play() - old_position = self.tracklist.tracklist_position - old_uri = self.playback.current_track.uri + tl_track = self.playback.current_tl_track + old_position = self.tracklist.tracklist_position(tl_track) + old_uri = tl_track.track.uri self.playback.on_end_of_track() + tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tracklist_position, old_position + 1) + self.tracklist.tracklist_position(tl_track), old_position + 1) self.assertNotEqual(self.playback.current_track.uri, old_uri) @populate_tracklist @@ -341,7 +346,8 @@ class PlaybackControllerTest(object): for i, track in enumerate(self.tracks): self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) - self.assertEqual(self.tracklist.tracklist_position, i) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tracklist_position(tl_track), i) self.playback.on_end_of_track() @@ -500,24 +506,28 @@ class PlaybackControllerTest(object): @populate_tracklist def test_initial_tracklist_position(self): - self.assertEqual(self.tracklist.tracklist_position, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tracklist_position(tl_track), None) @populate_tracklist def test_tracklist_position_during_play(self): self.playback.play() - self.assertEqual(self.tracklist.tracklist_position, 0) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tracklist_position(tl_track), 0) @populate_tracklist def test_tracklist_position_after_next(self): self.playback.play() self.playback.next() - self.assertEqual(self.tracklist.tracklist_position, 1) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tracklist_position(tl_track), 1) @populate_tracklist def test_tracklist_position_at_end_of_playlist(self): self.playback.play(self.tracklist.tl_tracks[-1]) self.playback.on_end_of_track() - self.assertEqual(self.tracklist.tracklist_position, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tracklist_position(tl_track), None) def test_on_tracklist_change_gets_called(self): callback = self.playback.on_tracklist_change From 5a87d219ff3bf0353bc639640a3716617a616d25 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 18:00:33 +0200 Subject: [PATCH 03/42] Correcting some self-arounds that were innecesary. --- mopidy/core/tracklist.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index ded8a3c4..d4f6f28d 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -142,7 +142,7 @@ class TracklistController(object): if tl_track is None: return None try: - return self.core.tracklist.tl_tracks.index(tl_track) + return self.tl_tracks.index(tl_track) except ValueError: return None @@ -152,15 +152,14 @@ class TracklistController(object): # Too many return statements current_tl_track = self.core.playback.current_tl_track - tl_tracks = self.core.tracklist.tl_tracks - if not tl_tracks: + if not self.tl_tracks: return None if self.random and not self._shuffled: if self.repeat or self._first_shuffle: logger.debug('Shuffling tracks') - self._shuffled = tl_tracks + self._shuffled = self.tl_tracks random.shuffle(self._shuffled) self._first_shuffle = False @@ -168,17 +167,17 @@ class TracklistController(object): return self._shuffled[0] if current_tl_track is None: - return tl_tracks[0] + return self.tl_tracks[0] position = self.tracklist_position(current_tl_track) if self.repeat and self.single: - return tl_tracks[position] + return self.tl_tracks[position] if self.repeat and not self.single: - return tl_tracks[(position + 1) % len(tl_tracks)] + return self.tl_tracks[(position + 1) % len(self.tl_tracks)] try: - return tl_tracks[position + 1] + return self.tl_tracks[position + 1] except IndexError: return None @@ -192,7 +191,7 @@ class TracklistController(object): """ def get_tl_track_at_next(self): - tl_tracks = self.core.tracklist.tl_tracks + tl_tracks = self.tl_tracks current_tl_track = self.core.playback.current_tl_track if not tl_tracks: @@ -234,14 +233,14 @@ class TracklistController(object): def get_tl_track_at_previous(self): current_tl_track = self.core.playback.current_tl_track - if self.repeat or self.core.tracklist.consume or self.random: + if self.repeat or self.consume or self.random: return current_tl_track position = self.tracklist_position(current_tl_track) if position in (None, 0): return None - return self.core.tracklist.tl_tracks[position - 1] + return self.tl_tracks[position - 1] tl_track_at_previous = property(get_tl_track_at_previous) """ From ec716fba82747d33ea40c5448241a1a0431c3dae Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 19:44:00 +0200 Subject: [PATCH 04/42] Converting tl_track_at_eot property in function with the track having to be given as an argument --- mopidy/core/playback.py | 5 +++-- mopidy/core/tracklist.py | 26 +++++++++----------------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 8b33140a..ed02e8ac 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -140,10 +140,11 @@ class PlaybackController(object): return original_tl_track = self.current_tl_track + next_track = self.core.tracklist.tl_track_at_eot(original_tl_track) - if self.core.tracklist.tl_track_at_eot: + if next_track: self._trigger_track_playback_ended() - self.play(self.core.tracklist.tl_track_at_eot) + self.play(next_track) else: self.stop(clear_current_track=True) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index d4f6f28d..42180799 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -135,9 +135,7 @@ class TracklistController(object): def tracklist_position(self, tl_track): """ - The position of the current track in the tracklist. - - Read-only. + The position of the given track in the tracklist. """ if tl_track is None: return None @@ -147,12 +145,15 @@ class TracklistController(object): return None - def get_tl_track_at_eot(self): + def tl_track_at_eot(self, tl_track): + """ + The track that will be played after the given track. + + Not necessarily the same track as :meth:`tl_track_at_next`. + """ # pylint: disable = R0911 # Too many return statements - current_tl_track = self.core.playback.current_tl_track - if not self.tl_tracks: return None @@ -166,10 +167,10 @@ class TracklistController(object): if self.random and self._shuffled: return self._shuffled[0] - if current_tl_track is None: + if tl_track is None: return self.tl_tracks[0] - position = self.tracklist_position(current_tl_track) + position = self.tracklist_position(tl_track) if self.repeat and self.single: return self.tl_tracks[position] @@ -181,15 +182,6 @@ class TracklistController(object): except IndexError: return None - tl_track_at_eot = property(get_tl_track_at_eot) - """ - The track that will be played at the end of the current track. - - Read-only. A :class:`mopidy.models.TlTrack`. - - Not necessarily the same track as :attr:`tl_track_at_next`. - """ - def get_tl_track_at_next(self): tl_tracks = self.tl_tracks current_tl_track = self.core.playback.current_tl_track From 6abcad3e5509e05ea76f23ec4630746b5ebdb902 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 21:38:16 +0200 Subject: [PATCH 05/42] Converting tl_track_at_next into a function that takes the track argument. Rewrote tests too. --- mopidy/core/playback.py | 7 ++-- mopidy/core/tracklist.py | 38 ++++++++----------- mopidy/frontends/mpris/objects.py | 6 ++- tests/backends/base/playback.py | 63 ++++++++++++++++++++----------- 4 files changed, 66 insertions(+), 48 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index ed02e8ac..742864b5 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -170,9 +170,10 @@ class PlaybackController(object): The current playback state will be kept. If it was playing, playing will continue. If it was paused, it will still be paused, etc. """ - if self.core.tracklist.tl_track_at_next: + tl_track = self.core.tracklist.tl_track_at_next(self.current_tl_track) + if tl_track: self._trigger_track_playback_ended() - self.change_track(self.core.tracklist.tl_track_at_next) + self.change_track(tl_track) else: self.stop(clear_current_track=True) @@ -203,7 +204,7 @@ class PlaybackController(object): elif self.current_tl_track is not None: tl_track = self.current_tl_track elif self.current_tl_track is None and on_error_step == 1: - tl_track = self.core.tracklist.tl_track_at_next + 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 diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 42180799..aa2f18ef 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -182,47 +182,41 @@ class TracklistController(object): except IndexError: return None - def get_tl_track_at_next(self): - tl_tracks = self.tl_tracks - current_tl_track = self.core.playback.current_tl_track + def tl_track_at_next(self, tl_track): + """ + The track that will be played if calling :meth:`next()`. - if not tl_tracks: + For normal playback this is the next track in the playlist. If repeat + is enabled the next track can loop around the playlist. When random is + enabled this should be a random track, all tracks should be played once + before the list repeats. + """ + + if not self.tl_tracks: return None if self.random and not self._shuffled: if self.repeat or self._first_shuffle: logger.debug('Shuffling tracks') - self._shuffled = tl_tracks + self._shuffled = self.tl_tracks random.shuffle(self._shuffled) self._first_shuffle = False if self.random and self._shuffled: return self._shuffled[0] - if current_tl_track is None: - return tl_tracks[0] + if tl_track is None: + return self.tl_tracks[0] - position = self.tracklist_position(current_tl_track) + position = self.tracklist_position(tl_track) if self.repeat: - return tl_tracks[(position + 1) % len(tl_tracks)] + return self.tl_tracks[(position + 1) % len(self.tl_tracks)] try: - return tl_tracks[position + 1] + return self.tl_tracks[position + 1] except IndexError: return None - tl_track_at_next = property(get_tl_track_at_next) - """ - The track that will be played if calling :meth:`next()`. - - Read-only. A :class:`mopidy.models.TlTrack`. - - For normal playback this is the next track in the playlist. If repeat - is enabled the next track can loop around the playlist. When random is - enabled this should be a random track, all tracks should be played once - before the list repeats. - """ - 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: diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index d4f28a83..5253f609 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -406,8 +406,9 @@ class MprisObject(dbus.service.Object): def get_CanGoNext(self): if not self.get_CanControl(): return False + tl_track = self.core.playback.current_tl_track.get() return ( - self.core.tracklist.tl_track_at_next.get() != + self.core.tracklist.tl_track_at_next(tl_track).get() != self.core.playback.current_tl_track.get()) def get_CanGoPrevious(self): @@ -420,9 +421,10 @@ class MprisObject(dbus.service.Object): def get_CanPlay(self): if not self.get_CanControl(): return False + tl_track = self.core.playback.current_tl_track.get() return ( self.core.playback.current_tl_track.get() is not None or - self.core.tracklist.tl_track_at_next.get() is not None) + self.core.tracklist.tl_track_at_next(tl_track).get() is not None) def get_CanPause(self): if not self.get_CanControl(): diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 32c0abf7..8f57e219 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -244,29 +244,34 @@ class PlaybackControllerTest(object): @populate_tracklist def test_next_track_before_play(self): - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) @populate_tracklist def test_next_track_during_play(self): self.playback.play() - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) @populate_tracklist def test_next_track_after_previous(self): self.playback.play() self.playback.next() self.playback.previous() - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) def test_next_track_empty_playlist(self): - self.assertEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) @populate_tracklist def test_next_track_at_end_of_playlist(self): self.playback.play() for _ in self.tracklist.tl_tracks[1:]: self.playback.next() - self.assertEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) @populate_tracklist def test_next_track_at_end_of_playlist_with_repeat(self): @@ -274,13 +279,15 @@ class PlaybackControllerTest(object): self.playback.play() for _ in self.tracks[1:]: self.playback.next() - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) @populate_tracklist def test_next_track_with_random(self): random.seed(1) self.tracklist.random = True - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) @populate_tracklist def test_next_with_consume(self): @@ -310,9 +317,11 @@ class PlaybackControllerTest(object): def test_next_track_with_random_after_append_playlist(self): random.seed(1) self.tracklist.random = True - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) @populate_tracklist def test_end_of_track(self): @@ -383,29 +392,34 @@ class PlaybackControllerTest(object): @populate_tracklist def test_end_of_track_track_before_play(self): - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_during_play(self): self.playback.play() - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) @populate_tracklist def test_end_of_track_track_after_previous(self): self.playback.play() self.playback.on_end_of_track() self.playback.previous() - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) def test_end_of_track_track_empty_playlist(self): - self.assertEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) @populate_tracklist def test_end_of_track_track_at_end_of_playlist(self): self.playback.play() for _ in self.tracklist.tl_tracks[1:]: self.playback.on_end_of_track() - self.assertEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) @populate_tracklist def test_end_of_track_track_at_end_of_playlist_with_repeat(self): @@ -413,13 +427,15 @@ class PlaybackControllerTest(object): self.playback.play() for _ in self.tracks[1:]: self.playback.on_end_of_track() - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[0]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_with_random(self): random.seed(1) self.tracklist.random = True - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) @populate_tracklist def test_end_of_track_with_consume(self): @@ -441,9 +457,11 @@ class PlaybackControllerTest(object): def test_end_of_track_track_with_random_after_append_playlist(self): random.seed(1) self.tracklist.random = True - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[2]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) - self.assertEqual(self.tracklist.tl_track_at_next, self.tl_tracks[1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) @populate_tracklist def test_previous_track_before_play(self): @@ -849,14 +867,16 @@ class PlaybackControllerTest(object): self.playback.play() for _ in self.tracks[1:]: self.playback.next() - self.assertEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) @populate_tracklist def test_random_until_end_of_playlist_and_play_from_start(self): self.tracklist.repeat = True for _ in self.tracks: self.playback.next() - self.assertNotEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertNotEqual(self.tracklist.tl_track_at_next(tl_track), None) self.assertEqual(self.playback.state, PlaybackState.STOPPED) self.playback.play() self.assertEqual(self.playback.state, PlaybackState.PLAYING) @@ -868,7 +888,8 @@ class PlaybackControllerTest(object): self.playback.play() for _ in self.tracks: self.playback.next() - self.assertNotEqual(self.tracklist.tl_track_at_next, None) + tl_track = self.playback.current_tl_track + self.assertNotEqual(self.tracklist.tl_track_at_next(tl_track), None) @populate_tracklist def test_played_track_during_random_not_played_again(self): From 6e61f2ef85b01f110d3f5623bbfba6a1899258cb Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Wed, 7 Aug 2013 21:53:46 +0200 Subject: [PATCH 06/42] 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): From 2c83225a1ec4b175ff92366c5dd0b4e427275469 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 11:56:35 +0200 Subject: [PATCH 07/42] Created a TracklistController to let it control wether if a track must be consumed or not --- mopidy/core/playback.py | 3 +-- mopidy/core/tracklist.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 8ae7b4d9..6a5b0dc7 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -148,8 +148,7 @@ class PlaybackController(object): else: self.stop(clear_current_track=True) - if self.core.tracklist.consume: - self.core.tracklist.remove(tlid=original_tl_track.tlid) + self.core.tracklist.mark_consumed(tlid=original_tl_track.tlid) def on_tracklist_change(self): """ diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 7207f1a9..8971745e 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -434,6 +434,19 @@ class TracklistController(object): """ return self._tl_tracks[start:end] + def mark_consumed(self, **kwargs): + """ + Marks the given track as played. + + :param tl_track: Track to mark + :type tl_track: :class:`mopidy.models.TlTrack` + :rtype: True if the track was actually removed from the tracklist + """ + if not self.consume: + return False + self.remove(**kwargs) + return True + def _trigger_tracklist_changed(self): self._first_shuffle = True self._shuffled = [] From ab85dd9d6284aae3bfba2bffca5bea91d388df34 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 12:12:37 +0200 Subject: [PATCH 08/42] Changed mark_consumed to a flexible mark() function that currently allows to mark songs as consumed (that have been played full time), played (that have been played for some time) and unplayable --- mopidy/core/playback.py | 8 +++----- mopidy/core/tracklist.py | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 6a5b0dc7..51254521 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -148,7 +148,7 @@ class PlaybackController(object): else: self.stop(clear_current_track=True) - self.core.tracklist.mark_consumed(tlid=original_tl_track.tlid) + self.core.tracklist.mark("consumed", original_tl_track) def on_tracklist_change(self): """ @@ -213,16 +213,14 @@ class PlaybackController(object): backend = self._get_backend() if not backend or not backend.playback.play(tl_track.track).get(): logger.warning('Track is not playable: %s', tl_track.track.uri) - if self.core.tracklist.random and self.core.tracklist._shuffled: - self.core.tracklist._shuffled.remove(tl_track) + self.core.tracklist.mark("unplayable", tl_track) if on_error_step == 1: self.next() elif on_error_step == -1: self.previous() return - if self.core.tracklist.random and self.current_tl_track in self.core.tracklist._shuffled: - self.core.tracklist._shuffled.remove(self.current_tl_track) + self.core.tracklist.mark("played", tl_track) self._trigger_track_playback_started() diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 8971745e..60a76845 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -434,18 +434,25 @@ class TracklistController(object): """ return self._tl_tracks[start:end] - def mark_consumed(self, **kwargs): + def mark(self, what, tl_track): """ - Marks the given track as played. + Marks the given track as specified. :param tl_track: Track to mark :type tl_track: :class:`mopidy.models.TlTrack` :rtype: True if the track was actually removed from the tracklist """ - if not self.consume: - return False - self.remove(**kwargs) - return True + if what == "consumed": + if not self.consume: + return False + self.remove(tlid=tl_track.tlid) + return True + elif what == "played": + if self.random and tl_track in self._shuffled: + self._shuffled.remove(tl_track) + elif what == "unplayable": + if self.random and self._shuffled: + self._shuffled.remove(tl_track) def _trigger_tracklist_changed(self): self._first_shuffle = True From 826084d9b8dea0257c4374c6d0c33bd2d4c8006c Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 12:34:46 +0200 Subject: [PATCH 09/42] Ignoring editor temporary files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6ef1ff32..dca43a7d 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ docs/_build/ mopidy.log* node_modules/ nosetests.xml +*~ From 2cb64b365dcccfa34fc45bd1fc189f602f7beb8c Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 13:07:42 +0200 Subject: [PATCH 10/42] docs: Documenting changed and created functions --- mopidy/core/tracklist.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 60a76845..05f8fa88 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -150,6 +150,9 @@ class TracklistController(object): The track that will be played after the given track. Not necessarily the same track as :meth:`tl_track_at_next`. + + :param tl_track: The reference track + :type tl_track: :class:`mopidy.models.TlTrack` """ # pylint: disable = R0911 # Too many return statements @@ -184,12 +187,16 @@ class TracklistController(object): def tl_track_at_next(self, tl_track): """ - The track that will be played if calling :meth:`next()`. + The track that will be played if calling + :meth:`mopidy.core.PlaybackController.next()`. For normal playback this is the next track in the playlist. If repeat is enabled the next track can loop around the playlist. When random is enabled this should be a random track, all tracks should be played once before the list repeats. + + :param tl_track: The reference track + :type tl_track: :class:`mopidy.models.TlTrack` """ if not self.tl_tracks: @@ -219,13 +226,17 @@ class TracklistController(object): def tl_track_at_previous(self, tl_track): """ - Returns the track that will be played if calling :meth:`previous()`. + Returns the track that will be played if calling + :meth:`mopidy.core.PlaybackController.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. + + :param tl_track: The reference track + :type tl_track: :class:`mopidy.models.TlTrack` """ if self.repeat or self.consume or self.random: return tl_track @@ -436,8 +447,13 @@ class TracklistController(object): def mark(self, what, tl_track): """ - Marks the given track as specified. + Marks the given track as specified. Currently supports:: + * `consumed` The track has been completely played. + * `played` The track has been played, at least a piece of it. + * `unplayable` The track is unplayable + :param what: What to be marked as + :type what: string :param tl_track: Track to mark :type tl_track: :class:`mopidy.models.TlTrack` :rtype: True if the track was actually removed from the tracklist From b15c4f198b63870e1364309d1145195e08f658cc Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 13:39:18 +0200 Subject: [PATCH 11/42] docs: Updating changelog.rst to reflect updates in core --- docs/changelog.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 43e99350..cf398047 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,22 @@ v0.15.0 (UNRELEASED) - Mopidy no longer supports Python 2.6. Currently, the only Python version supported by Mopidy is Python 2.7. (Fixes: :issue:`344`) +**Core** +- Tracklist has now the power to make decisions on which is the next/previous + song, along with previously playback associated features, such as randomness, + consumption, repeat and single. For that, a new method has been created to + mark songs, some Playback properties have been converted into functions and + both functions and properties have been moved into Tracklist to have more + modularity: + - Properties converted into functions that need arguments: + :meth:`get_tl_track_at_eot`, :meth:`get_tl_track_at_next`, + :meth:`get_tl_track_at_previous` and :meth:`tracklist_position` + - Properties moved: :attr:`random`, :attr:`repeat`, :attr:`consume` and + :attr:`single` + - Method created: :meth:`mark` +- Tracklist's get_tl_track_at_* and tracklist_position now need a tl_track as a + reference to give an answer. + **Command line options** - Converted from the optparse to the argparse library for handling command line From cb4130c2a7c951c12968f7a24f00e9de820f0540 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 13:45:36 +0200 Subject: [PATCH 12/42] core: Moving the trigger activation from one playback to tracklist --- mopidy/core/playback.py | 4 ---- mopidy/core/tracklist.py | 12 ++++++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 7ec7ff2a..8bdf8fd0 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -330,10 +330,6 @@ class PlaybackController(object): 'playback_state_changed', old_state=old_state, new_state=new_state) - def _trigger_options_changed(self): - logger.debug('Triggering options changed event') - listener.CoreListener.send('options_changed') - def _trigger_volume_changed(self, volume): logger.debug('Triggering volume changed event') listener.CoreListener.send('volume_changed', volume=volume) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 05f8fa88..3364356b 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -72,7 +72,7 @@ class TracklistController(object): def set_consume(self, value): if self.get_consume() != value: - self.core.playback._trigger_options_changed() + self._trigger_options_changed() return setattr(self, '_consume', value) consume = property(get_consume, set_consume) @@ -88,7 +88,7 @@ class TracklistController(object): def set_random(self, value): if self.get_random() != value: - self.core.playback._trigger_options_changed() + self._trigger_options_changed() return setattr(self, '_random', value) random = property(get_random, set_random) @@ -104,7 +104,7 @@ class TracklistController(object): def set_repeat(self, value): if self.get_repeat() != value: - self.core.playback._trigger_options_changed() + self._trigger_options_changed() return setattr(self, '_repeat', value) repeat = property(get_repeat, set_repeat) @@ -121,7 +121,7 @@ class TracklistController(object): def set_single(self, value): if self.get_single() != value: - self.core.playback._trigger_options_changed() + self._trigger_options_changed() return setattr(self, '_single', value) single = property(get_single, set_single) @@ -475,3 +475,7 @@ class TracklistController(object): self._shuffled = [] logger.debug('Triggering event: tracklist_changed()') listener.CoreListener.send('tracklist_changed') + + def _trigger_options_changed(self): + logger.debug('Triggering options changed event') + listener.CoreListener.send('options_changed') From 782a6a7d1f6385991cbe65ef5f9ee00a46a4d46f Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 8 Aug 2013 17:01:24 +0200 Subject: [PATCH 13/42] Renamed tl_track_at_* to more readable names --- docs/changelog.rst | 8 +++-- mopidy/core/playback.py | 10 +++--- mopidy/core/tracklist.py | 8 ++--- mopidy/frontends/mpris/objects.py | 6 ++-- tests/backends/base/playback.py | 56 +++++++++++++++---------------- 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index cf398047..c997d1f4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,11 +22,13 @@ v0.15.0 (UNRELEASED) both functions and properties have been moved into Tracklist to have more modularity: - Properties converted into functions that need arguments: - :meth:`get_tl_track_at_eot`, :meth:`get_tl_track_at_next`, - :meth:`get_tl_track_at_previous` and :meth:`tracklist_position` + :meth:`tracklist_position` - Properties moved: :attr:`random`, :attr:`repeat`, :attr:`consume` and :attr:`single` - - Method created: :meth:`mark` + - Method created from properties: :meth:`next_track` from + `tl_track_at_next`, :meth:`eot_track` from ´tl_track_at_eot` and + :meth:`previous_track` from `tl_track_at_previous` + - Method created to separe functionality: :meth:`mark` - Tracklist's get_tl_track_at_* and tracklist_position now need a tl_track as a reference to give an answer. diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 8bdf8fd0..bb797f09 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -140,7 +140,7 @@ class PlaybackController(object): return original_tl_track = self.current_tl_track - next_track = self.core.tracklist.tl_track_at_eot(original_tl_track) + next_track = self.core.tracklist.eot_track(original_tl_track) if next_track: self._trigger_track_playback_ended() @@ -169,7 +169,7 @@ class PlaybackController(object): The current playback state will be kept. If it was playing, playing will continue. If it was paused, it will still be paused, etc. """ - tl_track = self.core.tracklist.tl_track_at_next(self.current_tl_track) + tl_track = self.core.tracklist.next_track(self.current_tl_track) if tl_track: self._trigger_track_playback_ended() self.change_track(tl_track) @@ -203,9 +203,9 @@ class PlaybackController(object): elif self.current_tl_track is not None: tl_track = self.current_tl_track elif self.current_tl_track is None and on_error_step == 1: - tl_track = self.core.tracklist.tl_track_at_next(tl_track) + tl_track = self.core.tracklist.next_track(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) + tl_track = self.core.tracklist.previous_track(tl_track) if tl_track is not None: self.current_tl_track = tl_track @@ -234,7 +234,7 @@ class PlaybackController(object): """ self._trigger_track_playback_ended() tl_track = self.current_tl_track - self.change_track(self.core.tracklist.tl_track_at_previous(tl_track), on_error_step=-1) + self.change_track(self.core.tracklist.previous_track(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 3364356b..3a5775b3 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -145,11 +145,11 @@ class TracklistController(object): return None - def tl_track_at_eot(self, tl_track): + def eot_track(self, tl_track): """ The track that will be played after the given track. - Not necessarily the same track as :meth:`tl_track_at_next`. + Not necessarily the same track as :meth:`next_track`. :param tl_track: The reference track :type tl_track: :class:`mopidy.models.TlTrack` @@ -185,7 +185,7 @@ class TracklistController(object): except IndexError: return None - def tl_track_at_next(self, tl_track): + def next_track(self, tl_track): """ The track that will be played if calling :meth:`mopidy.core.PlaybackController.next()`. @@ -224,7 +224,7 @@ class TracklistController(object): except IndexError: return None - def tl_track_at_previous(self, tl_track): + def previous_track(self, tl_track): """ Returns the track that will be played if calling :meth:`mopidy.core.PlaybackController.previous()`. diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index 2ab4dd4a..22535261 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -408,7 +408,7 @@ class MprisObject(dbus.service.Object): return False tl_track = self.core.playback.current_tl_track.get() return ( - self.core.tracklist.tl_track_at_next(tl_track).get() != + self.core.tracklist.next_track(tl_track).get() != self.core.playback.current_tl_track.get()) def get_CanGoPrevious(self): @@ -416,7 +416,7 @@ class MprisObject(dbus.service.Object): return False tl_track = self.core.playback.current_tl_track return ( - self.core.tracklist.tl_track_at_previous(tl_track).get() != + self.core.tracklist.previous_track(tl_track).get() != tl_track) def get_CanPlay(self): @@ -425,7 +425,7 @@ class MprisObject(dbus.service.Object): tl_track = self.core.playback.current_tl_track.get() return ( self.core.playback.current_tl_track.get() is not None or - self.core.tracklist.tl_track_at_next(tl_track).get() is not None) + self.core.tracklist.next_track(tl_track).get() is not None) def get_CanPause(self): if not self.get_CanControl(): diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 61db2d71..4436ac6c 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -245,13 +245,13 @@ class PlaybackControllerTest(object): @populate_tracklist def test_next_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_next_track_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_next_track_after_previous(self): @@ -259,11 +259,11 @@ class PlaybackControllerTest(object): self.playback.next() self.playback.previous() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) def test_next_track_empty_playlist(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_next_track_at_end_of_playlist(self): @@ -271,7 +271,7 @@ class PlaybackControllerTest(object): for _ in self.tracklist.tl_tracks[1:]: self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_next_track_at_end_of_playlist_with_repeat(self): @@ -280,14 +280,14 @@ class PlaybackControllerTest(object): for _ in self.tracks[1:]: self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_next_track_with_random(self): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) @populate_tracklist def test_next_with_consume(self): @@ -318,10 +318,10 @@ class PlaybackControllerTest(object): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_end_of_track(self): @@ -393,13 +393,13 @@ class PlaybackControllerTest(object): @populate_tracklist def test_end_of_track_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_end_of_track_track_after_previous(self): @@ -407,11 +407,11 @@ class PlaybackControllerTest(object): self.playback.on_end_of_track() self.playback.previous() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) def test_end_of_track_track_empty_playlist(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_end_of_track_track_at_end_of_playlist(self): @@ -419,7 +419,7 @@ class PlaybackControllerTest(object): for _ in self.tracklist.tl_tracks[1:]: self.playback.on_end_of_track() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_end_of_track_track_at_end_of_playlist_with_repeat(self): @@ -428,14 +428,14 @@ class PlaybackControllerTest(object): for _ in self.tracks[1:]: self.playback.on_end_of_track() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_with_random(self): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) @populate_tracklist def test_end_of_track_with_consume(self): @@ -458,28 +458,28 @@ class PlaybackControllerTest(object): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_previous_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), None) + self.assertEqual(self.tracklist.previous_track(tl_track), None) @populate_tracklist def test_previous_track_after_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), None) + self.assertEqual(self.tracklist.previous_track(tl_track), None) @populate_tracklist def test_previous_track_after_next(self): self.playback.play() self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_previous_track_after_previous(self): @@ -488,11 +488,11 @@ class PlaybackControllerTest(object): self.playback.next() # At track 2 self.playback.previous() # At track 1 tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) def test_previous_track_empty_playlist(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_previous(tl_track), None) + self.assertEqual(self.tracklist.previous_track(tl_track), None) @populate_tracklist def test_previous_track_with_consume(self): @@ -501,7 +501,7 @@ class PlaybackControllerTest(object): self.playback.next() tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tl_track_at_previous(tl_track), tl_track) + self.tracklist.previous_track(tl_track), tl_track) @populate_tracklist def test_previous_track_with_random(self): @@ -510,7 +510,7 @@ class PlaybackControllerTest(object): self.playback.next() tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tl_track_at_previous(tl_track), tl_track) + self.tracklist.previous_track(tl_track), tl_track) @populate_tracklist def test_initial_current_track(self): @@ -873,7 +873,7 @@ class PlaybackControllerTest(object): for _ in self.tracks[1:]: self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_random_until_end_of_playlist_and_play_from_start(self): @@ -881,7 +881,7 @@ class PlaybackControllerTest(object): for _ in self.tracks: self.playback.next() tl_track = self.playback.current_tl_track - self.assertNotEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertNotEqual(self.tracklist.next_track(tl_track), None) self.assertEqual(self.playback.state, PlaybackState.STOPPED) self.playback.play() self.assertEqual(self.playback.state, PlaybackState.PLAYING) @@ -894,7 +894,7 @@ class PlaybackControllerTest(object): for _ in self.tracks: self.playback.next() tl_track = self.playback.current_tl_track - self.assertNotEqual(self.tracklist.tl_track_at_next(tl_track), None) + self.assertNotEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_played_track_during_random_not_played_again(self): From fac2c8af7d1173553ee9706769dc6f01cedecf93 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Fri, 9 Aug 2013 08:31:32 +0200 Subject: [PATCH 14/42] format: Correcting flake8 messages --- mopidy/core/playback.py | 4 ++-- mopidy/core/tracklist.py | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index bb797f09..eaa926eb 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals import logging -import random import urlparse from mopidy.audio import PlaybackState @@ -234,7 +233,8 @@ class PlaybackController(object): """ self._trigger_track_playback_ended() tl_track = self.current_tl_track - self.change_track(self.core.tracklist.previous_track(tl_track), on_error_step=-1) + self.change_track(self.core.tracklist.previous_track(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 3a5775b3..51d45e81 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -144,7 +144,6 @@ class TracklistController(object): except ValueError: return None - def eot_track(self, tl_track): """ The track that will be played after the given track. @@ -187,7 +186,7 @@ class TracklistController(object): def next_track(self, tl_track): """ - The track that will be played if calling + The track that will be played if calling :meth:`mopidy.core.PlaybackController.next()`. For normal playback this is the next track in the playlist. If repeat @@ -247,7 +246,6 @@ class TracklistController(object): return self.tl_tracks[position - 1] - def add(self, tracks=None, at_position=None, uri=None): """ Add the track or list of tracks to the tracklist. From 11d82056a9928b0c7a14a5d0662e0f682c93b095 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Fri, 9 Aug 2013 09:14:28 +0200 Subject: [PATCH 15/42] core: Merged tracklist_position to index inside TracklistController docs: Updated changelog.rst --- docs/changelog.rst | 4 ++-- mopidy/core/tracklist.py | 22 ++++-------------- mopidy/frontends/mpd/protocol/playback.py | 2 +- mopidy/frontends/mpd/protocol/status.py | 8 +++---- tests/backends/base/playback.py | 28 +++++++++++------------ tests/backends/base/tracklist.py | 6 ++--- tests/utils/jsonrpc_test.py | 16 ++++--------- 7 files changed, 34 insertions(+), 52 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c997d1f4..b95b6e00 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,8 +21,8 @@ v0.15.0 (UNRELEASED) mark songs, some Playback properties have been converted into functions and both functions and properties have been moved into Tracklist to have more modularity: - - Properties converted into functions that need arguments: - :meth:`tracklist_position` + - Properties merged into functions: :attr:`tracklist_position` merged to + :meth:`index` - Properties moved: :attr:`random`, :attr:`repeat`, :attr:`consume` and :attr:`single` - Method created from properties: :meth:`next_track` from diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 51d45e81..ebac89e8 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -133,14 +133,14 @@ class TracklistController(object): Playback continues after current song. """ - def tracklist_position(self, tl_track): + def index(self, tl_track): """ The position of the given track in the tracklist. """ if tl_track is None: return None try: - return self.tl_tracks.index(tl_track) + return self._tl_tracks.index(tl_track) except ValueError: return None @@ -172,7 +172,7 @@ class TracklistController(object): if tl_track is None: return self.tl_tracks[0] - position = self.tracklist_position(tl_track) + position = self.index(tl_track) if self.repeat and self.single: return self.tl_tracks[position] @@ -214,7 +214,7 @@ class TracklistController(object): if tl_track is None: return self.tl_tracks[0] - position = self.tracklist_position(tl_track) + position = self.index(tl_track) if self.repeat: return self.tl_tracks[(position + 1) % len(self.tl_tracks)] @@ -240,7 +240,7 @@ class TracklistController(object): if self.repeat or self.consume or self.random: return tl_track - position = self.tracklist_position(tl_track) + position = self.index(tl_track) if position in (None, 0): return None @@ -335,18 +335,6 @@ class TracklistController(object): lambda ct: getattr(ct.track, key) == value, matches) return matches - def index(self, tl_track): - """ - Get index of the given :class:`mopidy.models.TlTrack` in the tracklist. - - Raises :exc:`ValueError` if not found. - - :param tl_track: track to find the index of - :type tl_track: :class:`mopidy.models.TlTrack` - :rtype: int - """ - return self._tl_tracks.index(tl_track) - def move(self, start, end, to_position): """ Move the tracks in the slice ``[start:end]`` to ``to_position``. diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index 06a456d9..b9289d8a 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/mopidy/frontends/mpd/protocol/playback.py @@ -330,7 +330,7 @@ def seek(context, songpos, seconds): - issues ``seek 1 120`` without quotes around the arguments. """ tl_track = context.core.playback.current_tl_track.get() - if context.core.tracklist.tracklist_position(tl_track).get() != int(songpos): + if context.core.tracklist.index(tl_track).get() != int(songpos): playpos(context, songpos) context.core.playback.seek(int(seconds) * 1000).get() diff --git a/mopidy/frontends/mpd/protocol/status.py b/mopidy/frontends/mpd/protocol/status.py index 8f2207f6..49e08ee8 100644 --- a/mopidy/frontends/mpd/protocol/status.py +++ b/mopidy/frontends/mpd/protocol/status.py @@ -38,7 +38,7 @@ def currentsong(context): """ tl_track = context.core.playback.current_tl_track.get() if tl_track is not None: - position = context.core.tracklist.tracklist_position(tl_track).get() + position = context.core.tracklist.index(tl_track).get() return track_to_mpd_format(tl_track, position=position) @@ -184,8 +184,8 @@ def status(context): 'tracklist.single': context.core.tracklist.single, 'playback.state': context.core.playback.state, 'playback.current_tl_track': context.core.playback.current_tl_track, - 'tracklist.tracklist_position': ( - context.core.tracklist.tracklist_position( + 'tracklist.index': ( + context.core.tracklist.index( context.core.playback.current_tl_track.get())), 'playback.time_position': context.core.playback.time_position, } @@ -254,7 +254,7 @@ def _status_songid(futures): def _status_songpos(futures): - return futures['tracklist.tracklist_position'].get() + return futures['tracklist.index'].get() def _status_state(futures): diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 4436ac6c..0b40f8cf 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -180,14 +180,14 @@ class PlaybackControllerTest(object): self.playback.play() tl_track = self.playback.current_tl_track - old_position = self.tracklist.tracklist_position(tl_track) + old_position = self.tracklist.index(tl_track) old_uri = tl_track.track.uri self.playback.next() tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tracklist_position(tl_track), old_position + 1) + self.tracklist.index(tl_track), old_position + 1) self.assertNotEqual(self.playback.current_track.uri, old_uri) @populate_tracklist @@ -208,7 +208,7 @@ class PlaybackControllerTest(object): self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tracklist_position(tl_track), i) + self.assertEqual(self.tracklist.index(tl_track), i) self.playback.next() @@ -328,14 +328,14 @@ class PlaybackControllerTest(object): self.playback.play() tl_track = self.playback.current_tl_track - old_position = self.tracklist.tracklist_position(tl_track) + old_position = self.tracklist.index(tl_track) old_uri = tl_track.track.uri self.playback.on_end_of_track() tl_track = self.playback.current_tl_track self.assertEqual( - self.tracklist.tracklist_position(tl_track), old_position + 1) + self.tracklist.index(tl_track), old_position + 1) self.assertNotEqual(self.playback.current_track.uri, old_uri) @populate_tracklist @@ -356,7 +356,7 @@ class PlaybackControllerTest(object): self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tracklist_position(tl_track), i) + self.assertEqual(self.tracklist.index(tl_track), i) self.playback.on_end_of_track() @@ -528,29 +528,29 @@ class PlaybackControllerTest(object): self.assertEqual(self.playback.current_track, self.tracks[1]) @populate_tracklist - def test_initial_tracklist_position(self): + def test_initial_index(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tracklist_position(tl_track), None) + self.assertEqual(self.tracklist.index(tl_track), None) @populate_tracklist - def test_tracklist_position_during_play(self): + def test_index_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tracklist_position(tl_track), 0) + self.assertEqual(self.tracklist.index(tl_track), 0) @populate_tracklist - def test_tracklist_position_after_next(self): + def test_index_after_next(self): self.playback.play() self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tracklist_position(tl_track), 1) + self.assertEqual(self.tracklist.index(tl_track), 1) @populate_tracklist - def test_tracklist_position_at_end_of_playlist(self): + def test_index_at_end_of_playlist(self): self.playback.play(self.tracklist.tl_tracks[-1]) self.playback.on_end_of_track() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.tracklist_position(tl_track), None) + self.assertEqual(self.tracklist.index(tl_track), None) def test_on_tracklist_change_gets_called(self): callback = self.playback.on_tracklist_change diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 5140d3aa..dd56e9e3 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -165,9 +165,9 @@ class TracklistControllerTest(object): self.assertEquals(1, self.controller.index(tl_tracks[1])) self.assertEquals(2, self.controller.index(tl_tracks[2])) - def test_index_raises_value_error_if_item_not_found(self): - test = lambda: self.controller.index(TlTrack(0, Track())) - self.assertRaises(ValueError, test) + def test_index_returns_none_if_item_not_found(self): + index = self.controller.index(TlTrack(0, Track())) + self.assertEquals(None, index) @populate_tracklist def test_move_single(self): diff --git a/tests/utils/jsonrpc_test.py b/tests/utils/jsonrpc_test.py index a0709ebc..2df8b0ba 100644 --- a/tests/utils/jsonrpc_test.py +++ b/tests/utils/jsonrpc_test.py @@ -313,7 +313,7 @@ class JsonRpcBatchTest(JsonRpcTestBase): class JsonRpcSingleCommandErrorTest(JsonRpcTestBase): - def test_application_error_response(self): + def test_application_error_response_is_none(self): request = { 'jsonrpc': '2.0', 'method': 'core.tracklist.index', @@ -322,17 +322,11 @@ class JsonRpcSingleCommandErrorTest(JsonRpcTestBase): } response = self.jrw.handle_data(request) - self.assertNotIn('result', response) + print response + self.assertIn('result', response) - error = response['error'] - self.assertEqual(error['code'], 0) - self.assertEqual(error['message'], 'Application error') - - data = error['data'] - self.assertEqual(data['type'], 'ValueError') - self.assertIn('not in list', data['message']) - self.assertIn('traceback', data) - self.assertIn('Traceback (most recent call last):', data['traceback']) + result = response['result'] + self.assertEqual(result, None) def test_missing_jsonrpc_member_causes_invalid_request_error(self): request = { From cd83d7a0d937e9cb0c393aac5d1d230c6a2f6c6c Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Fri, 9 Aug 2013 10:08:52 +0200 Subject: [PATCH 16/42] tests: Removing accidental print statement in testcase --- tests/utils/jsonrpc_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/utils/jsonrpc_test.py b/tests/utils/jsonrpc_test.py index 2df8b0ba..7abaa512 100644 --- a/tests/utils/jsonrpc_test.py +++ b/tests/utils/jsonrpc_test.py @@ -322,7 +322,6 @@ class JsonRpcSingleCommandErrorTest(JsonRpcTestBase): } response = self.jrw.handle_data(request) - print response self.assertIn('result', response) result = response['result'] From 21f3a8784aa3bab528e5db4aa47e832ddc97067d Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Mon, 12 Aug 2013 15:40:50 +0200 Subject: [PATCH 17/42] mpris: Correcting get_CanGoPrevious to get() the future instead of passing it. The failure was invisible to testcases --- mopidy/frontends/mpris/objects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index 22535261..624caa99 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -414,7 +414,7 @@ class MprisObject(dbus.service.Object): def get_CanGoPrevious(self): if not self.get_CanControl(): return False - tl_track = self.core.playback.current_tl_track + tl_track = self.core.playback.current_tl_track.get() return ( self.core.tracklist.previous_track(tl_track).get() != tl_track) From 157556a001065eefd287c78dbe11e52402567c87 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Tue, 13 Aug 2013 09:17:21 +0200 Subject: [PATCH 18/42] docs: Documenting further the TracklistController functions --- mopidy/core/tracklist.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index ebac89e8..135f3e73 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -136,6 +136,10 @@ class TracklistController(object): def index(self, tl_track): """ The position of the given track in the tracklist. + + :param tl_track: The reference track + :type tl_track: :class:`mopidy.models.TlTrack` + :rtype: int """ if tl_track is None: return None @@ -152,6 +156,7 @@ class TracklistController(object): :param tl_track: The reference track :type tl_track: :class:`mopidy.models.TlTrack` + :rtype: :class:`mopidy.models.TlTrack` """ # pylint: disable = R0911 # Too many return statements @@ -196,6 +201,7 @@ class TracklistController(object): :param tl_track: The reference track :type tl_track: :class:`mopidy.models.TlTrack` + :rtype: :class:`mopidy.models.TlTrack` """ if not self.tl_tracks: @@ -236,6 +242,7 @@ class TracklistController(object): :param tl_track: The reference track :type tl_track: :class:`mopidy.models.TlTrack` + :rtype: :class:`mopidy.models.TlTrack` """ if self.repeat or self.consume or self.random: return tl_track From a7d8af544d202041d4c709c43653ed92e2c5f9a6 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Tue, 13 Aug 2013 09:41:15 +0200 Subject: [PATCH 19/42] tests: tracklist tests renaming the TracklistController holder from controller to tracklist for test uniformity --- tests/backends/base/tracklist.py | 194 +++++++++++++++---------------- 1 file changed, 97 insertions(+), 97 deletions(-) diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index dd56e9e3..4c20bb36 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -20,7 +20,7 @@ class TracklistControllerTest(object): self.backend = self.backend_class.start( config=self.config, audio=self.audio).proxy() self.core = core.Core(audio=self.audio, backends=[self.backend]) - self.controller = self.core.tracklist + self.tracklist = self.core.tracklist self.playback = self.core.playback assert len(self.tracks) == 3, 'Need three tracks to run tests.' @@ -29,212 +29,212 @@ class TracklistControllerTest(object): pykka.ActorRegistry.stop_all() def test_length(self): - self.assertEqual(0, len(self.controller.tl_tracks)) - self.assertEqual(0, self.controller.length) - self.controller.add(self.tracks) - self.assertEqual(3, len(self.controller.tl_tracks)) - self.assertEqual(3, self.controller.length) + self.assertEqual(0, len(self.tracklist.tl_tracks)) + self.assertEqual(0, self.tracklist.length) + self.tracklist.add(self.tracks) + self.assertEqual(3, len(self.tracklist.tl_tracks)) + self.assertEqual(3, self.tracklist.length) def test_add(self): for track in self.tracks: - tl_tracks = self.controller.add([track]) - self.assertEqual(track, self.controller.tracks[-1]) - self.assertEqual(tl_tracks[0], self.controller.tl_tracks[-1]) + tl_tracks = self.tracklist.add([track]) + self.assertEqual(track, self.tracklist.tracks[-1]) + self.assertEqual(tl_tracks[0], self.tracklist.tl_tracks[-1]) self.assertEqual(track, tl_tracks[0].track) def test_add_at_position(self): for track in self.tracks[:-1]: - tl_tracks = self.controller.add([track], 0) - self.assertEqual(track, self.controller.tracks[0]) - self.assertEqual(tl_tracks[0], self.controller.tl_tracks[0]) + tl_tracks = self.tracklist.add([track], 0) + self.assertEqual(track, self.tracklist.tracks[0]) + self.assertEqual(tl_tracks[0], self.tracklist.tl_tracks[0]) self.assertEqual(track, tl_tracks[0].track) @populate_tracklist def test_add_at_position_outside_of_playlist(self): for track in self.tracks: - tl_tracks = self.controller.add([track], len(self.tracks) + 2) - self.assertEqual(track, self.controller.tracks[-1]) - self.assertEqual(tl_tracks[0], self.controller.tl_tracks[-1]) + tl_tracks = self.tracklist.add([track], len(self.tracks) + 2) + self.assertEqual(track, self.tracklist.tracks[-1]) + self.assertEqual(tl_tracks[0], self.tracklist.tl_tracks[-1]) self.assertEqual(track, tl_tracks[0].track) @populate_tracklist def test_filter_by_tlid(self): - tl_track = self.controller.tl_tracks[1] + tl_track = self.tracklist.tl_tracks[1] self.assertEqual( - [tl_track], self.controller.filter(tlid=tl_track.tlid)) + [tl_track], self.tracklist.filter(tlid=tl_track.tlid)) @populate_tracklist def test_filter_by_uri(self): - tl_track = self.controller.tl_tracks[1] + tl_track = self.tracklist.tl_tracks[1] self.assertEqual( - [tl_track], self.controller.filter(uri=tl_track.track.uri)) + [tl_track], self.tracklist.filter(uri=tl_track.track.uri)) @populate_tracklist def test_filter_by_uri_returns_nothing_for_invalid_uri(self): - self.assertEqual([], self.controller.filter(uri='foobar')) + self.assertEqual([], self.tracklist.filter(uri='foobar')) def test_filter_by_uri_returns_single_match(self): track = Track(uri='a') - self.controller.add([Track(uri='z'), track, Track(uri='y')]) - self.assertEqual(track, self.controller.filter(uri='a')[0].track) + self.tracklist.add([Track(uri='z'), track, Track(uri='y')]) + self.assertEqual(track, self.tracklist.filter(uri='a')[0].track) def test_filter_by_uri_returns_multiple_matches(self): track = Track(uri='a') - self.controller.add([Track(uri='z'), track, track]) - tl_tracks = self.controller.filter(uri='a') + self.tracklist.add([Track(uri='z'), track, track]) + tl_tracks = self.tracklist.filter(uri='a') self.assertEqual(track, tl_tracks[0].track) self.assertEqual(track, tl_tracks[1].track) def test_filter_by_uri_returns_nothing_if_no_match(self): - self.controller.playlist = Playlist( + self.tracklist.playlist = Playlist( tracks=[Track(uri='z'), Track(uri='y')]) - self.assertEqual([], self.controller.filter(uri='a')) + self.assertEqual([], self.tracklist.filter(uri='a')) def test_filter_by_multiple_criteria_returns_elements_matching_all(self): track1 = Track(uri='a', name='x') track2 = Track(uri='b', name='x') track3 = Track(uri='b', name='y') - self.controller.add([track1, track2, track3]) + self.tracklist.add([track1, track2, track3]) self.assertEqual( - track1, self.controller.filter(uri='a', name='x')[0].track) + track1, self.tracklist.filter(uri='a', name='x')[0].track) self.assertEqual( - track2, self.controller.filter(uri='b', name='x')[0].track) + track2, self.tracklist.filter(uri='b', name='x')[0].track) self.assertEqual( - track3, self.controller.filter(uri='b', name='y')[0].track) + track3, self.tracklist.filter(uri='b', name='y')[0].track) def test_filter_by_criteria_that_is_not_present_in_all_elements(self): track1 = Track() track2 = Track(uri='b') track3 = Track() - self.controller.add([track1, track2, track3]) - self.assertEqual(track2, self.controller.filter(uri='b')[0].track) + self.tracklist.add([track1, track2, track3]) + self.assertEqual(track2, self.tracklist.filter(uri='b')[0].track) @populate_tracklist def test_clear(self): - self.controller.clear() - self.assertEqual(len(self.controller.tracks), 0) + self.tracklist.clear() + self.assertEqual(len(self.tracklist.tracks), 0) def test_clear_empty_playlist(self): - self.controller.clear() - self.assertEqual(len(self.controller.tracks), 0) + self.tracklist.clear() + self.assertEqual(len(self.tracklist.tracks), 0) @populate_tracklist def test_clear_when_playing(self): self.playback.play() self.assertEqual(self.playback.state, PlaybackState.PLAYING) - self.controller.clear() + self.tracklist.clear() self.assertEqual(self.playback.state, PlaybackState.STOPPED) def test_add_appends_to_the_tracklist(self): - self.controller.add([Track(uri='a'), Track(uri='b')]) - self.assertEqual(len(self.controller.tracks), 2) - self.controller.add([Track(uri='c'), Track(uri='d')]) - self.assertEqual(len(self.controller.tracks), 4) - self.assertEqual(self.controller.tracks[0].uri, 'a') - self.assertEqual(self.controller.tracks[1].uri, 'b') - self.assertEqual(self.controller.tracks[2].uri, 'c') - self.assertEqual(self.controller.tracks[3].uri, 'd') + self.tracklist.add([Track(uri='a'), Track(uri='b')]) + self.assertEqual(len(self.tracklist.tracks), 2) + self.tracklist.add([Track(uri='c'), Track(uri='d')]) + self.assertEqual(len(self.tracklist.tracks), 4) + self.assertEqual(self.tracklist.tracks[0].uri, 'a') + self.assertEqual(self.tracklist.tracks[1].uri, 'b') + self.assertEqual(self.tracklist.tracks[2].uri, 'c') + self.assertEqual(self.tracklist.tracks[3].uri, 'd') def test_add_does_not_reset_version(self): - version = self.controller.version - self.controller.add([]) - self.assertEqual(self.controller.version, version) + version = self.tracklist.version + self.tracklist.add([]) + self.assertEqual(self.tracklist.version, version) @populate_tracklist def test_add_preserves_playing_state(self): self.playback.play() track = self.playback.current_track - self.controller.add(self.controller.tracks[1:2]) + self.tracklist.add(self.tracklist.tracks[1:2]) self.assertEqual(self.playback.state, PlaybackState.PLAYING) self.assertEqual(self.playback.current_track, track) @populate_tracklist def test_add_preserves_stopped_state(self): - self.controller.add(self.controller.tracks[1:2]) + self.tracklist.add(self.tracklist.tracks[1:2]) self.assertEqual(self.playback.state, PlaybackState.STOPPED) self.assertEqual(self.playback.current_track, None) @populate_tracklist def test_add_returns_the_tl_tracks_that_was_added(self): - tl_tracks = self.controller.add(self.controller.tracks[1:2]) - self.assertEqual(tl_tracks[0].track, self.controller.tracks[1]) + tl_tracks = self.tracklist.add(self.tracklist.tracks[1:2]) + self.assertEqual(tl_tracks[0].track, self.tracklist.tracks[1]) def test_index_returns_index_of_track(self): - tl_tracks = self.controller.add(self.tracks) - self.assertEquals(0, self.controller.index(tl_tracks[0])) - self.assertEquals(1, self.controller.index(tl_tracks[1])) - self.assertEquals(2, self.controller.index(tl_tracks[2])) + tl_tracks = self.tracklist.add(self.tracks) + self.assertEquals(0, self.tracklist.index(tl_tracks[0])) + self.assertEquals(1, self.tracklist.index(tl_tracks[1])) + self.assertEquals(2, self.tracklist.index(tl_tracks[2])) def test_index_returns_none_if_item_not_found(self): - index = self.controller.index(TlTrack(0, Track())) + index = self.tracklist.index(TlTrack(0, Track())) self.assertEquals(None, index) @populate_tracklist def test_move_single(self): - self.controller.move(0, 0, 2) + self.tracklist.move(0, 0, 2) - tracks = self.controller.tracks + tracks = self.tracklist.tracks self.assertEqual(tracks[2], self.tracks[0]) @populate_tracklist def test_move_group(self): - self.controller.move(0, 2, 1) + self.tracklist.move(0, 2, 1) - tracks = self.controller.tracks + tracks = self.tracklist.tracks self.assertEqual(tracks[1], self.tracks[0]) self.assertEqual(tracks[2], self.tracks[1]) @populate_tracklist def test_moving_track_outside_of_playlist(self): - tracks = len(self.controller.tracks) - test = lambda: self.controller.move(0, 0, tracks + 5) + tracks = len(self.tracklist.tracks) + test = lambda: self.tracklist.move(0, 0, tracks + 5) self.assertRaises(AssertionError, test) @populate_tracklist def test_move_group_outside_of_playlist(self): - tracks = len(self.controller.tracks) - test = lambda: self.controller.move(0, 2, tracks + 5) + tracks = len(self.tracklist.tracks) + test = lambda: self.tracklist.move(0, 2, tracks + 5) self.assertRaises(AssertionError, test) @populate_tracklist def test_move_group_out_of_range(self): - tracks = len(self.controller.tracks) - test = lambda: self.controller.move(tracks + 2, tracks + 3, 0) + tracks = len(self.tracklist.tracks) + test = lambda: self.tracklist.move(tracks + 2, tracks + 3, 0) self.assertRaises(AssertionError, test) @populate_tracklist def test_move_group_invalid_group(self): - test = lambda: self.controller.move(2, 1, 0) + test = lambda: self.tracklist.move(2, 1, 0) self.assertRaises(AssertionError, test) def test_tracks_attribute_is_immutable(self): - tracks1 = self.controller.tracks - tracks2 = self.controller.tracks + tracks1 = self.tracklist.tracks + tracks2 = self.tracklist.tracks self.assertNotEqual(id(tracks1), id(tracks2)) @populate_tracklist def test_remove(self): - track1 = self.controller.tracks[1] - track2 = self.controller.tracks[2] - version = self.controller.version - self.controller.remove(uri=track1.uri) - self.assertLess(version, self.controller.version) - self.assertNotIn(track1, self.controller.tracks) - self.assertEqual(track2, self.controller.tracks[1]) + track1 = self.tracklist.tracks[1] + track2 = self.tracklist.tracks[2] + version = self.tracklist.version + self.tracklist.remove(uri=track1.uri) + self.assertLess(version, self.tracklist.version) + self.assertNotIn(track1, self.tracklist.tracks) + self.assertEqual(track2, self.tracklist.tracks[1]) @populate_tracklist def test_removing_track_that_does_not_exist_does_nothing(self): - self.controller.remove(uri='/nonexistant') + self.tracklist.remove(uri='/nonexistant') def test_removing_from_empty_playlist_does_nothing(self): - self.controller.remove(uri='/nonexistant') + self.tracklist.remove(uri='/nonexistant') @populate_tracklist def test_shuffle(self): random.seed(1) - self.controller.shuffle() + self.tracklist.shuffle() - shuffled_tracks = self.controller.tracks + shuffled_tracks = self.tracklist.tracks self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(set(self.tracks), set(shuffled_tracks)) @@ -242,9 +242,9 @@ class TracklistControllerTest(object): @populate_tracklist def test_shuffle_subset(self): random.seed(1) - self.controller.shuffle(1, 3) + self.tracklist.shuffle(1, 3) - shuffled_tracks = self.controller.tracks + shuffled_tracks = self.tracklist.tracks self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(self.tracks[0], shuffled_tracks[0]) @@ -252,21 +252,21 @@ class TracklistControllerTest(object): @populate_tracklist def test_shuffle_invalid_subset(self): - test = lambda: self.controller.shuffle(3, 1) + test = lambda: self.tracklist.shuffle(3, 1) self.assertRaises(AssertionError, test) @populate_tracklist def test_shuffle_superset(self): - tracks = len(self.controller.tracks) - test = lambda: self.controller.shuffle(1, tracks + 5) + tracks = len(self.tracklist.tracks) + test = lambda: self.tracklist.shuffle(1, tracks + 5) self.assertRaises(AssertionError, test) @populate_tracklist def test_shuffle_open_subset(self): random.seed(1) - self.controller.shuffle(1) + self.tracklist.shuffle(1) - shuffled_tracks = self.controller.tracks + shuffled_tracks = self.tracklist.tracks self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(self.tracks[0], shuffled_tracks[0]) @@ -274,22 +274,22 @@ class TracklistControllerTest(object): @populate_tracklist def test_slice_returns_a_subset_of_tracks(self): - track_slice = self.controller.slice(1, 3) + track_slice = self.tracklist.slice(1, 3) self.assertEqual(2, len(track_slice)) self.assertEqual(self.tracks[1], track_slice[0].track) self.assertEqual(self.tracks[2], track_slice[1].track) @populate_tracklist def test_slice_returns_empty_list_if_indexes_outside_tracks_list(self): - self.assertEqual(0, len(self.controller.slice(7, 8))) - self.assertEqual(0, len(self.controller.slice(-1, 1))) + self.assertEqual(0, len(self.tracklist.slice(7, 8))) + self.assertEqual(0, len(self.tracklist.slice(-1, 1))) def test_version_does_not_change_when_adding_nothing(self): - version = self.controller.version - self.controller.add([]) - self.assertEquals(version, self.controller.version) + version = self.tracklist.version + self.tracklist.add([]) + self.assertEquals(version, self.tracklist.version) def test_version_increases_when_adding_something(self): - version = self.controller.version - self.controller.add([Track()]) - self.assertLess(version, self.controller.version) + version = self.tracklist.version + self.tracklist.add([Track()]) + self.assertLess(version, self.tracklist.version) From b70cd9e787f8f266bf327e1e34dd80e6f7928707 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Tue, 13 Aug 2013 09:58:10 +0200 Subject: [PATCH 20/42] tests: moving more tests from playback to tracklist --- tests/backends/base/playback.py | 203 ------------------------------- tests/backends/base/tracklist.py | 203 +++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 203 deletions(-) diff --git a/tests/backends/base/playback.py b/tests/backends/base/playback.py index 0b40f8cf..7ee46097 100644 --- a/tests/backends/base/playback.py +++ b/tests/backends/base/playback.py @@ -242,87 +242,6 @@ class PlaybackControllerTest(object): self.assertNotEqual(self.playback.current_track, self.tracks[1]) self.assertEqual(self.playback.current_track, self.tracks[2]) - @populate_tracklist - def test_next_track_before_play(self): - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) - - @populate_tracklist - def test_next_track_during_play(self): - self.playback.play() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) - - @populate_tracklist - def test_next_track_after_previous(self): - self.playback.play() - self.playback.next() - self.playback.previous() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) - - def test_next_track_empty_playlist(self): - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), None) - - @populate_tracklist - def test_next_track_at_end_of_playlist(self): - self.playback.play() - for _ in self.tracklist.tl_tracks[1:]: - self.playback.next() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), None) - - @populate_tracklist - def test_next_track_at_end_of_playlist_with_repeat(self): - self.tracklist.repeat = True - self.playback.play() - for _ in self.tracks[1:]: - self.playback.next() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) - - @populate_tracklist - def test_next_track_with_random(self): - random.seed(1) - self.tracklist.random = True - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) - - @populate_tracklist - def test_next_with_consume(self): - self.tracklist.consume = True - self.playback.play() - self.playback.next() - self.assertIn(self.tracks[0], self.tracklist.tracks) - - @populate_tracklist - def test_next_with_single_and_repeat(self): - self.tracklist.single = True - self.tracklist.repeat = True - self.playback.play() - self.playback.next() - self.assertEqual(self.playback.current_track, self.tracks[1]) - - @populate_tracklist - def test_next_with_random(self): - # FIXME feels very fragile - random.seed(1) - self.tracklist.random = True - self.playback.play() - self.playback.next() - self.assertEqual(self.playback.current_track, self.tracks[1]) - - @populate_tracklist - def test_next_track_with_random_after_append_playlist(self): - random.seed(1) - 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.add(self.tracks[:1]) - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) - @populate_tracklist def test_end_of_track(self): self.playback.play() @@ -390,128 +309,6 @@ class PlaybackControllerTest(object): self.assertNotEqual(self.playback.current_track, self.tracks[1]) self.assertEqual(self.playback.current_track, self.tracks[2]) - @populate_tracklist - def test_end_of_track_track_before_play(self): - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) - - @populate_tracklist - def test_end_of_track_track_during_play(self): - self.playback.play() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) - - @populate_tracklist - def test_end_of_track_track_after_previous(self): - self.playback.play() - self.playback.on_end_of_track() - self.playback.previous() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) - - def test_end_of_track_track_empty_playlist(self): - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), None) - - @populate_tracklist - def test_end_of_track_track_at_end_of_playlist(self): - self.playback.play() - for _ in self.tracklist.tl_tracks[1:]: - self.playback.on_end_of_track() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), None) - - @populate_tracklist - def test_end_of_track_track_at_end_of_playlist_with_repeat(self): - self.tracklist.repeat = 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.next_track(tl_track), self.tl_tracks[0]) - - @populate_tracklist - def test_end_of_track_track_with_random(self): - random.seed(1) - self.tracklist.random = True - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) - - @populate_tracklist - def test_end_of_track_with_consume(self): - self.tracklist.consume = True - self.playback.play() - self.playback.on_end_of_track() - self.assertNotIn(self.tracks[0], self.tracklist.tracks) - - @populate_tracklist - def test_end_of_track_with_random(self): - # FIXME feels very fragile - random.seed(1) - self.tracklist.random = True - self.playback.play() - self.playback.on_end_of_track() - self.assertEqual(self.playback.current_track, self.tracks[1]) - - @populate_tracklist - def test_end_of_track_track_with_random_after_append_playlist(self): - random.seed(1) - 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.add(self.tracks[:1]) - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) - - @populate_tracklist - def test_previous_track_before_play(self): - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), None) - - @populate_tracklist - def test_previous_track_after_play(self): - self.playback.play() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), None) - - @populate_tracklist - def test_previous_track_after_next(self): - self.playback.play() - self.playback.next() - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) - - @populate_tracklist - def test_previous_track_after_previous(self): - self.playback.play() # At track 0 - self.playback.next() # At track 1 - self.playback.next() # At track 2 - self.playback.previous() # At track 1 - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) - - def test_previous_track_empty_playlist(self): - tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(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.previous_track(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.previous_track(tl_track), tl_track) - @populate_tracklist def test_initial_current_track(self): self.assertEqual(self.playback.current_track, None) diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 4c20bb36..33991f2a 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -125,6 +125,209 @@ class TracklistControllerTest(object): self.tracklist.clear() self.assertEqual(self.playback.state, PlaybackState.STOPPED) + @populate_tracklist + def test_next_track_before_play(self): + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + + @populate_tracklist + def test_next_track_during_play(self): + self.playback.play() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + + @populate_tracklist + def test_next_track_after_previous(self): + self.playback.play() + self.playback.next() + self.playback.previous() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + + def test_next_track_empty_playlist(self): + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), None) + + @populate_tracklist + def test_next_track_at_end_of_playlist(self): + self.playback.play() + for _ in self.tracklist.tl_tracks[1:]: + self.playback.next() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), None) + + @populate_tracklist + def test_next_track_at_end_of_playlist_with_repeat(self): + self.tracklist.repeat = True + self.playback.play() + for _ in self.tracks[1:]: + self.playback.next() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + + @populate_tracklist + def test_next_track_with_random(self): + random.seed(1) + self.tracklist.random = True + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) + + @populate_tracklist + def test_next_with_consume(self): + self.tracklist.consume = True + self.playback.play() + self.playback.next() + self.assertIn(self.tracks[0], self.tracklist.tracks) + + @populate_tracklist + def test_next_with_single_and_repeat(self): + self.tracklist.single = True + self.tracklist.repeat = True + self.playback.play() + self.playback.next() + self.assertEqual(self.playback.current_track, self.tracks[1]) + + @populate_tracklist + def test_next_with_random(self): + # FIXME feels very fragile + random.seed(1) + self.tracklist.random = True + self.playback.play() + self.playback.next() + self.assertEqual(self.playback.current_track, self.tracks[1]) + + @populate_tracklist + def test_next_track_with_random_after_append_playlist(self): + random.seed(1) + 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.add(self.tracks[:1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + + @populate_tracklist + def test_end_of_track_track_before_play(self): + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + + @populate_tracklist + def test_end_of_track_track_during_play(self): + self.playback.play() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + + @populate_tracklist + def test_end_of_track_track_after_previous(self): + self.playback.play() + self.playback.on_end_of_track() + self.playback.previous() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + + def test_end_of_track_track_empty_playlist(self): + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), None) + + @populate_tracklist + def test_end_of_track_track_at_end_of_playlist(self): + self.playback.play() + for _ in self.tracklist.tl_tracks[1:]: + self.playback.on_end_of_track() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), None) + + @populate_tracklist + def test_end_of_track_track_at_end_of_playlist_with_repeat(self): + self.tracklist.repeat = 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.next_track(tl_track), self.tl_tracks[0]) + + @populate_tracklist + def test_end_of_track_track_with_random(self): + random.seed(1) + self.tracklist.random = True + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) + + @populate_tracklist + def test_end_of_track_with_consume(self): + self.tracklist.consume = True + self.playback.play() + self.playback.on_end_of_track() + self.assertNotIn(self.tracks[0], self.tracklist.tracks) + + @populate_tracklist + def test_end_of_track_with_random(self): + # FIXME feels very fragile + random.seed(1) + self.tracklist.random = True + self.playback.play() + self.playback.on_end_of_track() + self.assertEqual(self.playback.current_track, self.tracks[1]) + + @populate_tracklist + def test_end_of_track_track_with_random_after_append_playlist(self): + random.seed(1) + 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.add(self.tracks[:1]) + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + + @populate_tracklist + def test_previous_track_before_play(self): + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.previous_track(tl_track), None) + + @populate_tracklist + def test_previous_track_after_play(self): + self.playback.play() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.previous_track(tl_track), None) + + @populate_tracklist + def test_previous_track_after_next(self): + self.playback.play() + self.playback.next() + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) + + @populate_tracklist + def test_previous_track_after_previous(self): + self.playback.play() # At track 0 + self.playback.next() # At track 1 + self.playback.next() # At track 2 + self.playback.previous() # At track 1 + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) + + def test_previous_track_empty_playlist(self): + tl_track = self.playback.current_tl_track + self.assertEqual(self.tracklist.previous_track(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.previous_track(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.previous_track(tl_track), tl_track) + def test_add_appends_to_the_tracklist(self): self.tracklist.add([Track(uri='a'), Track(uri='b')]) self.assertEqual(len(self.tracklist.tracks), 2) From 268c3b787986c6bf5dbc0082e3f987bd93f278c1 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Tue, 13 Aug 2013 10:05:01 +0200 Subject: [PATCH 21/42] tests: correcting indentation --- tests/backends/base/tracklist.py | 48 +++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 33991f2a..f1786ef9 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -128,13 +128,15 @@ class TracklistControllerTest(object): @populate_tracklist def test_next_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[0]) @populate_tracklist def test_next_track_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[1]) @populate_tracklist def test_next_track_after_previous(self): @@ -142,7 +144,8 @@ class TracklistControllerTest(object): self.playback.next() self.playback.previous() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[1]) def test_next_track_empty_playlist(self): tl_track = self.playback.current_tl_track @@ -163,14 +166,16 @@ class TracklistControllerTest(object): for _ in self.tracks[1:]: self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[0]) @populate_tracklist def test_next_track_with_random(self): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[2]) @populate_tracklist def test_next_with_consume(self): @@ -201,21 +206,25 @@ class TracklistControllerTest(object): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[1]) @populate_tracklist def test_end_of_track_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[1]) @populate_tracklist def test_end_of_track_track_after_previous(self): @@ -223,7 +232,8 @@ class TracklistControllerTest(object): self.playback.on_end_of_track() self.playback.previous() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[1]) def test_end_of_track_track_empty_playlist(self): tl_track = self.playback.current_tl_track @@ -244,14 +254,16 @@ class TracklistControllerTest(object): for _ in self.tracks[1:]: self.playback.on_end_of_track() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_with_random(self): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[2]) @populate_tracklist def test_end_of_track_with_consume(self): @@ -274,10 +286,12 @@ class TracklistControllerTest(object): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[2]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), self.tl_tracks[1]) + self.assertEqual(self.tracklist.next_track(tl_track), + self.tl_tracks[1]) @populate_tracklist def test_previous_track_before_play(self): @@ -295,7 +309,8 @@ class TracklistControllerTest(object): self.playback.play() self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.previous_track(tl_track), + self.tl_tracks[0]) @populate_tracklist def test_previous_track_after_previous(self): @@ -304,7 +319,8 @@ class TracklistControllerTest(object): self.playback.next() # At track 2 self.playback.previous() # At track 1 tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), self.tl_tracks[0]) + self.assertEqual(self.tracklist.previous_track(tl_track), + self.tl_tracks[0]) def test_previous_track_empty_playlist(self): tl_track = self.playback.current_tl_track From 72890bbe8db3847b6d6bab56520613a7cbd0d35b Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Thu, 12 Sep 2013 11:38:15 +0200 Subject: [PATCH 22/42] docs: Fixing changelog syntax --- docs/changelog.rst | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2a05801a..92de24d1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,20 +15,26 @@ v0.15.0 (UNRELEASED) supported by Mopidy is Python 2.7. (Fixes: :issue:`344`) **Core** + - Tracklist has now the power to make decisions on which is the next/previous song, along with previously playback associated features, such as randomness, consumption, repeat and single. For that, a new method has been created to mark songs, some Playback properties have been converted into functions and both functions and properties have been moved into Tracklist to have more modularity: - - Properties merged into functions: :attr:`tracklist_position` merged to - :meth:`index` - - Properties moved: :attr:`random`, :attr:`repeat`, :attr:`consume` and - :attr:`single` - - Method created from properties: :meth:`next_track` from - `tl_track_at_next`, :meth:`eot_track` from ´tl_track_at_eot` and - :meth:`previous_track` from `tl_track_at_previous` - - Method created to separe functionality: :meth:`mark` + + - Properties merged into functions: :attr:`tracklist_position` merged to + :meth:`index` + + - Properties moved: :attr:`random`, :attr:`repeat`, :attr:`consume` and + :attr:`single` + + - Method created from properties: :meth:`next_track` from + `tl_track_at_next`, :meth:`eot_track` from `tl_track_at_eot` and + :meth:`previous_track` from `tl_track_at_previous` + + - Method created to separe functionality: :meth:`mark` + - Tracklist's get_tl_track_at_* and tracklist_position now need a tl_track as a reference to give an answer. From aec164ed482ac61de4a9810a8ec50cc433283b04 Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Fri, 13 Sep 2013 12:47:28 +0200 Subject: [PATCH 23/42] git: Ignoring permanently *.orig files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index dca43a7d..79230110 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ mopidy.log* node_modules/ nosetests.xml *~ +*.orig From a14a19447bd9ff4b2b2f9f52ba5845afe5f3a92a Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Fri, 11 Oct 2013 12:41:11 +0200 Subject: [PATCH 24/42] Updating tidy-up-core to jodal specs and changing one name. Still need to update docs --- mopidy/core/playback.py | 6 +++--- mopidy/core/tracklist.py | 35 ++++++++++++----------------------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 7eedb750..0709c688 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -165,7 +165,7 @@ class PlaybackController(object): else: self.stop(clear_current_track=True) - self.core.tracklist.mark("consumed", original_tl_track) + self.core.tracklist.mark_consumed(original_tl_track) def on_tracklist_change(self): """ @@ -230,7 +230,7 @@ class PlaybackController(object): backend = self._get_backend() if not backend or not backend.playback.play(tl_track.track).get(): logger.warning('Track is not playable: %s', tl_track.track.uri) - self.core.tracklist.mark("unplayable", tl_track) + self.core.tracklist.mark_unplayable(tl_track) if on_error_step == 1: # TODO: can cause an endless loop for single track repeat. self.next() @@ -238,7 +238,7 @@ class PlaybackController(object): self.previous() return - self.core.tracklist.mark("played", tl_track) + self.core.tracklist.mark_starting(tl_track) self._trigger_track_playback_started() diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 135f3e73..154eb0d5 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -438,30 +438,19 @@ class TracklistController(object): """ return self._tl_tracks[start:end] - def mark(self, what, tl_track): - """ - Marks the given track as specified. Currently supports:: - * `consumed` The track has been completely played. - * `played` The track has been played, at least a piece of it. - * `unplayable` The track is unplayable + def mark_consumed(self, tl_track): + if not self.consume: + return False + self.remove(tlid=tl_track.tlid) + return True - :param what: What to be marked as - :type what: string - :param tl_track: Track to mark - :type tl_track: :class:`mopidy.models.TlTrack` - :rtype: True if the track was actually removed from the tracklist - """ - if what == "consumed": - if not self.consume: - return False - self.remove(tlid=tl_track.tlid) - return True - elif what == "played": - if self.random and tl_track in self._shuffled: - self._shuffled.remove(tl_track) - elif what == "unplayable": - if self.random and self._shuffled: - self._shuffled.remove(tl_track) + def mark_starting(self, tl_track): + if self.random and tl_track in self._shuffled: + self._shuffled.remove(tl_track) + + def mark_unplayable(self, tl_track): + if self.random and self._shuffled: + self._shuffled.remove(tl_track) def _trigger_tracklist_changed(self): self._first_shuffle = True From e9c20d2e59365881be7e02eab322d638c88fd1df Mon Sep 17 00:00:00 2001 From: Javier Domingo Cansino Date: Tue, 15 Oct 2013 12:11:01 +0200 Subject: [PATCH 25/42] Correcting flake error on unused import --- tests/backends/local/tracklist_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 6681c645..043f0905 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -8,7 +8,7 @@ import pykka from mopidy import audio, core from mopidy.backends.local import actor from mopidy.core import PlaybackState -from mopidy.models import Playlist, TlTrack, Track +from mopidy.models import Playlist, Track from tests import path_to_data_dir from tests.backends.local import generate_song, populate_tracklist From b6346f1c8636f7d2dc48986db467331301bcff11 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 13:14:52 +0200 Subject: [PATCH 26/42] core.playback: Rename next_{ => tl_}track To make the type of the variable obvious --- mopidy/core/playback.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 0709c688..19060859 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -157,11 +157,11 @@ class PlaybackController(object): return original_tl_track = self.current_tl_track - next_track = self.core.tracklist.eot_track(original_tl_track) + next_tl_track = self.core.tracklist.eot_track(original_tl_track) - if next_track: + if next_tl_track: self._trigger_track_playback_ended() - self.play(next_track) + self.play(next_tl_track) else: self.stop(clear_current_track=True) From d636f0d2289d1244fab67debdb598ceeea09ec4b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 13:15:25 +0200 Subject: [PATCH 27/42] core.playback: Simplify if stmt tracklist.tl_tracks is always a list --- mopidy/core/playback.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 19060859..8573fba2 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -173,10 +173,7 @@ class PlaybackController(object): Used by :class:`mopidy.core.TracklistController`. """ - - if (not self.core.tracklist.tl_tracks or - self.current_tl_track not in - self.core.tracklist.tl_tracks): + if self.current_tl_track not in self.core.tracklist.tl_tracks: self.stop(clear_current_track=True) def next(self): From ce55e0eca57f38bfd885c4552d81b6f8e02d9019 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 13:25:05 +0200 Subject: [PATCH 28/42] core.playback: Refactor play() logic --- mopidy/core/playback.py | 48 +++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 8573fba2..dba1ce4d 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -209,35 +209,41 @@ class PlaybackController(object): :type on_error_step: int, -1 or 1 """ - if tl_track is not None: - assert tl_track in self.core.tracklist.tl_tracks - elif tl_track is None: + assert on_error_step in (-1, 1) + + if tl_track is None: if self.state == PlaybackState.PAUSED: return self.resume() - elif self.current_tl_track is not None: - tl_track = self.current_tl_track - elif self.current_tl_track is None and on_error_step == 1: - tl_track = self.core.tracklist.next_track(tl_track) - elif self.current_tl_track is None and on_error_step == -1: - tl_track = self.core.tracklist.previous_track(tl_track) - if tl_track is not None: - self.current_tl_track = tl_track - self.state = PlaybackState.PLAYING - backend = self._get_backend() - if not backend or not backend.playback.play(tl_track.track).get(): - logger.warning('Track is not playable: %s', tl_track.track.uri) - self.core.tracklist.mark_unplayable(tl_track) + if self.current_tl_track is not None: + tl_track = self.current_tl_track + else: if on_error_step == 1: - # TODO: can cause an endless loop for single track repeat. - self.next() + tl_track = self.core.tracklist.next_track(tl_track) elif on_error_step == -1: - self.previous() + tl_track = self.core.tracklist.previous_track(tl_track) + + if tl_track is None: return - self.core.tracklist.mark_starting(tl_track) + assert tl_track in self.core.tracklist.tl_tracks - self._trigger_track_playback_started() + self.current_tl_track = tl_track + self.state = PlaybackState.PLAYING + backend = self._get_backend() + success = backend and backend.playback.play(tl_track.track).get() + + if success: + self.core.tracklist.mark_starting(tl_track) + self._trigger_track_playback_started() + else: + logger.warning('Track is not playable: %s', tl_track.track.uri) + self.core.tracklist.mark_unplayable(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() def previous(self): """ From e5e1b5fa63ed018f4e1346a6d16be83b2418638c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 13:35:15 +0200 Subject: [PATCH 29/42] core.playback: Formatting --- mopidy/core/playback.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index dba1ce4d..fbbab4e5 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -254,8 +254,8 @@ class PlaybackController(object): """ self._trigger_track_playback_ended() tl_track = self.current_tl_track - self.change_track(self.core.tracklist.previous_track(tl_track), - on_error_step=-1) + self.change_track( + self.core.tracklist.previous_track(tl_track), on_error_step=-1) def resume(self): """If paused, resume playing the current track.""" From ff89fc58a9b48bc60e62b80119cbff5d7b8035a3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 13:39:22 +0200 Subject: [PATCH 30/42] core.tracklist: Remove pylint comment --- mopidy/core/tracklist.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 154eb0d5..37428e53 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -158,9 +158,6 @@ class TracklistController(object): :type tl_track: :class:`mopidy.models.TlTrack` :rtype: :class:`mopidy.models.TlTrack` """ - # pylint: disable = R0911 - # Too many return statements - if not self.tl_tracks: return None From d9921d91274fe22778aaef71cfbec46a22379e1b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 15:08:48 +0200 Subject: [PATCH 31/42] core.tracklist: Test changed index() behavior --- tests/backends/local/tracklist_test.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/backends/local/tracklist_test.py b/tests/backends/local/tracklist_test.py index 043f0905..00d8f208 100644 --- a/tests/backends/local/tracklist_test.py +++ b/tests/backends/local/tracklist_test.py @@ -8,7 +8,7 @@ import pykka from mopidy import audio, core from mopidy.backends.local import actor from mopidy.core import PlaybackState -from mopidy.models import Playlist, Track +from mopidy.models import Playlist, TlTrack, Track from tests import path_to_data_dir from tests.backends.local import generate_song, populate_tracklist @@ -171,9 +171,13 @@ class LocalTracklistControllerTest(unittest.TestCase): def test_index_returns_index_of_track(self): tl_tracks = self.controller.add(self.tracks) - self.assertEquals(0, self.controller.index(tl_tracks[0])) - self.assertEquals(1, self.controller.index(tl_tracks[1])) - self.assertEquals(2, self.controller.index(tl_tracks[2])) + self.assertEqual(0, self.controller.index(tl_tracks[0])) + self.assertEqual(1, self.controller.index(tl_tracks[1])) + self.assertEqual(2, self.controller.index(tl_tracks[2])) + + def test_index_returns_none_if_item_not_found(self): + tl_track = TlTrack(0, Track()) + self.assertEqual(self.controller.index(tl_track), None) @populate_tracklist def test_move_single(self): From 9c2f6c2f2586cc7de7bc6505b7c56cc35f194fe1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 15:12:18 +0200 Subject: [PATCH 32/42] core.tracklist: Tweak docstrings --- mopidy/core/tracklist.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 37428e53..86f4d426 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -137,9 +137,9 @@ class TracklistController(object): """ The position of the given track in the tracklist. - :param tl_track: The reference track + :param tl_track: the track to find the index of :type tl_track: :class:`mopidy.models.TlTrack` - :rtype: int + :rtype: :class:`int` or :class:`None` """ if tl_track is None: return None @@ -154,9 +154,9 @@ class TracklistController(object): Not necessarily the same track as :meth:`next_track`. - :param tl_track: The reference track - :type tl_track: :class:`mopidy.models.TlTrack` - :rtype: :class:`mopidy.models.TlTrack` + :param tl_track: the reference track + :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` + :rtype: :class:`mopidy.models.TlTrack` or :class:`None` """ if not self.tl_tracks: return None @@ -196,9 +196,9 @@ class TracklistController(object): enabled this should be a random track, all tracks should be played once before the list repeats. - :param tl_track: The reference track - :type tl_track: :class:`mopidy.models.TlTrack` - :rtype: :class:`mopidy.models.TlTrack` + :param tl_track: the reference track + :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` + :rtype: :class:`mopidy.models.TlTrack` or :class:`None` """ if not self.tl_tracks: @@ -231,15 +231,13 @@ class TracklistController(object): Returns the track that will be played if calling :meth:`mopidy.core.PlaybackController.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. - :param tl_track: The reference track - :type tl_track: :class:`mopidy.models.TlTrack` - :rtype: :class:`mopidy.models.TlTrack` + :param tl_track: the reference track + :type tl_track: :class:`mopidy.models.TlTrack` or :class:`None` + :rtype: :class:`mopidy.models.TlTrack` or :class:`None` """ if self.repeat or self.consume or self.random: return tl_track From aaa3b2e93c0914be4bebdc238a768a206b99d139 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 15:12:38 +0200 Subject: [PATCH 33/42] core.tracklist: Remove redundant if stmt in index() --- mopidy/core/tracklist.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 86f4d426..33315ac8 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -141,8 +141,6 @@ class TracklistController(object): :type tl_track: :class:`mopidy.models.TlTrack` :rtype: :class:`int` or :class:`None` """ - if tl_track is None: - return None try: return self._tl_tracks.index(tl_track) except ValueError: From 0ea4fd6af038f0157fd8554a024a13e9948d6aee Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 20:12:14 +0200 Subject: [PATCH 34/42] core.tracklist: Rename mark_{consumed => played} --- mopidy/core/playback.py | 2 +- mopidy/core/tracklist.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index fbbab4e5..a8c1a78a 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -165,7 +165,7 @@ class PlaybackController(object): else: self.stop(clear_current_track=True) - self.core.tracklist.mark_consumed(original_tl_track) + self.core.tracklist.mark_played(original_tl_track) def on_tracklist_change(self): """ diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 33315ac8..100a603f 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -431,12 +431,6 @@ class TracklistController(object): """ return self._tl_tracks[start:end] - def mark_consumed(self, tl_track): - if not self.consume: - return False - self.remove(tlid=tl_track.tlid) - return True - def mark_starting(self, tl_track): if self.random and tl_track in self._shuffled: self._shuffled.remove(tl_track) @@ -445,6 +439,12 @@ class TracklistController(object): if self.random and self._shuffled: self._shuffled.remove(tl_track) + def mark_played(self, tl_track): + if not self.consume: + return False + self.remove(tlid=tl_track.tlid) + return True + def _trigger_tracklist_changed(self): self._first_shuffle = True self._shuffled = [] From 91e718e85f671203c4633d67618fd0378da13dd9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 20:13:25 +0200 Subject: [PATCH 35/42] core.tracklist: Rename mark_{starting => playing} --- mopidy/core/playback.py | 2 +- mopidy/core/tracklist.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index a8c1a78a..b0e0f2e1 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -234,7 +234,7 @@ class PlaybackController(object): success = backend and backend.playback.play(tl_track.track).get() if success: - self.core.tracklist.mark_starting(tl_track) + self.core.tracklist.mark_playing(tl_track) self._trigger_track_playback_started() else: logger.warning('Track is not playable: %s', tl_track.track.uri) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 100a603f..22e741da 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -431,7 +431,7 @@ class TracklistController(object): """ return self._tl_tracks[start:end] - def mark_starting(self, tl_track): + def mark_playing(self, tl_track): if self.random and tl_track in self._shuffled: self._shuffled.remove(tl_track) From 67a7e0021a619d3aaf750892477f9ad8d63b4ca8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 20:14:06 +0200 Subject: [PATCH 36/42] core.tracklist: Add docstrings to mark_* --- mopidy/core/tracklist.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 22e741da..125843b2 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -432,14 +432,17 @@ class TracklistController(object): return self._tl_tracks[start:end] def mark_playing(self, tl_track): + """Private method used by :class:`mopidy.core.PlaybackController`.""" if self.random and tl_track in self._shuffled: self._shuffled.remove(tl_track) def mark_unplayable(self, tl_track): + """Private method used by :class:`mopidy.core.PlaybackController`.""" if self.random and self._shuffled: self._shuffled.remove(tl_track) def mark_played(self, tl_track): + """Private method used by :class:`mopidy.core.PlaybackController`.""" if not self.consume: return False self.remove(tlid=tl_track.tlid) From 46aeb3bfcc913c0ff01f31dbd0d9171129f2d296 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 20:15:38 +0200 Subject: [PATCH 37/42] core.tracklist: Move logging into mark_unplayable() --- mopidy/core/playback.py | 1 - mopidy/core/tracklist.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index b0e0f2e1..d127fbbe 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -237,7 +237,6 @@ class PlaybackController(object): self.core.tracklist.mark_playing(tl_track) self._trigger_track_playback_started() else: - logger.warning('Track is not playable: %s', tl_track.track.uri) self.core.tracklist.mark_unplayable(tl_track) if on_error_step == 1: # TODO: can cause an endless loop for single track repeat. diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 125843b2..940eb0ec 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -438,6 +438,7 @@ class TracklistController(object): def mark_unplayable(self, tl_track): """Private method used by :class:`mopidy.core.PlaybackController`.""" + logger.warning('Track is not playable: %s', tl_track.track.uri) if self.random and self._shuffled: self._shuffled.remove(tl_track) From 9864f55b75a26c8c6c697bbd3ec5edd899d4ed6f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 20:52:21 +0200 Subject: [PATCH 38/42] core.tracklist: Improve if check in mark_unplayable() --- mopidy/core/tracklist.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 940eb0ec..aedc2785 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -439,7 +439,7 @@ class TracklistController(object): def mark_unplayable(self, tl_track): """Private method used by :class:`mopidy.core.PlaybackController`.""" logger.warning('Track is not playable: %s', tl_track.track.uri) - if self.random and self._shuffled: + if self.random and tl_track in self._shuffled: self._shuffled.remove(tl_track) def mark_played(self, tl_track): From 83db750e0a1071d54b7c8d45e9114d9ad343e84e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 15:12:58 +0200 Subject: [PATCH 39/42] core.tracklist: Formatting --- mopidy/core/tracklist.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index aedc2785..5d85e190 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -133,6 +133,8 @@ class TracklistController(object): Playback continues after current song. """ + ### Methods + def index(self, tl_track): """ The position of the given track in the tracklist. @@ -173,6 +175,7 @@ class TracklistController(object): return self.tl_tracks[0] position = self.index(tl_track) + if self.repeat and self.single: return self.tl_tracks[position] @@ -216,6 +219,7 @@ class TracklistController(object): return self.tl_tracks[0] position = self.index(tl_track) + if self.repeat: return self.tl_tracks[(position + 1) % len(self.tl_tracks)] @@ -241,6 +245,7 @@ class TracklistController(object): return tl_track position = self.index(tl_track) + if position in (None, 0): return None @@ -452,6 +457,7 @@ class TracklistController(object): def _trigger_tracklist_changed(self): self._first_shuffle = True self._shuffled = [] + logger.debug('Triggering event: tracklist_changed()') listener.CoreListener.send('tracklist_changed') From d7552b2fe2b5f3adb86fd25db37575474c400927 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 15:08:32 +0200 Subject: [PATCH 40/42] tests: Formatting --- tests/backends/local/playback_test.py | 66 +++++++++++++-------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/tests/backends/local/playback_test.py b/tests/backends/local/playback_test.py index 662d2772..67b43374 100644 --- a/tests/backends/local/playback_test.py +++ b/tests/backends/local/playback_test.py @@ -278,15 +278,15 @@ class LocalPlaybackControllerTest(unittest.TestCase): @populate_tracklist def test_next_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[0]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_next_track_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[1]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_next_track_after_previous(self): @@ -294,13 +294,12 @@ class LocalPlaybackControllerTest(unittest.TestCase): self.playback.next() self.playback.previous() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[1]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[1]) def test_next_track_empty_playlist(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_next_track_at_end_of_playlist(self): @@ -308,8 +307,7 @@ class LocalPlaybackControllerTest(unittest.TestCase): for _ in self.tracklist.tl_tracks[1:]: self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - None) + self.assertEqual(self.tracklist.next_track(tl_track), None) @populate_tracklist def test_next_track_at_end_of_playlist_with_repeat(self): @@ -318,16 +316,16 @@ class LocalPlaybackControllerTest(unittest.TestCase): for _ in self.tracks[1:]: self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[0]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_next_track_with_random(self): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[2]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[2]) @populate_tracklist def test_next_with_consume(self): @@ -362,8 +360,8 @@ class LocalPlaybackControllerTest(unittest.TestCase): self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[1]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_end_of_track(self): @@ -435,15 +433,15 @@ class LocalPlaybackControllerTest(unittest.TestCase): @populate_tracklist def test_end_of_track_track_before_play(self): tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[0]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_during_play(self): self.playback.play() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[1]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_end_of_track_track_after_previous(self): @@ -451,8 +449,8 @@ class LocalPlaybackControllerTest(unittest.TestCase): self.playback.on_end_of_track() self.playback.previous() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[1]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[1]) def test_end_of_track_track_empty_playlist(self): tl_track = self.playback.current_tl_track @@ -473,16 +471,16 @@ class LocalPlaybackControllerTest(unittest.TestCase): for _ in self.tracks[1:]: self.playback.on_end_of_track() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[0]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_end_of_track_track_with_random(self): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[2]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[2]) @populate_tracklist def test_end_of_track_with_consume(self): @@ -505,12 +503,12 @@ class LocalPlaybackControllerTest(unittest.TestCase): random.seed(1) self.tracklist.random = True tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[2]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[2]) self.tracklist.add(self.tracks[:1]) tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.next_track(tl_track), - self.tl_tracks[1]) + self.assertEqual( + self.tracklist.next_track(tl_track), self.tl_tracks[1]) @populate_tracklist def test_previous_track_before_play(self): @@ -528,8 +526,8 @@ class LocalPlaybackControllerTest(unittest.TestCase): self.playback.play() self.playback.next() tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), - self.tl_tracks[0]) + self.assertEqual( + self.tracklist.previous_track(tl_track), self.tl_tracks[0]) @populate_tracklist def test_previous_track_after_previous(self): @@ -538,8 +536,8 @@ class LocalPlaybackControllerTest(unittest.TestCase): self.playback.next() # At track 2 self.playback.previous() # At track 1 tl_track = self.playback.current_tl_track - self.assertEqual(self.tracklist.previous_track(tl_track), - self.tl_tracks[0]) + self.assertEqual( + self.tracklist.previous_track(tl_track), self.tl_tracks[0]) def test_previous_track_empty_playlist(self): tl_track = self.playback.current_tl_track From 033e3ab813639b788efc9805f3f06f292886f408 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 20:02:26 +0200 Subject: [PATCH 41/42] jsonrpc: Test application error responses again The test was modified to not fail after a refactoring, making it not test what it was intended to test at all. This reverts the changes and updates the test in another way, keeping the original intention. --- tests/utils/jsonrpc_test.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/utils/jsonrpc_test.py b/tests/utils/jsonrpc_test.py index 7abaa512..c6f516bb 100644 --- a/tests/utils/jsonrpc_test.py +++ b/tests/utils/jsonrpc_test.py @@ -34,6 +34,9 @@ class Calculator(object): def _secret(self): return 'Grand Unified Theory' + def fail(self): + raise ValueError('What did you expect?') + class JsonRpcTestBase(unittest.TestCase): def setUp(self): @@ -313,19 +316,26 @@ class JsonRpcBatchTest(JsonRpcTestBase): class JsonRpcSingleCommandErrorTest(JsonRpcTestBase): - def test_application_error_response_is_none(self): + def test_application_error_response(self): request = { 'jsonrpc': '2.0', - 'method': 'core.tracklist.index', - 'params': ['bogus'], + 'method': 'calc.fail', + 'params': [], 'id': 1, } response = self.jrw.handle_data(request) - self.assertIn('result', response) + self.assertNotIn('result', response) - result = response['result'] - self.assertEqual(result, None) + error = response['error'] + self.assertEqual(error['code'], 0) + self.assertEqual(error['message'], 'Application error') + + data = error['data'] + self.assertEqual(data['type'], 'ValueError') + self.assertIn('What did you expect?', data['message']) + self.assertIn('traceback', data) + self.assertIn('Traceback (most recent call last):', data['traceback']) def test_missing_jsonrpc_member_causes_invalid_request_error(self): request = { From 345be9d3edad5467f2d08d1e712d6b1a21da42a6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 19 Oct 2013 13:00:41 +0200 Subject: [PATCH 42/42] docs: Update changelog for tidy-up-core changes --- docs/changelog.rst | 63 ++++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d28e41bc..1cbad579 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -39,6 +39,45 @@ of the following extensions as well: **Core** +- Parts of the functionality in :class:`mopidy.core.PlaybackController` have + been moved to :class:`mopidy.core.TracklistController`: + + =================================== ================================== + Old location New location + =================================== ================================== + playback.get_consume() tracklist.get_consume() + playback.set_consume(v) tracklist.set_consume(v) + playback.consume tracklist.consume + + playback.get_random() tracklist.get_random() + playback.set_random(v) tracklist.set_random(v) + playback.random tracklist.random + + playback.get_repeat() tracklist.get_repeat() + playback.set_repeat(v) tracklist.set_repeat(v) + playback.repeat tracklist.repeat + + playback.get_single() tracklist.get_single() + playback.set_single(v) tracklist.set_single(v) + playback.single tracklist.single + + playback.get_tracklist_position() tracklist.index(tl_track) + playback.tracklist_position tracklist.index(tl_track) + + playback.get_tl_track_at_eot() tracklist.eot_track(tl_track) + playback.tl_track_at_eot tracklist.eot_track(tl_track) + + playback.get_tl_track_at_next() tracklist.next_track(tl_track) + playback.tl_track_at_next tracklist.next_track(tl_track) + + playback.get_tl_track_at_previous() tracklist.previous_track(tl_track) + playback.tl_track_at_previous tracklist.previous_track(tl_track) + =================================== ================================== + + The ``tl_track`` argument to the last four new functions are used as the + reference ``tl_track`` in the tracklist to find e.g. the next track. Usually, + this will be :attr:`~mopidy.core.PlaybackController.current_tl_track`. + - Added :attr:`mopidy.core.PlaybackController.mute` for muting and unmuting audio. (Fixes: :issue:`186`) @@ -68,30 +107,6 @@ A release with a number of small and medium fixes, with no specific focus. supported by Mopidy is Python 2.7. We're continuously working towards running Mopidy on Python 3. (Fixes: :issue:`344`) -**Core** - -- Tracklist has now the power to make decisions on which is the next/previous - song, along with previously playback associated features, such as randomness, - consumption, repeat and single. For that, a new method has been created to - mark songs, some Playback properties have been converted into functions and - both functions and properties have been moved into Tracklist to have more - modularity: - - - Properties merged into functions: :attr:`tracklist_position` merged to - :meth:`index` - - - Properties moved: :attr:`random`, :attr:`repeat`, :attr:`consume` and - :attr:`single` - - - Method created from properties: :meth:`next_track` from - `tl_track_at_next`, :meth:`eot_track` from `tl_track_at_eot` and - :meth:`previous_track` from `tl_track_at_previous` - - - Method created to separe functionality: :meth:`mark` - -- Tracklist's get_tl_track_at_* and tracklist_position now need a tl_track as a - reference to give an answer. - **Command line options** - Converted from the optparse to the argparse library for handling command line