From f241aa663de7b85c3f7418ded060defd44f760c9 Mon Sep 17 00:00:00 2001 From: Jonathan Springer Date: Mon, 7 Dec 2015 16:31:46 -0500 Subject: [PATCH 1/4] Add test to reproduce traceback in mediacontroller --- .../test_mediacontroller.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/functional/openlp_core_ui_media/test_mediacontroller.py b/tests/functional/openlp_core_ui_media/test_mediacontroller.py index d78f9a06b..25132f05b 100644 --- a/tests/functional/openlp_core_ui_media/test_mediacontroller.py +++ b/tests/functional/openlp_core_ui_media/test_mediacontroller.py @@ -120,6 +120,36 @@ class TestMediaController(TestCase, TestMixin): # THEN: it should return False self.assertFalse(ret, '_check_file_type should return False when the processor for service_item is None.') + @patch('openlp.core.ui.media.mediacontroller.get_media_players') + @patch('openlp.core.ui.media.mediacontroller.UiStrings') + def check_file_type_automatic_processor_test(self, mocked_uistrings, mocked_get_media_players): + """ + Test that we can play media when players are available and we have a automatic processor from the service item + """ + # GIVEN: A mocked UiStrings, get_media_players, controller, display and service_item + mocked_get_media_players.return_value = (['vlc', 'webkit'], '') + mocked_ret_uistrings = MagicMock() + mocked_ret_uistrings.Automatic = 1 + mocked_uistrings.return_value = mocked_ret_uistrings + media_controller = MediaController() + mocked_vlc = MagicMock() + mocked_vlc.video_extensions_list = ['*.mp4'] + media_controller.media_players = {'vlc': mocked_vlc, 'webkit': MagicMock()} + mocked_controller = MagicMock() + mocked_suffix = MagicMock() + mocked_suffix.return_value = 'mp4' + mocked_controller.media_info.file_info.suffix = mocked_suffix + mocked_display = MagicMock() + mocked_service_item = MagicMock() + mocked_service_item.processor = 1 + + # WHEN: calling _check_file_type when the processor for the service item is None + ret = media_controller._check_file_type(mocked_controller, mocked_display, mocked_service_item) + + # THEN: it should return False + self.assertTrue(ret, '_check_file_type should return True when mediaplayers are available and ' + 'the service item has an automatic processor.') + def media_play_msg_test(self): """ Test that the media controller responds to the request to play a loaded video From ae4858e4347109e68bad91d088ade64fa1ecb241 Mon Sep 17 00:00:00 2001 From: Jonathan Springer Date: Mon, 7 Dec 2015 16:34:10 -0500 Subject: [PATCH 2/4] Fix traceback in mediacontroller if a service item had a processor type of automatic --- openlp/core/ui/media/mediacontroller.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index 6d35f2478..2679e836e 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/openlp/core/ui/media/mediacontroller.py @@ -514,7 +514,10 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties): :param display: Which display to use :param service_item: The ServiceItem containing the details to be played. """ - used_players = get_media_players() + used_players = get_media_players()[0] + # If no player, we can't play + if not used_players: + return False default_player = used_players[0] if service_item.processor and service_item.processor != UiStrings().Automatic: # check to see if the player is usable else use the default one. @@ -522,9 +525,6 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties): used_players = default_player else: used_players = [service_item.processor.lower()] - # If no player, we can't play - if not used_players: - return False if controller.media_info.file_info.isFile(): suffix = '*.%s' % controller.media_info.file_info.suffix().lower() for title in used_players: From 3108adff17c2a0213830a997d0d1468cbd852331 Mon Sep 17 00:00:00 2001 From: Jonathan Springer Date: Thu, 10 Dec 2015 12:25:34 -0500 Subject: [PATCH 3/4] Add test for failing situation --- .../test_mediacontroller.py | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_core_ui_media/test_mediacontroller.py b/tests/functional/openlp_core_ui_media/test_mediacontroller.py index 25132f05b..c207d1d99 100644 --- a/tests/functional/openlp_core_ui_media/test_mediacontroller.py +++ b/tests/functional/openlp_core_ui_media/test_mediacontroller.py @@ -146,10 +146,40 @@ class TestMediaController(TestCase, TestMixin): # WHEN: calling _check_file_type when the processor for the service item is None ret = media_controller._check_file_type(mocked_controller, mocked_display, mocked_service_item) - # THEN: it should return False + # THEN: it should return True self.assertTrue(ret, '_check_file_type should return True when mediaplayers are available and ' 'the service item has an automatic processor.') + @patch('openlp.core.ui.media.mediacontroller.get_media_players') + @patch('openlp.core.ui.media.mediacontroller.UiStrings') + def check_file_type_processor_different_from_available_test(self, mocked_uistrings, mocked_get_media_players): + """ + Test that we can play media when players available are different from the processor from the service item + """ + # GIVEN: A mocked UiStrings, get_media_players, controller, display and service_item + mocked_get_media_players.return_value = (['phonon'], '') + mocked_ret_uistrings = MagicMock() + mocked_ret_uistrings.Automatic = 'automatic' + mocked_uistrings.return_value = mocked_ret_uistrings + media_controller = MediaController() + mocked_phonon = MagicMock() + mocked_phonon.video_extensions_list = ['*.mp4'] + media_controller.media_players = {'phonon': mocked_phonon} + mocked_controller = MagicMock() + mocked_suffix = MagicMock() + mocked_suffix.return_value = 'mp4' + mocked_controller.media_info.file_info.suffix = mocked_suffix + mocked_display = MagicMock() + mocked_service_item = MagicMock() + mocked_service_item.processor = 'vlc' + + # WHEN: calling _check_file_type when the processor for the service item is None + ret = media_controller._check_file_type(mocked_controller, mocked_display, mocked_service_item) + + # THEN: it should return True + self.assertTrue(ret, '_check_file_type should return True when the players available are different' + 'from the processor from the service item.') + def media_play_msg_test(self): """ Test that the media controller responds to the request to play a loaded video From fc3c433a21251081e1e13cdfcf02dc3edbfba127 Mon Sep 17 00:00:00 2001 From: Jonathan Springer Date: Thu, 10 Dec 2015 12:26:08 -0500 Subject: [PATCH 4/4] Fix failing situation by making default_players a list instead of a string --- openlp/core/ui/media/mediacontroller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index 2679e836e..1440c187f 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/openlp/core/ui/media/mediacontroller.py @@ -518,7 +518,7 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties): # If no player, we can't play if not used_players: return False - default_player = used_players[0] + default_player = [used_players[0]] if service_item.processor and service_item.processor != UiStrings().Automatic: # check to see if the player is usable else use the default one. if not service_item.processor.lower() in used_players: