Added test + some cleaning

openlp/plugins/presentations/lib/powerpointcontroller.py

@@ -39,9 +39,8 @@ if is_win():
     import pywintypes
 
 from openlp.core.lib import ScreenList
-from openlp.core.common import Registry
 from openlp.core.lib.ui import UiStrings, critical_error_message_box, translate
-from openlp.core.common import trace_error_handler
+from openlp.core.common import trace_error_handler, Registry
 from .presentationcontroller import PresentationController, PresentationDocument
 
 log = logging.getLogger(__name__)
@@ -375,15 +374,6 @@ class PowerpointDocument(PresentationDocument):
         """
         log.debug('get_slide_count')
         return self.slide_count
-        ret = 0
-        try:
-            ret = self.presentation.Slides.Count
-        except (AttributeError, pywintypes.com_error) as e:
-            log.exception('Caught exception while in get_slide_count')
-            log.exception(e)
-            trace_error_handler(log)
-            self.show_error_msg()
-        return ret
 
     def goto_slide(self, slide_no):
         """
@@ -393,10 +383,12 @@ class PowerpointDocument(PresentationDocument):
         """
         log.debug('goto_slide')
         try:
-            if Settings().value('presentations/powerpoint slide click advance') and self.get_slide_number() == self.index_map[slide_no]:
+            if Settings().value('presentations/powerpoint slide click advance') \
+                    and self.get_slide_number() == self.index_map[slide_no]:
                 click_index = self.presentation.SlideShowWindow.View.GetClickIndex()
                 click_count = self.presentation.SlideShowWindow.View.GetClickCount()
-                log.debug('We are already on this slide - go to next effect if any left, idx: %d, count: %d' % (click_index, click_count))
+                log.debug('We are already on this slide - go to next effect if any left, idx: %d, count: %d'
+                          % (click_index, click_count))
                 if click_index < click_count:
                     self.next_step()
             else:

openlp/plugins/presentations/lib/presentationtab.py

@@ -145,7 +145,8 @@ class PresentationTab(SettingsTab):
                 powerpoint_available = True
         self.override_app_check_box.setChecked(Settings().value(self.settings_section + '/override app'))
         # Load Powerpoint settings
-        self.ppt_slide_click_check_box.setChecked(Settings().value(self.settings_section + '/powerpoint slide click advance'))
+        self.ppt_slide_click_check_box.setChecked(Settings().value(self.settings_section +
+                                                                   '/powerpoint slide click advance'))
         self.ppt_slide_click_check_box.setEnabled(powerpoint_available)
         # load pdf-program settings
         enable_pdf_program = Settings().value(self.settings_section + '/enable_pdf_program')

tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py

@@ -33,11 +33,15 @@ from tests.utils.constants import TEST_RESOURCES_PATH
 
 from openlp.plugins.presentations.lib.powerpointcontroller import PowerpointController, PowerpointDocument,\
     _get_text_from_shapes
-from openlp.core.common import is_win
+from openlp.core.common import is_win, Settings
 
 if is_win():
     import pywintypes
 
+__default_settings__ = {
+    'presentations/powerpoint slide click advance': True
+}
+
 
 class TestPowerpointController(TestCase, TestMixin):
     """
@@ -104,6 +108,7 @@ class TestPowerpointDocument(TestCase, TestMixin):
         self.mock_presentation_document_get_temp_folder.return_value = 'temp folder'
         self.file_name = os.path.join(TEST_RESOURCES_PATH, 'presentations', 'test.pptx')
         self.real_controller = PowerpointController(self.mock_plugin)
+        Settings().extend_default_settings(__default_settings__)
 
     def tearDown(self):
         """
@@ -228,3 +233,23 @@ class TestPowerpointDocument(TestCase, TestMixin):
 
         # THEN: it should not fail but return empty string
         self.assertEqual(result, '', 'result should be empty')
+
+    def goto_slide_test(self):
+        """
+        Test that goto_slide goes to next effect if the slide is already displayed
+        """
+        # GIVEN: A Document with mocked controller, presentation, and mocked functions get_slide_number and next_step
+        doc = PowerpointDocument(self.mock_controller, self.mock_presentation)
+        doc.presentation = MagicMock()
+        doc.presentation.SlideShowWindow.View.GetClickIndex.return_value = 1
+        doc.presentation.SlideShowWindow.View.GetClickCount.return_value = 2
+        doc.get_slide_number = MagicMock()
+        doc.get_slide_number.return_value = 1
+        doc.next_step = MagicMock()
+        doc.index_map[1] = 1
+
+        # WHEN: Calling goto_slide
+        doc.goto_slide(1)
+
+        # THEN: next_step() should be call to try to advance to the next effect.
+        self.assertTrue(doc.next_step.called, 'next_step() should have been called!')