Merge branch 'develop' into fix/310-persist-mopidy-state-between-runs

Conflicts:
	docs/changelog.rst

Fixed conflict in doc/changelog.rst

.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:

AUTHORS

@@ -75,3 +75,4 @@
 - kozec <kozec@kozec.com>
 - Jelle van der Waa <jelle@vdwaa.nl>
 - Alex Malone <jalexmalone@gmail.com>
+- Daniel Hahler <git@thequod.de>

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

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)
 ===================

docs/clients/mpd.rst

@@ -167,5 +167,5 @@ projects are a real match made in heaven."
 Partify
 -------
 
-`Partify <https://github.com/fhats/partify>`_ is a web based MPD client focusing on
+`Partify <https://github.com/fhats/partify>`_ is a web based MPD client focussing on
 making music playing collaborative and social.

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
+html_theme = 'sphinx_rtd_theme'
-# 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_static_path = ['_static']
 
 html_use_modindex = True

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

docs/contributing.rst

@@ -126,3 +126,11 @@ Pull request guidelines
 
 #. Send a pull request to the ``develop`` branch. See the `GitHub pull request
    docs <https://help.github.com/articles/using-pull-requests>`_ 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`.

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.

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

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
+Not a full-featured web client, but rather a local library and web
-extension which allows other Web clients access to album art embedded
+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
 ============

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
 <https://github.com/xbianonpi/xbian/issues/378#issuecomment-37723392>`_.
 
 Run the following commands to remedy this and then install Mopidy as normal::

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
 =================

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

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

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):
         """

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
-
+        return backend.playback.seek(time_position).get()
-        success = backend.playback.seek(time_position).get()
-        if success:
-            self._trigger_seeked(time_position)
-        return success
 
     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)
 

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.

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):

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)
+    port = proxy_config.get('port')
-    if port < 0:
+    if not port or port < 0:
         port = 80
 
     if proxy_config.get('username') and proxy_config.get('password') and auth:

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'],
+        try:
-                                  disable_existing_loggers=False)
+            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:

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))

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):

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):

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
 

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)
+        if subsystem:
-        for listener in listeners:
+            listener.send(session.MpdSession, subsystem)
-            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')

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(

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
 

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
+    try:
-    tracks = list(playlist.tracks)
+        # Convert tracks to list and remove requested
-    tracks.pop(songpos)
+        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
+    try:
-    tracks = list(playlist.tracks)
+        # Convert tracks to list and perform move
-    track = tracks.pop(from_pos)
+        tracks = list(playlist.tracks)
-    tracks.insert(to_pos, track)
+        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)
+    _check_playlist_name(old_name)
-    uri_scheme = urllib.parse.urlparse(uri).scheme
+    _check_playlist_name(new_name)
-    old_playlist = uri is not None and context.core.playlists.lookup(uri).get()
+
+    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()

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):

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)
 

mopidy/zeroconf.py

@@ -55,17 +55,20 @@ class Zeroconf(object):
         self.bus = None
         self.server = None
         self.group = None
-        try:
+        self.display_hostname = None
-            self.bus = dbus.SystemBus()
+        self.name = None
-            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 = '%s' % self.server.GetHostName()
+        if dbus:
-        self.name = string.Template(name).safe_substitute(
+            try:
-            hostname=self.display_hostname, port=port)
+                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)' % (

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())

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)

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):
 

tests/data/comment-ext.m3u

@@ -1,5 +1,5 @@
 #EXTM3U
 # test
-#EXTINF:-1,song1
+#EXTINF:-1,Song #1
 # test
 song1.mp3

tests/data/one-ext.m3u

@@ -1,3 +1,3 @@
 #EXTM3U
-#EXTINF:-1,song1
+#EXTINF:-1,Song #1
 song1.mp3

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

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

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, len(self.controller.get_tl_tracks().get()))
-        self.assertEqual(0, self.controller.length)
+        self.assertEqual(0, self.controller.get_length().get())
         self.controller.add(self.tracks)
-        self.assertEqual(3, len(self.controller.tl_tracks))
+        self.assertEqual(3, len(self.controller.get_tl_tracks().get()))
-        self.assertEqual(3, self.controller.length)
+        self.assertEqual(3, self.controller.get_length().get())
 
     def test_add(self):
         for track in self.tracks:
-            tl_tracks = self.controller.add([track])
+            added = self.controller.add([track]).get()
-            self.assertEqual(track, self.controller.tracks[-1])
+            tracks = self.controller.get_tracks().get()
-            self.assertEqual(tl_tracks[0], self.controller.tl_tracks[-1])
+            tl_tracks = self.controller.get_tl_tracks().get()
-            self.assertEqual(track, tl_tracks[0].track)
+
+            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)
+            added = self.controller.add([track], 0).get()
-            self.assertEqual(track, self.controller.tracks[0])
+            tracks = self.controller.get_tracks().get()
-            self.assertEqual(tl_tracks[0], self.controller.tl_tracks[0])
+            tl_tracks = self.controller.get_tl_tracks().get()
-            self.assertEqual(track, tl_tracks[0].track)
+
+            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)
+            added = self.controller.add([track], len(self.tracks) + 2).get()
-            self.assertEqual(track, self.controller.tracks[-1])
+            tracks = self.controller.get_tracks().get()
-            self.assertEqual(tl_tracks[0], self.controller.tl_tracks[-1])
+            tl_tracks = self.controller.get_tl_tracks().get()
-            self.assertEqual(track, tl_tracks[0].track)
+
+            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]
+        tl_track = self.controller.get_tl_tracks().get()[1]
-        self.assertEqual(
+        result = self.controller.filter({'tlid': [tl_track.tlid]}).get()
-            [tl_track], self.controller.filter({'tlid': [tl_track.tlid]}))
+        self.assertEqual([tl_track], result)
 
     @populate_tracklist
     def test_filter_by_uri(self):
-        tl_track = self.controller.tl_tracks[1]
+        tl_track = self.controller.get_tl_tracks().get()[1]
-        self.assertEqual(
+        result = self.controller.filter({'uri': [tl_track.track.uri]}).get()
-            [tl_track], self.controller.filter({'uri': [tl_track.track.uri]}))
+        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)
+        result1 = self.controller.filter({'uri': ['a'], 'name': ['x']}).get()
-        self.assertEqual(
+        self.assertEqual(t1, result1[0].track)
-            t2, self.controller.filter({'uri': ['b'], 'name': ['x']})[0].track)
+
-        self.assertEqual(
+        result2 = self.controller.filter({'uri': ['b'], 'name': ['x']}).get()
-            t3, self.controller.filter({'uri': ['b'], 'name': ['y']})[0].track)
+        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(
+        result = self.controller.filter({'uri': ['b']}).get()
-            track2, self.controller.filter({'uri': ['b']})[0].track)
+        self.assertEqual(track2, result[0].track)
 
     @populate_tracklist
     def test_clear(self):
-        self.controller.clear()
+        self.controller.clear().get()
-        self.assertEqual(len(self.controller.tracks), 0)
+        self.assertEqual(len(self.controller.get_tracks().get()), 0)
 
     def test_clear_empty_playlist(self):
-        self.controller.clear()
+        self.controller.clear().get()
-        self.assertEqual(len(self.controller.tracks), 0)
+        self.assertEqual(len(self.controller.get_tracks().get()), 0)
 
     @populate_tracklist
     def test_clear_when_playing(self):
-        self.playback.play()
+        self.playback.play().get()
-        self.assertEqual(self.playback.state, PlaybackState.PLAYING)
+        self.assert_state_is(PlaybackState.PLAYING)
-        self.controller.clear()
+        self.controller.clear().get()
-        self.assertEqual(self.playback.state, PlaybackState.STOPPED)
+        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')
+        tracks = self.controller.get_tracks().get()
-        self.assertEqual(self.controller.tracks[1].uri, 'b')
+        self.assertEqual(len(tracks), 4)
-        self.assertEqual(self.controller.tracks[2].uri, 'c')
+        self.assertEqual(tracks[0].uri, 'a')
-        self.assertEqual(self.controller.tracks[3].uri, 'd')
+        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()
+        self.playback.play().get()
-        track = self.playback.current_track
+
-        self.controller.add(self.controller.tracks[1:2])
+        track = self.playback.get_current_track().get()
-        self.assertEqual(self.playback.state, PlaybackState.PLAYING)
+        tracks = self.controller.get_tracks().get()
-        self.assertEqual(self.playback.current_track, track)
+        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])
+        tracks = self.controller.get_tracks().get()
-        self.assertEqual(self.playback.state, PlaybackState.STOPPED)
+        self.controller.add(tracks[1:2]).get()
-        self.assertEqual(self.playback.current_track, None)
+
+        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])
+        tracks = self.controller.get_tracks().get()
-        self.assertEqual(tl_tracks[0].track, self.controller.tracks[1])
+
+        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
+        tracks1 = self.controller.tracks.get()
-        tracks2 = self.controller.tracks
+        tracks2 = self.controller.tracks.get()
         self.assertNotEqual(id(tracks1), id(tracks2))
 
     @populate_tracklist
     def test_remove(self):
-        track1 = self.controller.tracks[1]
+        track1 = self.controller.get_tracks().get()[1]
-        track2 = self.controller.tracks[2]
+        track2 = self.controller.get_tracks().get()[2]
-        version = self.controller.version
+        version = self.controller.get_version().get()
         self.controller.remove({'uri': [track1.uri]})
-        self.assertLess(version, self.controller.version)
+        self.assertLess(version, self.controller.get_version().get())
-        self.assertNotIn(track1, self.controller.tracks)
+        self.assertNotIn(track1, self.controller.get_tracks().get())
-        self.assertEqual(track2, self.controller.tracks[1])
+        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]
+        version = self.controller.get_version().get()
-        track1 = self.controller.tracks[1]
+        tracks = self.controller.get_tracks().get()
-        track2 = self.controller.tracks[2]
+        track0 = tracks[0]
-        version = self.controller.version
+        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)
+        tracks = self.controller.get_tracks().get()
-        self.assertNotIn(track2, self.controller.tracks)
+        self.assertLess(version, self.controller.get_version().get())
-        self.assertEqual(track1, self.controller.tracks[0])
+        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(7, 8).get()))
-        self.assertEqual(0, len(self.controller.slice(-1, 1)))
+        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())

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)
+song1_track = Track(name='song1', uri=song1_uri)
-song2_track = Track(uri=song2_uri)
+song2_track = Track(name='song2', uri=song2_uri)
-song3_track = Track(uri=song3_uri)
+song3_track = Track(name='φοο', uri=song3_uri)
-encoded_track = Track(uri=encoded_uri)
+song4_track = Track(name='foo bar', uri=song4_uri)
-song1_ext_track = song1_track.replace(name='song1')
+encoded_track = Track(name='æøå', uri=encoded_uri)
-song2_ext_track = song2_track.replace(name='song2', length=60000)
+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)

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)

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])

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')

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)

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'),
 ])