openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

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

Requested reviews:
  Tim Bentley (trb143)
Related bugs:
  Bug #1385979 in OpenLP: "Settings form -- list_item variable issue"
  https://bugs.launchpad.net/openlp/+bug/1385979

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

[bug 1385979] Check if the item is valid first

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1385979 (revision 2433)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/721/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/664/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/608/
[SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/548/
[SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/157/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/362/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/236/
-- 
https://code.launchpad.net/~raoul-snyman/openlp/bug-1385979/+merge/239908
Your team OpenLP Core is subscribed to branch lp:openlp.

=== modified file 'openlp/core/ui/settingsform.py'
--- openlp/core/ui/settingsform.py	2014-10-28 19:40:01 +0000
+++ openlp/core/ui/settingsform.py	2014-10-28 20:36:38 +0000
@@ -155,6 +155,9 @@
         """
         # Get the item we clicked on
         list_item = self.setting_list_widget.item(item_index)
+        # Quick exit to the left if the item doesn't exist (maybe -1?)
+        if not list_item:
+            return
         # Loop through the list of tabs in the stacked layout
         for tab_index in range(self.stacked_layout.count()):
             # Get the widget

=== modified file 'tests/functional/openlp_core_ui/test_settingsform.py'
--- tests/functional/openlp_core_ui/test_settingsform.py	2014-10-28 19:40:01 +0000
+++ tests/functional/openlp_core_ui/test_settingsform.py	2014-10-28 20:36:38 +0000
@@ -33,7 +33,6 @@
 from unittest import TestCase
 
 from openlp.core.common import Registry
-from openlp.core.ui.generaltab import GeneralTab
 from openlp.core.ui.settingsform import SettingsForm
 
 from tests.functional import MagicMock, patch
@@ -63,7 +62,7 @@
                 patch.object(settings_form.setting_list_widget, 'addItem') as mocked_add_item:
             settings_form.insert_tab(general_tab, is_visible=True)
 
-            # THEN: Stuff should happen
+            # THEN: The general tab should have been inserted into the stacked layout and an item inserted into the list
             mocked_add_widget.assert_called_with(general_tab)
             self.assertEqual(1, mocked_add_item.call_count, 'addItem should have been called')
 
@@ -81,7 +80,7 @@
                 patch.object(settings_form.setting_list_widget, 'addItem') as mocked_add_item:
             settings_form.insert_tab(general_tab, is_visible=False)
 
-            # THEN: Stuff should happen
+            # THEN: The general tab should have been inserted, but no list item should have been inserted into the list
             mocked_add_widget.assert_called_with(general_tab)
             self.assertEqual(0, mocked_add_item.call_count, 'addItem should not have been called')
 
@@ -112,3 +111,22 @@
         # THEN: The general tab's save() method should have been called, but not the themes tab
         mocked_general_save.assert_called_with()
         self.assertEqual(0, mocked_theme_save.call_count, 'The Themes tab\'s save() should not have been called')
+
+    def list_item_changed_invalid_item_test(self):
+        """
+        Test that the list_item_changed() slot handles a non-existent item
+        """
+        # GIVEN: A mocked tab inserted into a Settings Form
+        settings_form = SettingsForm(None)
+        general_tab = QtGui.QWidget(None)
+        general_tab.tab_title = 'mock'
+        general_tab.tab_title_visible = 'Mock'
+        general_tab.icon_path = ':/icon/openlp-logo-16x16.png'
+        settings_form.insert_tab(general_tab, is_visible=True)
+
+        with patch.object(settings_form.stacked_layout, 'count') as mocked_count:
+            # WHEN: The list_item_changed() slot is called with an invalid item index
+            settings_form.list_item_changed(100)
+
+            # THEN: The rest of the method should not have been called
+            self.assertEqual(0, mocked_count.call_count, 'The count method of the stacked layout should not be called')