diff --git a/.travis.yml b/.travis.yml index eb8aadfe..964ae89f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,18 +1,11 @@ -sudo: false +sudo: required +dist: trusty language: python python: - "2.7_with_system_site_packages" -addons: - apt: - sources: - - mopidy-stable - packages: - - graphviz-dev - - mopidy - env: - TOX_ENV=py27 - TOX_ENV=py27-tornado23 @@ -20,6 +13,11 @@ env: - TOX_ENV=docs - TOX_ENV=flake8 +before_install: + - "sudo sed -i '/127.0.1.1/d' /etc/hosts" # Workaround tornadoweb/tornado#1573 + - "sudo apt-get update -qq" + - "sudo apt-get install -y graphviz-dev gstreamer0.10-plugins-good python-gst0.10" + install: - "pip install tox" @@ -27,7 +25,7 @@ script: - "tox -e $TOX_ENV" after_success: - - "if [ $TOX_ENV == 'py27' ]; then pip install coveralls==1.0b1; coveralls; fi" + - "if [ $TOX_ENV == 'py27' ]; then pip install coveralls; coveralls; fi" branches: except: diff --git a/AUTHORS b/AUTHORS index 439b8ed7..38c394dc 100644 --- a/AUTHORS +++ b/AUTHORS @@ -75,3 +75,4 @@ - kozec - Jelle van der Waa - Alex Malone +- Daniel Hahler diff --git a/docs/api/core.rst b/docs/api/core.rst index 3aca2504..de686028 100644 --- a/docs/api/core.rst +++ b/docs/api/core.rst @@ -164,6 +164,8 @@ Playlists controller .. class:: mopidy.core.PlaylistsController +.. automethod:: mopidy.core.PlaylistsController.get_uri_schemes + Fetching -------- @@ -229,8 +231,8 @@ TracklistController .. autoattribute:: mopidy.core.TracklistController.repeat .. autoattribute:: mopidy.core.TracklistController.single -PlaylistsController -------------------- +PlaybackController +------------------ .. automethod:: mopidy.core.PlaybackController.get_mute .. automethod:: mopidy.core.PlaybackController.get_volume @@ -247,8 +249,8 @@ LibraryController .. automethod:: mopidy.core.LibraryController.find_exact -PlaybackController ------------------- +PlaylistsController +------------------- .. automethod:: mopidy.core.PlaylistsController.filter .. automethod:: mopidy.core.PlaylistsController.get_playlists diff --git a/docs/changelog.rst b/docs/changelog.rst index db76c501..09f35c14 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,15 +16,41 @@ Core API - Start ``tlid`` counting at 1 instead of 0 to keep in sync with MPD's ``songid``. +- :meth:`~mopidy.core.PlaybackController.get_time_position` now returns the + seek target while a seek is in progress. This gives better results than just + failing the position query. (Fixes: :issue:`312` PR: :issue:`1346`) + +- Add :meth:`mopidy.core.PlaylistsController.get_uri_schemes`. (PR: + :issue:`1362`) + - Persist state between runs. The amount of data to persist can be controlled by config value :confval:`core/restore_state` +Models +------ + +- **Deprecated:** :attr:`mopidy.models.Album.images` is deprecated. Use + :meth:`mopidy.core.LibraryController.get_images` instead. (Fixes: + :issue:`1325`) + +Extension support +----------------- + +- Log exception and continue if an extension crashes during setup. Previously, + we let Mopidy crash if an extension's setup crashed. (PR: :issue:`1337`) + Local backend -------------- - Made :confval:`local/data_dir` really deprecated. This change breaks older versions of Mopidy-Local-SQLite and Mopidy-Local-Images. +M3U backend +----------- + +- Derive track name from file name for non-extended M3U + playlists. (Fixes: :issue:`1364`, PR: :issue:`1369`) + MPD frontend ------------ @@ -42,6 +68,18 @@ MPD frontend - Start ``songid`` counting at 1 instead of 0 to match the original MPD server. +- Idle events are now emitted on ``seekeded`` events. This fix means that + clients relying on ``idle`` events now get notified about seeks. + (Fixes: :issue:`1331` :issue:`1347`) + +- Idle events are now emitted on ``playlists_loaded`` events. This fix means + that clients relying on ``idle`` events now get notified about playlist loads. + (Fixes: :issue:`1331` PR: :issue:`1347`) + +- Event handler for ``playlist_deleted`` has been unbroken. This unreported bug + would cause the MPD Frontend to crash preventing any further communication + via the MPD protocol. (PR: :issue:`1347`) + Zeroconf -------- @@ -61,6 +99,12 @@ Cleanups - Removed warning if :file:`~/.config/mopidy/settings.py` exists. We stopped using this settings file in 0.14, released in April 2013. +- The ``on_event`` handler in our listener helper now catches exceptions. This + means that any errors in event handling won't crash the actor in question. + +- Catch errors when loading :confval:`logging/config_file`. + (Fixes: :issue:`1320`) + Gapless ------- @@ -73,6 +117,11 @@ Gapless - Tests have been updated to always use a core actor so async state changes don't trip us up. +- Seek events are now triggered when the seek completes. Previously the event + was emitted when the seek was requested, not when it completed. Further + changes have been made to make seek work correctly for gapless related corner + cases. (Fixes: :issue:`1305` PR: :issue:`1346`) + v1.1.2 (UNRELEASED) =================== diff --git a/docs/clients/mpd.rst b/docs/clients/mpd.rst index c7d6ca7b..b070092a 100644 --- a/docs/clients/mpd.rst +++ b/docs/clients/mpd.rst @@ -167,5 +167,5 @@ projects are a real match made in heaven." Partify ------- -`Partify `_ is a web based MPD client focusing on +`Partify `_ is a web based MPD client focussing on making music playing collaborative and social. diff --git a/docs/conf.py b/docs/conf.py index 88ff29eb..bd553ae4 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -111,11 +111,7 @@ modindex_common_prefix = ['mopidy.'] # -- Options for HTML output -------------------------------------------------- -# 'sphinx_rtd_theme' is bundled with Sphinx 1.3, which we don't have when -# building the docs as part of the Debian packages on e.g. Debian wheezy. -# html_theme = 'sphinx_rtd_theme' -html_theme = 'default' -html_theme_path = ['_themes'] +html_theme = 'sphinx_rtd_theme' html_static_path = ['_static'] html_use_modindex = True diff --git a/docs/config.rst b/docs/config.rst index 5b7a2c29..3a2046f7 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -217,7 +217,7 @@ Proxy configuration ------------------- Not all parts of Mopidy or all Mopidy extensions respect the proxy -server configuration when connecting to the Internt. Currently, this is at +server configuration when connecting to the Internet. Currently, this is at least used when Mopidy's audio subsystem reads media directly from the network, like when listening to Internet radio streams, and by the Mopidy-Spotify extension. With time, we hope that more of the Mopidy ecosystem will respect diff --git a/docs/contributing.rst b/docs/contributing.rst index b5230b18..199c6b2a 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -126,3 +126,11 @@ Pull request guidelines #. Send a pull request to the ``develop`` branch. See the `GitHub pull request docs `_ for help. + +.. note:: + + If you are contributing a bug fix for a specific minor version of Mopidy + you should create the branch based on ``release-x.y`` instead of + ``develop``. When the release is done the changes will be merged back into + ``develop`` automatically as part of the normal release process. See + :ref:`creating-releases`. diff --git a/docs/ext/frontends.rst b/docs/ext/frontends.rst index 50dc348f..1e2ad3f4 100644 --- a/docs/ext/frontends.rst +++ b/docs/ext/frontends.rst @@ -81,4 +81,4 @@ Mopidy-Webhooks https://github.com/paddycarey/mopidy-webhooks Extension for sending HTTP POST requests with JSON payloads to a remote server -on when Mopidy core triggers an event and on regular intervals. +when Mopidy core triggers an event and on regular intervals. diff --git a/docs/ext/mixers.rst b/docs/ext/mixers.rst index 88fd27dd..5023f285 100644 --- a/docs/ext/mixers.rst +++ b/docs/ext/mixers.rst @@ -17,7 +17,7 @@ Mopidy-ALSAMixer https://github.com/mopidy/mopidy-alsamixer -Extension for controlling volume one a Linux system using ALSA. +Extension for controlling volume on a Linux system using ALSA. Mopidy-Arcam diff --git a/docs/ext/web.rst b/docs/ext/web.rst index bbdf4d0c..a6e2d748 100644 --- a/docs/ext/web.rst +++ b/docs/ext/web.rst @@ -35,8 +35,8 @@ Mopidy-Local-Images https://github.com/tkem/mopidy-local-images -Not a full-featured Web client, but rather a local library and Web -extension which allows other Web clients access to album art embedded +Not a full-featured web client, but rather a local library and web +extension which allows other web clients access to album art embedded in local media files. .. image:: /ext/local_images.jpg @@ -69,7 +69,7 @@ Mopidy-Mobile https://github.com/tkem/mopidy-mobile -A Mopidy Web client extension and hybrid mobile app, made with Ionic, +A Mopidy web client extension and hybrid mobile app, made with Ionic, AngularJS and Apache Cordova by Thomas Kemmer. .. image:: /ext/mobile.png @@ -132,18 +132,6 @@ To install, run:: pip install Mopidy-MusicBox-Webclient -Mopidy-Party -============ - -https://github.com/Lesterpig/mopidy-party - -Minimal web client designed for collaborative music management during parties. - -.. image:: /ext/mopidy_party.png - -To install, run:: - - pip install Mopidy-Party Mopidy-Party ============ diff --git a/docs/installation/raspberrypi.rst b/docs/installation/raspberrypi.rst index 495b0776..9bf9f550 100644 --- a/docs/installation/raspberrypi.rst +++ b/docs/installation/raspberrypi.rst @@ -181,7 +181,7 @@ Appendix C: Installation on XBian Similar to the Raspbmc issue outlined in Appendix B, it's not possible to install Mopidy on XBian without first resolving a dependency problem between ``gstreamer0.10-plugins-good`` and ``libtag1c2a``. More information can be -found in `this post +found in `this issue `_. Run the following commands to remedy this and then install Mopidy as normal:: diff --git a/docs/releasing.rst b/docs/releasing.rst index 4c2d8373..8d489146 100644 --- a/docs/releasing.rst +++ b/docs/releasing.rst @@ -6,6 +6,7 @@ Here we try to keep an up to date record of how Mopidy releases are made. This documentation serves both as a checklist, to reduce the project's dependency on key individuals, and as a stepping stone to more automation. +.. _creating-releases: Creating releases ================= diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index cc229827..817beb0f 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -92,6 +92,9 @@ class Core( def stream_changed(self, uri): self.playback._on_stream_changed(uri) + def position_changed(self, position): + self.playback._on_position_changed(position) + def state_changed(self, old_state, new_state, target_state): # XXX: This is a temporary fix for issue #232 while we wait for a more # permanent solution with the implementation of issue #234. When the diff --git a/mopidy/core/library.py b/mopidy/core/library.py index f254172f..30064e5a 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -149,7 +149,7 @@ class LibraryController(object): """Lookup the images for the given URIs Backends can use this to return image URIs for any URI they know about - be it tracks, albums, playlists... The lookup result is a dictionary + be it tracks, albums, playlists. The lookup result is a dictionary mapping the provided URIs to lists of images. Unknown URIs or URIs the corresponding backend couldn't find anything diff --git a/mopidy/core/listener.py b/mopidy/core/listener.py index 8feb0324..b8ef734d 100644 --- a/mopidy/core/listener.py +++ b/mopidy/core/listener.py @@ -31,7 +31,8 @@ class CoreListener(listener.Listener): :type event: string :param kwargs: any other arguments to the specific event handlers """ - getattr(self, event)(**kwargs) + # Just delegate to parent, entry mostly for docs. + super(CoreListener, self).on_event(event, **kwargs) def track_playback_paused(self, tl_track, time_position): """ diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 5efbd382..1779ed77 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -26,6 +26,10 @@ class PlaybackController(object): self._current_tl_track = None self._pending_tl_track = None + self._pending_position = None + self._last_position = None + self._previous = False + if self._audio: self._audio.set_about_to_finish_callback( self._on_about_to_finish_callback) @@ -127,6 +131,8 @@ class PlaybackController(object): def get_time_position(self): """Get time position in milliseconds.""" + if self._pending_position is not None: + return self._pending_position backend = self._get_backend(self.get_current_tl_track()) if backend: return backend.playback.get_time_position().get() @@ -197,15 +203,35 @@ class PlaybackController(object): def _on_end_of_stream(self): self.set_state(PlaybackState.STOPPED) + if self._current_tl_track: + self._trigger_track_playback_ended(self.get_time_position()) self._set_current_tl_track(None) - # TODO: self._trigger_track_playback_ended? def _on_stream_changed(self, uri): + if self._last_position is None: + position = self.get_time_position() + else: + # This code path handles the stop() case, uri should be none. + position, self._last_position = self._last_position, None + + if self._pending_position is None: + self._trigger_track_playback_ended(position) + self._stream_title = None if self._pending_tl_track: self._set_current_tl_track(self._pending_tl_track) self._pending_tl_track = None - self._trigger_track_playback_started() + + if self._pending_position is None: + self.set_state(PlaybackState.PLAYING) + self._trigger_track_playback_started() + else: + self._seek(self._pending_position) + + def _on_position_changed(self, position): + if self._pending_position == position: + self._trigger_seeked(position) + self._pending_position = None def _on_about_to_finish_callback(self): """Callback that performs a blocking actor call to the real callback. @@ -221,7 +247,8 @@ class PlaybackController(object): }) def _on_about_to_finish(self): - self._trigger_track_playback_ended(self.get_time_position()) + if self._state == PlaybackState.STOPPED: + return # TODO: check that we always have a current track original_tl_track = self.get_current_tl_track() @@ -235,8 +262,6 @@ class PlaybackController(object): if backend: backend.playback.change_track(next_tl_track.track).get() - self.core.tracklist._mark_played(original_tl_track) - def _on_tracklist_change(self): """ Tell the playback controller that the current playlist has changed. @@ -259,10 +284,6 @@ class PlaybackController(object): state = self.get_state() current = self._pending_tl_track or self._current_tl_track - # TODO: move to pending track? - self._trigger_track_playback_ended(self.get_time_position()) - self.core.tracklist._mark_played(self._current_tl_track) - while current: pending = self.core.tracklist.next_track(current) if self._change(pending, state): @@ -321,17 +342,9 @@ class PlaybackController(object): self.resume() return - original = self._current_tl_track current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) - if original != pending and self.get_state() != PlaybackState.STOPPED: - self._trigger_track_playback_ended(self.get_time_position()) - - if pending: - # TODO: remove? - self.set_state(PlaybackState.PLAYING) - while pending: # TODO: should we consume unplayable tracks in this loop? if self._change(pending, PlaybackState.PLAYING): @@ -341,8 +354,6 @@ class PlaybackController(object): current = pending pending = self.core.tracklist.next_track(current) - # TODO: move to top and get rid of original? - self.core.tracklist._mark_played(original) # TODO return result? def _change(self, pending_tl_track, state): @@ -387,8 +398,7 @@ class PlaybackController(object): The current playback state will be kept. If it was playing, playing will continue. If it was paused, it will still be paused, etc. """ - self._trigger_track_playback_ended(self.get_time_position()) - + self._previous = True state = self.get_state() current = self._pending_tl_track or self._current_tl_track @@ -440,11 +450,6 @@ class PlaybackController(object): if self.get_state() == PlaybackState.STOPPED: self.play() - # TODO: uncomment once we have tests for this. Should fix seek after - # about to finish doing wrong track. - # if self._current_tl_track and self._pending_tl_track: - # self.play(self._current_tl_track) - # We need to prefer the still playing track, but if nothing is playing # we fall back to the pending one. tl_track = self._current_tl_track or self._pending_tl_track @@ -458,23 +463,29 @@ class PlaybackController(object): self.next() return True + # Store our target position. + self._pending_position = time_position + + # Make sure we switch back to previous track if we get a seek while we + # have a pending track. + if self._current_tl_track and self._pending_tl_track: + self._change(self._current_tl_track, self.get_state()) + else: + return self._seek(time_position) + + def _seek(self, time_position): backend = self._get_backend(self.get_current_tl_track()) if not backend: return False - - success = backend.playback.seek(time_position).get() - if success: - self._trigger_seeked(time_position) - return success + return backend.playback.seek(time_position).get() def stop(self): """Stop playing.""" if self.get_state() != PlaybackState.STOPPED: + self._last_position = self.get_time_position() backend = self._get_backend(self.get_current_tl_track()) - time_position_before_stop = self.get_time_position() if not backend or backend.playback.stop().get(): self.set_state(PlaybackState.STOPPED) - self._trigger_track_playback_ended(time_position_before_stop) def _trigger_track_playback_paused(self): logger.debug('Triggering track playback paused event') @@ -495,20 +506,26 @@ class PlaybackController(object): time_position=self.get_time_position()) def _trigger_track_playback_started(self): - # TODO: replace with stream-changed - logger.debug('Triggering track playback started event') if self.get_current_tl_track() is None: return + logger.debug('Triggering track playback started event') tl_track = self.get_current_tl_track() self.core.tracklist._mark_playing(tl_track) self.core.history._add_track(tl_track.track) listener.CoreListener.send('track_playback_started', tl_track=tl_track) def _trigger_track_playback_ended(self, time_position_before_stop): - logger.debug('Triggering track playback ended event') if self.get_current_tl_track() is None: return + + logger.debug('Triggering track playback ended event') + + if not self._previous: + self.core.tracklist._mark_played(self._current_tl_track) + self._previous = False + + # TODO: Use the lowest of track duration and position. listener.CoreListener.send( 'track_playback_ended', tl_track=self.get_current_tl_track(), @@ -521,6 +538,7 @@ class PlaybackController(object): old_state=old_state, new_state=new_state) def _trigger_seeked(self, time_position): + # TODO: Trigger this from audio events? logger.debug('Triggering seeked event') listener.CoreListener.send('seeked', time_position=time_position) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index e3e2ac20..87790c25 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -33,6 +33,16 @@ class PlaylistsController(object): self.backends = backends self.core = core + def get_uri_schemes(self): + """ + Get the list of URI schemes that support playlists. + + :rtype: list of string + + .. versionadded:: 1.2 + """ + return list(sorted(self.backends.with_playlists.keys())) + def as_list(self): """ Get a list of the currently available playlists. diff --git a/mopidy/ext.py b/mopidy/ext.py index 7fd68f96..48a623bb 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -54,7 +54,7 @@ class Extension(object): def get_config_schema(self): """The extension's config validation schema - :returns: :class:`~mopidy.config.schema.ExtensionConfigSchema` + :returns: :class:`~mopidy.config.schemas.ConfigSchema` """ schema = config_lib.ConfigSchema(self.ext_name) schema['enabled'] = config_lib.Boolean() @@ -198,7 +198,12 @@ def load_extensions(): for entry_point in pkg_resources.iter_entry_points('mopidy.ext'): logger.debug('Loading entry point: %s', entry_point) - extension_class = entry_point.load(require=False) + try: + extension_class = entry_point.load(require=False) + except Exception as e: + logger.exception("Failed to load extension %s: %s" % ( + entry_point.name, e)) + continue try: if not issubclass(extension_class, Extension): diff --git a/mopidy/httpclient.py b/mopidy/httpclient.py index 682a78bd..6be127ca 100644 --- a/mopidy/httpclient.py +++ b/mopidy/httpclient.py @@ -21,8 +21,8 @@ def format_proxy(proxy_config, auth=True): if not proxy_config.get('hostname'): return None - port = proxy_config.get('port', 80) - if port < 0: + port = proxy_config.get('port') + if not port or port < 0: port = 80 if proxy_config.get('username') and proxy_config.get('password') and auth: diff --git a/mopidy/internal/log.py b/mopidy/internal/log.py index 9c40da4f..011a70d2 100644 --- a/mopidy/internal/log.py +++ b/mopidy/internal/log.py @@ -19,6 +19,8 @@ LOG_LEVELS = { TRACE_LOG_LEVEL = 5 logging.addLevelName(TRACE_LOG_LEVEL, 'TRACE') +logger = logging.getLogger(__name__) + class DelayedHandler(logging.Handler): @@ -54,8 +56,12 @@ def setup_logging(config, verbosity_level, save_debug_log): if config['logging']['config_file']: # Logging config from file must be read before other handlers are # added. If not, the other handlers will have no effect. - logging.config.fileConfig(config['logging']['config_file'], - disable_existing_loggers=False) + try: + path = config['logging']['config_file'] + logging.config.fileConfig(path, disable_existing_loggers=False) + except Exception as e: + # Catch everything as logging does not specify what can go wrong. + logger.error('Loading logging config %r failed. %s', path, e) setup_console_logging(config, verbosity_level) if save_debug_log: diff --git a/mopidy/listener.py b/mopidy/listener.py index 9bcab0e0..79c53570 100644 --- a/mopidy/listener.py +++ b/mopidy/listener.py @@ -41,4 +41,9 @@ class Listener(object): :type event: string :param kwargs: any other arguments to the specific event handlers """ - getattr(self, event)(**kwargs) + try: + getattr(self, event)(**kwargs) + except Exception: + # Ensure we don't crash the actor due to "bad" events. + logger.exception( + 'Triggering event failed: %s(%s)', event, ', '.join(kwargs)) diff --git a/mopidy/m3u/translator.py b/mopidy/m3u/translator.py index f60cedfe..764cf84b 100644 --- a/mopidy/m3u/translator.py +++ b/mopidy/m3u/translator.py @@ -99,7 +99,9 @@ def parse_m3u(file_path, media_dir=None): if extended and line.startswith('#EXTINF'): track = m3u_extinf_to_track(line) continue - + if not track.name: + name = os.path.basename(os.path.splitext(line)[0]) + track = track.replace(name=urllib.parse.unquote(name)) if urllib.parse.urlsplit(line).scheme: tracks.append(track.replace(uri=line)) elif os.path.normpath(line) == os.path.abspath(line): diff --git a/mopidy/models/__init__.py b/mopidy/models/__init__.py index e919bbbf..5a980866 100644 --- a/mopidy/models/__init__.py +++ b/mopidy/models/__init__.py @@ -148,6 +148,10 @@ class Album(ValidatedImmutableObject): :type musicbrainz_id: string :param images: album image URIs :type images: list of strings + + .. deprecated:: 1.2 + The ``images`` field is deprecated. + Use :meth:`mopidy.core.LibraryController.get_images` instead. """ #: The album URI. Read-only. @@ -172,10 +176,10 @@ class Album(ValidatedImmutableObject): musicbrainz_id = fields.Identifier() #: The album image URIs. Read-only. + #: + #: .. deprecated:: 1.2 + #: Use :meth:`mopidy.core.LibraryController.get_images` instead. images = fields.Collection(type=compat.string_types, container=frozenset) - # XXX If we want to keep the order of images we shouldn't use frozenset() - # as it doesn't preserve order. I'm deferring this issue until we got - # actual usage of this field with more than one image. class Track(ValidatedImmutableObject): diff --git a/mopidy/models/immutable.py b/mopidy/models/immutable.py index 8bbf568b..18de7d76 100644 --- a/mopidy/models/immutable.py +++ b/mopidy/models/immutable.py @@ -112,7 +112,7 @@ class ImmutableObject(object): for key, value in kwargs.items(): if not self._is_valid_field(key): raise TypeError( - 'copy() got an unexpected keyword argument "%s"' % key) + 'replace() got an unexpected keyword argument "%s"' % key) other._set_field(key, value) return other diff --git a/mopidy/mpd/actor.py b/mopidy/mpd/actor.py index 7439e73f..067d20c5 100644 --- a/mopidy/mpd/actor.py +++ b/mopidy/mpd/actor.py @@ -4,13 +4,30 @@ import logging import pykka -from mopidy import exceptions, zeroconf +from mopidy import exceptions, listener, zeroconf from mopidy.core import CoreListener from mopidy.internal import encoding, network, process from mopidy.mpd import session, uri_mapper logger = logging.getLogger(__name__) +_CORE_EVENTS_TO_IDLE_SUBSYSTEMS = { + 'track_playback_paused': None, + 'track_playback_resumed': None, + 'track_playback_started': None, + 'track_playback_ended': None, + 'playback_state_changed': 'player', + 'tracklist_changed': 'playlist', + 'playlists_loaded': 'stored_playlist', + 'playlist_changed': 'stored_playlist', + 'playlist_deleted': 'stored_playlist', + 'options_changed': 'options', + 'volume_changed': 'mixer', + 'mute_changed': 'output', + 'seeked': 'player', + 'stream_title_changed': 'playlist', +} + class MpdFrontend(pykka.ThreadingActor, CoreListener): @@ -24,6 +41,9 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): self.zeroconf_name = config['mpd']['zeroconf'] self.zeroconf_service = None + self._setup_server(config, core) + + def _setup_server(self, config, core): try: network.Server( self.hostname, self.port, @@ -56,31 +76,13 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): process.stop_actors_by_class(session.MpdSession) + def on_event(self, event, **kwargs): + if event not in _CORE_EVENTS_TO_IDLE_SUBSYSTEMS: + logger.warning( + 'Got unexpected event: %s(%s)', event, ', '.join(kwargs)) + else: + self.send_idle(_CORE_EVENTS_TO_IDLE_SUBSYSTEMS[event]) + def send_idle(self, subsystem): - listeners = pykka.ActorRegistry.get_by_class(session.MpdSession) - for listener in listeners: - getattr(listener.proxy(), 'on_idle')(subsystem) - - def playback_state_changed(self, old_state, new_state): - self.send_idle('player') - - def tracklist_changed(self): - self.send_idle('playlist') - - def playlist_changed(self, playlist): - self.send_idle('stored_playlist') - - def playlist_deleted(self, playlist): - self.send_idle('stored_playlist') - - def options_changed(self): - self.send_idle('options') - - def volume_changed(self, volume): - self.send_idle('mixer') - - def mute_changed(self, mute): - self.send_idle('output') - - def stream_title_changed(self, title): - self.send_idle('playlist') + if subsystem: + listener.send(session.MpdSession, subsystem) diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py index 099a2f18..175d8b32 100644 --- a/mopidy/mpd/dispatcher.py +++ b/mopidy/mpd/dispatcher.py @@ -47,6 +47,7 @@ class MpdDispatcher(object): return self._call_next_filter(request, response, filter_chain) def handle_idle(self, subsystem): + # TODO: validate against mopidy/mpd/protocol/status.SUBSYSTEMS self.context.events.add(subsystem) subsystems = self.context.subscriptions.intersection( diff --git a/mopidy/mpd/exceptions.py b/mopidy/mpd/exceptions.py index b64a6cf0..05762683 100644 --- a/mopidy/mpd/exceptions.py +++ b/mopidy/mpd/exceptions.py @@ -80,10 +80,23 @@ class MpdNoExistError(MpdAckError): error_code = MpdAckError.ACK_ERROR_NO_EXIST +class MpdExistError(MpdAckError): + error_code = MpdAckError.ACK_ERROR_EXIST + + class MpdSystemError(MpdAckError): error_code = MpdAckError.ACK_ERROR_SYSTEM +class MpdInvalidPlaylistName(MpdAckError): + error_code = MpdAckError.ACK_ERROR_ARG + + def __init__(self, *args, **kwargs): + super(MpdInvalidPlaylistName, self).__init__(*args, **kwargs) + self.message = ('playlist name is invalid: playlist names may not ' + 'contain slashes, newlines or carriage returns') + + class MpdNotImplemented(MpdAckError): error_code = 0 diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py index 647a1464..68ae1e9e 100644 --- a/mopidy/mpd/protocol/stored_playlists.py +++ b/mopidy/mpd/protocol/stored_playlists.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, division, unicode_literals import datetime import logging +import re import warnings from mopidy.compat import urllib @@ -10,6 +11,11 @@ from mopidy.mpd import exceptions, protocol, translator logger = logging.getLogger(__name__) +def _check_playlist_name(name): + if re.search('[/\n\r]', name): + raise exceptions.MpdInvalidPlaylistName() + + @protocol.commands.add('listplaylist') def listplaylist(context, name): """ @@ -149,6 +155,7 @@ def playlistadd(context, name, track_uri): ``NAME.m3u`` will be created if it does not exist. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) old_playlist = uri is not None and context.core.playlists.lookup(uri).get() if not old_playlist: @@ -219,6 +226,7 @@ def playlistclear(context, name): The playlist will be created if it does not exist. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() if not playlist: @@ -240,14 +248,18 @@ def playlistdelete(context, name, songpos): Deletes ``SONGPOS`` from the playlist ``NAME.m3u``. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() if not playlist: raise exceptions.MpdNoExistError('No such playlist') - # Convert tracks to list and remove requested - tracks = list(playlist.tracks) - tracks.pop(songpos) + try: + # Convert tracks to list and remove requested + tracks = list(playlist.tracks) + tracks.pop(songpos) + except IndexError: + raise exceptions.MpdArgError('Bad song index') # Replace tracks and save playlist playlist = playlist.replace(tracks=tracks) @@ -274,6 +286,10 @@ def playlistmove(context, name, from_pos, to_pos): documentation, but just the ``SONGPOS`` to move *from*, i.e. ``playlistmove {NAME} {FROM_SONGPOS} {TO_SONGPOS}``. """ + if from_pos == to_pos: + return + + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() if not playlist: @@ -281,10 +297,13 @@ def playlistmove(context, name, from_pos, to_pos): if from_pos == to_pos: return # Nothing to do - # Convert tracks to list and perform move - tracks = list(playlist.tracks) - track = tracks.pop(from_pos) - tracks.insert(to_pos, track) + try: + # Convert tracks to list and perform move + tracks = list(playlist.tracks) + track = tracks.pop(from_pos) + tracks.insert(to_pos, track) + except IndexError: + raise exceptions.MpdArgError('Bad song index') # Replace tracks and save playlist playlist = playlist.replace(tracks=tracks) @@ -303,16 +322,28 @@ def rename(context, old_name, new_name): Renames the playlist ``NAME.m3u`` to ``NEW_NAME.m3u``. """ - uri = context.lookup_playlist_uri_from_name(old_name) - uri_scheme = urllib.parse.urlparse(uri).scheme - old_playlist = uri is not None and context.core.playlists.lookup(uri).get() + _check_playlist_name(old_name) + _check_playlist_name(new_name) + + old_uri = context.lookup_playlist_uri_from_name(old_name) + if not old_uri: + raise exceptions.MpdNoExistError('No such playlist') + + old_playlist = context.core.playlists.lookup(old_uri).get() if not old_playlist: raise exceptions.MpdNoExistError('No such playlist') + new_uri = context.lookup_playlist_uri_from_name(new_name) + if new_uri and context.core.playlists.lookup(new_uri).get(): + raise exceptions.MpdExistError('Playlist already exists') + # TODO: should we purge the mapping in an else? + # Create copy of the playlist and remove original + uri_scheme = urllib.parse.urlparse(old_uri).scheme new_playlist = context.core.playlists.create(new_name, uri_scheme).get() new_playlist = new_playlist.replace(tracks=old_playlist.tracks) saved_playlist = context.core.playlists.save(new_playlist).get() + if saved_playlist is None: raise exceptions.MpdFailedToSavePlaylist(uri_scheme) context.core.playlists.delete(old_playlist.uri).get() @@ -327,7 +358,10 @@ def rm(context, name): Removes the playlist ``NAME.m3u`` from the playlist directory. """ + _check_playlist_name(name) uri = context.lookup_playlist_uri_from_name(name) + if not uri: + raise exceptions.MpdNoExistError('No such playlist') context.core.playlists.delete(uri).get() @@ -341,6 +375,7 @@ def save(context, name): Saves the current playlist to ``NAME.m3u`` in the playlist directory. """ + _check_playlist_name(name) tracks = context.core.tracklist.get_tracks().get() uri = context.lookup_playlist_uri_from_name(name) playlist = uri is not None and context.core.playlists.lookup(uri).get() diff --git a/mopidy/mpd/session.py b/mopidy/mpd/session.py index 68550f3b..d484d986 100644 --- a/mopidy/mpd/session.py +++ b/mopidy/mpd/session.py @@ -41,7 +41,7 @@ class MpdSession(network.LineProtocol): self.send_lines(response) - def on_idle(self, subsystem): + def on_event(self, subsystem): self.dispatcher.handle_idle(subsystem) def decode(self, line): diff --git a/mopidy/mpd/uri_mapper.py b/mopidy/mpd/uri_mapper.py index 9e7ec2dd..bb627a47 100644 --- a/mopidy/mpd/uri_mapper.py +++ b/mopidy/mpd/uri_mapper.py @@ -71,7 +71,7 @@ class MpdUriMapper(object): """ Helper function to retrieve a playlist URI from its unique MPD name. """ - if not self._uri_from_name: + if name not in self._uri_from_name: self.refresh_playlists_mapping() return self._uri_from_name.get(name) diff --git a/mopidy/zeroconf.py b/mopidy/zeroconf.py index 4ca49b69..9b7b3808 100644 --- a/mopidy/zeroconf.py +++ b/mopidy/zeroconf.py @@ -55,17 +55,20 @@ class Zeroconf(object): self.bus = None self.server = None self.group = None - try: - self.bus = dbus.SystemBus() - self.server = dbus.Interface( - self.bus.get_object('org.freedesktop.Avahi', '/'), - 'org.freedesktop.Avahi.Server') - except dbus.exceptions.DBusException as e: - logger.debug('%s: Server failed: %s', self, e) + self.display_hostname = None + self.name = None - self.display_hostname = '%s' % self.server.GetHostName() - self.name = string.Template(name).safe_substitute( - hostname=self.display_hostname, port=port) + if dbus: + try: + self.bus = dbus.SystemBus() + self.server = dbus.Interface( + self.bus.get_object('org.freedesktop.Avahi', '/'), + 'org.freedesktop.Avahi.Server') + self.display_hostname = '%s' % self.server.GetHostName() + self.name = string.Template(name).safe_substitute( + hostname=self.display_hostname, port=port) + except dbus.exceptions.DBusException as e: + logger.debug('%s: Server failed: %s', self, e) def __str__(self): return 'Zeroconf service "%s" (%s at [%s]:%d)' % ( diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index 314d1d42..0cfbdaf3 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -163,7 +163,7 @@ class AudioEventTest(BaseTest): self.listener = DummyAudioListener.start().proxy() def tearDown(self): # noqa: N802 - super(AudioEventTest, self).setUp() + super(AudioEventTest, self).tearDown() def assertEvent(self, event, **kwargs): # noqa: N802 self.assertIn((event, kwargs), self.listener.get_events().get()) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 801c79f5..128ec3ce 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -115,6 +115,23 @@ class TestPlayHandling(BaseTest): current_tl_track = self.core.playback.get_current_tl_track() self.assertEqual(tl_tracks[1], current_tl_track) + def test_resume_skips_to_next_on_unplayable_track(self): + """Checks that we handle backend.change_track failing when + resuming playback.""" + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.core.playback.pause() + + self.audio.trigger_fake_playback_failure(tl_tracks[1].track.uri) + + self.core.playback.next() + self.core.playback.resume() + self.replay_events() + + current_tl_track = self.core.playback.get_current_tl_track() + self.assertEqual(tl_tracks[2], current_tl_track) + def test_play_tlid(self): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -314,6 +331,8 @@ class TestCurrentAndPendingTlTrack(BaseTest): 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) class EventEmissionTest(BaseTest): + maxDiff = None + def test_play_when_stopped_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -321,14 +340,14 @@ class EventEmissionTest(BaseTest): self.replay_events() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'playback_state_changed', old_state='stopped', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[0]), - ]) + ], + listener_mock.send.mock_calls) def test_play_when_paused_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -344,7 +363,6 @@ class EventEmissionTest(BaseTest): self.replay_events() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'track_playback_ended', @@ -354,7 +372,8 @@ class EventEmissionTest(BaseTest): old_state='paused', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), - ]) + ], + listener_mock.send.mock_calls) def test_play_when_playing_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -366,9 +385,7 @@ class EventEmissionTest(BaseTest): self.core.playback.play(tl_tracks[2]) self.replay_events() - # TODO: Do we want to emit playing->playing for this case? self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'track_playback_ended', @@ -378,7 +395,8 @@ class EventEmissionTest(BaseTest): new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[2]), - ]) + ], + listener_mock.send.mock_calls) def test_pause_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -392,7 +410,6 @@ class EventEmissionTest(BaseTest): self.core.playback.pause() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'playback_state_changed', @@ -400,7 +417,8 @@ class EventEmissionTest(BaseTest): mock.call( 'track_playback_paused', tl_track=tl_tracks[0], time_position=1000), - ]) + ], + listener_mock.send.mock_calls) def test_resume_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -415,7 +433,6 @@ class EventEmissionTest(BaseTest): self.core.playback.resume() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'playback_state_changed', @@ -423,7 +440,8 @@ class EventEmissionTest(BaseTest): mock.call( 'track_playback_resumed', tl_track=tl_tracks[0], time_position=1000), - ]) + ], + listener_mock.send.mock_calls) def test_stop_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -431,13 +449,13 @@ class EventEmissionTest(BaseTest): self.core.playback.play(tl_tracks[0]) self.replay_events() self.core.playback.seek(1000) + self.replay_events() listener_mock.reset_mock() self.core.playback.stop() self.replay_events() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'playback_state_changed', @@ -445,7 +463,8 @@ class EventEmissionTest(BaseTest): mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=1000), - ]) + ], + listener_mock.send.mock_calls) def test_next_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -453,23 +472,26 @@ class EventEmissionTest(BaseTest): self.core.playback.play(tl_tracks[0]) self.replay_events() self.core.playback.seek(1000) + self.replay_events() listener_mock.reset_mock() self.core.playback.next() self.replay_events() - # TODO: should we be emitting playing -> playing? self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), - ]) + ], + listener_mock.send.mock_calls) - def test_on_end_of_track_emits_events(self, listener_mock): + def test_gapless_track_change_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.play(tl_tracks[0]) @@ -479,14 +501,17 @@ class EventEmissionTest(BaseTest): self.trigger_about_to_finish() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), - ]) + ], + listener_mock.send.mock_calls) def test_seek_emits_seeked_event(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -496,6 +521,7 @@ class EventEmissionTest(BaseTest): listener_mock.reset_mock() self.core.playback.seek(1000) + self.replay_events() listener_mock.send.assert_called_once_with( 'seeked', time_position=1000) @@ -511,14 +537,35 @@ class EventEmissionTest(BaseTest): self.replay_events() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'track_playback_ended', tl_track=tl_tracks[0], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[1]), - ]) + ], + listener_mock.send.mock_calls) + + def test_seek_race_condition_emits_events(self, listener_mock): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.trigger_about_to_finish(replay_until='stream_changed') + listener_mock.reset_mock() + + self.core.playback.seek(1000) + self.replay_events() + + # When we trigger seek after an about to finish the other code that + # emits track stopped/started and playback state changed events gets + # triggered as we have to switch back to the previous track. + # The correct behavior would be to only emit seeked. + self.assertListEqual( + [mock.call('seeked', time_position=1000)], + listener_mock.send.mock_calls) def test_previous_emits_events(self, listener_mock): tl_tracks = self.core.tracklist.get_tl_tracks() @@ -531,14 +578,17 @@ class EventEmissionTest(BaseTest): self.replay_events() self.assertListEqual( - listener_mock.send.mock_calls, [ mock.call( 'track_playback_ended', tl_track=tl_tracks[1], time_position=mock.ANY), + mock.call( + 'playback_state_changed', + old_state='playing', new_state='playing'), mock.call( 'track_playback_started', tl_track=tl_tracks[0]), - ]) + ], + listener_mock.send.mock_calls) class UnplayableURITest(BaseTest): @@ -612,12 +662,27 @@ class SeekTest(BaseTest): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback.play(tl_tracks[0]) + self.replay_events() + self.core.playback.pause() self.replay_events() self.core.playback.seek(1000) self.assertEqual(self.core.playback.state, core.PlaybackState.PAUSED) + def test_seek_race_condition_after_about_to_finish(self): + tl_tracks = self.core.tracklist.get_tl_tracks() + + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.trigger_about_to_finish(replay_until='stream_changed') + self.core.playback.seek(1000) + self.replay_events() + + current_tl_track = self.core.playback.get_current_tl_track() + self.assertEqual(current_tl_track, tl_tracks[0]) + class TestStream(BaseTest): @@ -807,7 +872,6 @@ class BackendSelectionTest(unittest.TestCase): self.core.playback.play(self.tl_tracks[0]) self.trigger_stream_changed() - self.core.playback.seek(10000) self.core.playback.time_position self.playback1.get_time_position.assert_called_once_with() @@ -817,7 +881,6 @@ class BackendSelectionTest(unittest.TestCase): self.core.playback.play(self.tl_tracks[1]) self.trigger_stream_changed() - self.core.playback.seek(10000) self.core.playback.time_position self.assertFalse(self.playback1.get_time_position.called) diff --git a/tests/core/test_playlists.py b/tests/core/test_playlists.py index 029254a8..c908af6a 100644 --- a/tests/core/test_playlists.py +++ b/tests/core/test_playlists.py @@ -248,6 +248,10 @@ class PlaylistTest(BasePlaylistsTest): self.assertFalse(self.sp1.save.called) self.assertFalse(self.sp2.save.called) + def test_get_uri_schemes(self): + result = self.core.playlists.get_uri_schemes() + self.assertEquals(result, ['dummy1', 'dummy2']) + class DeprecatedFilterPlaylistsTest(BasePlaylistsTest): diff --git a/tests/data/comment-ext.m3u b/tests/data/comment-ext.m3u index 95983d06..a9b675b8 100644 --- a/tests/data/comment-ext.m3u +++ b/tests/data/comment-ext.m3u @@ -1,5 +1,5 @@ #EXTM3U # test -#EXTINF:-1,song1 +#EXTINF:-1,Song #1 # test song1.mp3 diff --git a/tests/data/one-ext.m3u b/tests/data/one-ext.m3u index 7e94d5e9..a8a51c2f 100644 --- a/tests/data/one-ext.m3u +++ b/tests/data/one-ext.m3u @@ -1,3 +1,3 @@ #EXTM3U -#EXTINF:-1,song1 +#EXTINF:-1,Song #1 song1.mp3 diff --git a/tests/data/two-ext.m3u b/tests/data/two-ext.m3u index c2bf3e75..f50feb94 100644 --- a/tests/data/two-ext.m3u +++ b/tests/data/two-ext.m3u @@ -1,5 +1,5 @@ #EXTM3U -#EXTINF:-1,song1 +#EXTINF:-1,Song #1 song1.mp3 -#EXTINF:60,song2 +#EXTINF:60,Song #2 song2.mp3 diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 92fbe5b9..f0dcf20b 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -998,7 +998,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.playback.next().get() self.assert_next_tl_track_is_not(None) self.assert_state_is(PlaybackState.STOPPED) - self.playback.play() + self.playback.play().get() self.assert_state_is(PlaybackState.PLAYING) @populate_tracklist diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index b7ed7dcb..6c9532e8 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -38,7 +38,9 @@ class LocalTracklistProviderTest(unittest.TestCase): self.audio = dummy_audio.create_proxy() self.backend = actor.LocalBackend.start( config=self.config, audio=self.audio).proxy() - self.core = core.Core(self.config, mixer=None, backends=[self.backend]) + self.core = core.Core.start(audio=self.audio, + backends=[self.backend], + config=self.config).proxy() self.controller = self.core.tracklist self.playback = self.core.playback @@ -47,216 +49,254 @@ class LocalTracklistProviderTest(unittest.TestCase): def tearDown(self): # noqa: N802 pykka.ActorRegistry.stop_all() + def assert_state_is(self, state): + self.assertEqual(self.playback.get_state().get(), state) + + def assert_current_track_is(self, track): + self.assertEqual(self.playback.get_current_track().get(), track) + def test_length(self): - self.assertEqual(0, len(self.controller.tl_tracks)) - self.assertEqual(0, self.controller.length) + self.assertEqual(0, len(self.controller.get_tl_tracks().get())) + self.assertEqual(0, self.controller.get_length().get()) self.controller.add(self.tracks) - self.assertEqual(3, len(self.controller.tl_tracks)) - self.assertEqual(3, self.controller.length) + self.assertEqual(3, len(self.controller.get_tl_tracks().get())) + self.assertEqual(3, self.controller.get_length().get()) def test_add(self): for track in self.tracks: - tl_tracks = self.controller.add([track]) - self.assertEqual(track, self.controller.tracks[-1]) - self.assertEqual(tl_tracks[0], self.controller.tl_tracks[-1]) - self.assertEqual(track, tl_tracks[0].track) + added = self.controller.add([track]).get() + tracks = self.controller.get_tracks().get() + tl_tracks = self.controller.get_tl_tracks().get() + + self.assertEqual(track, tracks[-1]) + self.assertEqual(added[0], tl_tracks[-1]) + self.assertEqual(track, added[0].track) def test_add_at_position(self): for track in self.tracks[:-1]: - tl_tracks = self.controller.add([track], 0) - self.assertEqual(track, self.controller.tracks[0]) - self.assertEqual(tl_tracks[0], self.controller.tl_tracks[0]) - self.assertEqual(track, tl_tracks[0].track) + added = self.controller.add([track], 0).get() + tracks = self.controller.get_tracks().get() + tl_tracks = self.controller.get_tl_tracks().get() + + self.assertEqual(track, tracks[0]) + self.assertEqual(added[0], tl_tracks[0]) + self.assertEqual(track, added[0].track) @populate_tracklist def test_add_at_position_outside_of_playlist(self): for track in self.tracks: - tl_tracks = self.controller.add([track], len(self.tracks) + 2) - self.assertEqual(track, self.controller.tracks[-1]) - self.assertEqual(tl_tracks[0], self.controller.tl_tracks[-1]) - self.assertEqual(track, tl_tracks[0].track) + added = self.controller.add([track], len(self.tracks) + 2).get() + tracks = self.controller.get_tracks().get() + tl_tracks = self.controller.get_tl_tracks().get() + + self.assertEqual(track, tracks[-1]) + self.assertEqual(added[0], tl_tracks[-1]) + self.assertEqual(track, added[0].track) @populate_tracklist def test_filter_by_tlid(self): - tl_track = self.controller.tl_tracks[1] - self.assertEqual( - [tl_track], self.controller.filter({'tlid': [tl_track.tlid]})) + tl_track = self.controller.get_tl_tracks().get()[1] + result = self.controller.filter({'tlid': [tl_track.tlid]}).get() + self.assertEqual([tl_track], result) @populate_tracklist def test_filter_by_uri(self): - tl_track = self.controller.tl_tracks[1] - self.assertEqual( - [tl_track], self.controller.filter({'uri': [tl_track.track.uri]})) + tl_track = self.controller.get_tl_tracks().get()[1] + result = self.controller.filter({'uri': [tl_track.track.uri]}).get() + self.assertEqual([tl_track], result) @populate_tracklist def test_filter_by_uri_returns_nothing_for_invalid_uri(self): - self.assertEqual([], self.controller.filter({'uri': ['foobar']})) + self.assertEqual([], self.controller.filter({'uri': ['foobar']}).get()) def test_filter_by_uri_returns_single_match(self): t = Track(uri='a') self.controller.add([Track(uri='z'), t, Track(uri='y')]) - self.assertEqual(t, self.controller.filter({'uri': ['a']})[0].track) + + result = self.controller.filter({'uri': ['a']}).get() + self.assertEqual(t, result[0].track) def test_filter_by_uri_returns_multiple_matches(self): track = Track(uri='a') self.controller.add([Track(uri='z'), track, track]) - tl_tracks = self.controller.filter({'uri': ['a']}) + tl_tracks = self.controller.filter({'uri': ['a']}).get() self.assertEqual(track, tl_tracks[0].track) self.assertEqual(track, tl_tracks[1].track) def test_filter_by_uri_returns_nothing_if_no_match(self): self.controller.playlist = Playlist( tracks=[Track(uri='z'), Track(uri='y')]) - self.assertEqual([], self.controller.filter({'uri': ['a']})) + self.assertEqual([], self.controller.filter({'uri': ['a']}).get()) def test_filter_by_multiple_criteria_returns_elements_matching_all(self): t1 = Track(uri='a', name='x') t2 = Track(uri='b', name='x') t3 = Track(uri='b', name='y') self.controller.add([t1, t2, t3]) - self.assertEqual( - t1, self.controller.filter({'uri': ['a'], 'name': ['x']})[0].track) - self.assertEqual( - t2, self.controller.filter({'uri': ['b'], 'name': ['x']})[0].track) - self.assertEqual( - t3, self.controller.filter({'uri': ['b'], 'name': ['y']})[0].track) + + result1 = self.controller.filter({'uri': ['a'], 'name': ['x']}).get() + self.assertEqual(t1, result1[0].track) + + result2 = self.controller.filter({'uri': ['b'], 'name': ['x']}).get() + self.assertEqual(t2, result2[0].track) + + result3 = self.controller.filter({'uri': ['b'], 'name': ['y']}).get() + self.assertEqual(t3, result3[0].track) def test_filter_by_criteria_that_is_not_present_in_all_elements(self): track1 = Track() track2 = Track(uri='b') track3 = Track() + self.controller.add([track1, track2, track3]) - self.assertEqual( - track2, self.controller.filter({'uri': ['b']})[0].track) + result = self.controller.filter({'uri': ['b']}).get() + self.assertEqual(track2, result[0].track) @populate_tracklist def test_clear(self): - self.controller.clear() - self.assertEqual(len(self.controller.tracks), 0) + self.controller.clear().get() + self.assertEqual(len(self.controller.get_tracks().get()), 0) def test_clear_empty_playlist(self): - self.controller.clear() - self.assertEqual(len(self.controller.tracks), 0) + self.controller.clear().get() + self.assertEqual(len(self.controller.get_tracks().get()), 0) @populate_tracklist def test_clear_when_playing(self): - self.playback.play() - self.assertEqual(self.playback.state, PlaybackState.PLAYING) - self.controller.clear() - self.assertEqual(self.playback.state, PlaybackState.STOPPED) + self.playback.play().get() + self.assert_state_is(PlaybackState.PLAYING) + self.controller.clear().get() + self.assert_state_is(PlaybackState.STOPPED) def test_add_appends_to_the_tracklist(self): self.controller.add([Track(uri='a'), Track(uri='b')]) - self.assertEqual(len(self.controller.tracks), 2) + + tracks = self.controller.get_tracks().get() + self.assertEqual(len(tracks), 2) + self.controller.add([Track(uri='c'), Track(uri='d')]) - self.assertEqual(len(self.controller.tracks), 4) - self.assertEqual(self.controller.tracks[0].uri, 'a') - self.assertEqual(self.controller.tracks[1].uri, 'b') - self.assertEqual(self.controller.tracks[2].uri, 'c') - self.assertEqual(self.controller.tracks[3].uri, 'd') + + tracks = self.controller.get_tracks().get() + self.assertEqual(len(tracks), 4) + self.assertEqual(tracks[0].uri, 'a') + self.assertEqual(tracks[1].uri, 'b') + self.assertEqual(tracks[2].uri, 'c') + self.assertEqual(tracks[3].uri, 'd') def test_add_does_not_reset_version(self): - version = self.controller.version + version = self.controller.get_version().get() self.controller.add([]) - self.assertEqual(self.controller.version, version) + self.assertEqual(self.controller.get_version().get(), version) @populate_tracklist def test_add_preserves_playing_state(self): - self.playback.play() - track = self.playback.current_track - self.controller.add(self.controller.tracks[1:2]) - self.assertEqual(self.playback.state, PlaybackState.PLAYING) - self.assertEqual(self.playback.current_track, track) + self.playback.play().get() + + track = self.playback.get_current_track().get() + tracks = self.controller.get_tracks().get() + self.controller.add(tracks[1:2]).get() + + self.assert_state_is(PlaybackState.PLAYING) + self.assert_current_track_is(track) @populate_tracklist def test_add_preserves_stopped_state(self): - self.controller.add(self.controller.tracks[1:2]) - self.assertEqual(self.playback.state, PlaybackState.STOPPED) - self.assertEqual(self.playback.current_track, None) + tracks = self.controller.get_tracks().get() + self.controller.add(tracks[1:2]).get() + + self.assert_state_is(PlaybackState.STOPPED) + self.assert_current_track_is(None) @populate_tracklist def test_add_returns_the_tl_tracks_that_was_added(self): - tl_tracks = self.controller.add(self.controller.tracks[1:2]) - self.assertEqual(tl_tracks[0].track, self.controller.tracks[1]) + tracks = self.controller.get_tracks().get() + + added = self.controller.add(tracks[1:2]).get() + tracks = self.controller.get_tracks().get() + self.assertEqual(added[0].track, tracks[1]) @populate_tracklist def test_move_single(self): self.controller.move(0, 0, 2) - tracks = self.controller.tracks + tracks = self.controller.get_tracks().get() self.assertEqual(tracks[2], self.tracks[0]) @populate_tracklist def test_move_group(self): self.controller.move(0, 2, 1) - tracks = self.controller.tracks + tracks = self.controller.get_tracks().get() self.assertEqual(tracks[1], self.tracks[0]) self.assertEqual(tracks[2], self.tracks[1]) @populate_tracklist def test_moving_track_outside_of_playlist(self): - tracks = len(self.controller.tracks) + num_tracks = len(self.controller.get_tracks().get()) with self.assertRaises(AssertionError): - self.controller.move(0, 0, tracks + 5) + self.controller.move(0, 0, num_tracks + 5).get() @populate_tracklist def test_move_group_outside_of_playlist(self): - tracks = len(self.controller.tracks) + num_tracks = len(self.controller.get_tracks().get()) with self.assertRaises(AssertionError): - self.controller.move(0, 2, tracks + 5) + self.controller.move(0, 2, num_tracks + 5).get() @populate_tracklist def test_move_group_out_of_range(self): - tracks = len(self.controller.tracks) + num_tracks = len(self.controller.get_tracks().get()) with self.assertRaises(AssertionError): - self.controller.move(tracks + 2, tracks + 3, 0) + self.controller.move(num_tracks + 2, num_tracks + 3, 0).get() @populate_tracklist def test_move_group_invalid_group(self): with self.assertRaises(AssertionError): - self.controller.move(2, 1, 0) + self.controller.move(2, 1, 0).get() def test_tracks_attribute_is_immutable(self): - tracks1 = self.controller.tracks - tracks2 = self.controller.tracks + tracks1 = self.controller.tracks.get() + tracks2 = self.controller.tracks.get() self.assertNotEqual(id(tracks1), id(tracks2)) @populate_tracklist def test_remove(self): - track1 = self.controller.tracks[1] - track2 = self.controller.tracks[2] - version = self.controller.version + track1 = self.controller.get_tracks().get()[1] + track2 = self.controller.get_tracks().get()[2] + version = self.controller.get_version().get() self.controller.remove({'uri': [track1.uri]}) - self.assertLess(version, self.controller.version) - self.assertNotIn(track1, self.controller.tracks) - self.assertEqual(track2, self.controller.tracks[1]) + self.assertLess(version, self.controller.get_version().get()) + self.assertNotIn(track1, self.controller.get_tracks().get()) + self.assertEqual(track2, self.controller.get_tracks().get()[1]) @populate_tracklist def test_removing_track_that_does_not_exist_does_nothing(self): - self.controller.remove({'uri': ['/nonexistant']}) + self.controller.remove({'uri': ['/nonexistant']}).get() def test_removing_from_empty_playlist_does_nothing(self): - self.controller.remove({'uri': ['/nonexistant']}) + self.controller.remove({'uri': ['/nonexistant']}).get() @populate_tracklist def test_remove_lists(self): - track0 = self.controller.tracks[0] - track1 = self.controller.tracks[1] - track2 = self.controller.tracks[2] - version = self.controller.version + version = self.controller.get_version().get() + tracks = self.controller.get_tracks().get() + track0 = tracks[0] + track1 = tracks[1] + track2 = tracks[2] + self.controller.remove({'uri': [track0.uri, track2.uri]}) - self.assertLess(version, self.controller.version) - self.assertNotIn(track0, self.controller.tracks) - self.assertNotIn(track2, self.controller.tracks) - self.assertEqual(track1, self.controller.tracks[0]) + + tracks = self.controller.get_tracks().get() + self.assertLess(version, self.controller.get_version().get()) + self.assertNotIn(track0, tracks) + self.assertNotIn(track2, tracks) + self.assertEqual(track1, tracks[0]) @populate_tracklist def test_shuffle(self): random.seed(1) self.controller.shuffle() - shuffled_tracks = self.controller.tracks + shuffled_tracks = self.controller.get_tracks().get() self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(set(self.tracks), set(shuffled_tracks)) @@ -266,7 +306,7 @@ class LocalTracklistProviderTest(unittest.TestCase): random.seed(1) self.controller.shuffle(1, 3) - shuffled_tracks = self.controller.tracks + shuffled_tracks = self.controller.get_tracks().get() self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(self.tracks[0], shuffled_tracks[0]) @@ -275,20 +315,20 @@ class LocalTracklistProviderTest(unittest.TestCase): @populate_tracklist def test_shuffle_invalid_subset(self): with self.assertRaises(AssertionError): - self.controller.shuffle(3, 1) + self.controller.shuffle(3, 1).get() @populate_tracklist def test_shuffle_superset(self): - tracks = len(self.controller.tracks) + num_tracks = len(self.controller.get_tracks().get()) with self.assertRaises(AssertionError): - self.controller.shuffle(1, tracks + 5) + self.controller.shuffle(1, num_tracks + 5).get() @populate_tracklist def test_shuffle_open_subset(self): random.seed(1) self.controller.shuffle(1) - shuffled_tracks = self.controller.tracks + shuffled_tracks = self.controller.get_tracks().get() self.assertNotEqual(self.tracks, shuffled_tracks) self.assertEqual(self.tracks[0], shuffled_tracks[0]) @@ -296,22 +336,22 @@ class LocalTracklistProviderTest(unittest.TestCase): @populate_tracklist def test_slice_returns_a_subset_of_tracks(self): - track_slice = self.controller.slice(1, 3) + track_slice = self.controller.slice(1, 3).get() self.assertEqual(2, len(track_slice)) self.assertEqual(self.tracks[1], track_slice[0].track) self.assertEqual(self.tracks[2], track_slice[1].track) @populate_tracklist def test_slice_returns_empty_list_if_indexes_outside_tracks_list(self): - self.assertEqual(0, len(self.controller.slice(7, 8))) - self.assertEqual(0, len(self.controller.slice(-1, 1))) + self.assertEqual(0, len(self.controller.slice(7, 8).get())) + self.assertEqual(0, len(self.controller.slice(-1, 1).get())) def test_version_does_not_change_when_adding_nothing(self): - version = self.controller.version + version = self.controller.get_version().get() self.controller.add([]) - self.assertEqual(version, self.controller.version) + self.assertEqual(version, self.controller.get_version().get()) def test_version_increases_when_adding_something(self): - version = self.controller.version + version = self.controller.get_version().get() self.controller.add([Track()]) - self.assertLess(version, self.controller.version) + self.assertLess(version, self.controller.get_version().get()) diff --git a/tests/m3u/test_translator.py b/tests/m3u/test_translator.py index f1e14301..88387cb3 100644 --- a/tests/m3u/test_translator.py +++ b/tests/m3u/test_translator.py @@ -20,13 +20,15 @@ encoded_path = path_to_data_dir('æøå.mp3') song1_uri = path.path_to_uri(song1_path) song2_uri = path.path_to_uri(song2_path) song3_uri = path.path_to_uri(song3_path) +song4_uri = 'http://example.com/foo%20bar.mp3' encoded_uri = path.path_to_uri(encoded_path) -song1_track = Track(uri=song1_uri) -song2_track = Track(uri=song2_uri) -song3_track = Track(uri=song3_uri) -encoded_track = Track(uri=encoded_uri) -song1_ext_track = song1_track.replace(name='song1') -song2_ext_track = song2_track.replace(name='song2', length=60000) +song1_track = Track(name='song1', uri=song1_uri) +song2_track = Track(name='song2', uri=song2_uri) +song3_track = Track(name='φοο', uri=song3_uri) +song4_track = Track(name='foo bar', uri=song4_uri) +encoded_track = Track(name='æøå', uri=encoded_uri) +song1_ext_track = song1_track.replace(name='Song #1') +song2_ext_track = song2_track.replace(name='Song #2', length=60000) encoded_ext_track = encoded_track.replace(name='æøå') @@ -84,9 +86,11 @@ class M3UToUriTest(unittest.TestCase): def test_file_with_uri(self): with tempfile.NamedTemporaryFile(delete=False) as tmp: tmp.write(song1_uri) + tmp.write('\n') + tmp.write(song4_uri) try: tracks = self.parse(tmp.name) - self.assertEqual([song1_track], tracks) + self.assertEqual([song1_track, song4_track], tracks) finally: if os.path.exists(tmp.name): os.remove(tmp.name) diff --git a/tests/mpd/protocol/test_idle.py b/tests/mpd/protocol/test_idle.py index 075da845..f93bf8aa 100644 --- a/tests/mpd/protocol/test_idle.py +++ b/tests/mpd/protocol/test_idle.py @@ -10,7 +10,7 @@ from tests.mpd import protocol class IdleHandlerTest(protocol.BaseTestCase): def idle_event(self, subsystem): - self.session.on_idle(subsystem) + self.session.on_event(subsystem) def assertEqualEvents(self, events): # noqa: N802 self.assertEqual(set(events), self.context.events) diff --git a/tests/mpd/protocol/test_regression.py b/tests/mpd/protocol/test_regression.py index 1688d064..40a3f103 100644 --- a/tests/mpd/protocol/test_regression.py +++ b/tests/mpd/protocol/test_regression.py @@ -233,3 +233,24 @@ class IssueGH1120RegressionTest(protocol.BaseTestCase): response2 = self.send_request('lsinfo "/"') self.assertEqual(response1, response2) + + +class IssueGH1348RegressionTest(protocol.BaseTestCase): + + """ + The issue: http://github.com/mopidy/mopidy/issues/1348 + """ + + def test(self): + self.backend.library.dummy_library = [Track(uri='dummy:a')] + + # Create a dummy playlist and trigger population of mapping + self.send_request('playlistadd "testing1" "dummy:a"') + self.send_request('listplaylists') + + # Create an other playlist which isn't in the map + self.send_request('playlistadd "testing2" "dummy:a"') + self.assertEqual(['OK'], self.send_request('rm "testing2"')) + + playlists = self.backend.playlists.as_list().get() + self.assertEqual(['testing1'], [ref.name for ref in playlists]) diff --git a/tests/mpd/protocol/test_stored_playlists.py b/tests/mpd/protocol/test_stored_playlists.py index e212af09..fc3e8214 100644 --- a/tests/mpd/protocol/test_stored_playlists.py +++ b/tests/mpd/protocol/test_stored_playlists.py @@ -232,6 +232,10 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertEqual(0, len(self.core.tracklist.tracks.get())) self.assertEqualResponse('ACK [50@0] {load} No such playlist') + # No invalid name check for load. + self.send_request('load "unknown/playlist"') + self.assertEqualResponse('ACK [50@0] {load} No such playlist') + def test_playlistadd(self): tracks = [ Track(uri='dummy:a'), @@ -259,6 +263,12 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + def test_playlistadd_invalid_name_acks(self): + self.send_request('playlistadd "foo/bar" "dummy:a"') + self.assertInResponse('ACK [2@0] {playlistadd} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + def test_playlistclear(self): self.backend.playlists.set_dummy_playlists([ Playlist( @@ -276,6 +286,12 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + def test_playlistclear_invalid_name_acks(self): + self.send_request('playlistclear "foo/bar"') + self.assertInResponse('ACK [2@0] {playlistclear} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + def test_playlistdelete(self): tracks = [ Track(uri='dummy:a'), @@ -292,6 +308,21 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertEqual( 2, len(self.backend.playlists.get_items('dummy:a1').get())) + def test_playlistdelete_invalid_name_acks(self): + self.send_request('playlistdelete "foo/bar" "0"') + self.assertInResponse('ACK [2@0] {playlistdelete} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + + def test_playlistdelete_unknown_playlist_acks(self): + self.send_request('playlistdelete "foobar" "0"') + self.assertInResponse('ACK [50@0] {playlistdelete} No such playlist') + + def test_playlistdelete_unknown_index_acks(self): + self.send_request('save "foobar"') + self.send_request('playlistdelete "foobar" "0"') + self.assertInResponse('ACK [2@0] {playlistdelete} Bad song index') + def test_playlistmove(self): tracks = [ Track(uri='dummy:a'), @@ -309,6 +340,42 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): "dummy:c", self.backend.playlists.get_items('dummy:a1').get()[0].uri) + def test_playlistmove_invalid_name_acks(self): + self.send_request('playlistmove "foo/bar" "0" "1"') + self.assertInResponse('ACK [2@0] {playlistmove} playlist name is ' + 'invalid: playlist names may not contain ' + 'slashes, newlines or carriage returns') + + def test_playlistmove_unknown_playlist_acks(self): + self.send_request('playlistmove "foobar" "0" "1"') + self.assertInResponse('ACK [50@0] {playlistmove} No such playlist') + + def test_playlistmove_unknown_position_acks(self): + self.send_request('save "foobar"') + self.send_request('playlistmove "foobar" "0" "1"') + self.assertInResponse('ACK [2@0] {playlistmove} Bad song index') + + def test_playlistmove_same_index_shortcircuits_everything(self): + # Bad indexes on unknown playlist: + self.send_request('playlistmove "foobar" "0" "0"') + self.assertInResponse('OK') + + self.send_request('playlistmove "foobar" "100000" "100000"') + self.assertInResponse('OK') + + # Bad indexes on known playlist: + self.send_request('save "foobar"') + + self.send_request('playlistmove "foobar" "0" "0"') + self.assertInResponse('OK') + + self.send_request('playlistmove "foobar" "10" "10"') + self.assertInResponse('OK') + + # Invalid playlist name: + self.send_request('playlistmove "foo/bar" "0" "0"') + self.assertInResponse('OK') + def test_rename(self): self.backend.playlists.set_dummy_playlists([ Playlist( @@ -320,6 +387,31 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertIsNotNone( self.backend.playlists.lookup('dummy:new_name').get()) + def test_rename_unknown_playlist_acks(self): + self.send_request('rename "foo" "bar"') + self.assertInResponse('ACK [50@0] {rename} No such playlist') + + def test_rename_to_existing_acks(self): + self.send_request('save "foo"') + self.send_request('save "bar"') + + self.send_request('rename "foo" "bar"') + self.assertInResponse('ACK [56@0] {rename} Playlist already exists') + + def test_rename_invalid_name_acks(self): + expected = ('ACK [2@0] {rename} playlist name is invalid: playlist ' + 'names may not contain slashes, newlines or carriage ' + 'returns') + + self.send_request('rename "foo/bar" "bar"') + self.assertInResponse(expected) + + self.send_request('rename "foo" "foo/bar"') + self.assertInResponse(expected) + + self.send_request('rename "bar/foo" "foo/bar"') + self.assertInResponse(expected) + def test_rm(self): self.backend.playlists.set_dummy_playlists([ Playlist( @@ -330,8 +422,24 @@ class PlaylistsHandlerTest(protocol.BaseTestCase): self.assertInResponse('OK') self.assertIsNone(self.backend.playlists.lookup('dummy:a1').get()) + def test_rm_unknown_playlist_acks(self): + self.send_request('rm "name"') + self.assertInResponse('ACK [50@0] {rm} No such playlist') + + def test_rm_invalid_name_acks(self): + self.send_request('rm "foo/bar"') + self.assertInResponse('ACK [2@0] {rm} playlist name is invalid: ' + 'playlist names may not contain slashes, ' + 'newlines or carriage returns') + def test_save(self): self.send_request('save "name"') self.assertInResponse('OK') self.assertIsNotNone(self.backend.playlists.lookup('dummy:name').get()) + + def test_save_invalid_name_acks(self): + self.send_request('save "foo/bar"') + self.assertInResponse('ACK [2@0] {save} playlist name is invalid: ' + 'playlist names may not contain slashes, ' + 'newlines or carriage returns') diff --git a/tests/mpd/test_actor.py b/tests/mpd/test_actor.py new file mode 100644 index 00000000..843e46d3 --- /dev/null +++ b/tests/mpd/test_actor.py @@ -0,0 +1,44 @@ +from __future__ import absolute_import, unicode_literals + +import mock + +import pytest + +from mopidy.mpd import actor + +# NOTE: Should be kept in sync with all events from mopidy.core.listener + + +@pytest.mark.parametrize("event,expected", [ + (['track_playback_paused', 'tl_track', 'time_position'], None), + (['track_playback_resumed', 'tl_track', 'time_position'], None), + (['track_playback_started', 'tl_track'], None), + (['track_playback_ended', 'tl_track', 'time_position'], None), + (['playback_state_changed', 'old_state', 'new_state'], 'player'), + (['tracklist_changed'], 'playlist'), + (['playlists_loaded'], 'stored_playlist'), + (['playlist_changed', 'playlist'], 'stored_playlist'), + (['playlist_deleted', 'uri'], 'stored_playlist'), + (['options_changed'], 'options'), + (['volume_changed', 'volume'], 'mixer'), + (['mute_changed', 'mute'], 'output'), + (['seeked', 'time_position'], 'player'), + (['stream_title_changed', 'title'], 'playlist'), +]) +def test_idle_hooked_up_correctly(event, expected): + config = {'mpd': {'hostname': 'foobar', + 'port': 1234, + 'zeroconf': None, + 'max_connections': None, + 'connection_timeout': None}} + + with mock.patch.object(actor.MpdFrontend, '_setup_server'): + frontend = actor.MpdFrontend(core=mock.Mock(), config=config) + + with mock.patch('mopidy.listener.send') as send_mock: + frontend.on_event(event[0], **{e: None for e in event[1:]}) + + if expected is None: + assert not send_mock.call_args + else: + send_mock.assert_called_once_with(mock.ANY, expected) diff --git a/tests/test_httpclient.py b/tests/test_httpclient.py index 63591f80..30f03d8d 100644 --- a/tests/test_httpclient.py +++ b/tests/test_httpclient.py @@ -9,6 +9,7 @@ from mopidy import httpclient @pytest.mark.parametrize("config,expected", [ ({}, None), + ({'hostname': ''}, None), ({'hostname': 'proxy.lan'}, 'http://proxy.lan:80'), ({'scheme': None, 'hostname': 'proxy.lan'}, 'http://proxy.lan:80'), ({'scheme': 'https', 'hostname': 'proxy.lan'}, 'https://proxy.lan:80'), @@ -16,6 +17,8 @@ from mopidy import httpclient ({'password': 'pass', 'hostname': 'proxy.lan'}, 'http://proxy.lan:80'), ({'hostname': 'proxy.lan', 'port': 8080}, 'http://proxy.lan:8080'), ({'hostname': 'proxy.lan', 'port': -1}, 'http://proxy.lan:80'), + ({'hostname': 'proxy.lan', 'port': None}, 'http://proxy.lan:80'), + ({'hostname': 'proxy.lan', 'port': ''}, 'http://proxy.lan:80'), ({'username': 'user', 'password': 'pass', 'hostname': 'proxy.lan'}, 'http://user:pass@proxy.lan:80'), ])