Merge lp:~raoul-snyman/openlp/ubuntu-vlc-bug into lp:openlp/2.2

Proposed by Raoul Snyman
Status: Merged
Merged at revision: 2566
Proposed branch: lp:~raoul-snyman/openlp/ubuntu-vlc-bug
Merge into: lp:openlp/2.2
Diff against target: 70 lines (+22/-5)
2 files modified
openlp/core/ui/media/vlcplayer.py (+5/-0)
tests/functional/openlp_core_ui_media/test_vlcplayer.py (+17/-5)
To merge this branch: bzr merge lp:~raoul-snyman/openlp/ubuntu-vlc-bug
Reviewer Review Type Date Requested Status
Tomas Groth Approve
Review via email: mp+275936@code.launchpad.net

This proposal supersedes a proposal from 2015-10-27.

Commit message

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

Description of the change

[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/

To post a comment you must log in.
Revision history for this message
Tomas Groth (tomasgroth) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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:51:18 +0000
@@ -84,7 +84,9 @@
84 pass84 pass
85 if is_vlc_available:85 if is_vlc_available:
86 try:86 try:
87 vlc_instance = vlc.Instance()
87 VERSION = vlc.libvlc_get_version().decode('UTF-8')88 VERSION = vlc.libvlc_get_version().decode('UTF-8')
89 vlc_instance.release()
88 except:90 except:
89 VERSION = '0.0.0'91 VERSION = '0.0.0'
90 # LooseVersion does not work when a string contains letter and digits (e. g. 2.0.5 Twoflower).92 # LooseVersion does not work when a string contains letter and digits (e. g. 2.0.5 Twoflower).
@@ -95,6 +97,9 @@
95 if is_vlc_available:97 if is_vlc_available:
96 return vlc98 return vlc
97 else:99 else:
100 # The vlc module may have been imported. Remove it if it has.
101 if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
102 del sys.modules['openlp.core.ui.media.vendor.vlc']
98 return None103 return None
99104
100105
101106
=== 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:51:18 +0000
@@ -25,7 +25,7 @@
25import os25import os
26import sys26import sys
27from datetime import datetime, timedelta27from datetime import datetime, timedelta
28from unittest import TestCase28from unittest import TestCase, skip
2929
30from openlp.core.common import Registry30from openlp.core.common import Registry
31from openlp.core.ui.media import MediaState, MediaType31from openlp.core.ui.media import MediaState, MediaType
@@ -50,6 +50,22 @@
50 del sys.modules['openlp.core.ui.media.vendor.vlc']50 del sys.modules['openlp.core.ui.media.vendor.vlc']
51 MockDateTime.revert()51 MockDateTime.revert()
5252
53 @skip('No way to test this')
54 @patch('openlp.core.ui.media.vlcplayer.vlc')
55 def get_vlc_fails_and_removes_module_test(self, mocked_vlc):
56 """
57 Test that when the VLC import fails, it removes the module from sys.modules
58 """
59 # GIVEN: We're on OS X and we don't have the VLC plugin path set
60 mocked_vlc.Instance.side_effect = NameError
61 mocked_vlc.libvlc_get_version.return_value = b'0.0.0'
62
63 # WHEN: An checking if the player is available
64 get_vlc()
65
66 # THEN: The extra environment variable should be there
67 self.assertNotIn('openlp.core.ui.media.vendor.vlc', sys.modules)
68
53 @patch('openlp.core.ui.media.vlcplayer.is_macosx')69 @patch('openlp.core.ui.media.vlcplayer.is_macosx')
54 def fix_vlc_22_plugin_path_test(self, mocked_is_macosx):70 def fix_vlc_22_plugin_path_test(self, mocked_is_macosx):
55 """71 """
@@ -74,10 +90,6 @@
74 """90 """
75 # GIVEN: We're not on OS X and we don't have the VLC plugin path set91 # GIVEN: We're not on OS X and we don't have the VLC plugin path set
76 mocked_is_macosx.return_value = False92 mocked_is_macosx.return_value = False
77 if 'VLC_PLUGIN_PATH' in os.environ:
78 del os.environ['VLC_PLUGIN_PATH']
79 if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
80 del sys.modules['openlp.core.ui.media.vendor.vlc']
8193
82 # WHEN: An checking if the player is available94 # WHEN: An checking if the player is available
83 get_vlc()95 get_vlc()

Subscribers

People subscribed via source and target branches

to all changes: