openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/ubuntu-vlc-bug into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/ubuntu-vlc-bug/+merge/275935

[Media Players] Added better detection for VLC, with thanks to Tomas.

Note: I made a test, but this is untestable because of the hidden import inside the function.

lp:~raoul-snyman/openlp/ubuntu-vlc-bug (revision 2563)
[SUCCESS] https//ci.openlp.io/job/Branch-01-Pull/1162/
[SUCCESS] https//ci.openlp.io/job/Branch-02-Functional-Tests/1085/
[SUCCESS] https//ci.openlp.io/job/Branch-03-Interface-Tests/1026/
[SUCCESS] https//ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/873/
[SUCCESS] https//ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/469/
[SUCCESS] https//ci.openlp.io/job/Branch-05a-Code_Analysis/589/
[SUCCESS] https//ci.openlp.io/job/Branch-05b-Test_Coverage/460/
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/ubuntu-vlc-bug into lp:openlp.

=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2015-09-04 09:01:07 +0000
+++ openlp/core/ui/media/vlcplayer.py	2015-10-27 23:49:50 +0000
@@ -84,7 +84,9 @@
             pass
     if is_vlc_available:
         try:
+            vlc_instance = vlc.Instance()
             VERSION = vlc.libvlc_get_version().decode('UTF-8')
+            vlc_instance.release()
         except:
             VERSION = '0.0.0'
         # LooseVersion does not work when a string contains letter and digits (e. g. 2.0.5 Twoflower).
@@ -95,6 +97,9 @@
     if is_vlc_available:
         return vlc
     else:
+        # The vlc module may have been imported. Remove it if it has.
+        if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
+            del sys.modules['openlp.core.ui.media.vendor.vlc']
         return None
 
 

=== modified file 'tests/functional/openlp_core_ui_media/test_vlcplayer.py'
--- tests/functional/openlp_core_ui_media/test_vlcplayer.py	2015-05-25 20:58:05 +0000
+++ tests/functional/openlp_core_ui_media/test_vlcplayer.py	2015-10-27 23:49:50 +0000
@@ -25,7 +25,7 @@
 import os
 import sys
 from datetime import datetime, timedelta
-from unittest import TestCase
+from unittest import TestCase, skip
 
 from openlp.core.common import Registry
 from openlp.core.ui.media import MediaState, MediaType
@@ -50,6 +50,22 @@
             del sys.modules['openlp.core.ui.media.vendor.vlc']
         MockDateTime.revert()
 
+    @skip('No way to test this')
+    @patch('openlp.core.ui.media.vlcplayer.vlc')
+    def get_vlc_fails_and_removes_module_test(self, mocked_vlc):
+        """
+        Test that when the VLC import fails, it removes the module from sys.modules
+        """
+        # GIVEN: We're on OS X and we don't have the VLC plugin path set
+        mocked_vlc.Instance.side_effect = NameError
+        mocked_vlc.libvlc_get_version.return_value = b'0.0.0'
+
+        # WHEN: An checking if the player is available
+        get_vlc()
+
+        # THEN: The extra environment variable should be there
+        self.assertNotIn('openlp.core.ui.media.vendor.vlc', sys.modules)
+
     @patch('openlp.core.ui.media.vlcplayer.is_macosx')
     def fix_vlc_22_plugin_path_test(self, mocked_is_macosx):
         """
@@ -74,10 +90,6 @@
         """
         # GIVEN: We're not on OS X and we don't have the VLC plugin path set
         mocked_is_macosx.return_value = False
-        if 'VLC_PLUGIN_PATH' in os.environ:
-            del os.environ['VLC_PLUGIN_PATH']
-        if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
-            del sys.modules['openlp.core.ui.media.vendor.vlc']
 
         # WHEN: An checking if the player is available
         get_vlc()