From 3f7fbf67f33776b37bcab1fc7a3a7775247a0e26 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 12:45:12 +0100 Subject: [PATCH 01/10] Fix remaining gi.repository imports --- mopidy/commands.py | 3 +-- mopidy/internal/network.py | 3 +-- tests/internal/network/test_connection.py | 3 +-- tests/internal/network/test_server.py | 3 +-- tests/internal/test_path.py | 3 +-- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/mopidy/commands.py b/mopidy/commands.py index af861032..74905f8f 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -7,14 +7,13 @@ import logging import os import sys -from gi.repository import GLib, GObject - import pykka from mopidy import config as config_lib, exceptions from mopidy.audio import Audio from mopidy.core import Core from mopidy.internal import deps, process, timer, versioning +from mopidy.internal.gi import GLib, GObject logger = logging.getLogger(__name__) diff --git a/mopidy/internal/network.py b/mopidy/internal/network.py index c956d795..cefdf8ea 100644 --- a/mopidy/internal/network.py +++ b/mopidy/internal/network.py @@ -7,11 +7,10 @@ import socket import sys import threading -from gi.repository import GObject - import pykka from mopidy.internal import encoding +from mopidy.internal.gi import GObject logger = logging.getLogger(__name__) diff --git a/tests/internal/network/test_connection.py b/tests/internal/network/test_connection.py index 291bbc46..9ee0aaf3 100644 --- a/tests/internal/network/test_connection.py +++ b/tests/internal/network/test_connection.py @@ -5,13 +5,12 @@ import logging import socket import unittest -from gi.repository import GObject - from mock import Mock, call, patch, sentinel import pykka from mopidy.internal import network +from mopidy.internal.gi import GObject from tests import any_int, any_unicode diff --git a/tests/internal/network/test_server.py b/tests/internal/network/test_server.py index 1df25dbc..072e24de 100644 --- a/tests/internal/network/test_server.py +++ b/tests/internal/network/test_server.py @@ -4,11 +4,10 @@ import errno import socket import unittest -from gi.repository import GObject - from mock import Mock, patch, sentinel from mopidy.internal import network +from mopidy.internal.gi import GObject from tests import any_int diff --git a/tests/internal/test_path.py b/tests/internal/test_path.py index 751e7c6e..9e09c39a 100644 --- a/tests/internal/test_path.py +++ b/tests/internal/test_path.py @@ -7,10 +7,9 @@ import shutil import tempfile import unittest -from gi.repository import GLib - from mopidy import compat, exceptions from mopidy.internal import path +from mopidy.internal.gi import GLib import tests From 95b21599c7cf791649e223c16e967f97d2252a48 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 12:45:16 +0100 Subject: [PATCH 02/10] docs: Update mocks for docs build without all deps Fixes #1431 --- docs/conf.py | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 5dcebc31..3ad5c799 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -15,7 +15,6 @@ sys.path.insert(0, os.path.abspath(os.path.dirname(__file__) + '/../')) class Mock(object): - def __init__(self, *args, **kwargs): pass @@ -27,35 +26,20 @@ class Mock(object): @classmethod def __getattr__(self, name): - if name in ('__file__', '__path__'): - return '/dev/null' - elif name == 'get_system_config_dirs': - # glib.get_system_config_dirs() - return tuple - elif name == 'get_user_config_dir': - # glib.get_user_config_dir() + if name == 'get_system_config_dirs': # GLib.get_system_config_dirs() + return list + elif name == 'get_user_config_dir': # GLib.get_user_config_dir() return str - elif (name[0] == name[0].upper() and - # gst.Caps - not name.startswith('Caps') and - # gst.PadTemplate - not name.startswith('PadTemplate') and - # dbus.String() - not name == 'String'): - return type(name, (), {}) else: return Mock() + MOCK_MODULES = [ 'dbus', 'dbus.mainloop', 'dbus.mainloop.glib', 'dbus.service', - 'glib', - 'gobject', - 'gst', - 'gst.pbutils', - 'pygst', + 'mopidy.internal.gi', 'pykka', 'pykka.actor', 'pykka.future', From 78d10c4ab8dbbcc5f02dfb23459dc265e7c96ae0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 12:55:10 +0100 Subject: [PATCH 03/10] Reduce variation in Pykka imports Which lets us reduce the amount of mocked modules when building docs --- docs/conf.py | 3 --- mopidy/internal/process.py | 13 ++++++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 3ad5c799..208822a2 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -41,9 +41,6 @@ MOCK_MODULES = [ 'dbus.service', 'mopidy.internal.gi', 'pykka', - 'pykka.actor', - 'pykka.future', - 'pykka.registry', ] for mod_name in MOCK_MODULES: sys.modules[mod_name] = Mock() diff --git a/mopidy/internal/process.py b/mopidy/internal/process.py index e826e43c..0710a82f 100644 --- a/mopidy/internal/process.py +++ b/mopidy/internal/process.py @@ -4,8 +4,7 @@ import logging import signal import threading -from pykka import ActorDeadError -from pykka.registry import ActorRegistry +import pykka from mopidy.compat import thread @@ -31,14 +30,14 @@ def exit_handler(signum, frame): def stop_actors_by_class(klass): - actors = ActorRegistry.get_by_class(klass) + actors = pykka.ActorRegistry.get_by_class(klass) logger.debug('Stopping %d instance(s) of %s', len(actors), klass.__name__) for actor in actors: actor.stop() def stop_remaining_actors(): - num_actors = len(ActorRegistry.get_all()) + num_actors = len(pykka.ActorRegistry.get_all()) while num_actors: logger.error( 'There are actor threads still running, this is probably a bug') @@ -47,8 +46,8 @@ def stop_remaining_actors(): num_actors, threading.active_count() - num_actors, ', '.join([t.name for t in threading.enumerate()])) logger.debug('Stopping %d actor(s)...', num_actors) - ActorRegistry.stop_all() - num_actors = len(ActorRegistry.get_all()) + pykka.ActorRegistry.stop_all() + num_actors = len(pykka.ActorRegistry.get_all()) logger.debug('All actors stopped.') @@ -67,7 +66,7 @@ class BaseThread(threading.Thread): logger.info('Interrupted by user') except ImportError as e: logger.error(e) - except ActorDeadError as e: + except pykka.ActorDeadError as e: logger.warning(e) except Exception as e: logger.exception(e) From 4d0bc755a0645f089bb15c6d9e888ff9ce07ebbb Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 13:03:19 +0100 Subject: [PATCH 04/10] docs: Fix typo --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 53b98672..9d695afd 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -92,7 +92,7 @@ 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 +- Idle events are now emitted on ``seeked`` events. This fix means that clients relying on ``idle`` events now get notified about seeks. (Fixes: :issue:`1331` :issue:`1347`) From 1f4f0ab03bd92d779cd84473f3b2d411858df888 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 22:00:10 +0100 Subject: [PATCH 05/10] tests: Prefix some test classes with 'Test' We don't want to rely on them subclassing unittest.TestCase. --- tests/core/test_playback.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index bef06510..392891ed 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -621,12 +621,15 @@ class EventEmissionTest(BaseTest): listener_mock.send.mock_calls) -class UnplayableURITest(BaseTest): +class TestUnplayableURI(BaseTest): + + tracks = [ + Track(uri='unplayable://'), + ] def setUp(self): # noqa: N802 - super(UnplayableURITest, self).setUp() - self.core.tracklist.clear() - tl_tracks = self.core.tracklist.add([Track(uri='unplayable://')]) + super(TestUnplayableURI, self).setUp() + tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback._set_current_tl_track(tl_tracks[0]) def test_pause_changes_state_even_if_track_is_unplayable(self): @@ -768,7 +771,7 @@ class TestStream(BaseTest): self.assertEqual(self.playback.get_stream_title(), None) -class BackendSelectionTest(unittest.TestCase): +class TestBackendSelection(unittest.TestCase): def setUp(self): # noqa: N802 config = { @@ -917,7 +920,7 @@ class BackendSelectionTest(unittest.TestCase): self.playback2.get_time_position.assert_called_once_with() -class CorePlaybackWithOldBackendTest(unittest.TestCase): +class TestCorePlaybackWithOldBackend(unittest.TestCase): def test_type_error_from_old_backend_does_not_crash_core(self): config = { @@ -940,7 +943,7 @@ class CorePlaybackWithOldBackendTest(unittest.TestCase): b.playback.play.assert_called_once_with() -class Bug1177RegressionTest(unittest.TestCase): +class TestBug1177Regression(unittest.TestCase): def test(self): config = { 'core': { From cd83084804fe9b3787f8062ea36a56c6609e4a77 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 22:01:33 +0100 Subject: [PATCH 06/10] tests: Merge TestPlayUnknownHandling into TestUnplayableURI --- tests/core/test_playback.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index 392891ed..4f20830e 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -231,21 +231,6 @@ class TestPreviousHandling(BaseTest): self.assertIn(tl_tracks[1], self.core.tracklist.tl_tracks) -class TestPlayUnknownHandling(BaseTest): - - tracks = [Track(uri='unknown:a', length=1234), - Track(uri='dummy:b', length=1234)] - - # TODO: move to UnplayableTest? - def test_play_skips_to_next_on_track_without_playback_backend(self): - self.core.playback.play() - - self.replay_events() - - current_track = self.core.playback.get_current_track() - self.assertEqual(current_track, self.tracks[1]) - - class OnAboutToFinishTest(BaseTest): def test_on_about_to_finish_keeps_finished_track_in_tracklist(self): @@ -625,6 +610,7 @@ class TestUnplayableURI(BaseTest): tracks = [ Track(uri='unplayable://'), + Track(uri='dummy:b'), ] def setUp(self): # noqa: N802 @@ -632,6 +618,14 @@ class TestUnplayableURI(BaseTest): tl_tracks = self.core.tracklist.get_tl_tracks() self.core.playback._set_current_tl_track(tl_tracks[0]) + def test_play_skips_to_next_if_track_is_unplayable(self): + self.core.playback.play() + + self.replay_events() + + current_track = self.core.playback.get_current_track() + self.assertEqual(current_track, self.tracks[1]) + def test_pause_changes_state_even_if_track_is_unplayable(self): self.core.playback.pause() self.assertEqual(self.core.playback.state, core.PlaybackState.PAUSED) From e67e4c2c6ea7b5d00c1e6538f57d97183f626957 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 7 Feb 2016 22:06:48 +0100 Subject: [PATCH 07/10] core: Avoid use of deprecated property --- mopidy/core/tracklist.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 02508c97..2a4ec8b6 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -626,7 +626,7 @@ class TracklistController(object): def _mark_played(self, tl_track): """Internal method for :class:`mopidy.core.PlaybackController`.""" - if self.consume and tl_track is not None: + if self.get_consume() and tl_track is not None: self.remove({'tlid': [tl_track.tlid]}) return True return False From 6cbfe86677b108181deb756ecf8276fe9521f691 Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 8 Feb 2016 00:21:33 +0100 Subject: [PATCH 08/10] gst1: Send in an argument to Gst.init As of gst-python 1.5.2, the init call requires one argument. The argument is a list of the command line options. I don't think we need to send any. This relates to #1432. --- mopidy/internal/gi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/internal/gi.py b/mopidy/internal/gi.py index 1407a657..fb9af0c9 100644 --- a/mopidy/internal/gi.py +++ b/mopidy/internal/gi.py @@ -22,7 +22,7 @@ except ImportError: """)) raise else: - Gst.is_initialized() or Gst.init() + Gst.is_initialized() or Gst.init([]) REQUIRED_GST_VERSION = (1, 2, 3) From fefb6aa5a2996329e3a9e3ef95d7e5d1bb29fb5c Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 8 Feb 2016 00:24:38 +0100 Subject: [PATCH 09/10] gst1: Don't check Gst.is_initialized before calling Gst.init As of gst-python 1.5.2, Gst.is_initialized throws a NotInitialized exception if run before Gst.init. Gst.init should be a noop if run again after the first call, so this should be safe. This fixes #1432. --- mopidy/internal/gi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/internal/gi.py b/mopidy/internal/gi.py index fb9af0c9..229277c3 100644 --- a/mopidy/internal/gi.py +++ b/mopidy/internal/gi.py @@ -22,7 +22,7 @@ except ImportError: """)) raise else: - Gst.is_initialized() or Gst.init([]) + Gst.init([]) REQUIRED_GST_VERSION = (1, 2, 3) From 17d96edd41a1d29f6beb2152723df50864fbad89 Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Mon, 8 Feb 2016 00:28:29 +0100 Subject: [PATCH 10/10] gst1: Import GstPbutils after calling Gst.init With gst-python 1.6.2, importing GstPbutils before calling Gst.init gives some warnings. --- mopidy/internal/gi.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mopidy/internal/gi.py b/mopidy/internal/gi.py index 229277c3..7fa51f09 100644 --- a/mopidy/internal/gi.py +++ b/mopidy/internal/gi.py @@ -7,8 +7,7 @@ import textwrap try: import gi gi.require_version('Gst', '1.0') - gi.require_version('GstPbutils', '1.0') - from gi.repository import GLib, GObject, Gst, GstPbutils + from gi.repository import GLib, GObject, Gst except ImportError: print(textwrap.dedent(""" ERROR: A GObject Python package was not found. @@ -23,6 +22,8 @@ except ImportError: raise else: Gst.init([]) + gi.require_version('GstPbutils', '1.0') + from gi.repository import GstPbutils REQUIRED_GST_VERSION = (1, 2, 3)