diff --git a/docs/api/models.rst b/docs/api/models.rst index 23a08002..7192c284 100644 --- a/docs/api/models.rst +++ b/docs/api/models.rst @@ -76,6 +76,8 @@ Data model helpers .. autoclass:: mopidy.models.ImmutableObject +.. autoclass:: mopidy.models.Field + .. autoclass:: mopidy.models.ModelJSONEncoder .. autofunction:: mopidy.models.model_json_decoder diff --git a/docs/changelog.rst b/docs/changelog.rst index 4595587d..b0574246 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,7 +4,6 @@ Changelog This changelog is used to track all major changes to Mopidy. - v1.1.0 (UNRELEASED) =================== @@ -17,12 +16,24 @@ Core API - Update core controllers to handle backend exceptions in all calls that rely on multiple backends. (Issue: :issue:`667`) +Models +------ + +- Added type checks and other sanity checks to model construction and + serialization. (Fixes: :issue:`865`) + Internal changes ---------------- - Tests have been cleaned up to stop using deprecated APIs where feasible. (Partial fix: :issue:`1083`, PR: :issue:`1090`) +- It is now possible to import :mod:`mopidy.backends` without having GObject or + GStreamer installed. In other words, a lot of backend extensions should now + be able to run tests in a virtualenv with global site-packages disabled. This + removes a lot of potential error sources. (Fixes: :issue:`1068`, PR: + :issue:`1115`) + v1.0.1 (UNRELEASED) =================== diff --git a/mopidy/__init__.py b/mopidy/__init__.py index 388bb9f0..8322de32 100644 --- a/mopidy/__init__.py +++ b/mopidy/__init__.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, print_function, unicode_literals import platform import sys -import textwrap import warnings @@ -11,21 +10,6 @@ if not (2, 7) <= sys.version_info < (3,): 'ERROR: Mopidy requires Python 2.7, but found %s.' % platform.python_version()) -try: - import gobject # noqa -except ImportError: - print(textwrap.dedent(""" - ERROR: The gobject Python package was not found. - - Mopidy requires GStreamer (and GObject) to work. These are C libraries - with a number of dependencies themselves, and cannot be installed with - the regular Python tools like pip. - - Please see http://docs.mopidy.com/en/latest/installation/ for - instructions on how to install the required dependencies. - """)) - raise - warnings.filterwarnings('ignore', 'could not open display') diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 96e10e18..9ec9769f 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -4,8 +4,23 @@ import logging import os import signal import sys +import textwrap + +try: + import gobject # noqa +except ImportError: + print(textwrap.dedent(""" + ERROR: The gobject Python package was not found. + + Mopidy requires GStreamer (and GObject) to work. These are C libraries + with a number of dependencies themselves, and cannot be installed with + the regular Python tools like pip. + + Please see http://docs.mopidy.com/en/latest/installation/ for + instructions on how to install the required dependencies. + """)) + raise -import gobject gobject.threads_init() try: diff --git a/mopidy/core/library.py b/mopidy/core/library.py index 324786ad..35a43501 100644 --- a/mopidy/core/library.py +++ b/mopidy/core/library.py @@ -40,8 +40,8 @@ class LibraryController(object): Browse directories and tracks at the given ``uri``. ``uri`` is a string which represents some directory belonging to a - backend. To get the intial root directories for backends pass None as - the URI. + backend. To get the intial root directories for backends pass + :class:`None` as the URI. Returns a list of :class:`mopidy.models.Ref` objects for the directories and tracks at the given ``uri``. diff --git a/mopidy/ext.py b/mopidy/ext.py index f5f15058..3122611f 100644 --- a/mopidy/ext.py +++ b/mopidy/ext.py @@ -89,14 +89,7 @@ class Extension(object): the ``frontend`` and ``backend`` registry keys. This method can also be used for other setup tasks not involving the - extension registry. For example, to register custom GStreamer - elements:: - - def setup(self, registry): - from .mixer import SoundspotMixer - gobject.type_register(SoundspotMixer) - gst.element_register( - SoundspotMixer, 'soundspotmixer', gst.RANK_MARGINAL) + extension registry. :param registry: the extension registry :type registry: :class:`Registry` diff --git a/mopidy/listener.py b/mopidy/listener.py index 410558ac..35bd8b73 100644 --- a/mopidy/listener.py +++ b/mopidy/listener.py @@ -2,14 +2,18 @@ from __future__ import absolute_import, unicode_literals import logging -import gobject - import pykka logger = logging.getLogger(__name__) def send_async(cls, event, **kwargs): + # This file is imported by mopidy.backends, which again is imported by all + # backend extensions. By importing modules that are not easily installable + # close to their use, we make some extensions able to run their tests in a + # virtualenv with global site-packages disabled. + import gobject + gobject.idle_add(lambda: send(cls, event, **kwargs)) diff --git a/mopidy/m3u/playlists.py b/mopidy/m3u/playlists.py index c09eccdf..eaa3d980 100644 --- a/mopidy/m3u/playlists.py +++ b/mopidy/m3u/playlists.py @@ -76,6 +76,8 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider): 'Loaded %d M3U playlists from %s', len(playlists), self._playlists_dir) + # TODO Trigger playlists_loaded event? + def save(self, playlist): assert playlist.uri, 'Cannot save playlist without URI' assert playlist.uri in self._playlists, \ diff --git a/mopidy/models.py b/mopidy/models.py index 1ae26811..45961f30 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -2,30 +2,166 @@ from __future__ import absolute_import, unicode_literals import json +# TODO: split into base models, serialization and fields? + + +class Field(object): + + """ + Base field for use in :class:`ImmutableObject`. These fields are + responsible for type checking and other data sanitation in our models. + + For simplicity fields use the Python descriptor protocol to store the + values in the instance dictionary. Also note that fields are mutable if + the object they are attached to allow it. + + Default values will be validated with the exception of :class:`None`. + + :param default: default value for field + :param type: if set the field value must be of this type + :param choices: if set the field value must be one of these + """ + + def __init__(self, default=None, type=None, choices=None): + self._name = None # Set by FieldOwner + self._choices = choices + self._default = default + self._type = type + + if self._default is not None: + self.validate(self._default) + + def validate(self, value): + """Validate and possibly modify the field value before assignment""" + if self._type and not isinstance(value, self._type): + raise TypeError('Expected %s to be a %s, not %r' % + (self._name, self._type, value)) + if self._choices and value not in self._choices: + raise TypeError('Expected %s to be a one of %s, not %r' % + (self._name, self._choices, value)) + return value + + def __get__(self, instance, owner): + if not instance: + return self + return instance.__dict__.get(self._name, self._default) + + def __set__(self, instance, value): + if value is not None: + value = self.validate(value) + + if value is None or value == self._default: + self.__delete__(instance) + else: + instance.__dict__[self._name] = value + + def __delete__(self, instance): + instance.__dict__.pop(self._name, None) + + +class String(Field): + + """ + Specialized :class:`Field` which is wired up for bytes and unicode. + + :param default: default value for field + """ + + def __init__(self, default=None): + # TODO: normalize to unicode? + # TODO: only allow unicode? + # TODO: disallow empty strings? + super(String, self).__init__(type=basestring, default=default) + + +class Integer(Field): + + """ + :class:`Field` for storing integer numbers. + + :param default: default value for field + :param min: field value must be larger or equal to this value when set + :param max: field value must be smaller or equal to this value when set + """ + + def __init__(self, default=None, min=None, max=None): + self._min = min + self._max = max + super(Integer, self).__init__(type=(int, long), default=default) + + def validate(self, value): + value = super(Integer, self).validate(value) + if self._min is not None and value < self._min: + raise ValueError('Expected %s to be at least %d, not %d' % + (self._name, self._min, value)) + if self._max is not None and value > self._max: + raise ValueError('Expected %s to be at most %d, not %d' % + (self._name, self._max, value)) + return value + + +class Collection(Field): + + """ + :class:`Field` for storing collections of a given type. + + :param type: all items stored in the collection must be of this type + :param container: the type to store the items in + """ + + def __init__(self, type, container=tuple): + super(Collection, self).__init__(type=type, default=container()) + + def validate(self, value): + if isinstance(value, basestring): + raise TypeError('Expected %s to be a collection of %s, not %r' + % (self._name, self._type.__name__, value)) + for v in value: + if not isinstance(v, self._type): + raise TypeError('Expected %s to be a collection of %s, not %r' + % (self._name, self._type.__name__, value)) + return self._default.__class__(value) or None + + +class FieldOwner(type): + + """Helper to automatically assign field names to descriptors.""" + + def __new__(cls, name, bases, attrs): + attrs['_fields'] = [] + for key, value in attrs.items(): + if isinstance(value, Field): + attrs['_fields'].append(key) + value._name = key + attrs['_fields'].sort() + return super(FieldOwner, cls).__new__(cls, name, bases, attrs) + class ImmutableObject(object): """ Superclass for immutable objects whose fields can only be modified via the - constructor. + constructor. Fields should be :class:`Field` instances to ensure type + safety in our models. :param kwargs: kwargs to set as fields on the object :type kwargs: any """ + __metaclass__ = FieldOwner + def __init__(self, *args, **kwargs): for key, value in kwargs.items(): - if not hasattr(self, key) or callable(getattr(self, key)): + if key not in self._fields: raise TypeError( '__init__() got an unexpected keyword argument "%s"' % key) - if value == getattr(self, key): - continue # Don't explicitly set default values - self.__dict__[key] = value + super(ImmutableObject, self).__setattr__(key, value) def __setattr__(self, name, value): - if name.startswith('_'): - return super(ImmutableObject, self).__setattr__(name, value) + raise AttributeError('Object is immutable.') + + def __delattr__(self, name): raise AttributeError('Object is immutable.') def __repr__(self): @@ -71,26 +207,19 @@ class ImmutableObject(object): :type values: dict :rtype: new instance of the model being copied """ - data = {} - for key in self.__dict__.keys(): - public_key = key.lstrip('_') - value = values.pop(public_key, self.__dict__[key]) - data[public_key] = value - for key in values.keys(): - if hasattr(self, key): - value = values.pop(key) - data[key] = value - if values: - raise TypeError( - 'copy() got an unexpected keyword argument "%s"' % key) - return self.__class__(**data) + other = self.__class__() + other.__dict__.update(self.__dict__) + for key, value in values.items(): + if key not in self._fields: + raise TypeError( + 'copy() got an unexpected keyword argument "%s"' % key) + super(ImmutableObject, other).__setattr__(key, value) + return other def serialize(self): data = {} data['__model__'] = self.__class__.__name__ - for key in self.__dict__.keys(): - public_key = key.lstrip('_') - value = self.__dict__[key] + for key, value in self.__dict__.items(): if isinstance(value, (set, frozenset, list, tuple)): value = [ v.serialize() if isinstance(v, ImmutableObject) else v @@ -98,7 +227,7 @@ class ImmutableObject(object): elif isinstance(value, ImmutableObject): value = value.serialize() if not (isinstance(value, list) and len(value) == 0): - data[public_key] = value + data[key] = value return data @@ -161,14 +290,10 @@ class Ref(ImmutableObject): """ #: The object URI. Read-only. - uri = None + uri = String() #: The object name. Read-only. - name = None - - #: The object type, e.g. "artist", "album", "track", "playlist", - #: "directory". Read-only. - type = None + name = String() #: Constant used for comparison with the :attr:`type` field. ALBUM = 'album' @@ -185,6 +310,10 @@ class Ref(ImmutableObject): #: Constant used for comparison with the :attr:`type` field. TRACK = 'track' + #: The object type, e.g. "artist", "album", "track", "playlist", + #: "directory". Read-only. + type = Field(choices=(ALBUM, ARTIST, DIRECTORY, PLAYLIST, TRACK)) + @classmethod def album(cls, **kwargs): """Create a :class:`Ref` with ``type`` :attr:`ALBUM`.""" @@ -225,13 +354,13 @@ class Image(ImmutableObject): """ #: The image URI. Read-only. - uri = None + uri = String() #: Optional width of the image or :class:`None`. Read-only. - width = None + width = Integer(min=0) #: Optional height of the image or :class:`None`. Read-only. - height = None + height = Integer(min=0) class Artist(ImmutableObject): @@ -246,13 +375,13 @@ class Artist(ImmutableObject): """ #: The artist URI. Read-only. - uri = None + uri = String() #: The artist name. Read-only. - name = None + name = String() #: The MusicBrainz ID of the artist. Read-only. - musicbrainz_id = None + musicbrainz_id = String() class Album(ImmutableObject): @@ -277,37 +406,32 @@ class Album(ImmutableObject): """ #: The album URI. Read-only. - uri = None + uri = String() #: The album name. Read-only. - name = None + name = String() #: A set of album artists. Read-only. - artists = frozenset() + artists = Collection(type=Artist, container=frozenset) #: The number of tracks in the album. Read-only. - num_tracks = None + num_tracks = Integer(min=0) #: The number of discs in the album. Read-only. - num_discs = None + num_discs = Integer(min=0) #: The album release date. Read-only. - date = None + date = String() # TODO: add date type #: The MusicBrainz ID of the album. Read-only. - musicbrainz_id = None + musicbrainz_id = String() #: The album image URIs. Read-only. - images = frozenset() + images = Collection(type=basestring, 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. - def __init__(self, *args, **kwargs): - self.__dict__['artists'] = frozenset(kwargs.pop('artists', None) or []) - self.__dict__['images'] = frozenset(kwargs.pop('images', None) or []) - super(Album, self).__init__(*args, **kwargs) - class Track(ImmutableObject): @@ -345,61 +469,52 @@ class Track(ImmutableObject): """ #: The track URI. Read-only. - uri = None + uri = String() #: The track name. Read-only. - name = None + name = String() #: A set of track artists. Read-only. - artists = frozenset() + artists = Collection(type=Artist, container=frozenset) #: The track :class:`Album`. Read-only. - album = None + album = Field(type=Album) #: A set of track composers. Read-only. - composers = frozenset() + composers = Collection(type=Artist, container=frozenset) #: A set of track performers`. Read-only. - performers = frozenset() + performers = Collection(type=Artist, container=frozenset) #: The track genre. Read-only. - genre = None + genre = String() #: The track number in the album. Read-only. - track_no = None + track_no = Integer(min=0) #: The disc number in the album. Read-only. - disc_no = None + disc_no = Integer(min=0) #: The track release date. Read-only. - date = None + date = String() # TODO: add date type #: The track length in milliseconds. Read-only. - length = None + length = Integer(min=0) #: The track's bitrate in kbit/s. Read-only. - bitrate = None + bitrate = Integer(min=0) #: The track comment. Read-only. - comment = None + comment = String() #: The MusicBrainz ID of the track. Read-only. - musicbrainz_id = None + musicbrainz_id = String() #: Integer representing when the track was last modified. Exact meaning #: depends on source of track. For local files this is the modification #: time in milliseconds since Unix epoch. For other backends it could be an #: equivalent timestamp or simply a version counter. - last_modified = None - - def __init__(self, *args, **kwargs): - def get(key): - return frozenset(kwargs.pop(key, None) or []) - - self.__dict__['artists'] = get('artists') - self.__dict__['composers'] = get('composers') - self.__dict__['performers'] = get('performers') - super(Track, self).__init__(*args, **kwargs) + last_modified = Integer(min=0) class TlTrack(ImmutableObject): @@ -425,10 +540,10 @@ class TlTrack(ImmutableObject): """ #: The tracklist ID. Read-only. - tlid = None + tlid = Integer(min=0) #: The track. Read-only. - track = None + track = Field(type=Track) def __init__(self, *args, **kwargs): if len(args) == 2 and len(kwargs) == 0: @@ -456,23 +571,19 @@ class Playlist(ImmutableObject): """ #: The playlist URI. Read-only. - uri = None + uri = String() #: The playlist name. Read-only. - name = None + name = String() #: The playlist's tracks. Read-only. - tracks = tuple() + tracks = Collection(type=Track, container=tuple) #: The playlist modification time in milliseconds since Unix epoch. #: Read-only. #: #: Integer, or :class:`None` if unknown. - last_modified = None - - def __init__(self, *args, **kwargs): - self.__dict__['tracks'] = tuple(kwargs.pop('tracks', None) or []) - super(Playlist, self).__init__(*args, **kwargs) + last_modified = Integer(min=0) # TODO: def insert(self, pos, track): ... ? @@ -496,19 +607,13 @@ class SearchResult(ImmutableObject): """ # The search result URI. Read-only. - uri = None + uri = String() # The tracks matching the search query. Read-only. - tracks = tuple() + tracks = Collection(type=Track, container=tuple) # The artists matching the search query. Read-only. - artists = tuple() + artists = Collection(type=Artist, container=tuple) # The albums matching the search query. Read-only. - albums = tuple() - - def __init__(self, *args, **kwargs): - self.__dict__['tracks'] = tuple(kwargs.pop('tracks', None) or []) - self.__dict__['artists'] = tuple(kwargs.pop('artists', None) or []) - self.__dict__['albums'] = tuple(kwargs.pop('albums', None) or []) - super(SearchResult, self).__init__(*args, **kwargs) + albums = Collection(type=Album, container=tuple) diff --git a/mopidy/utils/path.py b/mopidy/utils/path.py index e845cd95..37b6cdb1 100644 --- a/mopidy/utils/path.py +++ b/mopidy/utils/path.py @@ -8,25 +8,15 @@ import threading import urllib import urlparse -import glib - from mopidy import compat, exceptions from mopidy.compat import queue -from mopidy.utils import encoding +from mopidy.utils import encoding, xdg logger = logging.getLogger(__name__) -XDG_DIRS = { - 'XDG_CACHE_DIR': glib.get_user_cache_dir(), - 'XDG_CONFIG_DIR': glib.get_user_config_dir(), - 'XDG_DATA_DIR': glib.get_user_data_dir(), - 'XDG_MUSIC_DIR': glib.get_user_special_dir(glib.USER_DIRECTORY_MUSIC), -} - -# XDG_MUSIC_DIR can be none, so filter out any bad data. -XDG_DIRS = dict((k, v) for k, v in XDG_DIRS.items() if v is not None) +XDG_DIRS = xdg.get_dirs() def get_or_create_dir(dir_path): diff --git a/mopidy/utils/xdg.py b/mopidy/utils/xdg.py new file mode 100644 index 00000000..adb43f39 --- /dev/null +++ b/mopidy/utils/xdg.py @@ -0,0 +1,66 @@ +from __future__ import absolute_import, unicode_literals + +import ConfigParser as configparser +import io +import os + + +def get_dirs(): + """Returns a dict of all the known XDG Base Directories for the current user. + + The keys ``XDG_CACHE_DIR``, ``XDG_CONFIG_DIR``, and ``XDG_DATA_DIR`` is + always available. + + Additional keys, like ``XDG_MUSIC_DIR``, may be available if the + ``$XDG_CONFIG_DIR/user-dirs.dirs`` file exists and is parseable. + + See http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html + for the XDG Base Directory specification. + """ + + dirs = { + 'XDG_CACHE_DIR': ( + os.environ.get('XDG_CACHE_HOME') or + os.path.expanduser(b'~/.cache')), + 'XDG_CONFIG_DIR': ( + os.environ.get('XDG_CONFIG_HOME') or + os.path.expanduser(b'~/.config')), + 'XDG_DATA_DIR': ( + os.environ.get('XDG_DATA_HOME') or + os.path.expanduser(b'~/.local/share')), + } + + dirs.update(_get_user_dirs(dirs['XDG_CONFIG_DIR'])) + + return dirs + + +def _get_user_dirs(xdg_config_dir): + """Returns a dict of XDG dirs read from + ``$XDG_CONFIG_HOME/user-dirs.dirs``. + + This is used at import time for most users of :mod:`mopidy`. By rolling our + own implementation instead of using :meth:`glib.get_user_special_dir` we + make it possible for many extensions to run their test suites, which are + importing parts of :mod:`mopidy`, in a virtualenv with global site-packages + disabled, and thus no :mod:`glib` available. + """ + + dirs_file = os.path.join(xdg_config_dir, 'user-dirs.dirs') + + if not os.path.exists(dirs_file): + return {} + + with open(dirs_file, 'rb') as fh: + data = fh.read() + + data = b'[XDG_USER_DIRS]\n' + data + data = data.replace(b'$HOME', os.path.expanduser(b'~')) + data = data.replace(b'"', b'') + + config = configparser.RawConfigParser() + config.readfp(io.BytesIO(data)) + + return { + k.decode('utf-8').upper(): os.path.abspath(v) + for k, v in config.items('XDG_USER_DIRS') if v is not None} diff --git a/tests/local/test_tracklist.py b/tests/local/test_tracklist.py index 22d4c954..ca36ac44 100644 --- a/tests/local/test_tracklist.py +++ b/tests/local/test_tracklist.py @@ -103,7 +103,7 @@ class LocalTracklistProviderTest(unittest.TestCase): def test_filter_by_uri_returns_nothing_if_no_match(self): self.controller.playlist = Playlist( - tracks=[Track(uri=['z']), Track(uri=['y'])]) + tracks=[Track(uri='z'), Track(uri='y')]) self.assertEqual([], self.controller.filter(uri=['a'])) def test_filter_by_multiple_criteria_returns_elements_matching_all(self): diff --git a/tests/models/test_fields.py b/tests/models/test_fields.py new file mode 100644 index 00000000..5347b83c --- /dev/null +++ b/tests/models/test_fields.py @@ -0,0 +1,207 @@ +from __future__ import absolute_import, unicode_literals + +import unittest + +from mopidy.models import * # noqa: F403 + + +def create_instance(field): + """Create an instance of a dummy class for testing fields.""" + + class Dummy(object): + __metaclass__ = FieldOwner + attr = field + + return Dummy() + + +class FieldDescriptorTest(unittest.TestCase): + def test_raw_field_accesible_through_class(self): + field = Field() + instance = create_instance(field) + self.assertEqual(field, instance.__class__.attr) + + def test_field_knows_its_name(self): + instance = create_instance(Field()) + self.assertEqual('attr', instance.__class__.attr._name) + + def test_field_has_none_as_default(self): + instance = create_instance(Field()) + self.assertIsNone(instance.attr) + + def test_field_does_not_store_default_in_dict(self): + instance = create_instance(Field()) + self.assertNotIn('attr', instance.__dict__) + + def test_field_assigment_and_retrival(self): + instance = create_instance(Field()) + instance.attr = 1234 + self.assertEqual(1234, instance.attr) + self.assertEqual(1234, instance.__dict__['attr']) + + def test_field_can_be_reassigned(self): + instance = create_instance(Field()) + instance.attr = 1234 + instance.attr = 5678 + self.assertEqual(5678, instance.attr) + + def test_field_can_be_deleted(self): + instance = create_instance(Field()) + instance.attr = 1234 + del instance.attr + self.assertEqual(None, instance.attr) + self.assertNotIn('attr', instance.__dict__) + + def test_field_can_be_set_to_none(self): + instance = create_instance(Field()) + instance.attr = 1234 + instance.attr = None + self.assertEqual(None, instance.attr) + self.assertNotIn('attr', instance.__dict__) + + def test_field_can_be_set_default(self): + default = object() + instance = create_instance(Field(default=default)) + instance.attr = 1234 + instance.attr = default + self.assertEqual(default, instance.attr) + self.assertNotIn('attr', instance.__dict__) + + +class FieldTest(unittest.TestCase): + def test_default_handling(self): + instance = create_instance(Field(default=1234)) + self.assertEqual(1234, instance.attr) + + def test_type_checking(self): + instance = create_instance(Field(type=set)) + instance.attr = set() + + with self.assertRaises(TypeError): + instance.attr = 1234 + + def test_choices_checking(self): + instance = create_instance(Field(choices=(1, 2, 3))) + instance.attr = 1 + + with self.assertRaises(TypeError): + instance.attr = 4 + + def test_default_respects_type_check(self): + with self.assertRaises(TypeError): + create_instance(Field(type=int, default='123')) + + def test_default_respects_choices_check(self): + with self.assertRaises(TypeError): + create_instance(Field(choices=(1, 2, 3), default=5)) + + +class StringTest(unittest.TestCase): + def test_default_handling(self): + instance = create_instance(String(default='abc')) + self.assertEqual('abc', instance.attr) + + def test_native_str_allowed(self): + instance = create_instance(String()) + instance.attr = str('abc') + self.assertEqual('abc', instance.attr) + + def test_bytes_allowed(self): + instance = create_instance(String()) + instance.attr = b'abc' + self.assertEqual(b'abc', instance.attr) + + def test_unicode_allowed(self): + instance = create_instance(String()) + instance.attr = u'abc' + self.assertEqual(u'abc', instance.attr) + + def test_other_disallowed(self): + instance = create_instance(String()) + with self.assertRaises(TypeError): + instance.attr = 1234 + + def test_empty_string(self): + instance = create_instance(String()) + instance.attr = '' + self.assertEqual('', instance.attr) + + +class IntegerTest(unittest.TestCase): + def test_default_handling(self): + instance = create_instance(Integer(default=1234)) + self.assertEqual(1234, instance.attr) + + def test_int_allowed(self): + instance = create_instance(Integer()) + instance.attr = int(123) + self.assertEqual(123, instance.attr) + + def test_long_allowed(self): + instance = create_instance(Integer()) + instance.attr = long(123) + self.assertEqual(123, instance.attr) + + def test_float_disallowed(self): + instance = create_instance(Integer()) + with self.assertRaises(TypeError): + instance.attr = 123.0 + + def test_numeric_string_disallowed(self): + instance = create_instance(Integer()) + with self.assertRaises(TypeError): + instance.attr = '123' + + def test_other_disallowed(self): + instance = create_instance(String()) + with self.assertRaises(TypeError): + instance.attr = tuple() + + def test_min_validation(self): + instance = create_instance(Integer(min=0)) + instance.attr = 0 + self.assertEqual(0, instance.attr) + + with self.assertRaises(ValueError): + instance.attr = -1 + + def test_max_validation(self): + instance = create_instance(Integer(max=10)) + instance.attr = 10 + self.assertEqual(10, instance.attr) + + with self.assertRaises(ValueError): + instance.attr = 11 + + +class CollectionTest(unittest.TestCase): + def test_container_instance_is_default(self): + instance = create_instance(Collection(type=int, container=frozenset)) + self.assertEqual(frozenset(), instance.attr) + + def test_empty_collection(self): + instance = create_instance(Collection(type=int, container=frozenset)) + instance.attr = [] + self.assertEqual(frozenset(), instance.attr) + self.assertNotIn('attr', instance.__dict__) + + def test_collection_gets_stored_in_container(self): + instance = create_instance(Collection(type=int, container=frozenset)) + instance.attr = [1, 2, 3] + self.assertEqual(frozenset([1, 2, 3]), instance.attr) + self.assertEqual(frozenset([1, 2, 3]), instance.__dict__['attr']) + + def test_collection_with_wrong_type(self): + instance = create_instance(Collection(type=int, container=frozenset)) + with self.assertRaises(TypeError): + instance.attr = [1, '2', 3] + + def test_collection_with_string(self): + instance = create_instance(Collection(type=int, container=frozenset)) + with self.assertRaises(TypeError): + instance.attr = '123' + + def test_strings_should_not_be_considered_a_collection(self): + instance = create_instance(Collection(type=str, container=tuple)) + with self.assertRaises(TypeError): + instance.attr = b'123' diff --git a/tests/test_models.py b/tests/models/test_models.py similarity index 99% rename from tests/test_models.py rename to tests/models/test_models.py index e9a8f439..77383a6e 100644 --- a/tests/test_models.py +++ b/tests/models/test_models.py @@ -74,6 +74,12 @@ class RefTest(unittest.TestCase): with self.assertRaises(AttributeError): ref.name = None + # TODO: add these for the more of the models? + def test_del_name(self): + ref = Ref(name='foo') + with self.assertRaises(AttributeError): + del ref.name + def test_invalid_kwarg(self): with self.assertRaises(TypeError): Ref(foo='baz') diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py index bf50687d..4e1baf0e 100644 --- a/tests/mpd/test_translator.py +++ b/tests/mpd/test_translator.py @@ -1,6 +1,5 @@ from __future__ import absolute_import, unicode_literals -import datetime import unittest from mopidy.models import Album, Artist, Playlist, TlTrack, Track @@ -20,8 +19,8 @@ class TrackMpdFormatTest(unittest.TestCase): composers=[Artist(name='a composer')], performers=[Artist(name='a performer')], genre='a genre', - date=datetime.date(1977, 1, 1), - disc_no='1', + date='1977-01-01', + disc_no=1, comment='a comment', length=137000, ) @@ -73,8 +72,8 @@ class TrackMpdFormatTest(unittest.TestCase): self.assertIn(('Performer', 'a performer'), result) self.assertIn(('Genre', 'a genre'), result) self.assertIn(('Track', '7/13'), result) - self.assertIn(('Date', datetime.date(1977, 1, 1)), result) - self.assertIn(('Disc', '1'), result) + self.assertIn(('Date', '1977-01-01'), result) + self.assertIn(('Disc', 1), result) self.assertIn(('Pos', 9), result) self.assertIn(('Id', 122), result) self.assertNotIn(('Comment', 'a comment'), result) diff --git a/tests/utils/test_xdg.py b/tests/utils/test_xdg.py new file mode 100644 index 00000000..eab595a4 --- /dev/null +++ b/tests/utils/test_xdg.py @@ -0,0 +1,69 @@ +from __future__ import unicode_literals + +import os + +import mock + +import pytest + +from mopidy.utils import xdg + + +@pytest.yield_fixture +def environ(): + patcher = mock.patch.dict(os.environ, clear=True) + yield patcher.start() + patcher.stop() + + +def test_cache_dir_default(environ): + assert xdg.get_dirs()['XDG_CACHE_DIR'] == os.path.expanduser(b'~/.cache') + + +def test_cache_dir_from_env(environ): + os.environ['XDG_CACHE_HOME'] = '/foo/bar' + + assert xdg.get_dirs()['XDG_CACHE_DIR'] == '/foo/bar' + + +def test_config_dir_default(environ): + assert xdg.get_dirs()['XDG_CONFIG_DIR'] == os.path.expanduser(b'~/.config') + + +def test_config_dir_from_env(environ): + os.environ['XDG_CONFIG_HOME'] = '/foo/bar' + + assert xdg.get_dirs()['XDG_CONFIG_DIR'] == '/foo/bar' + + +def test_data_dir_default(environ): + assert xdg.get_dirs()['XDG_DATA_DIR'] == os.path.expanduser( + b'~/.local/share') + + +def test_data_dir_from_env(environ): + os.environ['XDG_DATA_HOME'] = '/foo/bar' + + assert xdg.get_dirs()['XDG_DATA_DIR'] == '/foo/bar' + + +def test_user_dirs(environ, tmpdir): + os.environ['XDG_CONFIG_HOME'] = str(tmpdir) + + with open(os.path.join(str(tmpdir), 'user-dirs.dirs'), 'wb') as fh: + fh.write('# Some comments\n') + fh.write('XDG_MUSIC_DIR="$HOME/Music2"\n') + + result = xdg.get_dirs() + + assert result['XDG_MUSIC_DIR'] == os.path.expanduser(b'~/Music2') + assert 'XDG_DOWNLOAD_DIR' not in result + + +def test_user_dirs_when_no_dirs_file(environ, tmpdir): + os.environ['XDG_CONFIG_HOME'] = str(tmpdir) + + result = xdg.get_dirs() + + assert 'XDG_MUSIC_DIR' not in result + assert 'XDG_DOWNLOAD_DIR' not in result