Merge lp:~raoul-snyman/openlp/bug-1386896 into lp:openlp

Proposed by Raoul Snyman
Status: Merged
Merged at revision: 2435
Proposed branch: lp:~raoul-snyman/openlp/bug-1386896
Merge into: lp:openlp
Diff against target: 94 lines (+47/-3)
3 files modified
openlp/core/ui/media/vlcplayer.py (+2/-1)
openlp/core/ui/settingsform.py (+17/-2)
tests/functional/openlp_core_ui/test_settingsform.py (+28/-0)
To merge this branch: bzr merge lp:~raoul-snyman/openlp/bug-1386896
Reviewer Review Type Date Requested Status
Tim Bentley Approve
Review via email: mp+240187@code.launchpad.net

Description of the change

Two fixes:
  - Fix bug #1386896 by catching the OSError on Mac OS X and just ignoring it
  - Found a potential bug where the settings form was canceled, with one or more inactive plugins
  - Wrote a test for the above settings form bug

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1386896 (revision 2435)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/725/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/668/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/612/
[SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/552/
[SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/161/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/366/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/240/

To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openlp/core/ui/media/vlcplayer.py'
2--- openlp/core/ui/media/vlcplayer.py 2014-09-02 20:17:56 +0000
3+++ openlp/core/ui/media/vlcplayer.py 2014-10-30 23:19:46 +0000
4@@ -33,7 +33,6 @@
5 from distutils.version import LooseVersion
6 import logging
7 import os
8-import sys
9 import threading
10
11 from PyQt4 import QtGui
12@@ -55,6 +54,8 @@
13 if is_win():
14 if not isinstance(e, WindowsError) and e.winerror != 126:
15 raise
16+ elif is_macosx():
17+ pass
18 else:
19 raise
20
21
22=== modified file 'openlp/core/ui/settingsform.py'
23--- openlp/core/ui/settingsform.py 2014-10-28 20:48:20 +0000
24+++ openlp/core/ui/settingsform.py 2014-10-30 23:19:46 +0000
25@@ -57,6 +57,11 @@
26 self.processes = []
27 self.setupUi(self)
28 self.setting_list_widget.currentRowChanged.connect(self.list_item_changed)
29+ self.general_tab = None
30+ self.themes_tab = None
31+ self.projector_tab = None
32+ self.advanced_tab = None
33+ self.player_tab = None
34
35 def exec_(self):
36 """
37@@ -125,8 +130,18 @@
38 Process the form saving the settings
39 """
40 self.processes = []
41- for tabIndex in range(self.stacked_layout.count()):
42- self.stacked_layout.widget(tabIndex).cancel()
43+ # Same as accept(), we need to loop over the visible tabs, and skip the inactive ones
44+ for item_index in range(self.setting_list_widget.count()):
45+ # Get the list item
46+ list_item = self.setting_list_widget.item(item_index)
47+ if not list_item:
48+ continue
49+ # Now figure out if there's a tab for it, and save the tab.
50+ plugin_name = list_item.data(QtCore.Qt.UserRole)
51+ for tab_index in range(self.stacked_layout.count()):
52+ tab_widget = self.stacked_layout.widget(tab_index)
53+ if tab_widget.tab_title == plugin_name:
54+ tab_widget.cancel()
55 return QtGui.QDialog.reject(self)
56
57 def bootstrap_post_set_up(self):
58
59=== modified file 'tests/functional/openlp_core_ui/test_settingsform.py'
60--- tests/functional/openlp_core_ui/test_settingsform.py 2014-10-28 20:27:09 +0000
61+++ tests/functional/openlp_core_ui/test_settingsform.py 2014-10-30 23:19:46 +0000
62@@ -130,3 +130,31 @@
63
64 # THEN: The rest of the method should not have been called
65 self.assertEqual(0, mocked_count.call_count, 'The count method of the stacked layout should not be called')
66+
67+ def reject_with_inactive_items_test(self):
68+ """
69+ Test that the reject() method works correctly when some of the plugins are inactive
70+ """
71+ # GIVEN: A visible general tab and an invisible theme tab in a Settings Form
72+ settings_form = SettingsForm(None)
73+ general_tab = QtGui.QWidget(None)
74+ general_tab.tab_title = 'mock-general'
75+ general_tab.tab_title_visible = 'Mock General'
76+ general_tab.icon_path = ':/icon/openlp-logo-16x16.png'
77+ mocked_general_cancel = MagicMock()
78+ general_tab.cancel = mocked_general_cancel
79+ settings_form.insert_tab(general_tab, is_visible=True)
80+ themes_tab = QtGui.QWidget(None)
81+ themes_tab.tab_title = 'mock-themes'
82+ themes_tab.tab_title_visible = 'Mock Themes'
83+ themes_tab.icon_path = ':/icon/openlp-logo-16x16.png'
84+ mocked_theme_cancel = MagicMock()
85+ themes_tab.cancel = mocked_theme_cancel
86+ settings_form.insert_tab(themes_tab, is_visible=False)
87+
88+ # WHEN: The reject() method is called
89+ settings_form.reject()
90+
91+ # THEN: The general tab's cancel() method should have been called, but not the themes tab
92+ mocked_general_cancel.assert_called_with()
93+ self.assertEqual(0, mocked_theme_cancel.call_count, 'The Themes tab\'s cancel() should not have been called')
94\ No newline at end of file