From 2b00e831791cbab26c9aa19e28a42be6f7abdfa4 Mon Sep 17 00:00:00 2001 From: jcass Date: Sun, 6 Dec 2015 16:01:26 +0200 Subject: [PATCH 1/4] Mark track as playing and add to history if changing track while paused. --- docs/changelog.rst | 3 +++ mopidy/core/playback.py | 12 +++++++---- tests/core/test_playback.py | 43 ++++++++++++++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 7637de58..29f7ef82 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,6 +19,9 @@ Bug fix release. - MPD: Notify idling clients when a seek is performed. (Fixes: :issue:`1331`) +- Core: Fix error in :meth:`~mopidy.core.PlaybackController._change_track` + docstring. (Fixes: :issue:`1352`) + v1.1.1 (2015-09-14) =================== diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 9a11066b..e3a4f68d 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -207,12 +207,16 @@ class PlaybackController(object): if old_state == PlaybackState.PLAYING: self._play(on_error_step=on_error_step) elif old_state == PlaybackState.PAUSED: - # NOTE: this is just a quick hack to fix #1177 as this code has - # already been killed in the gapless branch. + # NOTE: this is just a quick hack to fix #1177 and #1352 as this + # code has already been killed in the gapless branch. backend = self._get_backend() if backend: - backend.playback.prepare_change() - backend.playback.change_track(tl_track.track).get() + success = ( + backend.playback.prepare_change().get and + backend.playback.change_track(tl_track.track).get()) + if success: + self.core.tracklist._mark_playing(tl_track) + self.core.history._add_track(tl_track.track) self.pause() # TODO: this is not really end of track, this is on_need_next_track diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 5a8c9649..6bacb3ed 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -8,7 +8,7 @@ import pykka from mopidy import backend, core from mopidy.internal import deprecation -from mopidy.models import Track +from mopidy.models import TlTrack, Track from tests import dummy_audio as audio @@ -789,3 +789,44 @@ class Bug1177RegressionTest(unittest.TestCase): c.playback.pause() c.playback.next() b.playback.change_track.assert_called_once_with(track2) + + +class Bug1352RegressionTest(unittest.TestCase): + def test(self): + config = { + 'core': { + 'max_tracklist_length': 10000, + } + } + + b = mock.Mock() + b.uri_schemes.get.return_value = ['dummy'] + b.playback = mock.Mock(spec=backend.PlaybackProvider) + b.playback.change_track.return_value.get.return_value = True + b.playback.play.return_value.get.return_value = True + + track1 = Track(uri='dummy:a', length=40000) + track2 = Track(uri='dummy:b', length=40000) + + tl_track2 = TlTrack(1, track2) + + c = core.Core(config, mixer=None, backends=[b]) + c.tracklist.add([track1, track2]) + + d = mock.Mock() + c.history._add_track = d + + e = mock.Mock() + c.tracklist._mark_playing = e + + c.playback.play() + b.playback.change_track.reset_mock() + d.reset_mock() + e.reset_mock() + + c.playback.pause() + c.playback.next() + b.playback.change_track.assert_called_once_with(track2) + + d.assert_called_once_with(track2) + e.assert_called_once_with(tl_track2) From 139634b93bd0886a4245b4286923c406ccefc595 Mon Sep 17 00:00:00 2001 From: jcass Date: Sun, 6 Dec 2015 19:27:02 +0200 Subject: [PATCH 2/4] Update changelog. --- docs/changelog.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 29f7ef82..7d901aef 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,8 +19,9 @@ Bug fix release. - MPD: Notify idling clients when a seek is performed. (Fixes: :issue:`1331`) -- Core: Fix error in :meth:`~mopidy.core.PlaybackController._change_track` - docstring. (Fixes: :issue:`1352`) +- Core: Fix :meth:`~mopidy.core.PlaybackController._change_track` to mark + track as playing and add it to the history if changing track while paused. + (Fixes: :issue:`1352`) v1.1.1 (2015-09-14) From fb7b466bee23674f6d6bd97391ee5222f1daf580 Mon Sep 17 00:00:00 2001 From: jcass Date: Mon, 7 Dec 2015 07:53:34 +0200 Subject: [PATCH 3/4] Result of prepare_change no longer affects whether a track is added to the history. Update changelog and test cases. --- docs/changelog.rst | 6 +++--- mopidy/core/playback.py | 5 ++--- tests/core/test_playback.py | 15 ++++++--------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 7d901aef..41d5cccc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,9 +19,9 @@ Bug fix release. - MPD: Notify idling clients when a seek is performed. (Fixes: :issue:`1331`) -- Core: Fix :meth:`~mopidy.core.PlaybackController._change_track` to mark - track as playing and add it to the history if changing track while paused. - (Fixes: :issue:`1352`) +- Core: Fix bug in playback controller. If changing to another track while + the player is paused, the new track would not be added to the history or + marked as currently playing. (Fixes: :issue:`1352`) v1.1.1 (2015-09-14) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index e3a4f68d..eeba5106 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -211,9 +211,8 @@ class PlaybackController(object): # code has already been killed in the gapless branch. backend = self._get_backend() if backend: - success = ( - backend.playback.prepare_change().get and - backend.playback.change_track(tl_track.track).get()) + backend.playback.prepare_change() + success = backend.playback.change_track(tl_track.track).get() if success: self.core.tracklist._mark_playing(tl_track) self.core.history._add_track(tl_track.track) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 6bacb3ed..f7b6dfda 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -813,20 +813,17 @@ class Bug1352RegressionTest(unittest.TestCase): c = core.Core(config, mixer=None, backends=[b]) c.tracklist.add([track1, track2]) - d = mock.Mock() - c.history._add_track = d + c.history._add_track = mock.PropertyMock() + c.tracklist._mark_playing = mock.PropertyMock() - e = mock.Mock() - c.tracklist._mark_playing = e c.playback.play() b.playback.change_track.reset_mock() - d.reset_mock() - e.reset_mock() + c.history._add_track.reset_mock() + c.tracklist._mark_playing.reset_mock() c.playback.pause() c.playback.next() b.playback.change_track.assert_called_once_with(track2) - - d.assert_called_once_with(track2) - e.assert_called_once_with(tl_track2) + c.history._add_track.assert_called_once_with(track2) + c.tracklist._mark_playing.assert_called_once_with(tl_track2) \ No newline at end of file From 3cd3b45512d8ff06dafaecd3b7ef2934e60495d5 Mon Sep 17 00:00:00 2001 From: jcass Date: Mon, 7 Dec 2015 07:56:35 +0200 Subject: [PATCH 4/4] Fix flake8 errors. --- tests/core/test_playback.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index f7b6dfda..61dfbb15 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -816,7 +816,6 @@ class Bug1352RegressionTest(unittest.TestCase): c.history._add_track = mock.PropertyMock() c.tracklist._mark_playing = mock.PropertyMock() - c.playback.play() b.playback.change_track.reset_mock() c.history._add_track.reset_mock() @@ -826,4 +825,4 @@ class Bug1352RegressionTest(unittest.TestCase): c.playback.next() b.playback.change_track.assert_called_once_with(track2) c.history._add_track.assert_called_once_with(track2) - c.tracklist._mark_playing.assert_called_once_with(tl_track2) \ No newline at end of file + c.tracklist._mark_playing.assert_called_once_with(tl_track2)