From b993b0d4a8bf8f73a7135d6f08a6306a93237e82 Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Mon, 30 Dec 2013 03:35:05 -0500 Subject: [PATCH] Style changes --- openlp/core/lib/serviceitem.py | 9 +- .../presentations/lib/impresscontroller.py | 4 +- .../presentations/lib/powerpointcontroller.py | 6 +- .../presentations/lib/pptviewcontroller.py | 19 ++-- .../lib/presentationcontroller.py | 6 +- openlp/plugins/remotes/lib/httprouter.py | 5 +- .../openlp_core_lib/test_image_manager.py | 11 +-- .../openlp_core_lib/test_serviceitem.py | 25 +++--- .../presentations/test_impresscontroller.py | 48 +++++++---- .../test_powerpointcontroller.py | 28 ++++-- .../test_powerpointviewercontroller.py | 25 +++--- .../test_presentationcontroller.py | 86 ++++++++----------- .../openlp_plugins/remotes/test_router.py | 66 ++++++-------- 13 files changed, 165 insertions(+), 173 deletions(-) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 3db9789eb..87130e09a 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -332,7 +332,7 @@ class ServiceItem(object): """ self.service_item_type = ServiceItemType.Command self._raw_frames.append({'title': file_name, 'image': image, 'path': path, - 'display_title': display_title, 'notes': notes}) + 'display_title': display_title, 'notes': notes}) self._new_item() def get_service_repr(self, lite_save): @@ -377,9 +377,8 @@ class ServiceItem(object): service_data = [slide['title'] for slide in self._raw_frames] elif self.service_item_type == ServiceItemType.Command: for slide in self._raw_frames: - service_data.append({'title': slide['title'], - 'image': slide['image'], 'path': slide['path'], - 'display_title': slide['display_title'], 'notes': slide['notes']}) + service_data.append({'title': slide['title'], 'image': slide['image'], 'path': slide['path'], + 'display_title': slide['display_title'], 'notes': slide['notes']}) return {'header': service_header, 'data': service_data} def set_from_service(self, serviceitem, path=None): @@ -452,7 +451,7 @@ class ServiceItem(object): if path: self.has_original_files = False self.add_from_command(path, text_image['title'], text_image['image'], - text_image.get('display_title',''), text_image.get('notes', '')) + text_image.get('display_title',''), text_image.get('notes', '')) else: self.add_from_command(text_image['path'], text_image['title'], text_image['image']) self._new_item() diff --git a/openlp/plugins/presentations/lib/impresscontroller.py b/openlp/plugins/presentations/lib/impresscontroller.py index 7480d3b1b..d288a646d 100644 --- a/openlp/plugins/presentations/lib/impresscontroller.py +++ b/openlp/plugins/presentations/lib/impresscontroller.py @@ -486,9 +486,7 @@ class ImpressDocument(PresentationDocument): notes = [] pages = self.document.getDrawPages() for slide_no in range(1, pages.getCount() + 1): - titles.append( - self.__get_text_from_page(slide_no, TextType.Title). - replace('\n', ' ') + '\n') + titles.append(self.__get_text_from_page(slide_no, TextType.Title).replace('\n', ' ') + '\n') note = self.__get_text_from_page(slide_no, TextType.Notes) if len(note) == 0: note = ' ' diff --git a/openlp/plugins/presentations/lib/powerpointcontroller.py b/openlp/plugins/presentations/lib/powerpointcontroller.py index e039f49ef..48af2edcb 100644 --- a/openlp/plugins/presentations/lib/powerpointcontroller.py +++ b/openlp/plugins/presentations/lib/powerpointcontroller.py @@ -352,9 +352,9 @@ def _get_text_from_shapes(shapes): """ text = '' for shape in shapes: - if shape.PlaceholderFormat.Type == constants.ppPlaceholderBody and \ - shape.HasTextFrame and shape.TextFrame.HasText: - text += shape.TextFrame.TextRange.Text + '\n' + if shape.PlaceholderFormat.Type == constants.ppPlaceholderBody: + if shape.HasTextFrame and shape.TextFrame.HasText: + text += shape.TextFrame.TextRange.Text + '\n' return text if os.name == "nt": diff --git a/openlp/plugins/presentations/lib/pptviewcontroller.py b/openlp/plugins/presentations/lib/pptviewcontroller.py index a8b9c3dae..f09d51b17 100644 --- a/openlp/plugins/presentations/lib/pptviewcontroller.py +++ b/openlp/plugins/presentations/lib/pptviewcontroller.py @@ -162,16 +162,14 @@ class PptviewDocument(PresentationDocument): filename = os.path.normpath(self.filepath) # let's make sure we have a valid zipped presentation if os.path.exists(filename) and zipfile.is_zipfile(filename): - namespaces = {"p": - "http://schemas.openxmlformats.org/presentationml/2006/main", - "a": "http://schemas.openxmlformats.org/drawingml/2006/main"} + namespaces = {"p": "http://schemas.openxmlformats.org/presentationml/2006/main", + "a": "http://schemas.openxmlformats.org/drawingml/2006/main"} # open the file with zipfile.ZipFile(filename) as zip_file: # find the presentation.xml to get the slide count with zip_file.open('ppt/presentation.xml') as pres: tree = ElementTree.parse(pres) - nodes = tree.getroot().findall(".//p:sldIdLst/p:sldId", - namespaces=namespaces) + nodes = tree.getroot().findall(".//p:sldIdLst/p:sldId", namespaces=namespaces) print ("slide count: " + str(len(nodes))) # initialize the lists titles = ['' for i in range(len(nodes))] @@ -188,8 +186,7 @@ class PptviewDocument(PresentationDocument): node_type = 'ctrTitle' list_to_add = titles # or a note - match = re.search("notesSlides/notesSlide(.+)\.xml", - zip_info.filename) + match = re.search("notesSlides/notesSlide(.+)\.xml", zip_info.filename) if match: index = int(match.group(1))-1 node_type = 'body' @@ -199,9 +196,8 @@ class PptviewDocument(PresentationDocument): with zip_file.open(zip_info) as zipped_file: tree = ElementTree.parse(zipped_file) text = '' - nodes = tree.getroot().findall(".//p:ph[@type='" + - node_type + "']../../..//p:txBody//a:t", - namespaces=namespaces) + nodes = tree.getroot().findall(".//p:ph[@type='" + node_type + "']../../..//p:txBody//a:t", + namespaces=namespaces) # if we found any content if nodes and len(nodes)>0: for node in nodes: @@ -211,8 +207,7 @@ class PptviewDocument(PresentationDocument): # Let's remove the \n from the titles and # just add one at the end if node_type == 'ctrTitle': - text = text.replace('\n', ' '). \ - replace('\x0b', ' ') + '\n' + text = text.replace('\n', ' ').replace('\x0b', ' ') + '\n' list_to_add[index] = text # now let's write the files self.save_titles_and_notes(titles, notes) diff --git a/openlp/plugins/presentations/lib/presentationcontroller.py b/openlp/plugins/presentations/lib/presentationcontroller.py index a5e06a599..081e89ab3 100644 --- a/openlp/plugins/presentations/lib/presentationcontroller.py +++ b/openlp/plugins/presentations/lib/presentationcontroller.py @@ -306,8 +306,7 @@ class PresentationDocument(object): log.exception('Failed to open/read existing titles file') titles = [] for slide_no, title in enumerate(titles, 1): - notes_file = os.path.join(self.get_thumbnail_folder(), - 'slideNotes%d.txt' % slide_no) + notes_file = os.path.join(self.get_thumbnail_folder(), 'slideNotes%d.txt' % slide_no) note = '' if os.path.exists(notes_file): try: @@ -330,8 +329,7 @@ class PresentationDocument(object): fo.writelines(titles) if notes: for slide_no, note in enumerate(notes, 1): - notes_file = os.path.join(self.get_thumbnail_folder(), - 'slideNotes%d.txt' % slide_no) + notes_file = os.path.join(self.get_thumbnail_folder(), 'slideNotes%d.txt' % slide_no) with open(notes_file, mode='w') as fn: fn.write(note) diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index fa36f912e..e9b61bee2 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -394,8 +394,7 @@ class HttpRouter(object): """ Serve an image file. If not found return 404. """ - log.debug('serve thumbnail %s/thumbnails%s/%s' % (controller_name, - dimensions, file_name)) + log.debug('serve thumbnail %s/thumbnails%s/%s' % (controller_name, dimensions, file_name)) supported_controllers = ['presentations'] if not dimensions: dimensions = '' @@ -406,7 +405,7 @@ class HttpRouter(object): full_path = urllib.parse.unquote(file_name) if not '..' in full_path: # no hacking please full_path = os.path.normpath(os.path.join(AppLocation.get_section_data_path(controller_name), - 'thumbnails/' + full_path)) + 'thumbnails/' + full_path)) if os.path.exists(full_path): path, just_file_name = os.path.split(full_path) self.image_manager.add_image(full_path, just_file_name, None, dimensions) diff --git a/tests/functional/openlp_core_lib/test_image_manager.py b/tests/functional/openlp_core_lib/test_image_manager.py index 912a6c2a2..a4f42c9b1 100644 --- a/tests/functional/openlp_core_lib/test_image_manager.py +++ b/tests/functional/openlp_core_lib/test_image_manager.py @@ -94,8 +94,7 @@ class TestImageManager(TestCase): image = self.image_manager.get_image(full_path, 'church.jpg', '80x80') # THEN: The return should be of type image - self.assertEqual(isinstance(image, QtGui.QImage), True, - 'The returned object should be a QImage') + self.assertEqual(isinstance(image, QtGui.QImage), True, 'The returned object should be a QImage') #print(len(self.image_manager._cache)) # WHEN: adding the same image with different dimensions @@ -103,18 +102,16 @@ class TestImageManager(TestCase): # THEN: the cache should contain two pictures self.assertEqual(len(self.image_manager._cache), 2, - 'Image manager should consider two dimensions of the same picture as different') + 'Image manager should consider two dimensions of the same picture as different') # WHEN: adding the same image with first dimensions self.image_manager.add_image(full_path, 'church.jpg', None, '80x80') # THEN: the cache should still contain only two pictures - self.assertEqual(len(self.image_manager._cache), 2, - 'Same dimensions should not be added again') + self.assertEqual(len(self.image_manager._cache), 2, 'Same dimensions should not be added again') # WHEN: calling with correct image, but wrong dimensions with self.assertRaises(KeyError) as context: self.image_manager.get_image(full_path, 'church.jpg', '120x120') - self.assertNotEquals(context.exception, '', - 'KeyError exception should have been thrown for missing dimension') + self.assertNotEquals(context.exception, '', 'KeyError exception should have been thrown for missing dimension') diff --git a/tests/functional/openlp_core_lib/test_serviceitem.py b/tests/functional/openlp_core_lib/test_serviceitem.py index 7663091ca..d58b240d2 100644 --- a/tests/functional/openlp_core_lib/test_serviceitem.py +++ b/tests/functional/openlp_core_lib/test_serviceitem.py @@ -99,7 +99,7 @@ class TestServiceItem(TestCase): self.assertEqual(VERSE[:-1], service_item.get_frames()[0]['text'], 'The returned text matches the input, except the last line feed') self.assertEqual(VERSE.split('\n', 1)[0], service_item.get_rendered_frame(1), - 'The first line has been returned') + 'The first line has been returned') self.assertEqual('Slide 1', service_item.get_frame_title(0), '"Slide 1" has been returned as the title') self.assertEqual('Slide 2', service_item.get_frame_title(1), '"Slide 2" has been returned as the title') self.assertEqual('', service_item.get_frame_title(2), 'Blank has been returned as the title of slide 3') @@ -216,14 +216,15 @@ class TestServiceItem(TestCase): image = MagicMock() display_title = 'DisplayTitle' notes = 'Note1\nNote2\n' - frame = {'title': presentation_name, 'image': image, - 'path': TEST_PATH, 'display_title': display_title, - 'notes': notes } + frame = {'title': presentation_name, 'image': image, 'path': TEST_PATH, + 'display_title': display_title, 'notes': notes} + # WHEN: adding presentation to service_item service_item.add_from_command(TEST_PATH, presentation_name, image, display_title, notes) + # THEN: verify that it is setup as a Command and that the frame data matches - assert service_item.service_item_type == ServiceItemType.Command - assert service_item.get_frames()[0] == frame + self.assertEqual(service_item.service_item_type, ServiceItemType.Command, 'It should be a Command') + self.assertEqual(service_item.get_frames()[0], frame, 'Frames should match') def add_from_comamnd_without_display_title_and_notes_test(self): """ @@ -233,12 +234,12 @@ class TestServiceItem(TestCase): service_item = ServiceItem(None) image_name = 'test.img' image = MagicMock() - frame = {'title': image_name, 'image': image, - 'path': TEST_PATH, 'display_title': None, - 'notes': None} + frame = {'title': image_name, 'image': image, 'path': TEST_PATH, + 'display_title': None, 'notes': None} + # WHEN: adding image to service_item service_item.add_from_command(TEST_PATH, image_name, image) + # THEN: verify that it is setup as a Command and that the frame data matches - assert service_item.service_item_type == ServiceItemType.Command - print(service_item.get_frames()[0]) - assert service_item.get_frames()[0] == frame + self.assertEqual(service_item.service_item_type, ServiceItemType.Command, 'It should be a Command') + self.assertEqual(service_item.get_frames()[0], frame, 'Frames should match') diff --git a/tests/functional/openlp_plugins/presentations/test_impresscontroller.py b/tests/functional/openlp_plugins/presentations/test_impresscontroller.py index 489310629..5679c68d2 100644 --- a/tests/functional/openlp_plugins/presentations/test_impresscontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_impresscontroller.py @@ -35,8 +35,7 @@ from mock import MagicMock from openlp.plugins.presentations.lib.impresscontroller import \ ImpressController, ImpressDocument, TextType -TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', - '..', '..', 'resources')) +TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources')) class TestLibModule(TestCase): @@ -57,19 +56,23 @@ class TestLibModule(TestCase): self.doc.document = MagicMock() self.doc.document.getDrawPages.return_value = MagicMock() self.doc.document.getDrawPages().getCount.return_value = 0 + # WHEN reading the titles and notes self.doc.create_titles_and_notes() + # THEN save_titles_and_notes should have been called with empty arrays self.doc.save_titles_and_notes.assert_called_once_with([], []) + # GIVEN: reset mock and set it to 2 pages self.doc.save_titles_and_notes.reset_mock() self.doc.document.getDrawPages().getCount.return_value = 2 + # WHEN: a new call to create_titles_and_notes self.doc.create_titles_and_notes() + # THEN: save_titles_and_notes should have been called once with # two arrays of two elements - self.doc.save_titles_and_notes.assert_called_once_with( - ['\n', '\n'], [' ', ' ']) + self.doc.save_titles_and_notes.assert_called_once_with(['\n', '\n'], [' ', ' ']) def get_text_from_page_out_of_bound_test(self): """ @@ -78,21 +81,27 @@ class TestLibModule(TestCase): # GIVEN: mocked LibreOffice Document with one slide, # two notes and three texts self.doc.document = self._mock_a_LibreOffice_document(1, 2, 3) + # WHEN: __get_text_from_page is called with an index of 0x00 result = self.doc._ImpressDocument__get_text_from_page(0, TextType.Notes) + # THEN: the result should be an empty string self.assertEqual(result, '', 'Result should be an empty string') + # WHEN: regardless of the type of text, index 0x00 is out of bounds result = self.doc._ImpressDocument__get_text_from_page(0, TextType.Title) + # THEN: result should be an empty string self.assertEqual(result, '', 'Result should be an empty string') + # WHEN: when called with 2, it should also be out of bounds result = self.doc._ImpressDocument__get_text_from_page(2, TextType.SlideText) + # THEN: result should be an empty string ... and, getByIndex should # have never been called self.assertEqual(result, '', 'Result should be an empty string') - self.assertEqual(self.doc.document.getDrawPages().getByIndex.call_count, - 0, 'There should be no call to getByIndex') + self.assertEqual(self.doc.document.getDrawPages().getByIndex.call_count, 0, + 'There should be no call to getByIndex') def get_text_from_page_wrong_type_test(self): """ @@ -101,12 +110,14 @@ class TestLibModule(TestCase): # GIVEN: mocked LibreOffice Document with one slide, two notes and # three texts self.doc.document = self._mock_a_LibreOffice_document(1, 2, 3) + # WHEN: called with TextType 3 result = self.doc._ImpressDocument__get_text_from_page(1, 3) + # THEN: result should be an empty string self.assertEqual(result, '', 'Result should be and empty string') - self.assertEqual(self.doc.document.getDrawPages().getByIndex.call_count, - 0, 'There should be no call to getByIndex') + self.assertEqual(self.doc.document.getDrawPages().getByIndex.call_count, 0, + 'There should be no call to getByIndex') def get_text_from_page_valid_params_test(self): """ @@ -115,22 +126,24 @@ class TestLibModule(TestCase): # GIVEN: mocked LibreOffice Document with one slide, # two notes and three texts self.doc.document = self._mock_a_LibreOffice_document(1, 2, 3) + # WHEN: __get_text_from_page is called to get the Notes result = self.doc._ImpressDocument__get_text_from_page(1, TextType.Notes) + # THEN: result should be 'Note\nNote\n' - self.assertEqual(result, 'Note\nNote\n', - 'Result should be \'Note\\n\' times the count of notes in the page') + self.assertEqual(result, 'Note\nNote\n', 'Result should be \'Note\\n\' times the count of notes in the page') + # WHEN: get the Title result = self.doc._ImpressDocument__get_text_from_page(1, TextType.Title) + # THEN: result should be 'Title\n' - self.assertEqual(result, 'Title\n', - 'Result should be exactly \'Title\\n\'') + self.assertEqual(result, 'Title\n', 'Result should be exactly \'Title\\n\'') + # WHEN: get all text - result = self.doc._ImpressDocument__get_text_from_page(1, - TextType.SlideText) + result = self.doc._ImpressDocument__get_text_from_page(1, TextType.SlideText) + # THEN: result should be 'Title\nString\nString\n' - self.assertEqual(result, 'Title\nString\nString\n', - 'Result should be exactly \'Title\\nString\\nString\\n\'') + self.assertEqual(result, 'Title\nString\nString\n', 'Result should be exactly \'Title\\nString\\nString\\n\'') def _mock_a_LibreOffice_document(self, page_count, note_count, text_count): pages = MagicMock() @@ -155,8 +168,7 @@ class TestLibModule(TestCase): page_shape = MagicMock() page_shape.supportsService.return_value = True if args[1] == 0: - page_shape.getShapeType.return_value = \ - 'com.sun.star.presentation.TitleTextShape' + page_shape.getShapeType.return_value = 'com.sun.star.presentation.TitleTextShape' page_shape.getString.return_value = 'Title' else: page_shape.getString.return_value = 'String' diff --git a/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py b/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py index 673f7b9f8..faa7c043b 100644 --- a/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py @@ -35,8 +35,7 @@ from mock import MagicMock from openlp.plugins.presentations.lib.powerpointcontroller import \ PowerpointController, PowerpointDocument, _get_text_from_shapes -TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', - '..', 'resources')) +TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', '..', 'resources')) class TestLibModule(TestCase): @@ -55,6 +54,7 @@ class TestLibModule(TestCase): # GIVEN: A boolean value set to true # WHEN: We "convert" it to a bool is_installed = self.ppc.check_available() + # THEN: We should get back a True bool self.assertEqual(is_installed, True, 'The result should be True') @@ -65,10 +65,12 @@ class TestLibModule(TestCase): """ # GIVEN: the filename print(self.file_name) + # WHEN: loading the filename self.doc = PowerpointDocument(self.ppc, self.file_name) self.doc.load_presentation() result = self.doc.is_loaded() + # THEN: result should be true self.assertEqual(result, True, 'The result should be True') @@ -85,11 +87,12 @@ class TestLibModule(TestCase): pres = MagicMock() pres.Slides = [slide, slide] self.doc.presentation = pres + # WHEN reading the titles and notes self.doc.create_titles_and_notes() + # THEN the save should have been called exactly once with 2 titles and 2 notes - self.doc.save_titles_and_notes.assert_called_once_with( - ['SlideText\n', 'SlideText\n'], [' ', ' ']) + self.doc.save_titles_and_notes.assert_called_once_with(['SlideText\n', 'SlideText\n'], [' ', ' ']) def create_titles_and_notes_with_no_slides_test(self): """ @@ -102,8 +105,10 @@ class TestLibModule(TestCase): pres = MagicMock() pres.Slides = [] self.doc.presentation = pres + # WHEN reading the titles and notes self.doc.create_titles_and_notes() + # THEN the save should have been called exactly once with empty titles and notes self.doc.save_titles_and_notes.assert_called_once_with([], []) @@ -111,21 +116,28 @@ class TestLibModule(TestCase): """ Test getting text from powerpoint shapes """ - # GIVEN: mocked + # GIVEN: mocked shapes shape = MagicMock() shape.PlaceholderFormat.Type = 2 shape.HasTextFrame = shape.TextFrame.HasText = True shape.TextFrame.TextRange.Text = 'slideText' shapes = [shape, shape] + + # WHEN: getting the text result = _get_text_from_shapes(shapes) - self.assertEqual(result, 'slideText\nslideText\n', - 'result should match \'slideText\nslideText\n\'') + + # THEN: it should return the text + self.assertEqual(result, 'slideText\nslideText\n','result should match \'slideText\nslideText\n\'') def get_text_from_shapes_with_no_shapes_test(self): """ Test getting text from powerpoint shapes with no shapes """ - # GIVEN: mocked + # GIVEN: empty shapes array shapes = [] + + # WHEN: getting the text result = _get_text_from_shapes(shapes) + + # THEN: it should not fail but return empty string self.assertEqual(result, '', 'result should be empty') diff --git a/tests/functional/openlp_plugins/presentations/test_powerpointviewercontroller.py b/tests/functional/openlp_plugins/presentations/test_powerpointviewercontroller.py index df20165c9..c538920bb 100644 --- a/tests/functional/openlp_plugins/presentations/test_powerpointviewercontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_powerpointviewercontroller.py @@ -93,13 +93,14 @@ class TestLibModule(TestCase): """ # GIVEN: mocked PresentationController.save_titles_and_notes self.doc.save_titles_and_notes = MagicMock() + # WHEN reading the titles and notes self.doc.create_titles_and_notes() + # THEN save_titles_and_notes should have been called once with empty arrays - self.doc.save_titles_and_notes.assert_called_once_with( - ['Test 1\n', '\n', 'Test 2\n', 'Test 4\n', 'Test 3\n'], - ['Notes for slide 1', 'Inserted', 'Notes for slide 2', - 'Notes \nfor slide 4', 'Notes for slide 3']) + self.doc.save_titles_and_notes.assert_called_once_with(['Test 1\n', '\n', 'Test 2\n', 'Test 4\n', 'Test 3\n'], + ['Notes for slide 1', 'Inserted', 'Notes for slide 2', + 'Notes \nfor slide 4', 'Notes for slide 3']) def create_titles_and_notes_nonexistent_file_test(self): """ @@ -108,18 +109,20 @@ class TestLibModule(TestCase): # GIVEN: mocked PresentationController.save_titles_and_notes and an nonexistent file with patch('builtins.open') as mocked_open, \ patch('openlp.plugins.presentations.lib.pptviewcontroller.os.path.exists') as mocked_exists, \ - patch('openlp.plugins.presentations.lib.presentationcontroller.check_directory_exists') as mocked_dir_exists: + patch('openlp.plugins.presentations.lib.presentationcontroller.check_directory_exists') as \ + mocked_dir_exists: mocked_exists.return_value = False mocked_dir_exists.return_value = False self.doc = PptviewDocument(self.ppc, 'Idontexist.pptx') self.doc.save_titles_and_notes = MagicMock() + # WHEN: reading the titles and notes self.doc.create_titles_and_notes() + # THEN: self.doc.save_titles_and_notes.assert_called_once_with(None, None) mocked_exists.assert_any_call('Idontexist.pptx') - self.assertEqual(mocked_open.call_count, 0, - 'There should be no calls to open a file') + self.assertEqual(mocked_open.call_count, 0, 'There should be no calls to open a file') def create_titles_and_notes_invalid_file_test(self): """ @@ -132,13 +135,13 @@ class TestLibModule(TestCase): mocked_is_zf.return_value = False mocked_exists.return_value = True mocked_open.filesize = 10 - self.doc = PptviewDocument(self.ppc, - os.path.join(TEST_PATH, "test.ppt")) + self.doc = PptviewDocument(self.ppc, os.path.join(TEST_PATH, "test.ppt")) self.doc.save_titles_and_notes = MagicMock() + # WHEN: reading the titles and notes self.doc.create_titles_and_notes() + # THEN: self.doc.save_titles_and_notes.assert_called_once_with(None, None) - self.assertEqual(mocked_is_zf.call_count, 1, - 'is_zipfile should have been called once') + self.assertEqual(mocked_is_zf.call_count, 1, 'is_zipfile should have been called once') \ No newline at end of file diff --git a/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py index 06d79bff5..d7427d8f8 100644 --- a/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py @@ -35,6 +35,8 @@ import os from openlp.plugins.presentations.lib.presentationcontroller import PresentationController, PresentationDocument from tests.functional import MagicMock, patch, mock_open +FOLDER_TO_PATCH = 'openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder' + class TestPresentationController(TestCase): """ Test the PresentationController. @@ -65,23 +67,19 @@ class TestPresentationController(TestCase): """ # GIVEN: two lists of length==2 and a mocked open and get_thumbnail_folder mocked_open = mock_open() - with patch('builtins.open', mocked_open), \ - patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder') \ - as mocked_get_thumbnail_folder: + with patch('builtins.open', mocked_open), patch(FOLDER_TO_PATCH) as mocked_get_thumbnail_folder: titles = ['uno', 'dos'] notes = ['one', 'two'] + # WHEN: calling save_titles_and_notes mocked_get_thumbnail_folder.return_value = 'test' self.document.save_titles_and_notes(titles, notes) + # THEN: the last call to open should have been for slideNotes2.txt - mocked_open.assert_any_call( - os.path.join('test', 'titles.txt'), mode='w') - mocked_open.assert_any_call( - os.path.join('test', 'slideNotes1.txt'), mode='w') - mocked_open.assert_any_call( - os.path.join('test', 'slideNotes2.txt'), mode='w') - self.assertEqual(mocked_open.call_count, 3, - 'There should be exactly three files opened') + mocked_open.assert_any_call(os.path.join('test', 'titles.txt'), mode='w') + mocked_open.assert_any_call(os.path.join('test', 'slideNotes1.txt'), mode='w') + mocked_open.assert_any_call(os.path.join('test', 'slideNotes2.txt'), mode='w') + self.assertEqual(mocked_open.call_count, 3, 'There should be exactly three files opened') mocked_open().writelines.assert_called_once_with(['uno', 'dos']) mocked_open().write.assert_called_any('one') mocked_open().write.assert_called_any('two') @@ -91,17 +89,16 @@ class TestPresentationController(TestCase): Test PresentationDocument.save_titles_and_notes method with no data """ # GIVEN: None and an empty list and a mocked open and get_thumbnail_folder - with patch('builtins.open') as mocked_open, \ - patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder') \ - as mocked_get_thumbnail_folder: + with patch('builtins.open') as mocked_open, patch(FOLDER_TO_PATCH) as mocked_get_thumbnail_folder: titles = None notes = None + # WHEN: calling save_titles_and_notes mocked_get_thumbnail_folder.return_value = 'test' self.document.save_titles_and_notes(titles, notes) + # THEN: No file should have been created - self.assertEqual(mocked_open.call_count, 0, - 'No file should be created') + self.assertEqual(mocked_open.call_count, 0, 'No file should be created') def get_titles_and_notes_test(self): @@ -109,30 +106,26 @@ class TestPresentationController(TestCase): Test PresentationDocument.get_titles_and_notes method """ # GIVEN: A mocked open, get_thumbnail_folder and exists + with patch('builtins.open', mock_open(read_data='uno\ndos\n')) as mocked_open, \ - patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder') \ - as mocked_get_thumbnail_folder, \ + patch(FOLDER_TO_PATCH) as mocked_get_thumbnail_folder, \ patch('openlp.plugins.presentations.lib.presentationcontroller.os.path.exists') as mocked_exists: mocked_get_thumbnail_folder.return_value = 'test' mocked_exists.return_value = True + # WHEN: calling get_titles_and_notes result_titles, result_notes = self.document.get_titles_and_notes() + # THEN: it should return two items for the titles and two empty strings for the notes - self.assertIs(type(result_titles), list, - 'result_titles should be of type list') - self.assertEqual(len(result_titles), 2, - 'There should be two items in the titles') - self.assertIs(type(result_notes), list, - 'result_notes should be of type list') - self.assertEqual(len(result_notes), 2, - 'There should be two items in the notes') - self.assertEqual(mocked_open.call_count, 3, - 'Three files should be opened') + self.assertIs(type(result_titles), list, 'result_titles should be of type list') + self.assertEqual(len(result_titles), 2, 'There should be two items in the titles') + self.assertIs(type(result_notes), list, 'result_notes should be of type list') + self.assertEqual(len(result_notes), 2, 'There should be two items in the notes') + self.assertEqual(mocked_open.call_count, 3, 'Three files should be opened') mocked_open.assert_any_call(os.path.join('test', 'titles.txt')) mocked_open.assert_any_call(os.path.join('test', 'slideNotes1.txt')) mocked_open.assert_any_call(os.path.join('test', 'slideNotes2.txt')) - self.assertEqual(mocked_exists.call_count, 3, - 'Three files should have been checked') + self.assertEqual(mocked_exists.call_count, 3, 'Three files should have been checked') def get_titles_and_notes_with_file_not_found_test(self): """ @@ -140,26 +133,21 @@ class TestPresentationController(TestCase): """ # GIVEN: A mocked open, get_thumbnail_folder and exists with patch('builtins.open') as mocked_open, \ - patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder') \ - as mocked_get_thumbnail_folder, \ + patch(FOLDER_TO_PATCH) as mocked_get_thumbnail_folder, \ patch('openlp.plugins.presentations.lib.presentationcontroller.os.path.exists') as mocked_exists: mocked_get_thumbnail_folder.return_value = 'test' mocked_exists.return_value = False + #WHEN: calling get_titles_and_notes result_titles, result_notes = self.document.get_titles_and_notes() + # THEN: it should return two empty lists - self.assertIs(type(result_titles), list, - 'result_titles should be of type list') - self.assertEqual(len(result_titles), 0, - 'there be no titles') - self.assertIs(type(result_notes), list, - 'result_notes should be a list') - self.assertEqual(len(result_notes), 0, - 'but the list should be empty') - self.assertEqual(mocked_open.call_count, 0, - 'No calls to open files') - self.assertEqual(mocked_exists.call_count, 1, - 'There should be one call to file exists') + self.assertIs(type(result_titles), list, 'result_titles should be of type list') + self.assertEqual(len(result_titles), 0, 'there be no titles') + self.assertIs(type(result_notes), list, 'result_notes should be a list') + self.assertEqual(len(result_notes), 0, 'but the list should be empty') + self.assertEqual(mocked_open.call_count, 0, 'No calls to open files') + self.assertEqual(mocked_exists.call_count, 1, 'There should be one call to file exists') def get_titles_and_notes_with_file_error_test(self): """ @@ -167,15 +155,15 @@ class TestPresentationController(TestCase): """ # GIVEN: A mocked open, get_thumbnail_folder and exists with patch('builtins.open') as mocked_open, \ - patch('openlp.plugins.presentations.lib.presentationcontroller.PresentationDocument.get_thumbnail_folder') \ - as mocked_get_thumbnail_folder, \ + patch(FOLDER_TO_PATCH) as mocked_get_thumbnail_folder, \ patch('openlp.plugins.presentations.lib.presentationcontroller.os.path.exists') as mocked_exists: mocked_get_thumbnail_folder.return_value = 'test' mocked_exists.return_value = True mocked_open.side_effect = IOError() + # WHEN: calling get_titles_and_notes result_titles, result_notes = self.document.get_titles_and_notes() - # THEN: it should return two empty lists - self.assertIs(type(result_titles), list, - 'result_titles should be a list') + + # THEN: it should return two empty lists + self.assertIs(type(result_titles), list, 'result_titles should be a list') diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 5eb46a0b6..6ba9ae297 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -120,7 +120,7 @@ class TestRouter(TestCase): Test the get_content_type logic """ # GIVEN: a set of files and their corresponding types - headers = [ ['test.html', 'text/html'], ['test.css', 'text/css'], + headers = [['test.html', 'text/html'], ['test.css', 'text/css'], ['test.js', 'application/javascript'], ['test.jpg', 'image/jpeg'], ['test.gif', 'image/gif'], ['test.ico', 'image/x-icon'], ['test.png', 'image/png'], ['test.whatever', 'text/plain'], @@ -166,7 +166,7 @@ class TestRouter(TestCase): self.router.html_dir = os.path.normpath('test/dir') self.router.template_vars = MagicMock() with patch('openlp.core.lib.os.path.exists') as mocked_exists, \ - patch('builtins.open', mock_open(read_data='123')): + patch('builtins.open', mock_open(read_data='123')): mocked_exists.return_value = True # WHEN: call serve_file with an existing html file @@ -187,10 +187,8 @@ class TestRouter(TestCase): self.router.wfile = MagicMock() self.router.serve_thumbnail() self.router.send_response.assert_called_once_with(404) - self.assertEqual(self.router.send_response.call_count, 1, - 'Send response called once') - self.assertEqual(self.router.end_headers.call_count, 1, - 'end_headers called once') + self.assertEqual(self.router.send_response.call_count, 1, 'Send response called once') + self.assertEqual(self.router.end_headers.call_count, 1, 'end_headers called once') def serve_thumbnail_with_invalid_params_test(self): """ @@ -201,27 +199,27 @@ class TestRouter(TestCase): self.router.send_header = MagicMock() self.router.end_headers = MagicMock() self.router.wfile = MagicMock() + # WHEN: pass a bad controller - self.router.serve_thumbnail('badcontroller', - 'tecnologia 1.pptx/slide1.png') + self.router.serve_thumbnail('badcontroller', 'tecnologia 1.pptx/slide1.png') + # THEN: a 404 should be returned - self.assertEqual(len(self.router.send_header.mock_calls), 1, - 'One header') - self.assertEqual(len(self.router.send_response.mock_calls), 1, - 'One response') - self.assertEqual(len(self.router.wfile.mock_calls), 1, - 'Once call to write to the socket') + self.assertEqual(len(self.router.send_header.mock_calls), 1, 'One header') + self.assertEqual(len(self.router.send_response.mock_calls), 1, 'One response') + self.assertEqual(len(self.router.wfile.mock_calls), 1, 'Once call to write to the socket') self.router.send_response.assert_called_once_with(404) + # WHEN: pass a bad filename self.router.send_response.reset_mock() - self.router.serve_thumbnail('presentations', - 'tecnologia 1.pptx/badfilename.png') + self.router.serve_thumbnail('presentations', 'tecnologia 1.pptx/badfilename.png') + # THEN: return a 404 self.router.send_response.assert_called_once_with(404) + # WHEN: a dangerous URL is passed self.router.send_response.reset_mock() - self.router.serve_thumbnail('presentations', - '../tecnologia 1.pptx/slide1.png') + self.router.serve_thumbnail('presentations', '../tecnologia 1.pptx/slide1.png') + # THEN: return a 404 self.router.send_response.assert_called_once_with(404) @@ -236,36 +234,28 @@ class TestRouter(TestCase): self.router.wfile = MagicMock() mocked_image_manager = MagicMock() Registry.create() - Registry().register('image_manager',mocked_image_manager) + Registry().register('image_manager', mocked_image_manager) file_name = 'another%20test/slide1.png' full_path = os.path.normpath(os.path.join('thumbnails',file_name)) width = 120 height = 90 with patch('openlp.core.lib.os.path.exists') as mocked_exists, \ patch('builtins.open', mock_open(read_data='123')), \ - patch('openlp.plugins.remotes.lib.httprouter.AppLocation') \ - as mocked_location, \ - patch('openlp.plugins.remotes.lib.httprouter.image_to_byte')\ - as mocked_image_to_byte: + patch('openlp.plugins.remotes.lib.httprouter.AppLocation') as mocked_location, \ + patch('openlp.plugins.remotes.lib.httprouter.image_to_byte') as mocked_image_to_byte: mocked_exists.return_value = True mocked_image_to_byte.return_value = '123' mocked_location.get_section_data_path.return_value = '' + # WHEN: pass good controller and filename - result = self.router.serve_thumbnail('presentations', - '{0}x{1}'.format(width, height), - file_name) + result = self.router.serve_thumbnail('presentations', '{0}x{1}'.format(width, height), file_name) + # THEN: a file should be returned - self.assertEqual(self.router.send_header.call_count, 1, - 'One header') - self.assertEqual(self.router.send_response.call_count, 1, - 'Send response called once') - self.assertEqual(self.router.end_headers.call_count, 1, - 'end_headers called once') + self.assertEqual(self.router.send_header.call_count, 1, 'One header') + self.assertEqual(self.router.send_response.call_count, 1, 'Send response called once') + self.assertEqual(self.router.end_headers.call_count, 1, 'end_headers called once') mocked_exists.assert_called_with(urllib.parse.unquote(full_path)) self.assertEqual(mocked_image_to_byte.call_count, 1, 'Called once') - mocked_image_manager.assert_called_any( - os.path.normpath('thumbnails\\another test'), 'slide1.png', - None, '120x90') - mocked_image_manager.assert_called_any( - os.path.normpath('thumbnails\\another test'),'slide1.png', - '120x90') + mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'), + 'slide1.png', None, '120x90') + mocked_image_manager.assert_called_any(os.path.normpath('thumbnails\\another test'), 'slide1.png', '120x90')