diff --git a/mopidy/core/mixer.py b/mopidy/core/mixer.py index fde7ee5a..94188d50 100644 --- a/mopidy/core/mixer.py +++ b/mopidy/core/mixer.py @@ -1,10 +1,24 @@ from __future__ import absolute_import, unicode_literals +import contextlib import logging +from mopidy import exceptions from mopidy.utils import validation +@contextlib.contextmanager +def _mixer_error_handling(mixer): + try: + yield + except exceptions.ValidationError as e: + logger.error('%s mixer returned bad data: %s', + mixer.actor_ref.actor_class.__name__, e) + except Exception: + logger.exception('%s mixer caused an exception.', + mixer.actor_ref.actor_class.__name__) + + logger = logging.getLogger(__name__) @@ -21,8 +35,15 @@ class MixerController(object): The volume scale is linear. """ - if self._mixer is not None: - return self._mixer.get_volume().get() + if self._mixer is None: + return None + + with _mixer_error_handling(self._mixer): + volume = self._mixer.get_volume().get() + volume is None or validation.check_integer(volume, min=0, max=100) + return volume + + return None def set_volume(self, volume): """Set the volume. @@ -37,8 +58,12 @@ class MixerController(object): if self._mixer is None: return False - else: - return self._mixer.set_volume(volume).get() + + with _mixer_error_handling(self._mixer): + # TODO: log non-bool return values? + return bool(self._mixer.set_volume(volume).get()) + + return False def get_mute(self): """Get mute state. @@ -46,8 +71,15 @@ class MixerController(object): :class:`True` if muted, :class:`False` unmuted, :class:`None` if unknown. """ - if self._mixer is not None: - return self._mixer.get_mute().get() + if self._mixer is None: + return None + + with _mixer_error_handling(self._mixer): + mute = self._mixer.get_mute().get() + mute is None or validation.check_instance(mute, bool) + return mute + + return None def set_mute(self, mute): """Set mute state. @@ -59,5 +91,9 @@ class MixerController(object): validation.check_boolean(mute) if self._mixer is None: return False - else: - return self._mixer.set_mute(bool(mute)).get() + + with _mixer_error_handling(self._mixer): + # TODO: log non-bool return values? + return bool(self._mixer.set_mute(bool(mute)).get()) + + return None diff --git a/tests/core/test_mixer.py b/tests/core/test_mixer.py index c4ef7fe9..61e054d0 100644 --- a/tests/core/test_mixer.py +++ b/tests/core/test_mixer.py @@ -92,3 +92,51 @@ class CoreNoneMixerListenerTest(unittest.TestCase): def test_forwards_mixer_mute_changed_event_to_frontends(self, send): self.core.mixer.set_mute(mute=True) self.assertEqual(send.call_count, 0) + + +class CoreBadMixerTest(unittest.TestCase): + + def setUp(self): # noqa: N802 + self.mixer = mock.Mock() + self.mixer.actor_ref.actor_class.__name__ = 'DummyMixer' + self.core = core.Core(mixer=self.mixer, backends=[]) + + def test_get_volume_raises_exception(self): + self.mixer.get_volume.return_value.get.side_effect = Exception + self.assertEqual(self.core.mixer.get_volume(), None) + + def test_get_volume_returns_negative(self): + self.mixer.get_volume.return_value.get.return_value = -1 + self.assertEqual(self.core.mixer.get_volume(), None) + + def test_get_volume_returns_out_of_bound(self): + self.mixer.get_volume.return_value.get.return_value = 1000 + self.assertEqual(self.core.mixer.get_volume(), None) + + def test_get_volume_returns_wrong_type(self): + self.mixer.get_volume.return_value.get.return_value = '12' + self.assertEqual(self.core.mixer.get_volume(), None) + + def test_set_volume_exception(self): + self.mixer.set_volume.return_value.get.side_effect = Exception + self.assertFalse(self.core.mixer.set_volume(30)) + + def test_set_volume_non_bool_return_value(self): + self.mixer.set_volume.return_value.get.return_value = 'done' + self.assertIs(self.core.mixer.set_volume(30), True) + + def test_get_mute_raises_exception(self): + self.mixer.get_mute.return_value.get.side_effect = Exception + self.assertEqual(self.core.mixer.get_mute(), None) + + def test_get_mute_returns_wrong_type(self): + self.mixer.get_mute.return_value.get.return_value = '12' + self.assertEqual(self.core.mixer.get_mute(), None) + + def test_set_mute_exception(self): + self.mixer.set_mute.return_value.get.side_effect = Exception + self.assertFalse(self.core.mixer.set_mute(True)) + + def test_set_mute_non_bool_return_value(self): + self.mixer.set_mute.return_value.get.return_value = 'done' + self.assertIs(self.core.mixer.set_mute(True), True)