From 7e66b719ea2542a067a6108a1a15a9bc9d09c048 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 26 Mar 2015 21:54:23 +0100 Subject: [PATCH 01/25] audio: pipeline.add_many() is deprecated --- mopidy/audio/scan.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 3880d91a..4234e748 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -73,7 +73,8 @@ def _setup_pipeline(uri, proxy_config=None): sink = gst.element_factory_make('fakesink') pipeline = gst.element_factory_make('pipeline') - pipeline.add_many(src, typefind, decodebin, sink) + for e in (src, typefind, decodebin, sink): + pipeline.add(e) gst.element_link_many(src, typefind, decodebin) if proxy_config: From b9d7ea37bee64aec523eb6eaca53e6f3107439fa Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 26 Mar 2015 21:54:46 +0100 Subject: [PATCH 02/25] commands: Exception.message is deprecated --- mopidy/commands.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mopidy/commands.py b/mopidy/commands.py index ebb2c891..dd91f5de 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -38,7 +38,8 @@ def config_override_type(value): class _ParserError(Exception): - pass + def __init__(self, message): + self.message = message class _HelpError(Exception): From b31f0c421f50dd9ea619c2d6c751646b05a1381f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 26 Mar 2015 21:58:44 +0100 Subject: [PATCH 03/25] tests: Make tests warning safe --- tests/core/test_events.py | 6 +++++- tests/mpd/protocol/__init__.py | 8 ++++++-- tests/mpd/protocol/test_current_playlist.py | 7 ++++++- tests/mpd/protocol/test_playback.py | 15 +++++++++------ tests/mpd/test_dispatcher.py | 6 +++++- tests/mpd/test_exceptions.py | 9 --------- tests/mpd/test_status.py | 9 +++++++-- tests/utils/test_jsonrpc.py | 6 +++++- 8 files changed, 43 insertions(+), 23 deletions(-) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index 942f9b5f..a197972b 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import mock @@ -16,7 +17,10 @@ from tests import dummy_backend class BackendEventsTest(unittest.TestCase): def setUp(self): # noqa: N802 self.backend = dummy_backend.create_proxy() - self.core = core.Core.start(backends=[self.backend]).proxy() + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + self.core = core.Core.start(backends=[self.backend]).proxy() def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() diff --git a/tests/mpd/protocol/__init__.py b/tests/mpd/protocol/__init__.py index 88e3567b..cbbc1991 100644 --- a/tests/mpd/protocol/__init__.py +++ b/tests/mpd/protocol/__init__.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import mock @@ -40,8 +41,11 @@ class BaseTestCase(unittest.TestCase): else: self.mixer = None self.backend = dummy_backend.create_proxy() - self.core = core.Core.start( - mixer=self.mixer, backends=[self.backend]).proxy() + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + self.core = core.Core.start( + mixer=self.mixer, backends=[self.backend]).proxy() self.uri_map = uri_mapper.MpdUriMapper(self.core) self.connection = MockConnection() diff --git a/tests/mpd/protocol/test_current_playlist.py b/tests/mpd/protocol/test_current_playlist.py index d6fdce8e..c96febb7 100644 --- a/tests/mpd/protocol/test_current_playlist.py +++ b/tests/mpd/protocol/test_current_playlist.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, unicode_literals +import warnings + from mopidy.models import Ref, Track from tests.mpd import protocol @@ -247,7 +249,10 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): 'ACK [50@0] {moveid} No such song') def test_playlist_returns_same_as_playlistinfo(self): - playlist_response = self.send_request('playlist') + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', message='.*playlistinfo.*') + playlist_response = self.send_request('playlist') + playlistinfo_response = self.send_request('playlistinfo') self.assertEqual(playlist_response, playlistinfo_response) diff --git a/tests/mpd/protocol/test_playback.py b/tests/mpd/protocol/test_playback.py index 22527e1e..8bac48cc 100644 --- a/tests/mpd/protocol/test_playback.py +++ b/tests/mpd/protocol/test_playback.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings from mopidy.core import PlaybackState from mopidy.models import Track @@ -200,13 +201,15 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') - self.send_request('pause') - self.assertEqual(PAUSED, self.core.playback.state.get()) - self.assertInResponse('OK') + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', message='.*pause command w/o.*') + self.send_request('pause') + self.assertEqual(PAUSED, self.core.playback.state.get()) + self.assertInResponse('OK') - self.send_request('pause') - self.assertEqual(PLAYING, self.core.playback.state.get()) - self.assertInResponse('OK') + self.send_request('pause') + self.assertEqual(PLAYING, self.core.playback.state.get()) + self.assertInResponse('OK') def test_play_without_pos(self): self.core.tracklist.add([Track(uri='dummy:a')]) diff --git a/tests/mpd/test_dispatcher.py b/tests/mpd/test_dispatcher.py index d6b11e43..2c21df67 100644 --- a/tests/mpd/test_dispatcher.py +++ b/tests/mpd/test_dispatcher.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import pykka @@ -20,9 +21,12 @@ class MpdDispatcherTest(unittest.TestCase): } } self.backend = dummy_backend.create_proxy() - self.core = core.Core.start(backends=[self.backend]).proxy() self.dispatcher = MpdDispatcher(config=config) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + self.core = core.Core.start(backends=[self.backend]).proxy() + def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() diff --git a/tests/mpd/test_exceptions.py b/tests/mpd/test_exceptions.py index 7bb64096..123bae5d 100644 --- a/tests/mpd/test_exceptions.py +++ b/tests/mpd/test_exceptions.py @@ -8,15 +8,6 @@ from mopidy.mpd.exceptions import ( class MpdExceptionsTest(unittest.TestCase): - def test_key_error_wrapped_in_mpd_ack_error(self): - try: - try: - raise KeyError('Track X not found') - except KeyError as e: - raise MpdAckError(e.message) - except MpdAckError as e: - self.assertEqual(e.message, 'Track X not found') - def test_mpd_not_implemented_is_a_mpd_ack_error(self): try: raise MpdNotImplemented diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py index e130353b..89030651 100644 --- a/tests/mpd/test_status.py +++ b/tests/mpd/test_status.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import pykka @@ -25,8 +26,12 @@ class StatusHandlerTest(unittest.TestCase): def setUp(self): # noqa: N802 self.mixer = dummy_mixer.create_proxy() self.backend = dummy_backend.create_proxy() - self.core = core.Core.start( - mixer=self.mixer, backends=[self.backend]).proxy() + + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + self.core = core.Core.start( + mixer=self.mixer, backends=[self.backend]).proxy() + self.dispatcher = dispatcher.MpdDispatcher(core=self.core) self.context = self.dispatcher.context diff --git a/tests/utils/test_jsonrpc.py b/tests/utils/test_jsonrpc.py index fb59d06b..890c2aba 100644 --- a/tests/utils/test_jsonrpc.py +++ b/tests/utils/test_jsonrpc.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import json import unittest +import warnings import mock @@ -52,9 +53,12 @@ class Calculator(object): class JsonRpcTestBase(unittest.TestCase): def setUp(self): # noqa: N802 self.backend = dummy_backend.create_proxy() - self.core = core.Core.start(backends=[self.backend]).proxy() self.calc = Calculator() + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + self.core = core.Core.start(backends=[self.backend]).proxy() + self.jrw = jsonrpc.JsonRpcWrapper( objects={ 'hello': lambda: 'Hello, world!', From 5a3fb64250accca6dddb2dbcf480881b28e253f5 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 26 Mar 2015 22:45:22 +0100 Subject: [PATCH 04/25] core: Emit deprecation warning for library.find_exact --- mopidy/core/library.py | 2 + mopidy/mpd/protocol/music_db.py | 6 +- tests/core/test_library.py | 164 ++++++++++++++++++-------------- tests/local/test_library.py | 130 +++++++++++++------------ 4 files changed, 166 insertions(+), 136 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 89a2037a..9aaa386e 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -4,6 +4,7 @@ import collections import logging import operator import urlparse +import warnings import pykka @@ -132,6 +133,7 @@ class LibraryController(object): .. deprecated:: 1.0 Use :meth:`search` with ``exact`` set. """ + warnings.warn('library.find_exact() is deprecated', DeprecationWarning) return self.search(query=query, uris=uris, exact=True, **kwargs) def lookup(self, uri=None, uris=None): diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index a942abf5..59a1f9b6 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -100,7 +100,7 @@ def count(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: raise exceptions.MpdArgError('incorrect arguments') - results = context.core.library.find_exact(**query).get() + results = context.core.library.search(query=query, exact=True).get() result_tracks = _get_tracks(results) return [ ('songs', len(result_tracks)), @@ -141,7 +141,7 @@ def find(context, *args): except ValueError: return - results = context.core.library.find_exact(**query).get() + results = context.core.library.search(query=query, exact=True).get() result_tracks = [] if ('artist' not in query and 'albumartist' not in query and @@ -168,7 +168,7 @@ def findadd(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: return - results = context.core.library.find_exact(**query).get() + results = context.core.library.search(query=query, exact=True).get() context.core.tracklist.add(_get_tracks(results)) diff --git a/tests/core/test_library.py b/tests/core/test_library.py index 51313daa..4a96042d 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import mock @@ -206,75 +207,6 @@ class CoreLibraryTest(unittest.TestCase): self.library1.refresh.assert_called_once_with(None) self.library2.refresh.assert_called_twice_with(None) - def test_find_exact_combines_results_from_all_backends(self): - track1 = Track(uri='dummy1:a') - track2 = Track(uri='dummy2:a') - result1 = SearchResult(tracks=[track1]) - result2 = SearchResult(tracks=[track2]) - - self.library1.search.return_value.get.return_value = result1 - self.library2.search.return_value.get.return_value = result2 - - result = self.core.library.find_exact(any=['a']) - - self.assertIn(result1, result) - self.assertIn(result2, result) - self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) - self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) - - def test_find_exact_with_uris_selects_dummy1_backend(self): - self.core.library.find_exact( - any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy3:']) - - self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=True) - self.assertFalse(self.library2.search.called) - - def test_find_exact_with_uris_selects_both_backends(self): - self.core.library.find_exact( - any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy2:']) - - self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=True) - self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy2:'], exact=True) - - def test_find_exact_filters_out_none(self): - track1 = Track(uri='dummy1:a') - result1 = SearchResult(tracks=[track1]) - - self.library1.search.return_value.get.return_value = result1 - self.library2.search.return_value.get.return_value = None - - result = self.core.library.find_exact(any=['a']) - - self.assertIn(result1, result) - self.assertNotIn(None, result) - self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) - self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) - - def test_find_accepts_query_dict_instead_of_kwargs(self): - track1 = Track(uri='dummy1:a') - track2 = Track(uri='dummy2:a') - result1 = SearchResult(tracks=[track1]) - result2 = SearchResult(tracks=[track2]) - - self.library1.search.return_value.get.return_value = result1 - self.library2.search.return_value.get.return_value = result2 - - result = self.core.library.find_exact(dict(any=['a'])) - - self.assertIn(result1, result) - self.assertIn(result2, result) - self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) - self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) - def test_search_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') track2 = Track(uri='dummy2:a') @@ -355,8 +287,90 @@ class CoreLibraryTest(unittest.TestCase): self.library1.search.assert_called_once_with( query={'any': ['foobar']}, uris=None, exact=False) + +class DeprecatedCoreLibraryTest(CoreLibraryTest): + def setUp(self): # noqa: N802 + super(DeprecatedCoreLibraryTest, self).setUp() + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', '.*library.find_exact.*') + + def tearDown(self): # noqa: N802 + super(DeprecatedCoreLibraryTest, self).tearDown() + warnings.filters = self._warnings_filters + + def test_find_exact_combines_results_from_all_backends(self): + track1 = Track(uri='dummy1:a') + track2 = Track(uri='dummy2:a') + result1 = SearchResult(tracks=[track1]) + result2 = SearchResult(tracks=[track2]) + + self.library1.search.return_value.get.return_value = result1 + self.library2.search.return_value.get.return_value = result2 + + result = self.core.library.find_exact(any=['a']) + + self.assertIn(result1, result) + self.assertIn(result2, result) + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=None, exact=True) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=None, exact=True) + + def test_find_exact_with_uris_selects_dummy1_backend(self): + self.core.library.find_exact( + any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy3:']) + + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=True) + self.assertFalse(self.library2.search.called) + + def test_find_exact_with_uris_selects_both_backends(self): + self.core.library.find_exact( + any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy2:']) + + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=True) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=['dummy2:'], exact=True) + + def test_find_exact_filters_out_none(self): + track1 = Track(uri='dummy1:a') + result1 = SearchResult(tracks=[track1]) + + self.library1.search.return_value.get.return_value = result1 + self.library2.search.return_value.get.return_value = None + + result = self.core.library.find_exact(any=['a']) + + self.assertIn(result1, result) + self.assertNotIn(None, result) + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=None, exact=True) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=None, exact=True) + + def test_find_accepts_query_dict_instead_of_kwargs(self): + track1 = Track(uri='dummy1:a') + track2 = Track(uri='dummy2:a') + result1 = SearchResult(tracks=[track1]) + result2 = SearchResult(tracks=[track2]) + + self.library1.search.return_value.get.return_value = result1 + self.library2.search.return_value.get.return_value = result2 + + result = self.core.library.find_exact(dict(any=['a'])) + + self.assertIn(result1, result) + self.assertIn(result2, result) + self.library1.search.assert_called_once_with( + query=dict(any=['a']), uris=None, exact=True) + self.library2.search.assert_called_once_with( + query=dict(any=['a']), uris=None, exact=True) + def test_find_exact_normalises_bad_queries(self): self.core.library.find_exact({'any': 'foobar'}) + self.library1.search.assert_called_once_with( query={'any': ['foobar']}, uris=None, exact=True) @@ -369,8 +383,18 @@ class LegacyFindExactToSearchLibraryTest(unittest.TestCase): self.backend.library = mock.Mock(spec=backend.LibraryProvider) self.core = core.Core(mixer=None, backends=[self.backend]) + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', '.*library.find_exact.*') + + def tearDown(self): # noqa: N802 + warnings.filters = self._warnings_filters + def test_core_find_exact_calls_backend_search_with_exact(self): - self.core.library.find_exact(query={'any': ['a']}) + with warnings.catch_warnings(): + warnings.simplefilter('ignore') + self.core.library.find_exact(query={'any': ['a']}) + self.backend.library.search.assert_called_once_with( query=dict(any=['a']), uris=None, exact=True) diff --git a/tests/local/test_library.py b/tests/local/test_library.py index 39f0e53e..7ab67fa6 100644 --- a/tests/local/test_library.py +++ b/tests/local/test_library.py @@ -84,6 +84,10 @@ class LocalLibraryProviderTest(unittest.TestCase): pykka.ActorRegistry.stop_all() actor.LocalBackend.libraries = [] + def find_exact(self, **query): + # TODO: remove this helper? + return self.library.search(query=query, exact=True) + def test_refresh(self): self.library.refresh() @@ -149,228 +153,228 @@ class LocalLibraryProviderTest(unittest.TestCase): # TODO: move to search_test module def test_find_exact_no_hits(self): - result = self.library.find_exact(track_name=['unknown track']) + result = self.find_exact(track_name=['unknown track']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(artist=['unknown artist']) + result = self.find_exact(artist=['unknown artist']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(albumartist=['unknown albumartist']) + result = self.find_exact(albumartist=['unknown albumartist']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(composer=['unknown composer']) + result = self.find_exact(composer=['unknown composer']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(performer=['unknown performer']) + result = self.find_exact(performer=['unknown performer']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(album=['unknown album']) + result = self.find_exact(album=['unknown album']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(date=['1990']) + result = self.find_exact(date=['1990']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(genre=['unknown genre']) + result = self.find_exact(genre=['unknown genre']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(track_no=['9']) + result = self.find_exact(track_no=['9']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(track_no=['no_match']) + result = self.find_exact(track_no=['no_match']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(comment=['fake comment']) + result = self.find_exact(comment=['fake comment']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(uri=['fake uri']) + result = self.find_exact(uri=['fake uri']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(any=['unknown any']) + result = self.find_exact(any=['unknown any']) self.assertEqual(list(result[0].tracks), []) def test_find_exact_uri(self): track_1_uri = 'local:track:path1' - result = self.library.find_exact(uri=track_1_uri) + result = self.find_exact(uri=track_1_uri) self.assertEqual(list(result[0].tracks), self.tracks[:1]) track_2_uri = 'local:track:path2' - result = self.library.find_exact(uri=track_2_uri) + result = self.find_exact(uri=track_2_uri) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_track_name(self): - result = self.library.find_exact(track_name=['track1']) + result = self.find_exact(track_name=['track1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(track_name=['track2']) + result = self.find_exact(track_name=['track2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_artist(self): - result = self.library.find_exact(artist=['artist1']) + result = self.find_exact(artist=['artist1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(artist=['artist2']) + result = self.find_exact(artist=['artist2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) - result = self.library.find_exact(artist=['artist3']) + result = self.find_exact(artist=['artist3']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) def test_find_exact_composer(self): - result = self.library.find_exact(composer=['artist5']) + result = self.find_exact(composer=['artist5']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) - result = self.library.find_exact(composer=['artist6']) + result = self.find_exact(composer=['artist6']) self.assertEqual(list(result[0].tracks), []) def test_find_exact_performer(self): - result = self.library.find_exact(performer=['artist6']) + result = self.find_exact(performer=['artist6']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) - result = self.library.find_exact(performer=['artist5']) + result = self.find_exact(performer=['artist5']) self.assertEqual(list(result[0].tracks), []) def test_find_exact_album(self): - result = self.library.find_exact(album=['album1']) + result = self.find_exact(album=['album1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(album=['album2']) + result = self.find_exact(album=['album2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_albumartist(self): # Artist is both track artist and album artist - result = self.library.find_exact(albumartist=['artist1']) + result = self.find_exact(albumartist=['artist1']) self.assertEqual(list(result[0].tracks), [self.tracks[0]]) # Artist is both track and album artist - result = self.library.find_exact(albumartist=['artist2']) + result = self.find_exact(albumartist=['artist2']) self.assertEqual(list(result[0].tracks), [self.tracks[1]]) # Artist is just album artist - result = self.library.find_exact(albumartist=['artist3']) + result = self.find_exact(albumartist=['artist3']) self.assertEqual(list(result[0].tracks), [self.tracks[2]]) def test_find_exact_track_no(self): - result = self.library.find_exact(track_no=['1']) + result = self.find_exact(track_no=['1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(track_no=['2']) + result = self.find_exact(track_no=['2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_genre(self): - result = self.library.find_exact(genre=['genre1']) + result = self.find_exact(genre=['genre1']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) - result = self.library.find_exact(genre=['genre2']) + result = self.find_exact(genre=['genre2']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) def test_find_exact_date(self): - result = self.library.find_exact(date=['2001']) + result = self.find_exact(date=['2001']) self.assertEqual(list(result[0].tracks), []) - result = self.library.find_exact(date=['2001-02-03']) + result = self.find_exact(date=['2001-02-03']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(date=['2002']) + result = self.find_exact(date=['2002']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_find_exact_comment(self): - result = self.library.find_exact( + result = self.find_exact( comment=['This is a fantastic track']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) - result = self.library.find_exact( + result = self.find_exact( comment=['This is a fantastic']) self.assertEqual(list(result[0].tracks), []) def test_find_exact_any(self): # Matches on track artist - result = self.library.find_exact(any=['artist1']) + result = self.find_exact(any=['artist1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(any=['artist2']) + result = self.find_exact(any=['artist2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) # Matches on track name - result = self.library.find_exact(any=['track1']) + result = self.find_exact(any=['track1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.find_exact(any=['track2']) + result = self.find_exact(any=['track2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) # Matches on track album - result = self.library.find_exact(any=['album1']) + result = self.find_exact(any=['album1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) # Matches on track album artists - result = self.library.find_exact(any=['artist3']) + result = self.find_exact(any=['artist3']) self.assertEqual(len(result[0].tracks), 2) self.assertIn(self.tracks[2], result[0].tracks) self.assertIn(self.tracks[3], result[0].tracks) # Matches on track composer - result = self.library.find_exact(any=['artist5']) + result = self.find_exact(any=['artist5']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) # Matches on track performer - result = self.library.find_exact(any=['artist6']) + result = self.find_exact(any=['artist6']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) # Matches on track genre - result = self.library.find_exact(any=['genre1']) + result = self.find_exact(any=['genre1']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) - result = self.library.find_exact(any=['genre2']) + result = self.find_exact(any=['genre2']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) # Matches on track date - result = self.library.find_exact(any=['2002']) + result = self.find_exact(any=['2002']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) # Matches on track comment - result = self.library.find_exact( + result = self.find_exact( any=['This is a fantastic track']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) # Matches on URI - result = self.library.find_exact(any=['local:track:path1']) + result = self.find_exact(any=['local:track:path1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) def test_find_exact_wrong_type(self): with self.assertRaises(LookupError): - self.library.find_exact(wrong=['test']) + self.find_exact(wrong=['test']) def test_find_exact_with_empty_query(self): with self.assertRaises(LookupError): - self.library.find_exact(artist=['']) + self.find_exact(artist=['']) with self.assertRaises(LookupError): - self.library.find_exact(albumartist=['']) + self.find_exact(albumartist=['']) with self.assertRaises(LookupError): - self.library.find_exact(track_name=['']) + self.find_exact(track_name=['']) with self.assertRaises(LookupError): - self.library.find_exact(composer=['']) + self.find_exact(composer=['']) with self.assertRaises(LookupError): - self.library.find_exact(performer=['']) + self.find_exact(performer=['']) with self.assertRaises(LookupError): - self.library.find_exact(album=['']) + self.find_exact(album=['']) with self.assertRaises(LookupError): - self.library.find_exact(track_no=['']) + self.find_exact(track_no=['']) with self.assertRaises(LookupError): - self.library.find_exact(genre=['']) + self.find_exact(genre=['']) with self.assertRaises(LookupError): - self.library.find_exact(date=['']) + self.find_exact(date=['']) with self.assertRaises(LookupError): - self.library.find_exact(comment=['']) + self.find_exact(comment=['']) with self.assertRaises(LookupError): - self.library.find_exact(any=['']) + self.find_exact(any=['']) def test_search_no_hits(self): result = self.library.search(track_name=['unknown track']) From a54551d9855a00ced41dcddce94c54563bebbe17 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 26 Mar 2015 23:12:26 +0100 Subject: [PATCH 05/25] core: Mark get_playlists and filter as deprecated --- mopidy/core/playlists.py | 6 +++ tests/core/test_playlists.py | 94 +++++++++++++++++++++++------------- tests/m3u/test_playlists.py | 66 +++++++++++++++---------- 3 files changed, 106 insertions(+), 60 deletions(-) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 669e1f35..b6f2e726 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import logging import urlparse +import warnings import pykka @@ -80,6 +81,9 @@ class PlaylistsController(object): .. deprecated:: 1.0 Use :meth:`as_list` and :meth:`get_items` instead. """ + warnings.warn( + 'playlists.get_playlists() is deprecated', DeprecationWarning) + playlist_refs = self.as_list() if include_tracks: @@ -166,6 +170,8 @@ class PlaylistsController(object): .. deprecated:: 1.0 Use :meth:`as_list` and filter yourself. """ + warnings.warn('playlists.filter() is deprecated', DeprecationWarning) + criteria = criteria or kwargs matches = self.playlists for (key, value) in criteria.iteritems(): diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index 081f73e6..fa8c3531 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import mock @@ -83,30 +84,6 @@ class PlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.delete.called) self.assertFalse(self.sp2.delete.called) - def test_get_playlists_combines_result_from_backends(self): - result = self.core.playlists.get_playlists() - - self.assertIn(self.pl1a, result) - self.assertIn(self.pl1b, result) - self.assertIn(self.pl2a, result) - self.assertIn(self.pl2b, result) - - def test_get_playlists_includes_tracks_by_default(self): - result = self.core.playlists.get_playlists() - - self.assertEqual(result[0].name, 'A') - self.assertEqual(len(result[0].tracks), 1) - self.assertEqual(result[1].name, 'B') - self.assertEqual(len(result[1].tracks), 1) - - def test_get_playlist_can_strip_tracks_from_returned_playlists(self): - result = self.core.playlists.get_playlists(include_tracks=False) - - self.assertEqual(result[0].name, 'A') - self.assertEqual(len(result[0].tracks), 0) - self.assertEqual(result[1].name, 'B') - self.assertEqual(len(result[1].tracks), 0) - def test_create_without_uri_scheme_uses_first_backend(self): playlist = Playlist() self.sp1.create().get.return_value = playlist @@ -164,16 +141,6 @@ class PlaylistsTest(unittest.TestCase): self.assertFalse(self.sp1.delete.called) self.assertFalse(self.sp2.delete.called) - def test_filter_returns_matching_playlists(self): - result = self.core.playlists.filter(name='A') - - self.assertEqual(2, len(result)) - - def test_filter_accepts_dict_instead_of_kwargs(self): - result = self.core.playlists.filter({'name': 'A'}) - - self.assertEqual(2, len(result)) - def test_lookup_selects_the_dummy1_backend(self): self.core.playlists.lookup('dummy1:a') @@ -259,3 +226,62 @@ class PlaylistsTest(unittest.TestCase): self.assertIsNone(result) self.assertFalse(self.sp1.save.called) self.assertFalse(self.sp2.save.called) + + +class DeprecatedFilterPlaylistsTest(BasePlaylistsTest): + def setUp(self): # noqa: N802 + super(DeprecatedFilterPlaylistsTest, self).setUp() + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', '.*filter.*') + warnings.filterwarnings('ignore', '.*get_playlists.*') + + def tearDown(self): # noqa: N802 + super(DeprecatedFilterPlaylistsTest, self).tearDown() + warnings.filters = self._warnings_filters + + def test_filter_returns_matching_playlists(self): + result = self.core.playlists.filter(name='A') + + self.assertEqual(2, len(result)) + + def test_filter_accepts_dict_instead_of_kwargs(self): + result = self.core.playlists.filter({'name': 'A'}) + + self.assertEqual(2, len(result)) + + +class DeprecatedGetPlaylistsTest(BasePlaylistsTest): + def setUp(self): # noqa: N802 + super(DeprecatedGetPlaylistsTest, self).setUp() + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', '.*get_playlists.*') + + def tearDown(self): # noqa: N802 + super(DeprecatedGetPlaylistsTest, self).tearDown() + warnings.filters = self._warnings_filters + + def test_get_playlists_combines_result_from_backends(self): + result = self.core.playlists.get_playlists() + + self.assertIn(self.pl1a, result) + self.assertIn(self.pl1b, result) + self.assertIn(self.pl2a, result) + self.assertIn(self.pl2b, result) + + def test_get_playlists_includes_tracks_by_default(self): + result = self.core.playlists.get_playlists() + + self.assertEqual(result[0].name, 'A') + self.assertEqual(len(result[0].tracks), 1) + self.assertEqual(result[1].name, 'B') + self.assertEqual(len(result[1].tracks), 1) + + def test_get_playlist_can_strip_tracks_from_returned_playlists(self): + result = self.core.playlists.get_playlists(include_tracks=False) + + self.assertEqual(result[0].name, 'A') + self.assertEqual(len(result[0].tracks), 0) + self.assertEqual(result[1].name, 'B') + self.assertEqual(len(result[1].tracks), 0) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index 355aabf5..cf3265c3 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -4,6 +4,7 @@ import os import shutil import tempfile import unittest +import warnings import pykka @@ -141,8 +142,8 @@ class M3UPlaylistsProviderTest(unittest.TestCase): def test_create_adds_playlist_to_playlists_collection(self): playlist = self.core.playlists.create('test') - self.assert_(self.core.playlists.playlists) - self.assertIn(playlist, self.core.playlists.playlists) + playlists = self.core.playlists.as_list() + self.assertIn(playlist.uri, [ref.uri for ref in playlists]) def test_as_list_empty_to_start_with(self): self.assertEqual(len(self.core.playlists.as_list()), 0) @@ -171,30 +172,6 @@ class M3UPlaylistsProviderTest(unittest.TestCase): self.core.playlists.delete(playlist.uri) self.assertIsNone(self.core.playlists.lookup(playlist.uri)) - def test_filter_without_criteria(self): - self.assertEqual( - self.core.playlists.get_playlists(), self.core.playlists.filter()) - - def test_filter_with_wrong_criteria(self): - self.assertEqual([], self.core.playlists.filter(name='foo')) - - def test_filter_with_right_criteria(self): - playlist = self.core.playlists.create('test') - playlists = self.core.playlists.filter(name='test') - self.assertEqual([playlist], playlists) - - def test_filter_by_name_returns_single_match(self): - self.core.playlists.create('a') - playlist = self.core.playlists.create('b') - - self.assertEqual([playlist], self.core.playlists.filter(name='b')) - - def test_filter_by_name_returns_no_matches(self): - self.core.playlists.create('a') - self.core.playlists.create('b') - - self.assertEqual([], self.core.playlists.filter(name='c')) - def test_lookup_finds_playlist_by_uri(self): original_playlist = self.core.playlists.create('test') @@ -292,3 +269,40 @@ class M3UPlaylistsProviderTest(unittest.TestCase): item_refs = self.core.playlists.get_items('dummy:unknown') self.assertIsNone(item_refs) + + +class DeprecatedM3UPlaylistsProviderTest(M3UPlaylistsProviderTest): + def setUp(self): # noqa: N802 + super(DeprecatedM3UPlaylistsProviderTest, self).setUp() + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', '.*filter.*') + warnings.filterwarnings('ignore', '.*get_playlists.*') + + def tearDown(self): # noqa: N802 + super(DeprecatedM3UPlaylistsProviderTest, self).tearDown() + warnings.filters = self._warnings_filters + + def test_filter_without_criteria(self): + self.assertEqual(self.core.playlists.get_playlists(), + self.core.playlists.filter()) + + def test_filter_with_wrong_criteria(self): + self.assertEqual([], self.core.playlists.filter(name='foo')) + + def test_filter_with_right_criteria(self): + playlist = self.core.playlists.create('test') + playlists = self.core.playlists.filter(name='test') + self.assertEqual([playlist], playlists) + + def test_filter_by_name_returns_single_match(self): + self.core.playlists.create('a') + playlist = self.core.playlists.create('b') + + self.assertEqual([playlist], self.core.playlists.filter(name='b')) + + def test_filter_by_name_returns_no_matches(self): + self.core.playlists.create('a') + self.core.playlists.create('b') + + self.assertEqual([], self.core.playlists.filter(name='c')) From 447629cbf9ab6d2c0e50aa87ad16a9caa91531fc Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 26 Mar 2015 23:48:33 +0100 Subject: [PATCH 06/25] audio: Add deprecation warning to emit_end_of_stream --- mopidy/audio/actor.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index b4c78ecb..7b3e7985 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import logging import os +import warnings import gobject @@ -605,6 +606,8 @@ class Audio(pykka.ThreadingActor): .. deprecated:: 1.0 Use :meth:`emit_data` with a :class:`None` buffer instead. """ + warnings.warn('audio.emit_end_of_stream() is deprecated.', + DeprecationWarning) self._appsrc.push(None) def set_about_to_finish_callback(self, callback): From f5f9899db923f1cdc4e150dda59f6f87fc863cf7 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 27 Mar 2015 19:30:10 +0100 Subject: [PATCH 07/25] tests: Make bases test classes in core --- tests/core/test_library.py | 11 +++++++---- tests/core/test_playlists.py | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/core/test_library.py b/tests/core/test_library.py index 4a96042d..bebdd8a6 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -9,7 +9,7 @@ from mopidy import backend, core from mopidy.models import Image, Ref, SearchResult, Track -class CoreLibraryTest(unittest.TestCase): +class BaseCoreLibraryTest(unittest.TestCase): def setUp(self): # noqa: N802 dummy1_root = Ref.directory(uri='dummy1:directory', name='dummy1') self.backend1 = mock.Mock() @@ -38,6 +38,9 @@ class CoreLibraryTest(unittest.TestCase): self.core = core.Core(mixer=None, backends=[ self.backend1, self.backend2, self.backend3]) + +# TODO: split by method +class CoreLibraryTest(BaseCoreLibraryTest): def test_get_images_returns_empty_dict_for_no_uris(self): self.assertEqual({}, self.core.library.get_images([])) @@ -288,15 +291,15 @@ class CoreLibraryTest(unittest.TestCase): query={'any': ['foobar']}, uris=None, exact=False) -class DeprecatedCoreLibraryTest(CoreLibraryTest): +class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): def setUp(self): # noqa: N802 - super(DeprecatedCoreLibraryTest, self).setUp() + super(DeprecatedFindExactCoreLibraryTest, self).setUp() self._warnings_filters = warnings.filters warnings.filters = warnings.filters[:] warnings.filterwarnings('ignore', '.*library.find_exact.*') def tearDown(self): # noqa: N802 - super(DeprecatedCoreLibraryTest, self).tearDown() + super(DeprecatedFindExactCoreLibraryTest, self).tearDown() warnings.filters = self._warnings_filters def test_find_exact_combines_results_from_all_backends(self): diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index fa8c3531..dbe05733 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -9,7 +9,7 @@ from mopidy import backend, core from mopidy.models import Playlist, Ref, Track -class PlaylistsTest(unittest.TestCase): +class BasePlaylistsTest(unittest.TestCase): def setUp(self): # noqa: N802 self.plr1a = Ref.playlist(name='A', uri='dummy1:pl:a') self.plr1b = Ref.playlist(name='B', uri='dummy1:pl:b') @@ -50,6 +50,8 @@ class PlaylistsTest(unittest.TestCase): self.core = core.Core(mixer=None, backends=[ self.backend3, self.backend1, self.backend2]) + +class PlaylistTest(BasePlaylistsTest): def test_as_list_combines_result_from_backends(self): result = self.core.playlists.as_list() From 0ab52a73faa0feb274423dfd85f551a84046d319 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 27 Mar 2015 19:32:47 +0100 Subject: [PATCH 08/25] core: Mark library.lookup by uri deprecated Updates core, mpd and tests to not use deprecated calls or safely catch them when running with -W error. --- mopidy/core/library.py | 4 ++ mopidy/core/tracklist.py | 12 +++--- mopidy/mpd/dispatcher.py | 11 +++--- mopidy/mpd/protocol/current_playlist.py | 3 +- mopidy/mpd/protocol/music_db.py | 11 +++--- tests/core/test_library.py | 50 +++++++++++++++---------- tests/local/test_library.py | 9 +++-- 7 files changed, 60 insertions(+), 40 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 9aaa386e..38e231f1 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -162,6 +162,10 @@ class LibraryController(object): if none_set or both_set: raise ValueError("One of 'uri' or 'uris' must be set") + if uri: + warnings.warn('library.lookup() "uri" argument is deprecated.', + DeprecationWarning) + if uri is not None: uris = [uri] diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 9186ae42..51ce7fbf 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -334,12 +334,12 @@ class TracklistController(object): if tracks is None: if uri is not None: - tracks = self.core.library.lookup(uri=uri) - elif uris is not None: - tracks = [] - track_map = self.core.library.lookup(uris=uris) - for uri in uris: - tracks.extend(track_map[uri]) + uris = [uri] + + tracks = [] + track_map = self.core.library.lookup(uris=uris) + for uri in uris: + tracks.extend(track_map[uri]) tl_tracks = [] diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py index d156b891..4d1c6196 100644 --- a/mopidy/mpd/dispatcher.py +++ b/mopidy/mpd/dispatcher.py @@ -267,10 +267,10 @@ class MpdContext(object): given path. If ``lookup`` is true and the ``path`` is to a track, the returned - ``data`` is a future which will contain the - :class:`mopidy.models.Track` model. If ``lookup`` is false and the - ``path`` is to a track, the returned ``data`` will be a - :class:`mopidy.models.Ref` for the track. + ``data`` is a future which will contain the results from looking up + the URI with :meth:`mopidy.core.LibraryController.lookup` If ``lookup`` + is false and the ``path`` is to a track, the returned ``data`` will be + a :class:`mopidy.models.Ref` for the track. For all entries that are not tracks, the returned ``data`` will be :class:`None`. @@ -302,7 +302,8 @@ class MpdContext(object): if ref.type == ref.TRACK: if lookup: - yield (path, self.core.library.lookup(ref.uri)) + # TODO: can we lookup all the refs at once now? + yield (path, self.core.library.lookup(uris=[ref.uri])) else: yield (path, ref) else: diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index d8e1a9d8..ccf6f788 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -29,7 +29,8 @@ def add(context, uri): tracks = [] for path, lookup_future in context.browse(uri): if lookup_future: - tracks.extend(lookup_future.get()) + for result in lookup_future.get().values(): + tracks.extend(result) except exceptions.MpdNoExistError as e: e.message = 'directory or file not found' raise diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 59a1f9b6..644da88d 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -331,8 +331,9 @@ def listallinfo(context, uri=None): if not lookup_future: result.append(('directory', path)) else: - for track in lookup_future.get(): - result.extend(translator.track_to_mpd_format(track)) + for uri, tracks in lookup_future.get().items(): + for track in tracks: + result.extend(translator.track_to_mpd_format(track)) return result @@ -358,9 +359,9 @@ def lsinfo(context, uri=None): if not lookup_future: result.append(('directory', path.lstrip('/'))) else: - tracks = lookup_future.get() - if tracks: - result.extend(translator.track_to_mpd_format(tracks[0])) + for uri, tracks in lookup_future.get().items(): + if tracks: + result.extend(translator.track_to_mpd_format(tracks[0])) if uri in (None, '', '/'): result.extend(protocol.stored_playlists.listplaylists(context)) diff --git a/tests/core/test_library.py b/tests/core/test_library.py index bebdd8a6..bdefd8f0 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -149,18 +149,6 @@ class CoreLibraryTest(BaseCoreLibraryTest): Ref.track(uri='dummy1:track:/foo/baz.mp3', name='Baz'), ]) - def test_lookup_selects_dummy1_backend(self): - self.core.library.lookup('dummy1:a') - - self.library1.lookup.assert_called_once_with('dummy1:a') - self.assertFalse(self.library2.lookup.called) - - def test_lookup_selects_dummy2_backend(self): - self.core.library.lookup('dummy2:a') - - self.assertFalse(self.library1.lookup.called) - self.library2.lookup.assert_called_once_with('dummy2:a') - def test_lookup_fails_with_uri_and_uris_set(self): with self.assertRaises(ValueError): self.core.library.lookup('dummy1:a', ['dummy2:a']) @@ -172,13 +160,6 @@ class CoreLibraryTest(BaseCoreLibraryTest): result = self.core.library.lookup(uris=['dummy1:a', 'dummy2:a']) self.assertEqual(result, {'dummy2:a': [5678], 'dummy1:a': [1234]}) - def test_lookup_uri_returns_empty_list_for_dummy3_track(self): - result = self.core.library.lookup('dummy3:a') - - self.assertEqual(result, []) - self.assertFalse(self.library1.lookup.called) - self.assertFalse(self.library2.lookup.called) - def test_lookup_uris_returns_empty_list_for_dummy3_track(self): result = self.core.library.lookup(uris=['dummy3:a']) @@ -378,6 +359,37 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): query={'any': ['foobar']}, uris=None, exact=True) +class DeprecatedLookupCoreLibraryTest(BaseCoreLibraryTest): + def setUp(self): # noqa: N802 + super(DeprecatedLookupCoreLibraryTest, self).setUp() + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', 'library.lookup.*"uri" argument.*') + + def tearDown(self): # noqa: N802 + super(DeprecatedLookupCoreLibraryTest, self).tearDown() + warnings.filters = self._warnings_filters + + def test_lookup_selects_dummy1_backend(self): + self.core.library.lookup('dummy1:a') + + self.library1.lookup.assert_called_once_with('dummy1:a') + self.assertFalse(self.library2.lookup.called) + + def test_lookup_selects_dummy2_backend(self): + self.core.library.lookup('dummy2:a') + + self.assertFalse(self.library1.lookup.called) + self.library2.lookup.assert_called_once_with('dummy2:a') + + def test_lookup_uri_returns_empty_list_for_dummy3_track(self): + result = self.core.library.lookup('dummy3:a') + + self.assertEqual(result, []) + self.assertFalse(self.library1.lookup.called) + self.assertFalse(self.library2.lookup.called) + + class LegacyFindExactToSearchLibraryTest(unittest.TestCase): def setUp(self): # noqa: N802 self.backend = mock.Mock() diff --git a/tests/local/test_library.py b/tests/local/test_library.py index 7ab67fa6..dfab2c89 100644 --- a/tests/local/test_library.py +++ b/tests/local/test_library.py @@ -128,12 +128,13 @@ class LocalLibraryProviderTest(unittest.TestCase): pass # TODO def test_lookup(self): - tracks = self.library.lookup(self.tracks[0].uri) - self.assertEqual(tracks, self.tracks[0:1]) + uri = self.tracks[0].uri + result = self.library.lookup(uris=[uri]) + self.assertEqual(result[uri], self.tracks[0:1]) def test_lookup_unknown_track(self): - tracks = self.library.lookup('fake uri') - self.assertEqual(tracks, []) + tracks = self.library.lookup(uris=['fake uri']) + self.assertEqual(tracks, {'fake uri': []}) # test backward compatibility with local libraries returning a # single Track From 49fc9941a14c5de9248c4b52fd2afd0d22a48e3f Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 27 Mar 2015 19:59:28 +0100 Subject: [PATCH 09/25] core: Mark searching via keyword argument based query deprecated --- mopidy/core/library.py | 6 ++ mopidy/mpd/protocol/music_db.py | 6 +- tests/core/test_library.py | 52 +++++++------- tests/local/test_library.py | 124 ++++++++++++++++---------------- 4 files changed, 99 insertions(+), 89 deletions(-) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 38e231f1..f87c9aa7 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -248,6 +248,12 @@ class LibraryController(object): The ``exact`` keyword argument, which replaces :meth:`find_exact`. """ query = _normalize_query(query or kwargs) + + if kwargs: + warnings.warn( + 'library.search() with keyword argument query is deprecated', + DeprecationWarning) + futures = {} for backend, backend_uris in self._get_backends_to_uris(uris).items(): futures[backend] = backend.library.search( diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 644da88d..b0919a9a 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -413,7 +413,7 @@ def search(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: return - results = context.core.library.search(**query).get() + results = context.core.library.search(query).get() artists = [_artist_as_track(a) for a in _get_artists(results)] albums = [_album_as_track(a) for a in _get_albums(results)] tracks = _get_tracks(results) @@ -437,7 +437,7 @@ def searchadd(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: return - results = context.core.library.search(**query).get() + results = context.core.library.search(query).get() context.core.tracklist.add(_get_tracks(results)) @@ -464,7 +464,7 @@ def searchaddpl(context, *args): query = _query_from_mpd_search_parameters(parameters, _SEARCH_MAPPING) except ValueError: return - results = context.core.library.search(**query).get() + results = context.core.library.search(query).get() uri = context.lookup_playlist_uri_from_name(playlist_name) playlist = uri is not None and context.core.playlists.lookup(uri).get() diff --git a/tests/core/test_library.py b/tests/core/test_library.py index bdefd8f0..eb3d4dc3 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -202,31 +202,31 @@ class CoreLibraryTest(BaseCoreLibraryTest): self.library2.search().get.return_value = result2 self.library2.search.reset_mock() - result = self.core.library.search(any=['a']) + result = self.core.library.search({'any': ['a']}) self.assertIn(result1, result) self.assertIn(result2, result) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=False) + query={'any': ['a']}, uris=None, exact=False) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=False) + query={'any': ['a']}, uris=None, exact=False) def test_search_with_uris_selects_dummy1_backend(self): self.core.library.search( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo', 'dummy3:']) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo', 'dummy3:']) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=False) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo'], exact=False) self.assertFalse(self.library2.search.called) def test_search_with_uris_selects_both_backends(self): self.core.library.search( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo', 'dummy2:']) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo', 'dummy2:']) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=False) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo'], exact=False) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy2:'], exact=False) + query={'any': ['a']}, uris=['dummy2:'], exact=False) def test_search_filters_out_none(self): track1 = Track(uri='dummy1:a') @@ -237,14 +237,14 @@ class CoreLibraryTest(BaseCoreLibraryTest): self.library2.search().get.return_value = None self.library2.search.reset_mock() - result = self.core.library.search(any=['a']) + result = self.core.library.search({'any': ['a']}) self.assertIn(result1, result) self.assertNotIn(None, result) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=False) + query={'any': ['a']}, uris=None, exact=False) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=False) + query={'any': ['a']}, uris=None, exact=False) def test_search_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -257,14 +257,14 @@ class CoreLibraryTest(BaseCoreLibraryTest): self.library2.search().get.return_value = result2 self.library2.search.reset_mock() - result = self.core.library.search(dict(any=['a'])) + result = self.core.library.search({'any': ['a']}) self.assertIn(result1, result) self.assertIn(result2, result) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=False) + query={'any': ['a']}, uris=None, exact=False) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=False) + query={'any': ['a']}, uris=None, exact=False) def test_search_normalises_bad_queries(self): self.core.library.search({'any': 'foobar'}) @@ -292,7 +292,7 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): self.library1.search.return_value.get.return_value = result1 self.library2.search.return_value.get.return_value = result2 - result = self.core.library.find_exact(any=['a']) + result = self.core.library.find_exact({'any': ['a']}) self.assertIn(result1, result) self.assertIn(result2, result) @@ -303,20 +303,20 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): def test_find_exact_with_uris_selects_dummy1_backend(self): self.core.library.find_exact( - any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy3:']) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo', 'dummy3:']) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=True) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo'], exact=True) self.assertFalse(self.library2.search.called) def test_find_exact_with_uris_selects_both_backends(self): self.core.library.find_exact( - any=['a'], uris=['dummy1:', 'dummy1:foo', 'dummy2:']) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo', 'dummy2:']) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy1:', 'dummy1:foo'], exact=True) + query={'any': ['a']}, uris=['dummy1:', 'dummy1:foo'], exact=True) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=['dummy2:'], exact=True) + query={'any': ['a']}, uris=['dummy2:'], exact=True) def test_find_exact_filters_out_none(self): track1 = Track(uri='dummy1:a') @@ -325,14 +325,14 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): self.library1.search.return_value.get.return_value = result1 self.library2.search.return_value.get.return_value = None - result = self.core.library.find_exact(any=['a']) + result = self.core.library.find_exact({'any': ['a']}) self.assertIn(result1, result) self.assertNotIn(None, result) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) + query={'any': ['a']}, uris=None, exact=True) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) + query={'any': ['a']}, uris=None, exact=True) def test_find_accepts_query_dict_instead_of_kwargs(self): track1 = Track(uri='dummy1:a') @@ -343,14 +343,14 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): self.library1.search.return_value.get.return_value = result1 self.library2.search.return_value.get.return_value = result2 - result = self.core.library.find_exact(dict(any=['a'])) + result = self.core.library.find_exact({'any': ['a']}) self.assertIn(result1, result) self.assertIn(result2, result) self.library1.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) + query={'any': ['a']}, uris=None, exact=True) self.library2.search.assert_called_once_with( - query=dict(any=['a']), uris=None, exact=True) + query={'any': ['a']}, uris=None, exact=True) def test_find_exact_normalises_bad_queries(self): self.core.library.find_exact({'any': 'foobar'}) diff --git a/tests/local/test_library.py b/tests/local/test_library.py index dfab2c89..0198ec9e 100644 --- a/tests/local/test_library.py +++ b/tests/local/test_library.py @@ -88,6 +88,10 @@ class LocalLibraryProviderTest(unittest.TestCase): # TODO: remove this helper? return self.library.search(query=query, exact=True) + def search(self, **query): + # TODO: remove this helper? + return self.library.search(query=query) + def test_refresh(self): self.library.refresh() @@ -378,213 +382,213 @@ class LocalLibraryProviderTest(unittest.TestCase): self.find_exact(any=['']) def test_search_no_hits(self): - result = self.library.search(track_name=['unknown track']) + result = self.search(track_name=['unknown track']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(artist=['unknown artist']) + result = self.search(artist=['unknown artist']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(albumartist=['unknown albumartist']) + result = self.search(albumartist=['unknown albumartist']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(composer=['unknown composer']) + result = self.search(composer=['unknown composer']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(performer=['unknown performer']) + result = self.search(performer=['unknown performer']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(album=['unknown album']) + result = self.search(album=['unknown album']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(track_no=['9']) + result = self.search(track_no=['9']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(track_no=['no_match']) + result = self.search(track_no=['no_match']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(genre=['unknown genre']) + result = self.search(genre=['unknown genre']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(date=['unknown date']) + result = self.search(date=['unknown date']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(comment=['unknown comment']) + result = self.search(comment=['unknown comment']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(uri=['unknown uri']) + result = self.search(uri=['unknown uri']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(any=['unknown anything']) + result = self.search(any=['unknown anything']) self.assertEqual(list(result[0].tracks), []) def test_search_uri(self): - result = self.library.search(uri=['TH1']) + result = self.search(uri=['TH1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(uri=['TH2']) + result = self.search(uri=['TH2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_track_name(self): - result = self.library.search(track_name=['Rack1']) + result = self.search(track_name=['Rack1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(track_name=['Rack2']) + result = self.search(track_name=['Rack2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_artist(self): - result = self.library.search(artist=['Tist1']) + result = self.search(artist=['Tist1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(artist=['Tist2']) + result = self.search(artist=['Tist2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_albumartist(self): # Artist is both track artist and album artist - result = self.library.search(albumartist=['Tist1']) + result = self.search(albumartist=['Tist1']) self.assertEqual(list(result[0].tracks), [self.tracks[0]]) # Artist is both track artist and album artist - result = self.library.search(albumartist=['Tist2']) + result = self.search(albumartist=['Tist2']) self.assertEqual(list(result[0].tracks), [self.tracks[1]]) # Artist is just album artist - result = self.library.search(albumartist=['Tist3']) + result = self.search(albumartist=['Tist3']) self.assertEqual(list(result[0].tracks), [self.tracks[2]]) def test_search_composer(self): - result = self.library.search(composer=['Tist5']) + result = self.search(composer=['Tist5']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) def test_search_performer(self): - result = self.library.search(performer=['Tist6']) + result = self.search(performer=['Tist6']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) def test_search_album(self): - result = self.library.search(album=['Bum1']) + result = self.search(album=['Bum1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(album=['Bum2']) + result = self.search(album=['Bum2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_genre(self): - result = self.library.search(genre=['Enre1']) + result = self.search(genre=['Enre1']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) - result = self.library.search(genre=['Enre2']) + result = self.search(genre=['Enre2']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) def test_search_date(self): - result = self.library.search(date=['2001']) + result = self.search(date=['2001']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(date=['2001-02-03']) + result = self.search(date=['2001-02-03']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(date=['2001-02-04']) + result = self.search(date=['2001-02-04']) self.assertEqual(list(result[0].tracks), []) - result = self.library.search(date=['2002']) + result = self.search(date=['2002']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_track_no(self): - result = self.library.search(track_no=['1']) + result = self.search(track_no=['1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(track_no=['2']) + result = self.search(track_no=['2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) def test_search_comment(self): - result = self.library.search(comment=['fantastic']) + result = self.search(comment=['fantastic']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) - result = self.library.search(comment=['antasti']) + result = self.search(comment=['antasti']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) def test_search_any(self): # Matches on track artist - result = self.library.search(any=['Tist1']) + result = self.search(any=['Tist1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) # Matches on track composer - result = self.library.search(any=['Tist5']) + result = self.search(any=['Tist5']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) # Matches on track performer - result = self.library.search(any=['Tist6']) + result = self.search(any=['Tist6']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) # Matches on track - result = self.library.search(any=['Rack1']) + result = self.search(any=['Rack1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) - result = self.library.search(any=['Rack2']) + result = self.search(any=['Rack2']) self.assertEqual(list(result[0].tracks), self.tracks[1:2]) # Matches on track album - result = self.library.search(any=['Bum1']) + result = self.search(any=['Bum1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) # Matches on track album artists - result = self.library.search(any=['Tist3']) + result = self.search(any=['Tist3']) self.assertEqual(len(result[0].tracks), 2) self.assertIn(self.tracks[2], result[0].tracks) self.assertIn(self.tracks[3], result[0].tracks) # Matches on track genre - result = self.library.search(any=['Enre1']) + result = self.search(any=['Enre1']) self.assertEqual(list(result[0].tracks), self.tracks[4:5]) - result = self.library.search(any=['Enre2']) + result = self.search(any=['Enre2']) self.assertEqual(list(result[0].tracks), self.tracks[5:6]) # Matches on track comment - result = self.library.search(any=['fanta']) + result = self.search(any=['fanta']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) - result = self.library.search(any=['is a fan']) + result = self.search(any=['is a fan']) self.assertEqual(list(result[0].tracks), self.tracks[3:4]) # Matches on URI - result = self.library.search(any=['TH1']) + result = self.search(any=['TH1']) self.assertEqual(list(result[0].tracks), self.tracks[:1]) def test_search_wrong_type(self): with self.assertRaises(LookupError): - self.library.search(wrong=['test']) + self.search(wrong=['test']) def test_search_with_empty_query(self): with self.assertRaises(LookupError): - self.library.search(artist=['']) + self.search(artist=['']) with self.assertRaises(LookupError): - self.library.search(albumartist=['']) + self.search(albumartist=['']) with self.assertRaises(LookupError): - self.library.search(composer=['']) + self.search(composer=['']) with self.assertRaises(LookupError): - self.library.search(performer=['']) + self.search(performer=['']) with self.assertRaises(LookupError): - self.library.search(track_name=['']) + self.search(track_name=['']) with self.assertRaises(LookupError): - self.library.search(album=['']) + self.search(album=['']) with self.assertRaises(LookupError): - self.library.search(genre=['']) + self.search(genre=['']) with self.assertRaises(LookupError): - self.library.search(date=['']) + self.search(date=['']) with self.assertRaises(LookupError): - self.library.search(comment=['']) + self.search(comment=['']) with self.assertRaises(LookupError): - self.library.search(uri=['']) + self.search(uri=['']) with self.assertRaises(LookupError): - self.library.search(any=['']) + self.search(any=['']) def test_default_get_images_impl_no_images(self): result = self.library.get_images([track.uri for track in self.tracks]) From d3b275e1a4bc5ac002e0fdf48db27959232147fa Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 27 Mar 2015 20:50:15 +0100 Subject: [PATCH 10/25] core: Mark tracklist.add by URI as deprecated --- mopidy/core/tracklist.py | 5 +++++ mopidy/mpd/protocol/current_playlist.py | 5 +++-- tests/core/test_tracklist.py | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 51ce7fbf..54206881 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -3,6 +3,7 @@ from __future__ import absolute_import, unicode_literals import collections import logging import random +import warnings from mopidy import compat from mopidy.core import listener @@ -332,6 +333,10 @@ class TracklistController(object): assert tracks is not None or uri is not None or uris is not None, \ 'tracks, uri or uris must be provided' + if uri: + warnings.warn('tracklist.add() "uri" argument is deprecated.', + DeprecationWarning) + if tracks is None: if uri is not None: uris = [uri] diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index ccf6f788..4e8ce5e1 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -22,7 +22,7 @@ def add(context, uri): if not uri.strip('/'): return - if context.core.tracklist.add(uri=uri).get(): + if context.core.tracklist.add(uris=[uri]).get(): return try: @@ -63,7 +63,8 @@ def addid(context, uri, songpos=None): raise exceptions.MpdNoExistError('No such song') if songpos is not None and songpos > context.core.tracklist.length.get(): raise exceptions.MpdArgError('Bad song index') - tl_tracks = context.core.tracklist.add(uri=uri, at_position=songpos).get() + tl_tracks = context.core.tracklist.add( + uris=[uri], at_position=songpos).get() if not tl_tracks: raise exceptions.MpdNoExistError('No such song') return ('Id', tl_tracks[0].tlid) diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 415d1fa0..c8cbf97f 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, unicode_literals import unittest +import warnings import mock @@ -28,7 +29,9 @@ class TracklistTest(unittest.TestCase): track = Track(uri='dummy1:x', name='x') self.library.lookup.return_value.get.return_value = [track] - tl_tracks = self.core.tracklist.add(uri='dummy1:x') + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', r'tracklist.add.*"uri".*') + tl_tracks = self.core.tracklist.add(uri='dummy1:x') self.library.lookup.assert_called_once_with('dummy1:x') self.assertEqual(1, len(tl_tracks)) From a8860faa35d30a87db7a2f592aaeb0a35e7d0368 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 27 Mar 2015 23:40:38 +0100 Subject: [PATCH 11/25] tests: Cleanup mpd.protocol.current_playlist tests - Split into smaller test cases more or less per command - Created a BasePopulatedTracklistTestCase with a sensible setUp - Modified test cases to work with the common tracklist state - Replaced all calls to tracklist.add(tracks=...) with uris=... - Test tracklist ordering in more compact way that also gives better error messages --- tests/mpd/protocol/test_current_playlist.py | 375 +++++++------------- 1 file changed, 125 insertions(+), 250 deletions(-) diff --git a/tests/mpd/protocol/test_current_playlist.py b/tests/mpd/protocol/test_current_playlist.py index c96febb7..84d905be 100644 --- a/tests/mpd/protocol/test_current_playlist.py +++ b/tests/mpd/protocol/test_current_playlist.py @@ -7,18 +7,25 @@ from mopidy.models import Ref, Track from tests.mpd import protocol -class CurrentPlaylistHandlerTest(protocol.BaseTestCase): - def test_add(self): - needle = Track(uri='dummy://foo') - self.backend.library.dummy_library = [ - Track(), Track(), needle, Track()] - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) +class AddCommandsTest(protocol.BaseTestCase): + def setUp(self): # noqa: N802 + super(AddCommandsTest, self).setUp() - self.send_request('add "dummy://foo"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 6) - self.assertEqual(self.core.tracklist.tracks.get()[5], needle) + self.tracks = [Track(uri='dummy:/a', name='a'), + Track(uri='dummy:/foo/b', name='b')] + + self.refs = {'/a': Ref.track(uri='dummy:/a', name='a'), + '/foo': Ref.directory(uri='dummy:/foo', name='foo'), + '/foo/b': Ref.track(uri='dummy:/foo/b', name='b')} + + self.backend.library.dummy_library = self.tracks + + def test_add(self): + for track in [self.tracks[0], self.tracks[0], self.tracks[1]]: + self.send_request('add "%s"' % track.uri) + + self.assertEqual(len(self.core.tracklist.tracks.get()), 3) + self.assertEqual(self.core.tracklist.tracks.get()[2], self.tracks[1]) self.assertEqualResponse('OK') def test_add_with_uri_not_found_in_library_should_ack(self): @@ -27,220 +34,150 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): 'ACK [50@0] {add} directory or file not found') def test_add_with_empty_uri_should_not_add_anything_and_ok(self): - self.backend.library.dummy_library = [Track(uri='dummy:/a', name='a')] self.backend.library.dummy_browse_result = { - 'dummy:/': [Ref.track(uri='dummy:/a', name='a')]} + 'dummy:/': [self.refs['/a']]} self.send_request('add ""') self.assertEqual(len(self.core.tracklist.tracks.get()), 0) self.assertInResponse('OK') def test_add_with_library_should_recurse(self): - tracks = [Track(uri='dummy:/a', name='a'), - Track(uri='dummy:/foo/b', name='b')] - - self.backend.library.dummy_library = tracks self.backend.library.dummy_browse_result = { - 'dummy:/': [Ref.track(uri='dummy:/a', name='a'), - Ref.directory(uri='dummy:/foo', name='foo')], - 'dummy:/foo': [Ref.track(uri='dummy:/foo/b', name='b')]} + 'dummy:/': [self.refs['/a'], self.refs['/foo']], + 'dummy:/foo': [self.refs['/foo/b']]} self.send_request('add "/dummy"') - self.assertEqual(self.core.tracklist.tracks.get(), tracks) + self.assertEqual(self.core.tracklist.tracks.get(), self.tracks) self.assertInResponse('OK') def test_add_root_should_not_add_anything_and_ok(self): - self.backend.library.dummy_library = [Track(uri='dummy:/a', name='a')] self.backend.library.dummy_browse_result = { - 'dummy:/': [Ref.track(uri='dummy:/a', name='a')]} + 'dummy:/': [self.refs['/a']]} self.send_request('add "/"') self.assertEqual(len(self.core.tracklist.tracks.get()), 0) self.assertInResponse('OK') def test_addid_without_songpos(self): - needle = Track(uri='dummy://foo') - self.backend.library.dummy_library = [ - Track(), Track(), needle, Track()] - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) + for track in [self.tracks[0], self.tracks[0], self.tracks[1]]: + self.send_request('addid "%s"' % track.uri) + tl_tracks = self.core.tracklist.tl_tracks.get() - self.send_request('addid "dummy://foo"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 6) - self.assertEqual(self.core.tracklist.tracks.get()[5], needle) - self.assertInResponse( - 'Id: %d' % self.core.tracklist.tl_tracks.get()[5].tlid) + self.assertEqual(len(tl_tracks), 3) + self.assertEqual(tl_tracks[2].track, self.tracks[1]) + self.assertInResponse('Id: %d' % tl_tracks[2].tlid) self.assertInResponse('OK') + def test_addid_with_songpos(self): + for track in [self.tracks[0], self.tracks[0]]: + self.send_request('add "%s"' % track.uri) + self.send_request('addid "%s" "1"' % self.tracks[1].uri) + tl_tracks = self.core.tracklist.tl_tracks.get() + + self.assertEqual(len(tl_tracks), 3) + self.assertEqual(tl_tracks[1].track, self.tracks[1]) + self.assertInResponse('Id: %d' % tl_tracks[1].tlid) + self.assertInResponse('OK') + + def test_addid_with_songpos_out_of_bounds_should_ack(self): + self.send_request('addid "%s" "3"' % self.tracks[0].uri) + self.assertEqualResponse('ACK [2@0] {addid} Bad song index') + def test_addid_with_empty_uri_acks(self): self.send_request('addid ""') self.assertEqualResponse('ACK [50@0] {addid} No such song') - def test_addid_with_songpos(self): - needle = Track(uri='dummy://foo') - self.backend.library.dummy_library = [ - Track(), Track(), needle, Track()] - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) - - self.send_request('addid "dummy://foo" "3"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 6) - self.assertEqual(self.core.tracklist.tracks.get()[3], needle) - self.assertInResponse( - 'Id: %d' % self.core.tracklist.tl_tracks.get()[3].tlid) - self.assertInResponse('OK') - - def test_addid_with_songpos_out_of_bounds_should_ack(self): - needle = Track(uri='dummy://foo') - self.backend.library.dummy_library = [ - Track(), Track(), needle, Track()] - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) - - self.send_request('addid "dummy://foo" "6"') - self.assertEqualResponse('ACK [2@0] {addid} Bad song index') - def test_addid_with_uri_not_found_in_library_should_ack(self): self.send_request('addid "dummy://foo"') self.assertEqualResponse('ACK [50@0] {addid} No such song') - def test_clear(self): - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) +class BasePopulatedTracklistTestCase(protocol.BaseTestCase): + def setUp(self): # noqa: N802 + super(BasePopulatedTracklistTestCase, self).setUp() + tracks = [Track(uri='dummy:/%s' % x, name=x) for x in 'abcdef'] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=[t.uri for t in tracks]) + + +class DeleteCommandsTest(BasePopulatedTracklistTestCase): + def test_clear(self): self.send_request('clear') self.assertEqual(len(self.core.tracklist.tracks.get()), 0) self.assertEqual(self.core.playback.current_track.get(), None) self.assertInResponse('OK') def test_delete_songpos(self): - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) + tl_tracks = self.core.tracklist.tl_tracks.get() + self.send_request('delete "%d"' % tl_tracks[1].tlid) self.assertEqual(len(self.core.tracklist.tracks.get()), 5) - - self.send_request( - 'delete "%d"' % self.core.tracklist.tl_tracks.get()[2].tlid) - self.assertEqual(len(self.core.tracklist.tracks.get()), 4) self.assertInResponse('OK') def test_delete_songpos_out_of_bounds(self): - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) - - self.send_request('delete "5"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) + self.send_request('delete "8"') + self.assertEqual(len(self.core.tracklist.tracks.get()), 6) self.assertEqualResponse('ACK [2@0] {delete} Bad song index') def test_delete_open_range(self): - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) - self.send_request('delete "1:"') self.assertEqual(len(self.core.tracklist.tracks.get()), 1) self.assertInResponse('OK') - def test_delete_closed_range(self): - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) + # TODO: check how this should work. + # def test_delete_open_upper_range(self): + # self.send_request('delete ":8"') + # self.assertEqual(len(self.core.tracklist.tracks.get()), 0) + # self.assertInResponse('OK') + def test_delete_closed_range(self): self.send_request('delete "1:3"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 3) + self.assertEqual(len(self.core.tracklist.tracks.get()), 4) self.assertInResponse('OK') - def test_delete_range_out_of_bounds(self): - self.core.tracklist.add( - [Track(), Track(), Track(), Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) - - self.send_request('delete "5:7"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 5) + def test_delete_entire_range_out_of_bounds(self): + self.send_request('delete "8:9"') + self.assertEqual(len(self.core.tracklist.tracks.get()), 6) self.assertEqualResponse('ACK [2@0] {delete} Bad song index') - def test_deleteid(self): - self.core.tracklist.add([Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 2) + def test_delete_upper_range_out_of_bounds(self): + self.send_request('delete "5:9"') + self.assertEqual(len(self.core.tracklist.tracks.get()), 5) + self.assertEqualResponse('OK') + def test_deleteid(self): self.send_request('deleteid "1"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 1) + self.assertEqual(len(self.core.tracklist.tracks.get()), 5) self.assertInResponse('OK') def test_deleteid_does_not_exist(self): - self.core.tracklist.add([Track(), Track()]) - self.assertEqual(len(self.core.tracklist.tracks.get()), 2) - self.send_request('deleteid "12345"') - self.assertEqual(len(self.core.tracklist.tracks.get()), 2) + self.assertEqual(len(self.core.tracklist.tracks.get()), 6) self.assertEqualResponse('ACK [50@0] {deleteid} No such song') - def test_move_songpos(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) +class MoveCommandsTest(BasePopulatedTracklistTestCase): + def test_move_songpos(self): self.send_request('move "1" "0"') - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'b') - self.assertEqual(tracks[1].name, 'a') - self.assertEqual(tracks[2].name, 'c') - self.assertEqual(tracks[3].name, 'd') - self.assertEqual(tracks[4].name, 'e') - self.assertEqual(tracks[5].name, 'f') + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result, ['b', 'a', 'c', 'd', 'e', 'f']) self.assertInResponse('OK') def test_move_open_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) - self.send_request('move "2:" "0"') - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'c') - self.assertEqual(tracks[1].name, 'd') - self.assertEqual(tracks[2].name, 'e') - self.assertEqual(tracks[3].name, 'f') - self.assertEqual(tracks[4].name, 'a') - self.assertEqual(tracks[5].name, 'b') + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result, ['c', 'd', 'e', 'f', 'a', 'b']) self.assertInResponse('OK') def test_move_closed_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) - self.send_request('move "1:3" "0"') - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'b') - self.assertEqual(tracks[1].name, 'c') - self.assertEqual(tracks[2].name, 'a') - self.assertEqual(tracks[3].name, 'd') - self.assertEqual(tracks[4].name, 'e') - self.assertEqual(tracks[5].name, 'f') + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result, ['b', 'c', 'a', 'd', 'e', 'f']) self.assertInResponse('OK') def test_moveid(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) - self.send_request('moveid "4" "2"') - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'a') - self.assertEqual(tracks[1].name, 'b') - self.assertEqual(tracks[2].name, 'e') - self.assertEqual(tracks[3].name, 'c') - self.assertEqual(tracks[4].name, 'd') - self.assertEqual(tracks[5].name, 'f') + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result, ['a', 'b', 'e', 'c', 'd', 'f']) self.assertInResponse('OK') def test_moveid_with_tlid_not_found_in_tracklist_should_ack(self): @@ -248,14 +185,8 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertEqualResponse( 'ACK [50@0] {moveid} No such song') - def test_playlist_returns_same_as_playlistinfo(self): - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', message='.*playlistinfo.*') - playlist_response = self.send_request('playlist') - - playlistinfo_response = self.send_request('playlistinfo') - self.assertEqual(playlist_response, playlistinfo_response) +class PlaylistFindCommandTest(protocol.BaseTestCase): def test_playlistfind(self): self.send_request('playlistfind "tag" "needle"') self.assertEqualResponse('ACK [0@0] {playlistfind} Not implemented') @@ -269,25 +200,25 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertEqualResponse('OK') def test_playlistfind_by_filename_in_tracklist(self): - self.core.tracklist.add([Track(uri='file:///exists')]) + track = Track(uri='dummy:///exists') + self.backend.library.dummy_library = [track] + self.core.tracklist.add(uris=[track.uri]) - self.send_request('playlistfind filename "file:///exists"') - self.assertInResponse('file: file:///exists') + self.send_request('playlistfind filename "dummy:///exists"') + self.assertInResponse('file: dummy:///exists') self.assertInResponse('Id: 0') self.assertInResponse('Pos: 0') self.assertInResponse('OK') - def test_playlistid_without_songid(self): - self.core.tracklist.add([Track(name='a'), Track(name='b')]) +class PlaylistIdCommandTest(BasePopulatedTracklistTestCase): + def test_playlistid_without_songid(self): self.send_request('playlistid') self.assertInResponse('Title: a') self.assertInResponse('Title: b') self.assertInResponse('OK') def test_playlistid_with_songid(self): - self.core.tracklist.add([Track(name='a'), Track(name='b')]) - self.send_request('playlistid "1"') self.assertNotInResponse('Title: a') self.assertNotInResponse('Id: 0') @@ -296,17 +227,20 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playlistid_with_not_existing_songid_fails(self): - self.core.tracklist.add([Track(name='a'), Track(name='b')]) - self.send_request('playlistid "25"') self.assertEqualResponse('ACK [50@0] {playlistid} No such song') - def test_playlistinfo_without_songpos_or_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) +class PlaylistInfoCommandTest(BasePopulatedTracklistTestCase): + def test_playlist_returns_same_as_playlistinfo(self): + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', message='.*playlistinfo.*') + playlist_response = self.send_request('playlist') + + playlistinfo_response = self.send_request('playlistinfo') + self.assertEqual(playlist_response, playlistinfo_response) + + def test_playlistinfo_without_songpos_or_range(self): self.send_request('playlistinfo') self.assertInResponse('Title: a') self.assertInResponse('Pos: 0') @@ -325,10 +259,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): def test_playlistinfo_with_songpos(self): # Make the track's CPID not match the playlist position self.core.tracklist.tlid = 17 - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) self.send_request('playlistinfo "4"') self.assertNotInResponse('Title: a') @@ -351,11 +281,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertEqual(response1, response2) def test_playlistinfo_with_open_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) - self.send_request('playlistinfo "2:"') self.assertNotInResponse('Title: a') self.assertNotInResponse('Pos: 0') @@ -372,11 +297,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playlistinfo_with_closed_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) - self.send_request('playlistinfo "2:4"') self.assertNotInResponse('Title: a') self.assertNotInResponse('Title: b') @@ -398,6 +318,8 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.send_request('playlistinfo "0"') self.assertInResponse('OK') + +class PlaylistSearchCommandTest(protocol.BaseTestCase): def test_playlistsearch(self): self.send_request('playlistsearch "any" "needle"') self.assertEqualResponse('ACK [0@0] {playlistsearch} Not implemented') @@ -406,10 +328,9 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.send_request('playlistsearch any "needle"') self.assertEqualResponse('ACK [0@0] {playlistsearch} Not implemented') - def test_plchanges_with_lower_version_returns_changes(self): - self.core.tracklist.add( - [Track(name='a'), Track(name='b'), Track(name='c')]) +class PlChangeCommandTest(BasePopulatedTracklistTestCase): + def test_plchanges_with_lower_version_returns_changes(self): self.send_request('plchanges "0"') self.assertInResponse('Title: a') self.assertInResponse('Title: b') @@ -417,9 +338,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_plchanges_with_equal_version_returns_nothing(self): - self.core.tracklist.add( - [Track(name='a'), Track(name='b'), Track(name='c')]) - self.assertEqual(self.core.tracklist.version.get(), 1) self.send_request('plchanges "1"') self.assertNotInResponse('Title: a') @@ -428,9 +346,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_plchanges_with_greater_version_returns_nothing(self): - self.core.tracklist.add( - [Track(name='a'), Track(name='b'), Track(name='c')]) - self.assertEqual(self.core.tracklist.version.get(), 1) self.send_request('plchanges "2"') self.assertNotInResponse('Title: a') @@ -439,9 +354,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_plchanges_with_minus_one_returns_entire_playlist(self): - self.core.tracklist.add( - [Track(name='a'), Track(name='b'), Track(name='c')]) - self.send_request('plchanges "-1"') self.assertInResponse('Title: a') self.assertInResponse('Title: b') @@ -449,9 +361,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_plchanges_without_quotes_works(self): - self.core.tracklist.add( - [Track(name='a'), Track(name='b'), Track(name='c')]) - self.send_request('plchanges 0') self.assertInResponse('Title: a') self.assertInResponse('Title: b') @@ -459,8 +368,6 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_plchangesposid(self): - self.core.tracklist.add([Track(), Track(), Track()]) - self.send_request('plchangesposid "0"') tl_tracks = self.core.tracklist.tl_tracks.get() self.assertInResponse('cpos: 0') @@ -471,11 +378,10 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('Id: %d' % tl_tracks[2].tlid) self.assertInResponse('OK') + +# TODO: we only seem to be testing that don't touch the non shuffled region :/ +class ShuffleCommandTest(BasePopulatedTracklistTestCase): def test_shuffle_without_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) version = self.core.tracklist.version.get() self.send_request('shuffle') @@ -483,77 +389,46 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_shuffle_with_open_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) version = self.core.tracklist.version.get() self.send_request('shuffle "4:"') self.assertLess(version, self.core.tracklist.version.get()) - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'a') - self.assertEqual(tracks[1].name, 'b') - self.assertEqual(tracks[2].name, 'c') - self.assertEqual(tracks[3].name, 'd') + + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result[:4], ['a', 'b', 'c', 'd']) self.assertInResponse('OK') def test_shuffle_with_closed_range(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) version = self.core.tracklist.version.get() self.send_request('shuffle "1:3"') self.assertLess(version, self.core.tracklist.version.get()) - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'a') - self.assertEqual(tracks[3].name, 'd') - self.assertEqual(tracks[4].name, 'e') - self.assertEqual(tracks[5].name, 'f') + + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result[:1], ['a']) + self.assertEqual(result[3:], ['d', 'e', 'f']) self.assertInResponse('OK') - def test_swap(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) +class SwapCommandTest(BasePopulatedTracklistTestCase): + def test_swap(self): self.send_request('swap "1" "4"') - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'a') - self.assertEqual(tracks[1].name, 'e') - self.assertEqual(tracks[2].name, 'c') - self.assertEqual(tracks[3].name, 'd') - self.assertEqual(tracks[4].name, 'b') - self.assertEqual(tracks[5].name, 'f') + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result, ['a', 'e', 'c', 'd', 'b', 'f']) self.assertInResponse('OK') def test_swapid(self): - self.core.tracklist.add([ - Track(name='a'), Track(name='b'), Track(name='c'), - Track(name='d'), Track(name='e'), Track(name='f'), - ]) - self.send_request('swapid "1" "4"') - tracks = self.core.tracklist.tracks.get() - self.assertEqual(tracks[0].name, 'a') - self.assertEqual(tracks[1].name, 'e') - self.assertEqual(tracks[2].name, 'c') - self.assertEqual(tracks[3].name, 'd') - self.assertEqual(tracks[4].name, 'b') - self.assertEqual(tracks[5].name, 'f') + result = [t.name for t in self.core.tracklist.tracks.get()] + self.assertEqual(result, ['a', 'e', 'c', 'd', 'b', 'f']) self.assertInResponse('OK') def test_swapid_with_first_id_unknown_should_ack(self): - self.core.tracklist.add([Track()]) - self.send_request('swapid "0" "4"') + self.send_request('swapid "0" "8"') self.assertEqualResponse( 'ACK [50@0] {swapid} No such song') def test_swapid_with_second_id_unknown_should_ack(self): - self.core.tracklist.add([Track()]) - self.send_request('swapid "4" "0"') + self.send_request('swapid "8" "0"') self.assertEqualResponse( 'ACK [50@0] {swapid} No such song') From faf8174ef72131a7e308df3062eaa5b79b7f6447 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 28 Mar 2015 00:03:06 +0100 Subject: [PATCH 12/25] tests: Update mpd.test_status to not use tracklist.add(tracks=...) --- tests/mpd/test_status.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py index 89030651..675626f6 100644 --- a/tests/mpd/test_status.py +++ b/tests/mpd/test_status.py @@ -38,6 +38,10 @@ class StatusHandlerTest(unittest.TestCase): def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() + def set_tracklist(self, track): + self.backend.library.dummy_library = [track] + self.core.tracklist.add(uris=[track.uri]).get() + def test_stats_method(self): result = status.stats(self.context) self.assertIn('artists', result) @@ -140,21 +144,22 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(result['state'], 'pause') def test_status_method_when_playlist_loaded_contains_song(self): - self.core.tracklist.add([Track(uri='dummy:a')]) + self.set_tracklist(Track(uri='dummy:/a')) + self.core.playback.play() result = dict(status.status(self.context)) self.assertIn('song', result) self.assertGreaterEqual(int(result['song']), 0) def test_status_method_when_playlist_loaded_contains_tlid_as_songid(self): - self.core.tracklist.add([Track(uri='dummy:a')]) + self.set_tracklist(Track(uri='dummy:/a')) self.core.playback.play() result = dict(status.status(self.context)) self.assertIn('songid', result) self.assertEqual(int(result['songid']), 0) def test_status_method_when_playing_contains_time_with_no_length(self): - self.core.tracklist.add([Track(uri='dummy:a', length=None)]) + self.set_tracklist(Track(uri='dummy:/a', length=None)) self.core.playback.play() result = dict(status.status(self.context)) self.assertIn('time', result) @@ -164,7 +169,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertLessEqual(position, total) def test_status_method_when_playing_contains_time_with_length(self): - self.core.tracklist.add([Track(uri='dummy:a', length=10000)]) + self.set_tracklist(Track(uri='dummy:/a', length=10000)) self.core.playback.play() result = dict(status.status(self.context)) self.assertIn('time', result) @@ -174,7 +179,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertLessEqual(position, total) def test_status_method_when_playing_contains_elapsed(self): - self.core.tracklist.add([Track(uri='dummy:a', length=60000)]) + self.set_tracklist(Track(uri='dummy:/a', length=60000)) self.core.playback.play() self.core.playback.pause() self.core.playback.seek(59123) @@ -183,7 +188,7 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(result['elapsed'], '59.123') def test_status_method_when_starting_playing_contains_elapsed_zero(self): - self.core.tracklist.add([Track(uri='dummy:a', length=10000)]) + self.set_tracklist(Track(uri='dummy:/a', length=10000)) self.core.playback.play() self.core.playback.pause() result = dict(status.status(self.context)) @@ -191,8 +196,8 @@ class StatusHandlerTest(unittest.TestCase): self.assertEqual(result['elapsed'], '0.000') def test_status_method_when_playing_contains_bitrate(self): - self.core.tracklist.add([Track(uri='dummy:a', bitrate=320)]) + self.set_tracklist(Track(uri='dummy:/a', bitrate=3200)) self.core.playback.play() result = dict(status.status(self.context)) self.assertIn('bitrate', result) - self.assertEqual(int(result['bitrate']), 320) + self.assertEqual(int(result['bitrate']), 3200) From 7d42d028c6562ada21e7d54eaf0cfe2bfa03f6d3 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 28 Mar 2015 00:28:34 +0100 Subject: [PATCH 13/25] tests: Stop using tracklist tracks in mpd playback tests --- tests/mpd/protocol/test_playback.py | 70 +++++++++-------------------- 1 file changed, 22 insertions(+), 48 deletions(-) diff --git a/tests/mpd/protocol/test_playback.py b/tests/mpd/protocol/test_playback.py index 8bac48cc..4d6e727d 100644 --- a/tests/mpd/protocol/test_playback.py +++ b/tests/mpd/protocol/test_playback.py @@ -173,13 +173,19 @@ class PlaybackOptionsHandlerTest(protocol.BaseTestCase): class PlaybackControlHandlerTest(protocol.BaseTestCase): + def setUp(self): # noqa: N802 + super(PlaybackControlHandlerTest, self).setUp() + self.tracks = [Track(uri='dummy:a', length=40000), + Track(uri='dummy:b', length=40000)] + self.backend.library.dummy_library = self.tracks + self.core.tracklist.add(uris=[t.uri for t in self.tracks]).get() + def test_next(self): + self.core.tracklist.clear().get() self.send_request('next') self.assertInResponse('OK') def test_pause_off(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('play "0"') self.send_request('pause "1"') self.send_request('pause "0"') @@ -187,16 +193,12 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_pause_on(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('play "0"') self.send_request('pause "1"') self.assertEqual(PAUSED, self.core.playback.state.get()) self.assertInResponse('OK') def test_pause_toggle(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('play "0"') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') @@ -212,36 +214,28 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_play_without_pos(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('play') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') def test_play_with_pos(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('play "0"') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') def test_play_with_pos_without_quotes(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('play 0') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') def test_play_with_pos_out_of_bounds(self): - self.core.tracklist.add([]) - + self.core.tracklist.clear().get() self.send_request('play "0"') self.assertEqual(STOPPED, self.core.playback.state.get()) self.assertInResponse('ACK [2@0] {play} Bad song index') def test_play_minus_one_plays_first_in_playlist_if_no_current_track(self): self.assertEqual(self.core.playback.current_track.get(), None) - self.core.tracklist.add([Track(uri='dummy:a'), Track(uri='dummy:b')]) self.send_request('play "-1"') self.assertEqual(PLAYING, self.core.playback.state.get()) @@ -250,7 +244,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_play_minus_one_plays_current_track_if_current_track_is_set(self): - self.core.tracklist.add([Track(uri='dummy:a'), Track(uri='dummy:b')]) self.assertEqual(self.core.playback.current_track.get(), None) self.core.playback.play() self.core.playback.next() @@ -272,7 +265,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_play_minus_is_ignored_if_playing(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.seek(30000) self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) @@ -285,7 +277,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_play_minus_one_resumes_if_paused(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.seek(30000) self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) @@ -300,22 +291,17 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playid(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('playid "0"') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') def test_playid_without_quotes(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('playid 0') self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') def test_playid_minus_1_plays_first_in_playlist_if_no_current_track(self): self.assertEqual(self.core.playback.current_track.get(), None) - self.core.tracklist.add([Track(uri='dummy:a'), Track(uri='dummy:b')]) self.send_request('playid "-1"') self.assertEqual(PLAYING, self.core.playback.state.get()) @@ -324,7 +310,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playid_minus_1_plays_current_track_if_current_track_is_set(self): - self.core.tracklist.add([Track(uri='dummy:a'), Track(uri='dummy:b')]) self.assertEqual(self.core.playback.current_track.get(), None) self.core.playback.play() self.core.playback.next() @@ -346,7 +331,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playid_minus_is_ignored_if_playing(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.seek(30000) self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) @@ -359,7 +343,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playid_minus_one_resumes_if_paused(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.seek(30000) self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) @@ -374,40 +357,36 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_playid_which_does_not_exist(self): - self.core.tracklist.add([Track(uri='dummy:a')]) - self.send_request('playid "12345"') self.assertInResponse('ACK [50@0] {playid} No such song') def test_previous(self): + self.core.tracklist.clear().get() self.send_request('previous') self.assertInResponse('OK') def test_seek_in_current_track(self): - seek_track = Track(uri='dummy:a', length=40000) - self.core.tracklist.add([seek_track]) self.core.playback.play() self.send_request('seek "0" "30"') - self.assertEqual(self.core.playback.current_track.get(), seek_track) + current_track = self.core.playback.current_track.get() + self.assertEqual(current_track, self.tracks[0]) self.assertGreaterEqual(self.core.playback.time_position, 30000) self.assertInResponse('OK') def test_seek_in_another_track(self): - seek_track = Track(uri='dummy:b', length=40000) - self.core.tracklist.add( - [Track(uri='dummy:a', length=40000), seek_track]) self.core.playback.play() - self.assertNotEqual(self.core.playback.current_track.get(), seek_track) + current_track = self.core.playback.current_track.get() + self.assertNotEqual(current_track, self.tracks[1]) self.send_request('seek "1" "30"') - self.assertEqual(self.core.playback.current_track.get(), seek_track) + current_track = self.core.playback.current_track.get() + self.assertEqual(current_track, self.tracks[1]) self.assertInResponse('OK') def test_seek_without_quotes(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.play() self.send_request('seek 0 30') @@ -416,31 +395,27 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_seekid_in_current_track(self): - seek_track = Track(uri='dummy:a', length=40000) - self.core.tracklist.add([seek_track]) self.core.playback.play() self.send_request('seekid "0" "30"') - self.assertEqual(self.core.playback.current_track.get(), seek_track) + current_track = self.core.playback.current_track.get() + self.assertEqual(current_track, self.tracks[0]) self.assertGreaterEqual( self.core.playback.time_position.get(), 30000) self.assertInResponse('OK') def test_seekid_in_another_track(self): - seek_track = Track(uri='dummy:b', length=40000) - self.core.tracklist.add( - [Track(uri='dummy:a', length=40000), seek_track]) self.core.playback.play() self.send_request('seekid "1" "30"') - self.assertEqual(1, self.core.playback.current_tl_track.get().tlid) - self.assertEqual(seek_track, self.core.playback.current_track.get()) + current_tl_track = self.core.playback.current_tl_track.get() + self.assertEqual(current_tl_track.tlid, 1) + self.assertEqual(current_tl_track.track, self.tracks[1]) self.assertInResponse('OK') def test_seekcur_absolute_value(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.play() self.send_request('seekcur "30"') @@ -449,7 +424,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_seekcur_positive_diff(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.play() self.core.playback.seek(10000) self.assertGreaterEqual(self.core.playback.time_position.get(), 10000) @@ -460,7 +434,6 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_seekcur_negative_diff(self): - self.core.tracklist.add([Track(uri='dummy:a', length=40000)]) self.core.playback.play() self.core.playback.seek(30000) self.assertGreaterEqual(self.core.playback.time_position.get(), 30000) @@ -471,6 +444,7 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_stop(self): + self.core.tracklist.clear().get() self.send_request('stop') self.assertEqual(STOPPED, self.core.playback.state.get()) self.assertInResponse('OK') From 79b0584887075eb1732770d1732ae07147ec21b6 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 28 Mar 2015 00:29:24 +0100 Subject: [PATCH 14/25] tests: Stop using tracklist add tracks in mpd status test --- tests/mpd/protocol/test_status.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/mpd/protocol/test_status.py b/tests/mpd/protocol/test_status.py index 09df3526..bec54466 100644 --- a/tests/mpd/protocol/test_status.py +++ b/tests/mpd/protocol/test_status.py @@ -11,11 +11,13 @@ class StatusHandlerTest(protocol.BaseTestCase): self.assertEqualResponse('ACK [0@0] {clearerror} Not implemented') def test_currentsong(self): - track = Track() - self.core.tracklist.add([track]) + track = Track(uri='dummy:/a') + self.backend.library.dummy_library = [track] + self.core.tracklist.add(uris=[track.uri]).get() + self.core.playback.play() self.send_request('currentsong') - self.assertInResponse('file: ') + self.assertInResponse('file: dummy:/a') self.assertInResponse('Time: 0') self.assertInResponse('Artist: ') self.assertInResponse('Title: ') From f7399c18495f1d3e66006cec28da3ca7d0a1b7ec Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 29 Mar 2015 18:20:28 +0200 Subject: [PATCH 15/25] tests: Stop using playlist filters in mpd music_db tests --- tests/mpd/protocol/test_music_db.py | 32 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index b9fbcdf6..df8fa866 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -104,31 +104,35 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase): self.core.playlists.save(playlist) self.backend.library.dummy_search_result = SearchResult( tracks=[Track(uri='dummy:a', name='A')]) - playlists = self.core.playlists.filter(name='my favs').get() - self.assertEqual(len(playlists), 1) - self.assertEqual(len(playlists[0].tracks), 2) + + items = self.core.playlists.get_items(playlist.uri).get() + self.assertEqual(len(items), 2) self.send_request('searchaddpl "my favs" "title" "a"') - playlists = self.core.playlists.filter(name='my favs').get() - self.assertEqual(len(playlists), 1) - self.assertEqual(len(playlists[0].tracks), 3) - self.assertEqual(playlists[0].tracks[0].uri, 'dummy:x') - self.assertEqual(playlists[0].tracks[1].uri, 'dummy:y') - self.assertEqual(playlists[0].tracks[2].uri, 'dummy:a') + items = self.core.playlists.get_items(playlist.uri).get() + self.assertEqual(len(items), 3) + self.assertEqual(items[0].uri, 'dummy:x') + self.assertEqual(items[1].uri, 'dummy:y') + self.assertEqual(items[2].uri, 'dummy:a') self.assertInResponse('OK') def test_searchaddpl_creates_missing_playlist(self): self.backend.library.dummy_search_result = SearchResult( tracks=[Track(uri='dummy:a', name='A')]) - self.assertEqual( - len(self.core.playlists.filter(name='my favs').get()), 0) + + playlists = self.core.playlists.as_list().get() + self.assertNotIn('my favs', {p.name for p in playlists}) self.send_request('searchaddpl "my favs" "title" "a"') - playlists = self.core.playlists.filter(name='my favs').get() - self.assertEqual(len(playlists), 1) - self.assertEqual(playlists[0].tracks[0].uri, 'dummy:a') + playlists = self.core.playlists.as_list().get() + playlist = {p.name: p for p in playlists}['my favs'] + + items = self.core.playlists.get_items(playlist.uri).get() + + self.assertEqual(len(items), 1) + self.assertEqual(items[0].uri, 'dummy:a') self.assertInResponse('OK') def test_listall_without_uri(self): From c85689edad4aa8cf9f627294c6b8e9b643a9be27 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 29 Mar 2015 22:53:52 +0200 Subject: [PATCH 16/25] mpd: Make mpd warnings safe with respect to tracklist.add(tracks=...) --- mopidy/mpd/protocol/current_playlist.py | 20 ++++--- mopidy/mpd/protocol/music_db.py | 17 +++++- mopidy/mpd/protocol/stored_playlists.py | 6 +- tests/mpd/protocol/test_music_db.py | 4 ++ tests/mpd/protocol/test_regression.py | 31 +++++++--- tests/mpd/protocol/test_stored_playlists.py | 66 ++++++++++++++------- 6 files changed, 104 insertions(+), 40 deletions(-) diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index 4e8ce5e1..080b6f2c 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -26,18 +26,17 @@ def add(context, uri): return try: - tracks = [] - for path, lookup_future in context.browse(uri): - if lookup_future: - for result in lookup_future.get().values(): - tracks.extend(result) + uris = [] + for path, ref in context.browse(uri, lookup=False): + if ref: + uris.append(ref.uri) except exceptions.MpdNoExistError as e: e.message = 'directory or file not found' raise - if not tracks: + if not uris: raise exceptions.MpdNoExistError('directory or file not found') - context.core.tracklist.add(tracks=tracks) + context.core.tracklist.add(uris=uris).get() @protocol.commands.add('addid', songpos=protocol.UINT) @@ -351,8 +350,13 @@ def swap(context, songpos1, songpos2): tracks.insert(songpos1, song2) del tracks[songpos2] tracks.insert(songpos2, song1) + + # TODO: do we need a tracklist.replace() context.core.tracklist.clear() - context.core.tracklist.add(tracks) + + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + context.core.tracklist.add(tracks=tracks).get() @protocol.commands.add('swapid', tlid1=protocol.UINT, tlid2=protocol.UINT) diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index b0919a9a..3f1dd2bc 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import functools import itertools +import warnings from mopidy.models import Track from mopidy.mpd import exceptions, protocol, translator @@ -168,8 +169,14 @@ def findadd(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: return + results = context.core.library.search(query=query, exact=True).get() - context.core.tracklist.add(_get_tracks(results)) + + with warnings.catch_warnings(): + # TODO: for now just use tracks as other wise we have to lookup the + # tracks we just got from the search. + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks" argument.*') + context.core.tracklist.add(tracks=_get_tracks(results)).get() @protocol.commands.add('list') @@ -437,8 +444,14 @@ def searchadd(context, *args): query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING) except ValueError: return + results = context.core.library.search(query).get() - context.core.tracklist.add(_get_tracks(results)) + + with warnings.catch_warnings(): + # TODO: for now just use tracks as other wise we have to lookup the + # tracks we just got from the search. + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + context.core.tracklist.add(_get_tracks(results)).get() @protocol.commands.add('searchaddpl') diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py index 9d9f66e0..a5d4b180 100644 --- a/mopidy/mpd/protocol/stored_playlists.py +++ b/mopidy/mpd/protocol/stored_playlists.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, division, unicode_literals import datetime +import warnings from mopidy.mpd import exceptions, protocol, translator @@ -127,7 +128,10 @@ def load(context, name, playlist_slice=slice(0, None)): playlist = uri is not None and context.core.playlists.lookup(uri).get() if not playlist: raise exceptions.MpdNoExistError('No such playlist') - context.core.tracklist.add(playlist.tracks[playlist_slice]) + + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + context.core.tracklist.add(playlist.tracks[playlist_slice]).get() @protocol.commands.add('playlistadd') diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py index df8fa866..ee8d386d 100644 --- a/tests/mpd/protocol/test_music_db.py +++ b/tests/mpd/protocol/test_music_db.py @@ -7,6 +7,8 @@ from mopidy.mpd.protocol import music_db from tests.mpd import protocol +# TODO: split into more modules for faster parallel tests? + class QueryFromMpdSearchFormatTest(unittest.TestCase): def test_dates_are_extracted(self): @@ -32,6 +34,8 @@ class QueryFromMpdListFormatTest(unittest.TestCase): pass # TODO +# TODO: why isn't core.playlists.filter getting deprecation warnings? + class MusicDatabaseHandlerTest(protocol.BaseTestCase): def test_count(self): self.send_request('count "artist" "needle"') diff --git a/tests/mpd/protocol/test_regression.py b/tests/mpd/protocol/test_regression.py index 6fb59afd..b8a5d1d5 100644 --- a/tests/mpd/protocol/test_regression.py +++ b/tests/mpd/protocol/test_regression.py @@ -18,14 +18,17 @@ class IssueGH17RegressionTest(protocol.BaseTestCase): - Press next until you get to the unplayable track """ def test(self): - self.core.tracklist.add([ + tracks = [ Track(uri='dummy:a'), Track(uri='dummy:b'), Track(uri='dummy:error'), Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f'), - ]) + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=[t.uri for t in tracks]).get() + random.seed(1) # Playlist order: abcfde self.send_request('play') @@ -59,9 +62,13 @@ class IssueGH18RegressionTest(protocol.BaseTestCase): """ def test(self): - self.core.tracklist.add([ + tracks = [ Track(uri='dummy:a'), Track(uri='dummy:b'), Track(uri='dummy:c'), - Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f')]) + Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f'), + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=[t.uri for t in tracks]).get() + random.seed(1) self.send_request('play') @@ -95,9 +102,13 @@ class IssueGH22RegressionTest(protocol.BaseTestCase): """ def test(self): - self.core.tracklist.add([ + tracks = [ Track(uri='dummy:a'), Track(uri='dummy:b'), Track(uri='dummy:c'), - Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f')]) + Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f'), + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=[t.uri for t in tracks]).get() + random.seed(1) self.send_request('play') @@ -124,9 +135,13 @@ class IssueGH69RegressionTest(protocol.BaseTestCase): def test(self): self.core.playlists.create('foo') - self.core.tracklist.add([ + + tracks = [ Track(uri='dummy:a'), Track(uri='dummy:b'), Track(uri='dummy:c'), - Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f')]) + Track(uri='dummy:d'), Track(uri='dummy:e'), Track(uri='dummy:f'), + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=[t.uri for t in tracks]).get() self.send_request('play') self.send_request('stop') diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index cca32b0d..6018686e 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -130,54 +130,78 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') def test_load_appends_to_tracklist(self): - self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) + tracks = [ + Track(uri='dummy:a'), + Track(uri='dummy:b'), + Track(uri='dummy:c'), + Track(uri='dummy:d'), + Track(uri='dummy:e'), + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=['dummy:a', 'dummy:b']).get() + self.assertEqual(len(self.core.tracklist.tracks.get()), 2) self.backend.playlists.set_dummy_playlists([ - Playlist(name='A-list', uri='dummy:A-list', tracks=[ - Track(uri='c'), Track(uri='d'), Track(uri='e')])]) + Playlist(name='A-list', uri='dummy:A-list', tracks=tracks[2:])]) self.send_request('load "A-list"') tracks = self.core.tracklist.tracks.get() self.assertEqual(5, len(tracks)) - self.assertEqual('a', tracks[0].uri) - self.assertEqual('b', tracks[1].uri) - self.assertEqual('c', tracks[2].uri) - self.assertEqual('d', tracks[3].uri) - self.assertEqual('e', tracks[4].uri) + self.assertEqual('dummy:a', tracks[0].uri) + self.assertEqual('dummy:b', tracks[1].uri) + self.assertEqual('dummy:c', tracks[2].uri) + self.assertEqual('dummy:d', tracks[3].uri) + self.assertEqual('dummy:e', tracks[4].uri) self.assertInResponse('OK') def test_load_with_range_loads_part_of_playlist(self): - self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) + tracks = [ + Track(uri='dummy:a'), + Track(uri='dummy:b'), + Track(uri='dummy:c'), + Track(uri='dummy:d'), + Track(uri='dummy:e'), + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=['dummy:a', 'dummy:b']).get() + self.assertEqual(len(self.core.tracklist.tracks.get()), 2) self.backend.playlists.set_dummy_playlists([ - Playlist(name='A-list', uri='dummy:A-list', tracks=[ - Track(uri='c'), Track(uri='d'), Track(uri='e')])]) + Playlist(name='A-list', uri='dummy:A-list', tracks=tracks[2:])]) self.send_request('load "A-list" "1:2"') tracks = self.core.tracklist.tracks.get() self.assertEqual(3, len(tracks)) - self.assertEqual('a', tracks[0].uri) - self.assertEqual('b', tracks[1].uri) - self.assertEqual('d', tracks[2].uri) + self.assertEqual('dummy:a', tracks[0].uri) + self.assertEqual('dummy:b', tracks[1].uri) + self.assertEqual('dummy:d', tracks[2].uri) self.assertInResponse('OK') def test_load_with_range_without_end_loads_rest_of_playlist(self): - self.core.tracklist.add([Track(uri='a'), Track(uri='b')]) + tracks = [ + Track(uri='dummy:a'), + Track(uri='dummy:b'), + Track(uri='dummy:c'), + Track(uri='dummy:d'), + Track(uri='dummy:e'), + ] + self.backend.library.dummy_library = tracks + self.core.tracklist.add(uris=['dummy:a', 'dummy:b']).get() + self.assertEqual(len(self.core.tracklist.tracks.get()), 2) self.backend.playlists.set_dummy_playlists([ - Playlist(name='A-list', uri='dummy:A-list', tracks=[ - Track(uri='c'), Track(uri='d'), Track(uri='e')])]) + Playlist(name='A-list', uri='dummy:A-list', tracks=tracks[2:])]) self.send_request('load "A-list" "1:"') tracks = self.core.tracklist.tracks.get() self.assertEqual(4, len(tracks)) - self.assertEqual('a', tracks[0].uri) - self.assertEqual('b', tracks[1].uri) - self.assertEqual('d', tracks[2].uri) - self.assertEqual('e', tracks[3].uri) + self.assertEqual('dummy:a', tracks[0].uri) + self.assertEqual('dummy:b', tracks[1].uri) + self.assertEqual('dummy:d', tracks[2].uri) + self.assertEqual('dummy:e', tracks[3].uri) self.assertInResponse('OK') def test_load_unknown_playlist_acks(self): From dc673d554c08daa37c576527ff3455c66882636e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 29 Mar 2015 23:02:27 +0200 Subject: [PATCH 17/25] tests: Ignore deprecated tracklist.add(tracks=...) in local tests Note, this is mostly because these tests are just core tests in disguise and need a lot more love than I can give them right now. --- tests/local/__init__.py | 6 +++++- tests/local/test_playback.py | 6 ++++++ tests/local/test_tracklist.py | 6 ++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/local/__init__.py b/tests/local/__init__.py index b1520768..bfd60044 100644 --- a/tests/local/__init__.py +++ b/tests/local/__init__.py @@ -1,5 +1,7 @@ from __future__ import absolute_import, unicode_literals +import warnings + def generate_song(i): return 'local:track:song%s.wav' % i @@ -7,7 +9,9 @@ def generate_song(i): def populate_tracklist(func): def wrapper(self): - self.tl_tracks = self.core.tracklist.add(self.tracks) + with warnings.catch_warnings(): + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + self.tl_tracks = self.core.tracklist.add(self.tracks) return func(self) wrapper.__name__ = func.__name__ diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 6ea82f2d..28ded52a 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import time import unittest +import warnings import mock @@ -55,8 +56,13 @@ class LocalPlaybackProviderTest(unittest.TestCase): assert self.tracks[0].length >= 2000, \ 'First song needs to be at least 2000 miliseconds' + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() + warnings.filters = self._warnings_filters def test_uri_scheme(self): self.assertNotIn('file', self.core.uri_schemes) diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index db5de58b..48257ff4 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals import random import unittest +import warnings import pykka @@ -36,8 +37,13 @@ class LocalTracklistProviderTest(unittest.TestCase): assert len(self.tracks) == 3, 'Need three tracks to run tests.' + self._warnings_filters = warnings.filters + warnings.filters = warnings.filters[:] + warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() + warnings.filters = self._warnings_filters def test_length(self): self.assertEqual(0, len(self.controller.tl_tracks)) From f4c93619d1bae7534cc2be5212e60137c22063f6 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 29 Mar 2015 23:04:02 +0200 Subject: [PATCH 18/25] core: Make core tracklist.add(tracks=...) deprecation safe --- mopidy/core/tracklist.py | 6 ++++++ tests/core/test_events.py | 14 ++++++------ tests/core/test_playback.py | 41 ++++++++++++++++++++++++++++++------ tests/core/test_tracklist.py | 39 +++++++++++++++++++--------------- 4 files changed, 69 insertions(+), 31 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 54206881..182cffba 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -333,6 +333,12 @@ class TracklistController(object): assert tracks is not None or uri is not None or uris is not None, \ 'tracks, uri or uris must be provided' + # TODO: assert that tracks are track instances + + if tracks: + warnings.warn('tracklist.add() "tracks" argument is deprecated.', + DeprecationWarning) + if uri: warnings.warn('tracklist.add() "uri" argument is deprecated.', DeprecationWarning) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index a197972b..09244b0d 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -17,6 +17,8 @@ from tests import dummy_backend class BackendEventsTest(unittest.TestCase): def setUp(self): # noqa: N802 self.backend = dummy_backend.create_proxy() + self.backend.library.dummy_library = [ + Track(uri='dummy:a'), Track(uri='dummy:b')] with warnings.catch_warnings(): warnings.simplefilter('ignore') @@ -45,12 +47,12 @@ class BackendEventsTest(unittest.TestCase): def test_tracklist_add_sends_tracklist_changed_event(self, send): send.reset_mock() - self.core.tracklist.add([Track(uri='dummy:a')]).get() + self.core.tracklist.add(uris=['dummy:a']).get() self.assertEqual(send.call_args[0][0], 'tracklist_changed') def test_tracklist_clear_sends_tracklist_changed_event(self, send): - self.core.tracklist.add([Track(uri='dummy:a')]).get() + self.core.tracklist.add(uris=['dummy:a']).get() send.reset_mock() self.core.tracklist.clear().get() @@ -58,8 +60,7 @@ class BackendEventsTest(unittest.TestCase): self.assertEqual(send.call_args[0][0], 'tracklist_changed') def test_tracklist_move_sends_tracklist_changed_event(self, send): - self.core.tracklist.add( - [Track(uri='dummy:a'), Track(uri='dummy:b')]).get() + self.core.tracklist.add(uris=['dummy:a', 'dummy:b']).get() send.reset_mock() self.core.tracklist.move(0, 1, 1).get() @@ -67,7 +68,7 @@ class BackendEventsTest(unittest.TestCase): self.assertEqual(send.call_args[0][0], 'tracklist_changed') def test_tracklist_remove_sends_tracklist_changed_event(self, send): - self.core.tracklist.add([Track(uri='dummy:a')]).get() + self.core.tracklist.add(uris=['dummy:a']).get() send.reset_mock() self.core.tracklist.remove(uri=['dummy:a']).get() @@ -75,8 +76,7 @@ class BackendEventsTest(unittest.TestCase): self.assertEqual(send.call_args[0][0], 'tracklist_changed') def test_tracklist_shuffle_sends_tracklist_changed_event(self, send): - self.core.tracklist.add( - [Track(uri='dummy:a'), Track(uri='dummy:b')]).get() + self.core.tracklist.add(uris=['dummy:a', 'dummy:b']).get() send.reset_mock() self.core.tracklist.shuffle().get() diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 7c4db0d6..d09950b2 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -13,6 +13,7 @@ from tests import dummy_audio as audio # TODO: split into smaller easier to follow tests. setup is way to complex. +# TODO: just mock tracklist? class CorePlaybackTest(unittest.TestCase): def setUp(self): # noqa: N802 self.backend1 = mock.Mock() @@ -42,14 +43,32 @@ class CorePlaybackTest(unittest.TestCase): Track(uri='dummy1:c', length=None), # No duration ] + self.uris = [ + 'dummy1:a', 'dummy2:a', 'dummy3:a', 'dummy1:b', 'dummy1:c'] + self.core = core.Core(mixer=None, backends=[ self.backend1, self.backend2, self.backend3]) - self.core.tracklist.add(self.tracks) + + def lookup(uris): + result = {uri: [] for uri in uris} + for track in self.tracks: + if track.uri in result: + result[track.uri].append(track) + return result + + self.lookup_patcher = mock.patch.object(self.core.library, 'lookup') + self.lookup_mock = self.lookup_patcher.start() + self.lookup_mock.side_effect = lookup + + self.core.tracklist.add(uris=self.uris) self.tl_tracks = self.core.tracklist.tl_tracks self.unplayable_tl_track = self.tl_tracks[2] self.duration_less_tl_track = self.tl_tracks[4] + def tearDown(self): # noqa: N802 + self.lookup_patcher.stop() + def trigger_end_of_track(self): self.core.playback._on_end_of_track() @@ -136,7 +155,7 @@ class CorePlaybackTest(unittest.TestCase): self.playback2.change_track.return_value.get.return_value = False self.core.tracklist.clear() - self.core.tracklist.add(self.tracks[:2]) + self.core.tracklist.add(uris=self.uris[:2]) tl_tracks = self.core.tracklist.tl_tracks self.core.playback.play(tl_tracks[0]) @@ -591,11 +610,16 @@ class TestStream(unittest.TestCase): self.tracks = [Track(uri='dummy:a', length=1234), Track(uri='dummy:b', length=1234)] - self.core.tracklist.add(self.tracks) + self.lookup_patcher = mock.patch.object(self.core.library, 'lookup') + self.lookup_mock = self.lookup_patcher.start() + self.lookup_mock.return_value = {t.uri: [t] for t in self.tracks} + + self.core.tracklist.add(uris=[t.uri for t in self.tracks]) self.events = [] - self.patcher = mock.patch('mopidy.audio.listener.AudioListener.send') - self.send_mock = self.patcher.start() + self.send_patcher = mock.patch( + 'mopidy.audio.listener.AudioListener.send') + self.send_mock = self.send_patcher.start() def send(event, **kwargs): self.events.append((event, kwargs)) @@ -604,7 +628,8 @@ class TestStream(unittest.TestCase): def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() - self.patcher.stop() + self.lookup_patcher.stop() + self.send_patcher.stop() def replay_audio_events(self): while self.events: @@ -664,7 +689,9 @@ class CorePlaybackWithOldBackendTest(unittest.TestCase): b.uri_schemes.get.return_value = ['dummy1'] b.playback = mock.Mock(spec=backend.PlaybackProvider) b.playback.play.side_effect = TypeError + b.library.lookup.return_value.get.return_value = [ + Track(uri='dummy1:a', length=40000)] c = core.Core(mixer=None, backends=[b]) - c.tracklist.add([Track(uri='dummy1:a', length=40000)]) + c.tracklist.add(uris=['dummy1:a']) c.playback.play() # No TypeError == test passed. diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index c8cbf97f..60d70547 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -17,44 +17,49 @@ class TracklistTest(unittest.TestCase): Track(uri='dummy1:c', name='bar'), ] + def lookup(uri): + future = mock.Mock() + future.get.return_value = [t for t in self.tracks if t.uri == uri] + return future + self.backend = mock.Mock() self.backend.uri_schemes.get.return_value = ['dummy1'] self.library = mock.Mock(spec=backend.LibraryProvider) + self.library.lookup.side_effect = lookup self.backend.library = self.library self.core = core.Core(mixer=None, backends=[self.backend]) - self.tl_tracks = self.core.tracklist.add(self.tracks) + self.tl_tracks = self.core.tracklist.add(uris=[ + t.uri for t in self.tracks]) def test_add_by_uri_looks_up_uri_in_library(self): - track = Track(uri='dummy1:x', name='x') - self.library.lookup.return_value.get.return_value = [track] + self.library.lookup.reset_mock() + self.core.tracklist.clear() with warnings.catch_warnings(): warnings.filterwarnings('ignore', r'tracklist.add.*"uri".*') - tl_tracks = self.core.tracklist.add(uri='dummy1:x') + tl_tracks = self.core.tracklist.add(uris=['dummy1:a']) - self.library.lookup.assert_called_once_with('dummy1:x') + self.library.lookup.assert_called_once_with('dummy1:a') self.assertEqual(1, len(tl_tracks)) - self.assertEqual(track, tl_tracks[0].track) + self.assertEqual(self.tracks[0], tl_tracks[0].track) self.assertEqual(tl_tracks, self.core.tracklist.tl_tracks[-1:]) def test_add_by_uris_looks_up_uris_in_library(self): - track1 = Track(uri='dummy1:x', name='x') - track2 = Track(uri='dummy1:y1', name='y1') - track3 = Track(uri='dummy1:y2', name='y2') - self.library.lookup.return_value.get.side_effect = [ - [track1], [track2, track3]] + self.library.lookup.reset_mock() + self.core.tracklist.clear() - tl_tracks = self.core.tracklist.add(uris=['dummy1:x', 'dummy1:y']) + tl_tracks = self.core.tracklist.add(uris=[t.uri for t in self.tracks]) self.library.lookup.assert_has_calls([ - mock.call('dummy1:x'), - mock.call('dummy1:y'), + mock.call('dummy1:a'), + mock.call('dummy1:b'), + mock.call('dummy1:c'), ]) self.assertEqual(3, len(tl_tracks)) - self.assertEqual(track1, tl_tracks[0].track) - self.assertEqual(track2, tl_tracks[1].track) - self.assertEqual(track3, tl_tracks[2].track) + self.assertEqual(self.tracks[0], tl_tracks[0].track) + self.assertEqual(self.tracks[1], tl_tracks[1].track) + self.assertEqual(self.tracks[2], tl_tracks[2].track) self.assertEqual( tl_tracks, self.core.tracklist.tl_tracks[-len(tl_tracks):]) From d44e8ff6f7bff0fad1ab4d752dd8a8a55048db75 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 29 Mar 2015 23:27:42 +0200 Subject: [PATCH 19/25] core: Add warning when doing library.search with a query. Tests and code that rely on this are not yet "warnings safe". --- mopidy/core/library.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mopidy/core/library.py b/mopidy/core/library.py index f87c9aa7..fdafaed0 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -254,6 +254,11 @@ class LibraryController(object): 'library.search() with keyword argument query is deprecated', DeprecationWarning) + if not query: + warnings.warn( + 'library.search() with an empty "query" argument deprecated', + DeprecationWarning) + futures = {} for backend, backend_uris in self._get_backends_to_uris(uris).items(): futures[backend] = backend.library.search( From bd1e822fea13a5b25c554be78e76b9629922159a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Mar 2015 21:04:56 +0200 Subject: [PATCH 20/25] utils: Create warn and ignore deprecation warning helpers This moves all the deprecation warnings messages to a central place so that it is easy to match against them without having to redefine the same regex all over the place. Each message has been given a message id which is more or less module.func:extra-info. This is not intended to be parsed, just used in tests when using the ignore helper. --- mopidy/audio/actor.py | 6 +-- mopidy/core/library.py | 17 +++---- mopidy/core/playback.py | 26 +++++------ mopidy/core/playlists.py | 11 ++--- mopidy/core/tracklist.py | 26 +++++------ mopidy/mpd/protocol/current_playlist.py | 9 ++-- mopidy/mpd/protocol/playback.py | 7 +-- mopidy/utils/deprecation.py | 61 +++++++++++++++++++++++++ 8 files changed, 101 insertions(+), 62 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 7b3e7985..802c67d1 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, unicode_literals import logging import os -import warnings import gobject @@ -17,7 +16,7 @@ from mopidy import exceptions from mopidy.audio import playlists, utils from mopidy.audio.constants import PlaybackState from mopidy.audio.listener import AudioListener -from mopidy.utils import process +from mopidy.utils import deprecation, process logger = logging.getLogger(__name__) @@ -606,8 +605,7 @@ class Audio(pykka.ThreadingActor): .. deprecated:: 1.0 Use :meth:`emit_data` with a :class:`None` buffer instead. """ - warnings.warn('audio.emit_end_of_stream() is deprecated.', - DeprecationWarning) + deprecation.warn('audio.emit_end_of_stream') self._appsrc.push(None) def set_about_to_finish_callback(self, callback): diff --git a/mopidy/core/library.py b/mopidy/core/library.py index fdafaed0..995c5e58 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -4,10 +4,12 @@ import collections import logging import operator import urlparse -import warnings import pykka +from mopidy.utils import deprecation + + logger = logging.getLogger(__name__) @@ -133,7 +135,7 @@ class LibraryController(object): .. deprecated:: 1.0 Use :meth:`search` with ``exact`` set. """ - warnings.warn('library.find_exact() is deprecated', DeprecationWarning) + deprecation.warn('core.library.find_exact') return self.search(query=query, uris=uris, exact=True, **kwargs) def lookup(self, uri=None, uris=None): @@ -163,8 +165,7 @@ class LibraryController(object): raise ValueError("One of 'uri' or 'uris' must be set") if uri: - warnings.warn('library.lookup() "uri" argument is deprecated.', - DeprecationWarning) + deprecation.warn('core.library.lookup:uri_arg') if uri is not None: uris = [uri] @@ -250,14 +251,10 @@ class LibraryController(object): query = _normalize_query(query or kwargs) if kwargs: - warnings.warn( - 'library.search() with keyword argument query is deprecated', - DeprecationWarning) + deprecation.warn('core.library.search:kwargs_query') if not query: - warnings.warn( - 'library.search() with an empty "query" argument deprecated', - DeprecationWarning) + deprecation.warn('core.library.search:empty_query') futures = {} for backend, backend_uris in self._get_backends_to_uris(uris).items(): diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 61bbc60c..abf9ae8a 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -2,12 +2,10 @@ from __future__ import absolute_import, unicode_literals import logging import urlparse -import warnings from mopidy.audio import PlaybackState from mopidy.core import listener -from mopidy.utils.deprecation import deprecated_property - +from mopidy.utils import deprecation logger = logging.getLogger(__name__) @@ -48,7 +46,7 @@ class PlaybackController(object): """ self._current_tl_track = value - current_tl_track = deprecated_property(get_current_tl_track) + current_tl_track = deprecation.deprecated_property(get_current_tl_track) """ .. deprecated:: 1.0 Use :meth:`get_current_tl_track` instead. @@ -66,7 +64,7 @@ class PlaybackController(object): if tl_track is not None: return tl_track.track - current_track = deprecated_property(get_current_track) + current_track = deprecation.deprecated_property(get_current_track) """ .. deprecated:: 1.0 Use :meth:`get_current_track` instead. @@ -103,7 +101,7 @@ class PlaybackController(object): self._trigger_playback_state_changed(old_state, new_state) - state = deprecated_property(get_state, set_state) + state = deprecation.deprecated_property(get_state, set_state) """ .. deprecated:: 1.0 Use :meth:`get_state` and :meth:`set_state` instead. @@ -117,7 +115,7 @@ class PlaybackController(object): else: return 0 - time_position = deprecated_property(get_time_position) + time_position = deprecation.deprecated_property(get_time_position) """ .. deprecated:: 1.0 Use :meth:`get_time_position` instead. @@ -129,8 +127,7 @@ class PlaybackController(object): Use :meth:`core.mixer.get_volume() ` instead. """ - warnings.warn( - 'playback.get_volume() is deprecated', DeprecationWarning) + deprecation.warn('core.playback.get_volume') return self.core.mixer.get_volume() def set_volume(self, volume): @@ -139,11 +136,10 @@ class PlaybackController(object): Use :meth:`core.mixer.set_volume() ` instead. """ - warnings.warn( - 'playback.set_volume() is deprecated', DeprecationWarning) + deprecation.warn('core.playback.set_volume') return self.core.mixer.set_volume(volume) - volume = deprecated_property(get_volume, set_volume) + volume = deprecation.deprecated_property(get_volume, set_volume) """ .. deprecated:: 1.0 Use :meth:`core.mixer.get_volume() @@ -158,7 +154,7 @@ class PlaybackController(object): Use :meth:`core.mixer.get_mute() ` instead. """ - warnings.warn('playback.get_mute() is deprecated', DeprecationWarning) + deprecation.warn('core.playback.get_mute') return self.core.mixer.get_mute() def set_mute(self, mute): @@ -167,10 +163,10 @@ class PlaybackController(object): Use :meth:`core.mixer.set_mute() ` instead. """ - warnings.warn('playback.set_mute() is deprecated', DeprecationWarning) + deprecation.warn('core.playback.set_mute') return self.core.mixer.set_mute(mute) - mute = deprecated_property(get_mute, set_mute) + mute = deprecation.deprecated_property(get_mute, set_mute) """ .. deprecated:: 1.0 Use :meth:`core.mixer.get_mute() diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index b6f2e726..2c997d84 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -2,14 +2,12 @@ from __future__ import absolute_import, unicode_literals import logging import urlparse -import warnings import pykka from mopidy.core import listener from mopidy.models import Playlist -from mopidy.utils.deprecation import deprecated_property - +from mopidy.utils import deprecation logger = logging.getLogger(__name__) @@ -81,8 +79,7 @@ class PlaylistsController(object): .. deprecated:: 1.0 Use :meth:`as_list` and :meth:`get_items` instead. """ - warnings.warn( - 'playlists.get_playlists() is deprecated', DeprecationWarning) + deprecation.warn('core.playlists.get_playlists') playlist_refs = self.as_list() @@ -97,7 +94,7 @@ class PlaylistsController(object): return [ Playlist(uri=r.uri, name=r.name) for r in playlist_refs] - playlists = deprecated_property(get_playlists) + playlists = deprecation.deprecated_property(get_playlists) """ .. deprecated:: 1.0 Use :meth:`as_list` and :meth:`get_items` instead. @@ -170,7 +167,7 @@ class PlaylistsController(object): .. deprecated:: 1.0 Use :meth:`as_list` and filter yourself. """ - warnings.warn('playlists.filter() is deprecated', DeprecationWarning) + deprecation.warn('core.playlists.filter') criteria = criteria or kwargs matches = self.playlists diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 182cffba..9a251b75 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -3,13 +3,11 @@ from __future__ import absolute_import, unicode_literals import collections import logging import random -import warnings from mopidy import compat from mopidy.core import listener from mopidy.models import TlTrack -from mopidy.utils.deprecation import deprecated_property - +from mopidy.utils import deprecation logger = logging.getLogger(__name__) @@ -31,7 +29,7 @@ class TracklistController(object): """Get tracklist as list of :class:`mopidy.models.TlTrack`.""" return self._tl_tracks[:] - tl_tracks = deprecated_property(get_tl_tracks) + tl_tracks = deprecation.deprecated_property(get_tl_tracks) """ .. deprecated:: 1.0 Use :meth:`get_tl_tracks` instead. @@ -41,7 +39,7 @@ class TracklistController(object): """Get tracklist as list of :class:`mopidy.models.Track`.""" return [tl_track.track for tl_track in self._tl_tracks] - tracks = deprecated_property(get_tracks) + tracks = deprecation.deprecated_property(get_tracks) """ .. deprecated:: 1.0 Use :meth:`get_tracks` instead. @@ -51,7 +49,7 @@ class TracklistController(object): """Get length of the tracklist.""" return len(self._tl_tracks) - length = deprecated_property(get_length) + length = deprecation.deprecated_property(get_length) """ .. deprecated:: 1.0 Use :meth:`get_length` instead. @@ -71,7 +69,7 @@ class TracklistController(object): self.core.playback._on_tracklist_change() self._trigger_tracklist_changed() - version = deprecated_property(get_version) + version = deprecation.deprecated_property(get_version) """ .. deprecated:: 1.0 Use :meth:`get_version` instead. @@ -99,7 +97,7 @@ class TracklistController(object): self._trigger_options_changed() return setattr(self, '_consume', value) - consume = deprecated_property(get_consume, set_consume) + consume = deprecation.deprecated_property(get_consume, set_consume) """ .. deprecated:: 1.0 Use :meth:`get_consume` and :meth:`set_consume` instead. @@ -131,7 +129,7 @@ class TracklistController(object): random.shuffle(self._shuffled) return setattr(self, '_random', value) - random = deprecated_property(get_random, set_random) + random = deprecation.deprecated_property(get_random, set_random) """ .. deprecated:: 1.0 Use :meth:`get_random` and :meth:`set_random` instead. @@ -164,7 +162,7 @@ class TracklistController(object): self._trigger_options_changed() return setattr(self, '_repeat', value) - repeat = deprecated_property(get_repeat, set_repeat) + repeat = deprecation.deprecated_property(get_repeat, set_repeat) """ .. deprecated:: 1.0 Use :meth:`get_repeat` and :meth:`set_repeat` instead. @@ -194,7 +192,7 @@ class TracklistController(object): self._trigger_options_changed() return setattr(self, '_single', value) - single = deprecated_property(get_single, set_single) + single = deprecation.deprecated_property(get_single, set_single) """ .. deprecated:: 1.0 Use :meth:`get_single` and :meth:`set_single` instead. @@ -336,12 +334,10 @@ class TracklistController(object): # TODO: assert that tracks are track instances if tracks: - warnings.warn('tracklist.add() "tracks" argument is deprecated.', - DeprecationWarning) + deprecation.warn('core.tracklist.add:tracks_arg') if uri: - warnings.warn('tracklist.add() "uri" argument is deprecated.', - DeprecationWarning) + deprecation.warn('core.tracklist.add:uri_arg') if tracks is None: if uri is not None: diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index 080b6f2c..38ad4017 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -1,8 +1,7 @@ from __future__ import absolute_import, unicode_literals -import warnings - from mopidy.mpd import exceptions, protocol, translator +from mopidy.utils import deprecation @protocol.commands.add('add') @@ -163,8 +162,7 @@ def playlist(context): Do not use this, instead use ``playlistinfo``. """ - warnings.warn( - 'Do not use this, instead use playlistinfo', DeprecationWarning) + deprecation.warn('mpd.protocol.current_playlist.playlist') return playlistinfo(context) @@ -354,8 +352,7 @@ def swap(context, songpos1, songpos2): # TODO: do we need a tracklist.replace() context.core.tracklist.clear() - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + with deprecation.ignore('core.tracklist.add:tracks_arg'): context.core.tracklist.add(tracks=tracks).get() diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index 86f2e36b..6beb4277 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -1,9 +1,8 @@ from __future__ import absolute_import, unicode_literals -import warnings - from mopidy.core import PlaybackState from mopidy.mpd import exceptions, protocol +from mopidy.utils import deprecation @protocol.commands.add('consume', state=protocol.BOOL) @@ -134,9 +133,7 @@ def pause(context, state=None): - Calls ``pause`` without any arguments to toogle pause. """ if state is None: - warnings.warn( - 'The use of pause command w/o the PAUSE argument is deprecated.', - DeprecationWarning) + deprecation.warn('mpd.protocol.playback.pause:state_arg') if (context.core.playback.state.get() == PlaybackState.PLAYING): context.core.playback.pause() diff --git a/mopidy/utils/deprecation.py b/mopidy/utils/deprecation.py index bf4756d7..a22a248c 100644 --- a/mopidy/utils/deprecation.py +++ b/mopidy/utils/deprecation.py @@ -1,5 +1,66 @@ from __future__ import unicode_literals +import contextlib +import re +import warnings + +# Messages used in deprecation warnings are collected here so we can target +# them easily when ignoring warnings. +_MESSAGES = { + # Deprecated features mpd: + 'mpd.protocol.playback.pause:state_arg': + 'The use of pause command w/o the PAUSE argument is deprecated.', + 'mpd.protocol.current_playlist.playlist': + 'Do not use this, instead use playlistinfo', + + # Deprecated features in audio: + 'audio.emit_end_of_stream': 'audio.emit_end_of_stream() is deprecated', + + # Deprecated features in core libary: + 'core.library.find_exact': 'library.find_exact() is deprecated', + 'core.library.lookup:uri_arg': + 'library.lookup() "uri" argument is deprecated', + 'core.library.search:kwargs_query': + 'library.search() with keyword argument query is deprecated', + 'core.library.search:empty_query': + 'library.search() with an empty "query" argument deprecated', + + # Deprecated features in core playback: + 'core.playback.get_mute': 'playback.get_mute() is deprecated', + 'core.playback.set_mute': 'playback.set_mute() is deprecated', + 'core.playback.get_volume': 'playback.get_volume() is deprecated', + 'core.playback.set_volume': 'playback.set_volume() is deprecated', + + # Deprecated features in core playlists: + 'core.playlists.filter': 'playlists.filter() is deprecated', + 'core.playlists.get_playlists': 'playlists.get_playlists() is deprecated', + + # Deprecated features in core tracklist: + 'core.tracklist.add:tracks_arg': + 'tracklist.add() "tracks" argument is deprecated', + 'core.tracklist.add:uri_arg': + 'tracklist.add() "uri" argument is deprecated', +} + + +def warn(msg_id): + warnings.warn(_MESSAGES.get(msg_id, msg_id), DeprecationWarning) + + +@contextlib.contextmanager +def ignore(ids=None): + with warnings.catch_warnings(): + if isinstance(ids, basestring): + ids = [ids] + + if ids: + for msg_id in ids: + msg = re.escape(_MESSAGES.get(msg_id, msg_id)) + warnings.filterwarnings('ignore', msg, DeprecationWarning) + else: + warnings.filterwarnings('ignore', category=DeprecationWarning) + yield + def deprecated_property( getter=None, setter=None, message='Property is deprecated'): From 9ede14f4a13e8c9b68a39a8a4e93d59b2538c437 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Mar 2015 23:50:54 +0200 Subject: [PATCH 21/25] tests: Convert to using deprecation helpers across the board. --- tests/core/test_events.py | 5 +-- tests/core/test_library.py | 42 ++++++--------------- tests/core/test_playlists.py | 28 ++++---------- tests/core/test_tracklist.py | 5 +-- tests/local/__init__.py | 5 +-- tests/local/test_playback.py | 11 +++--- tests/local/test_tracklist.py | 11 +++--- tests/m3u/test_playlists.py | 16 +++----- tests/mpd/protocol/__init__.py | 5 +-- tests/mpd/protocol/test_current_playlist.py | 6 +-- tests/mpd/protocol/test_playback.py | 5 +-- tests/mpd/test_dispatcher.py | 5 +-- tests/mpd/test_status.py | 5 +-- tests/utils/test_jsonrpc.py | 6 +-- 14 files changed, 53 insertions(+), 102 deletions(-) diff --git a/tests/core/test_events.py b/tests/core/test_events.py index 09244b0d..443c1b7e 100644 --- a/tests/core/test_events.py +++ b/tests/core/test_events.py @@ -1,7 +1,6 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import mock @@ -9,6 +8,7 @@ import pykka from mopidy import core from mopidy.models import Track +from mopidy.utils import deprecation from tests import dummy_backend @@ -20,8 +20,7 @@ class BackendEventsTest(unittest.TestCase): self.backend.library.dummy_library = [ Track(uri='dummy:a'), Track(uri='dummy:b')] - with warnings.catch_warnings(): - warnings.simplefilter('ignore') + with deprecation.ignore(): self.core = core.Core.start(backends=[self.backend]).proxy() def tearDown(self): # noqa: N802 diff --git a/tests/core/test_library.py b/tests/core/test_library.py index eb3d4dc3..7e3c8698 100644 --- a/tests/core/test_library.py +++ b/tests/core/test_library.py @@ -1,12 +1,12 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import mock from mopidy import backend, core from mopidy.models import Image, Ref, SearchResult, Track +from mopidy.utils import deprecation class BaseCoreLibraryTest(unittest.TestCase): @@ -273,15 +273,9 @@ class CoreLibraryTest(BaseCoreLibraryTest): class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): - def setUp(self): # noqa: N802 - super(DeprecatedFindExactCoreLibraryTest, self).setUp() - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', '.*library.find_exact.*') - - def tearDown(self): # noqa: N802 - super(DeprecatedFindExactCoreLibraryTest, self).tearDown() - warnings.filters = self._warnings_filters + def run(self, result=None): + with deprecation.ignore('core.library.find_exact'): + return super(DeprecatedFindExactCoreLibraryTest, self).run(result) def test_find_exact_combines_results_from_all_backends(self): track1 = Track(uri='dummy1:a') @@ -360,15 +354,9 @@ class DeprecatedFindExactCoreLibraryTest(BaseCoreLibraryTest): class DeprecatedLookupCoreLibraryTest(BaseCoreLibraryTest): - def setUp(self): # noqa: N802 - super(DeprecatedLookupCoreLibraryTest, self).setUp() - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', 'library.lookup.*"uri" argument.*') - - def tearDown(self): # noqa: N802 - super(DeprecatedLookupCoreLibraryTest, self).tearDown() - warnings.filters = self._warnings_filters + def run(self, result=None): + with deprecation.ignore('core.library.lookup:uri_arg'): + return super(DeprecatedLookupCoreLibraryTest, self).run(result) def test_lookup_selects_dummy1_backend(self): self.core.library.lookup('dummy1:a') @@ -391,6 +379,10 @@ class DeprecatedLookupCoreLibraryTest(BaseCoreLibraryTest): class LegacyFindExactToSearchLibraryTest(unittest.TestCase): + def run(self, result=None): + with deprecation.ignore('core.library.find_exact'): + return super(LegacyFindExactToSearchLibraryTest, self).run(result) + def setUp(self): # noqa: N802 self.backend = mock.Mock() self.backend.actor_ref.actor_class.__name__ = 'DummyBackend' @@ -398,18 +390,8 @@ class LegacyFindExactToSearchLibraryTest(unittest.TestCase): self.backend.library = mock.Mock(spec=backend.LibraryProvider) self.core = core.Core(mixer=None, backends=[self.backend]) - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', '.*library.find_exact.*') - - def tearDown(self): # noqa: N802 - warnings.filters = self._warnings_filters - def test_core_find_exact_calls_backend_search_with_exact(self): - with warnings.catch_warnings(): - warnings.simplefilter('ignore') - self.core.library.find_exact(query={'any': ['a']}) - + self.core.library.find_exact(query={'any': ['a']}) self.backend.library.search.assert_called_once_with( query=dict(any=['a']), uris=None, exact=True) diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index dbe05733..b842ae44 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -1,12 +1,12 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import mock from mopidy import backend, core from mopidy.models import Playlist, Ref, Track +from mopidy.utils import deprecation class BasePlaylistsTest(unittest.TestCase): @@ -231,16 +231,10 @@ class PlaylistTest(BasePlaylistsTest): class DeprecatedFilterPlaylistsTest(BasePlaylistsTest): - def setUp(self): # noqa: N802 - super(DeprecatedFilterPlaylistsTest, self).setUp() - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', '.*filter.*') - warnings.filterwarnings('ignore', '.*get_playlists.*') - - def tearDown(self): # noqa: N802 - super(DeprecatedFilterPlaylistsTest, self).tearDown() - warnings.filters = self._warnings_filters + def run(self, result=None): + with deprecation.ignore(ids=['core.playlists.filter', + 'core.playlists.get_playlists']): + return super(DeprecatedFilterPlaylistsTest, self).run(result) def test_filter_returns_matching_playlists(self): result = self.core.playlists.filter(name='A') @@ -254,15 +248,9 @@ class DeprecatedFilterPlaylistsTest(BasePlaylistsTest): class DeprecatedGetPlaylistsTest(BasePlaylistsTest): - def setUp(self): # noqa: N802 - super(DeprecatedGetPlaylistsTest, self).setUp() - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', '.*get_playlists.*') - - def tearDown(self): # noqa: N802 - super(DeprecatedGetPlaylistsTest, self).tearDown() - warnings.filters = self._warnings_filters + def run(self, result=None): + with deprecation.ignore('core.playlists.get_playlists'): + return super(DeprecatedGetPlaylistsTest, self).run(result) def test_get_playlists_combines_result_from_backends(self): result = self.core.playlists.get_playlists() diff --git a/tests/core/test_tracklist.py b/tests/core/test_tracklist.py index 60d70547..96de0f80 100644 --- a/tests/core/test_tracklist.py +++ b/tests/core/test_tracklist.py @@ -1,12 +1,12 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import mock from mopidy import backend, core from mopidy.models import Track +from mopidy.utils import deprecation class TracklistTest(unittest.TestCase): @@ -36,8 +36,7 @@ class TracklistTest(unittest.TestCase): self.library.lookup.reset_mock() self.core.tracklist.clear() - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', r'tracklist.add.*"uri".*') + with deprecation.ignore('core.tracklist.add:uri_arg'): tl_tracks = self.core.tracklist.add(uris=['dummy1:a']) self.library.lookup.assert_called_once_with('dummy1:a') diff --git a/tests/local/__init__.py b/tests/local/__init__.py index bfd60044..3841a1e4 100644 --- a/tests/local/__init__.py +++ b/tests/local/__init__.py @@ -1,6 +1,6 @@ from __future__ import absolute_import, unicode_literals -import warnings +from mopidy.utils import deprecation def generate_song(i): @@ -9,8 +9,7 @@ def generate_song(i): def populate_tracklist(func): def wrapper(self): - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') + with deprecation.ignore('core.tracklist.add:tracks_arg'): self.tl_tracks = self.core.tracklist.add(self.tracks) return func(self) diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 28ded52a..2bda46d3 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, unicode_literals import time import unittest -import warnings import mock @@ -12,6 +11,7 @@ from mopidy import core from mopidy.core import PlaybackState from mopidy.local import actor from mopidy.models import Track +from mopidy.utils import deprecation from tests import dummy_audio, path_to_data_dir from tests.local import generate_song, populate_tracklist @@ -43,6 +43,10 @@ class LocalPlaybackProviderTest(unittest.TestCase): def trigger_end_of_track(self): self.playback._on_end_of_track() + def run(self, result=None): + with deprecation.ignore('core.tracklist.add:tracks_arg'): + return super(LocalPlaybackProviderTest, self).run(result) + def setUp(self): # noqa: N802 self.audio = dummy_audio.create_proxy() self.backend = actor.LocalBackend.start( @@ -56,13 +60,8 @@ class LocalPlaybackProviderTest(unittest.TestCase): assert self.tracks[0].length >= 2000, \ 'First song needs to be at least 2000 miliseconds' - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') - def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() - warnings.filters = self._warnings_filters def test_uri_scheme(self): self.assertNotIn('file', self.core.uri_schemes) diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index 48257ff4..22d4c954 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, unicode_literals import random import unittest -import warnings import pykka @@ -10,6 +9,7 @@ from mopidy import core from mopidy.core import PlaybackState from mopidy.local import actor from mopidy.models import Playlist, TlTrack, Track +from mopidy.utils import deprecation from tests import dummy_audio, path_to_data_dir from tests.local import generate_song, populate_tracklist @@ -27,6 +27,10 @@ class LocalTracklistProviderTest(unittest.TestCase): tracks = [ Track(uri=generate_song(i), length=4464) for i in range(1, 4)] + def run(self, result=None): + with deprecation.ignore('core.tracklist.add:tracks_arg'): + return super(LocalTracklistProviderTest, self).run(result) + def setUp(self): # noqa: N802 self.audio = dummy_audio.create_proxy() self.backend = actor.LocalBackend.start( @@ -37,13 +41,8 @@ class LocalTracklistProviderTest(unittest.TestCase): assert len(self.tracks) == 3, 'Need three tracks to run tests.' - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') - def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() - warnings.filters = self._warnings_filters def test_length(self): self.assertEqual(0, len(self.controller.tl_tracks)) diff --git a/tests/m3u/test_playlists.py b/tests/m3u/test_playlists.py index cf3265c3..ecb3d40e 100644 --- a/tests/m3u/test_playlists.py +++ b/tests/m3u/test_playlists.py @@ -4,7 +4,6 @@ import os import shutil import tempfile import unittest -import warnings import pykka @@ -12,6 +11,7 @@ from mopidy import core from mopidy.m3u import actor from mopidy.m3u.translator import playlist_uri_to_path from mopidy.models import Playlist, Track +from mopidy.utils import deprecation from tests import dummy_audio, path_to_data_dir from tests.m3u import generate_song @@ -272,16 +272,10 @@ class M3UPlaylistsProviderTest(unittest.TestCase): class DeprecatedM3UPlaylistsProviderTest(M3UPlaylistsProviderTest): - def setUp(self): # noqa: N802 - super(DeprecatedM3UPlaylistsProviderTest, self).setUp() - self._warnings_filters = warnings.filters - warnings.filters = warnings.filters[:] - warnings.filterwarnings('ignore', '.*filter.*') - warnings.filterwarnings('ignore', '.*get_playlists.*') - - def tearDown(self): # noqa: N802 - super(DeprecatedM3UPlaylistsProviderTest, self).tearDown() - warnings.filters = self._warnings_filters + def run(self, result=None): + with deprecation.ignore(ids=['core.playlists.filter', + 'core.playlists.get_playlists']): + return super(DeprecatedM3UPlaylistsProviderTest, self).run(result) def test_filter_without_criteria(self): self.assertEqual(self.core.playlists.get_playlists(), diff --git a/tests/mpd/protocol/__init__.py b/tests/mpd/protocol/__init__.py index cbbc1991..9ebe99b0 100644 --- a/tests/mpd/protocol/__init__.py +++ b/tests/mpd/protocol/__init__.py @@ -1,7 +1,6 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import mock @@ -9,6 +8,7 @@ import pykka from mopidy import core from mopidy.mpd import session, uri_mapper +from mopidy.utils import deprecation from tests import dummy_backend, dummy_mixer @@ -42,8 +42,7 @@ class BaseTestCase(unittest.TestCase): self.mixer = None self.backend = dummy_backend.create_proxy() - with warnings.catch_warnings(): - warnings.simplefilter('ignore') + with deprecation.ignore(): self.core = core.Core.start( mixer=self.mixer, backends=[self.backend]).proxy() diff --git a/tests/mpd/protocol/test_current_playlist.py b/tests/mpd/protocol/test_current_playlist.py index 84d905be..4fa1926a 100644 --- a/tests/mpd/protocol/test_current_playlist.py +++ b/tests/mpd/protocol/test_current_playlist.py @@ -1,8 +1,7 @@ from __future__ import absolute_import, unicode_literals -import warnings - from mopidy.models import Ref, Track +from mopidy.utils import deprecation from tests.mpd import protocol @@ -233,8 +232,7 @@ class PlaylistIdCommandTest(BasePopulatedTracklistTestCase): class PlaylistInfoCommandTest(BasePopulatedTracklistTestCase): def test_playlist_returns_same_as_playlistinfo(self): - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', message='.*playlistinfo.*') + with deprecation.ignore('mpd.protocol.current_playlist.playlist'): playlist_response = self.send_request('playlist') playlistinfo_response = self.send_request('playlistinfo') diff --git a/tests/mpd/protocol/test_playback.py b/tests/mpd/protocol/test_playback.py index 4d6e727d..328fe136 100644 --- a/tests/mpd/protocol/test_playback.py +++ b/tests/mpd/protocol/test_playback.py @@ -1,10 +1,10 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings from mopidy.core import PlaybackState from mopidy.models import Track +from mopidy.utils import deprecation from tests.mpd import protocol @@ -203,8 +203,7 @@ class PlaybackControlHandlerTest(protocol.BaseTestCase): self.assertEqual(PLAYING, self.core.playback.state.get()) self.assertInResponse('OK') - with warnings.catch_warnings(): - warnings.filterwarnings('ignore', message='.*pause command w/o.*') + with deprecation.ignore('mpd.protocol.playback.pause:state_arg'): self.send_request('pause') self.assertEqual(PAUSED, self.core.playback.state.get()) self.assertInResponse('OK') diff --git a/tests/mpd/test_dispatcher.py b/tests/mpd/test_dispatcher.py index 2c21df67..9e2838b7 100644 --- a/tests/mpd/test_dispatcher.py +++ b/tests/mpd/test_dispatcher.py @@ -1,13 +1,13 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import pykka from mopidy import core from mopidy.mpd.dispatcher import MpdDispatcher from mopidy.mpd.exceptions import MpdAckError +from mopidy.utils import deprecation from tests import dummy_backend @@ -23,8 +23,7 @@ class MpdDispatcherTest(unittest.TestCase): self.backend = dummy_backend.create_proxy() self.dispatcher = MpdDispatcher(config=config) - with warnings.catch_warnings(): - warnings.simplefilter('ignore') + with deprecation.ignore(): self.core = core.Core.start(backends=[self.backend]).proxy() def tearDown(self): # noqa: N802 diff --git a/tests/mpd/test_status.py b/tests/mpd/test_status.py index 675626f6..080cbfc6 100644 --- a/tests/mpd/test_status.py +++ b/tests/mpd/test_status.py @@ -1,7 +1,6 @@ from __future__ import absolute_import, unicode_literals import unittest -import warnings import pykka @@ -10,6 +9,7 @@ from mopidy.core import PlaybackState from mopidy.models import Track from mopidy.mpd import dispatcher from mopidy.mpd.protocol import status +from mopidy.utils import deprecation from tests import dummy_backend, dummy_mixer @@ -27,8 +27,7 @@ class StatusHandlerTest(unittest.TestCase): self.mixer = dummy_mixer.create_proxy() self.backend = dummy_backend.create_proxy() - with warnings.catch_warnings(): - warnings.simplefilter('ignore') + with deprecation.ignore(): self.core = core.Core.start( mixer=self.mixer, backends=[self.backend]).proxy() diff --git a/tests/utils/test_jsonrpc.py b/tests/utils/test_jsonrpc.py index 890c2aba..411c0db4 100644 --- a/tests/utils/test_jsonrpc.py +++ b/tests/utils/test_jsonrpc.py @@ -2,14 +2,13 @@ from __future__ import absolute_import, unicode_literals import json import unittest -import warnings import mock import pykka from mopidy import core, models -from mopidy.utils import jsonrpc +from mopidy.utils import deprecation, jsonrpc from tests import dummy_backend @@ -55,8 +54,7 @@ class JsonRpcTestBase(unittest.TestCase): self.backend = dummy_backend.create_proxy() self.calc = Calculator() - with warnings.catch_warnings(): - warnings.simplefilter('ignore') + with deprecation.ignore(): self.core = core.Core.start(backends=[self.backend]).proxy() self.jrw = jsonrpc.JsonRpcWrapper( From f78973074eb67939018cdb25f821252cc028c278 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Mar 2015 23:51:36 +0200 Subject: [PATCH 22/25] mpd: Only loop over tracks in lsinfo/listallinfo --- mopidy/mpd/protocol/music_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 3f1dd2bc..fc726255 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -338,7 +338,7 @@ def listallinfo(context, uri=None): if not lookup_future: result.append(('directory', path)) else: - for uri, tracks in lookup_future.get().items(): + for tracks in lookup_future.get().values(): for track in tracks: result.extend(translator.track_to_mpd_format(track)) return result @@ -366,7 +366,7 @@ def lsinfo(context, uri=None): if not lookup_future: result.append(('directory', path.lstrip('/'))) else: - for uri, tracks in lookup_future.get().items(): + for tracks in lookup_future.get().values(): if tracks: result.extend(translator.track_to_mpd_format(tracks[0])) From 887c0774fb2987e08a153fc6b138d07e78e3a3f1 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Mon, 30 Mar 2015 23:56:59 +0200 Subject: [PATCH 23/25] review: Update wording deprecation messages --- mopidy/utils/deprecation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/deprecation.py b/mopidy/utils/deprecation.py index a22a248c..31d0fdc1 100644 --- a/mopidy/utils/deprecation.py +++ b/mopidy/utils/deprecation.py @@ -21,9 +21,9 @@ _MESSAGES = { 'core.library.lookup:uri_arg': 'library.lookup() "uri" argument is deprecated', 'core.library.search:kwargs_query': - 'library.search() with keyword argument query is deprecated', + 'library.search() with "kwargs" as query is deprecated', 'core.library.search:empty_query': - 'library.search() with an empty "query" argument deprecated', + 'library.search() with empty "query" is argument deprecated', # Deprecated features in core playback: 'core.playback.get_mute': 'playback.get_mute() is deprecated', From e2faf7f08370b7e42dcfc1a92ac5209ebcd4ea24 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Mar 2015 00:01:34 +0200 Subject: [PATCH 24/25] docs: Update docstring and changelog --- docs/changelog.rst | 15 +++++++++++++++ mopidy/core/library.py | 15 +++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b976c169..47b6bb8c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,21 @@ Changelog This changelog is used to track all major changes to Mopidy. +v1.1.0 (unreleased) +=================== + +Core API +-------- + +- Calling :meth:`mopidy.core.library.LibraryController.search`` with ``kwargs`` + as the query is no longer supported (PR: :issue:`1090`) + +Internal changes +---------------- + +- Tests have been cleaned up to stop using deprecated APIs where feasible. + (Partial fix: :issue:`1083`, PR: :issue:`1090`) + v1.0.0 (2015-03-25) =================== diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 995c5e58..c787e013 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -209,12 +209,6 @@ class LibraryController(object): """ Search the library for tracks where ``field`` contains ``values``. - .. deprecated:: 1.0 - Previously, if the query was empty, and the backend could support - it, all available tracks were returned. This has not changed, but - it is strongly discouraged. No new code should rely on this - behavior. - If ``uris`` is given, the search is limited to results from within the URI roots. For example passing ``uris=['file:']`` will limit the search to the local backend. @@ -247,6 +241,15 @@ class LibraryController(object): .. versionadded:: 1.0 The ``exact`` keyword argument, which replaces :meth:`find_exact`. + + .. deprecated:: 1.0 + Previously, if the query was empty, and the backend could support + it, all available tracks were returned. This has not changed, but + it is strongly discouraged. No new code should rely on this + behavior. + + .. deprecated:: 1.1 + Providing the search query via ``kwargs`` is no longer supported. """ query = _normalize_query(query or kwargs) From 28237df30304bcb939f761fb4cf91cc36b35b5bf Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Tue, 31 Mar 2015 21:04:23 +0200 Subject: [PATCH 25/25] core: Fix deprecation message --- mopidy/utils/deprecation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/utils/deprecation.py b/mopidy/utils/deprecation.py index 31d0fdc1..57042347 100644 --- a/mopidy/utils/deprecation.py +++ b/mopidy/utils/deprecation.py @@ -23,7 +23,7 @@ _MESSAGES = { 'core.library.search:kwargs_query': 'library.search() with "kwargs" as query is deprecated', 'core.library.search:empty_query': - 'library.search() with empty "query" is argument deprecated', + 'library.search() with empty "query" argument deprecated', # Deprecated features in core playback: 'core.playback.get_mute': 'playback.get_mute() is deprecated',