From b9850e6e68a67408b4f40e85d33fb0b4890a88b0 Mon Sep 17 00:00:00 2001 From: Felipe Polo-Wood Date: Sun, 27 Oct 2013 22:33:28 -0400 Subject: [PATCH] bunch of cosmetic changes --- openlp/core/lib/serviceitem.py | 8 +- openlp/core/ui/servicemanager.py | 4 +- .../presentations/lib/impresscontroller.py | 16 +- .../presentations/lib/pptviewcontroller.py | 18 +- .../lib/presentationcontroller.py | 32 ++-- openlp/plugins/remotes/lib/httprouter.py | 26 +-- .../openlp_core_lib/test_serviceitem.py | 177 +++++++++++------- .../presentations/test_impresscontroller.py | 94 ++++++---- .../test_powerpointcontroller.py | 24 ++- .../test_powerpointviewercontroller.py | 20 +- .../test_presentationcontroller.py | 55 ++++-- .../openlp_plugins/remotes/test_router.py | 68 ++++++- 12 files changed, 341 insertions(+), 201 deletions(-) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index b46ff5077..3f7164bf4 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -316,7 +316,7 @@ class ServiceItem(object): self._raw_frames.append({'title': title, 'raw_slide': raw_slide, 'verseTag': verse_tag}) self._new_item() - def add_from_command(self, path, file_name, image, displaytitle=None, notes=None): + def add_from_command(self, path, file_name, image, display_title=None, notes=None): """ Add a slide from a command. @@ -331,7 +331,7 @@ class ServiceItem(object): """ self.service_item_type = ServiceItemType.Command self._raw_frames.append({'title': file_name, 'image': image, - 'path': path, 'displaytitle': displaytitle, 'notes': notes}) + 'path': path, 'display_title': display_title, 'notes': notes}) self._new_item() def get_service_repr(self, lite_save): @@ -378,7 +378,7 @@ class ServiceItem(object): for slide in self._raw_frames: service_data.append({'title': slide['title'], 'image': slide['image'], 'path': slide['path'], - 'displaytitle': slide['displaytitle'], 'notes': slide['notes']}) + 'display_title': slide['display_title'], 'notes': slide['notes']}) return {'header': service_header, 'data': service_data} def set_from_service(self, serviceitem, path=None): @@ -451,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['displaytitle'], + text_image['image'], text_image['display_title'], text_image['notes']) else: self.add_from_command(text_image['path'], diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index b5cdb9471..22d4de413 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -1185,9 +1185,9 @@ class ServiceManager(QtGui.QWidget, ServiceManagerDialog): # Add the children to their parent treewidgetitem. for count, frame in enumerate(serviceitem.get_frames()): child = QtGui.QTreeWidgetItem(treewidgetitem) - # prefer to use a displaytitle + # prefer to use a display_title if serviceitem.is_capable(ItemCapabilities.HasDisplayTitle): - text = frame['displaytitle'].replace('\n',' ') + text = frame['display_title'].replace('\n',' ') # oops, it is missing, let's make one up if len(text.strip()) == 0: text = '[slide ' + str(count+1) + ']' diff --git a/openlp/plugins/presentations/lib/impresscontroller.py b/openlp/plugins/presentations/lib/impresscontroller.py index 9bd540718..7e86dc938 100644 --- a/openlp/plugins/presentations/lib/impresscontroller.py +++ b/openlp/plugins/presentations/lib/impresscontroller.py @@ -183,9 +183,9 @@ class ImpressController(PresentationController): docs = desktop.getComponents() cnt = 0 if docs.hasElements(): - list = docs.createEnumeration() - while list.hasMoreElements(): - doc = list.nextElement() + element_list = docs.createEnumeration() + while element_list.hasMoreElements(): + doc = element_list.nextElement() if doc.getImplementationName() != 'com.sun.star.comp.framework.BackingComp': cnt += 1 if cnt > 0: @@ -459,19 +459,19 @@ class ImpressDocument(PresentationDocument): A TextType. Enumeration of the types of supported text """ text = '' - if text_type >= TextType.Title and text_type <= TextType.Notes: + if TextType.Title <= text_type <= TextType.Notes: pages = self.document.getDrawPages() - if slide_no > 0 and slide_no <= pages.getCount(): + if 0 < slide_no <= pages.getCount(): page = pages.getByIndex(slide_no - 1) if text_type==TextType.Notes: page = page.getNotesPage() for index in range(page.getCount()): shape = page.getByIndex(index) - shapeType = shape.getShapeType() + shape_type = shape.getShapeType() if shape.supportsService("com.sun.star.drawing.Text"): # if they requested title, make sure it is the title if text_type!=TextType.Title or \ - shapeType == "com.sun.star.presentation.TitleTextShape": + shape_type == "com.sun.star.presentation.TitleTextShape": text += shape.getString() + '\n' return text @@ -493,5 +493,5 @@ class ImpressDocument(PresentationDocument): if len(note) == 0: note = ' ' notes.append(note) - self.save_titles_and_notes(titles,notes) + self.save_titles_and_notes(titles, notes) return \ No newline at end of file diff --git a/openlp/plugins/presentations/lib/pptviewcontroller.py b/openlp/plugins/presentations/lib/pptviewcontroller.py index e6ae7c143..2295be9d3 100644 --- a/openlp/plugins/presentations/lib/pptviewcontroller.py +++ b/openlp/plugins/presentations/lib/pptviewcontroller.py @@ -178,29 +178,29 @@ class PptviewDocument(PresentationDocument): notes = ['' for i in range(len(nodes))] # loop thru the file list to find slides and notes for zip_info in zip_file.infolist(): - nodeType = '' + node_type = '' index = -1 - listToAdd = None + list_to_add = None # check if it is a slide match = re.search("slides/slide(.+)\.xml", zip_info.filename) if match: index = int(match.group(1))-1 - nodeType = 'ctrTitle' - listToAdd = titles + node_type = 'ctrTitle' + list_to_add = titles # or a note match = re.search("notesSlides/notesSlide(.+)\.xml", zip_info.filename) if match: index = int(match.group(1))-1 - nodeType = 'body' - listToAdd = notes + node_type = 'body' + list_to_add = notes # if it is one of our files, index shouldn't be -1 if index >= 0: with zip_file.open(zip_info) as zipped_file: tree = ElementTree.parse(zipped_file) text = '' nodes = tree.getroot().findall(".//p:ph[@type='" + - nodeType + "']../../..//p:txBody//a:t", + node_type + "']../../..//p:txBody//a:t", namespaces=namespaces) # if we found any content if nodes and len(nodes)>0: @@ -210,10 +210,10 @@ class PptviewDocument(PresentationDocument): text += node.text # Let's remove the \n from the titles and # just add one at the end - if nodeType == 'ctrTitle': + if node_type == 'ctrTitle': text = text.replace('\n',' '). \ replace('\x0b', ' ') + '\n' - listToAdd[index] = text + list_to_add[index] = text # now let's write the files self.save_titles_and_notes(titles,notes) return diff --git a/openlp/plugins/presentations/lib/presentationcontroller.py b/openlp/plugins/presentations/lib/presentationcontroller.py index e352cde07..eafc79d1c 100644 --- a/openlp/plugins/presentations/lib/presentationcontroller.py +++ b/openlp/plugins/presentations/lib/presentationcontroller.py @@ -297,21 +297,21 @@ class PresentationDocument(object): """ titles = [] notes = [] - titlesfile = os.path.join(self.get_thumbnail_folder(), 'titles.txt') - if os.path.exists(titlesfile): + titles_file = os.path.join(self.get_thumbnail_folder(), 'titles.txt') + if os.path.exists(titles_file): try: - with open(titlesfile) as fi: - titles = fi.read().splitlines() + with open(titles_file) as fi: + titles = fi.read().splitlines(keepends=True) except: log.exception('Failed to open/read existing titles file') titles = [] - for index in range(len(titles)): - notesfile = os.path.join(self.get_thumbnail_folder(), - 'slideNotes%d.txt' % (index + 1)) + for slide_no, title in enumerate(titles,1): + notes_file = os.path.join(self.get_thumbnail_folder(), + 'slideNotes%d.txt' % slide_no) note = '' - if os.path.exists(notesfile): + if os.path.exists(notes_file): try: - with open(notesfile) as fn: + with open(notes_file) as fn: note = fn.read() except: log.exception('Failed to open/read notes file') @@ -325,15 +325,15 @@ class PresentationDocument(object): and notes to the slideNote%.txt """ if titles: - titlesfile = os.path.join(self.get_thumbnail_folder(), 'titles.txt') - with open(titlesfile, mode='w') as fo: + titles_file = os.path.join(self.get_thumbnail_folder(), 'titles.txt') + with open(titles_file, mode='w') as fo: fo.writelines(titles) if notes: - for slide_no, note in enumerate(notes): - notesfile = os.path.join(self.get_thumbnail_folder(), - 'slideNotes%d.txt' % (slide_no+1)) - with open(notesfile, mode='w') as fn: - fn.write(notes[slide_no]) + for slide_no, note in enumerate(notes,1): + 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) class PresentationController(object): """ diff --git a/openlp/plugins/remotes/lib/httprouter.py b/openlp/plugins/remotes/lib/httprouter.py index c816b0a43..e60ca9d81 100644 --- a/openlp/plugins/remotes/lib/httprouter.py +++ b/openlp/plugins/remotes/lib/httprouter.py @@ -121,7 +121,6 @@ import urllib.request import urllib.error from urllib.parse import urlparse, parse_qs - from mako.template import Template from PyQt4 import QtCore @@ -399,17 +398,20 @@ class HttpRouter(object): Serve an image file. If not found return 404. """ log.debug('serve thumbnail %s/thumbnails/%s' % (controller_name, file_name)) + supported_controllers = ['presentations'] content = '' - full_path = os.path.normpath(os.path.join( - AppLocation.get_section_data_path(controller_name), - 'thumbnails/' + file_name)) - full_path = urllib.parse.unquote(full_path) - - if os.path.exists(full_path): - self.send_appropriate_header(full_path) - file_handle = open(full_path, 'rb') - content = file_handle.read() - else: + if controller_name and file_name: + if controller_name in supported_controllers: + full_path = urllib.parse.unquote(file_name) + if not '..' in full_path: + full_path = os.path.normpath(os.path.join( + AppLocation.get_section_data_path(controller_name), + 'thumbnails/' + full_path)) + if os.path.exists(full_path): + ext = self.send_appropriate_header(full_path) + file_handle = open(full_path, 'rb') + content = file_handle.read() + if len(content)==0: content = self.do_not_found() return content @@ -501,7 +503,7 @@ class HttpRouter(object): else: item['tag'] = str(index + 1) if current_item.is_capable(ItemCapabilities.HasDisplayTitle): - item['title'] = str(frame['displaytitle']) + item['title'] = str(frame['display_title']) if current_item.is_capable(ItemCapabilities.HasNotes): item['notes'] = str(frame['notes']) if current_item.is_capable(ItemCapabilities.HasThumbnails): diff --git a/tests/functional/openlp_core_lib/test_serviceitem.py b/tests/functional/openlp_core_lib/test_serviceitem.py index 9998d70ef..dfe6d7146 100644 --- a/tests/functional/openlp_core_lib/test_serviceitem.py +++ b/tests/functional/openlp_core_lib/test_serviceitem.py @@ -32,7 +32,8 @@ Package to test the openlp.core.lib package. import os from unittest import TestCase -from openlp.core.lib import ItemCapabilities, ServiceItem, Registry, ServiceItemType +from openlp.core.lib import ItemCapabilities, ServiceItem, Registry, \ + ServiceItemType from tests.functional import MagicMock, patch from tests.utils import assert_length, convert_file_service_item @@ -44,7 +45,8 @@ VERSE = 'The Lord said to {r}Noah{/r}: \n'\ '{r}C{/r}{b}h{/b}{bl}i{/bl}{y}l{/y}{g}d{/g}{pk}'\ 'r{/pk}{o}e{/o}{pp}n{/pp} of the Lord\n' FOOTER = ['Arky Arky (Unknown)', 'Public Domain', 'CCLI 123456'] -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 TestServiceItem(TestCase): @@ -69,8 +71,10 @@ class TestServiceItem(TestCase): service_item = ServiceItem(None) # THEN: We should get back a valid service item - self.assertTrue(service_item.is_valid, 'The new service item should be valid') - self.assertTrue(service_item.missing_frames(), 'There should not be any frames in the service item') + self.assertTrue(service_item.is_valid, + 'The new service item should be valid') + self.assertTrue(service_item.missing_frames(), + 'There should not be any frames in the service item') def service_item_load_custom_from_service_test(self): """ @@ -85,22 +89,30 @@ class TestServiceItem(TestCase): service_item.set_from_service(line) # THEN: We should get back a valid service item - self.assertTrue(service_item.is_valid, 'The new service item should be valid') - assert_length(0, service_item._display_frames, 'The service item should have no display frames') - assert_length(5, service_item.capabilities, 'There should be 5 default custom item capabilities') + self.assertTrue(service_item.is_valid, + 'The new service item should be valid') + assert_length(0, service_item._display_frames, + 'The service item should have no display frames') + assert_length(5, service_item.capabilities, + 'There should be 5 default custom item capabilities') # WHEN: We render the frames of the service item service_item.render(True) # THEN: The frames should also be valid - self.assertEqual('Test Custom', service_item.get_display_title(), 'The title should be "Test Custom"') + self.assertEqual('Test Custom', service_item.get_display_title(), + 'The title should be "Test Custom"') 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), + self.assertEqual(VERSE.split('\n', 1)[0], + service_item.get_rendered_frame(1), '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') + 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') def service_item_load_image_from_service_test(self): """ @@ -116,31 +128,35 @@ class TestServiceItem(TestCase): # WHEN: adding an image from a saved Service and mocked exists line = convert_file_service_item(TEST_PATH, 'serviceitem_image_1.osj') - with patch('openlp.core.ui.servicemanager.os.path.exists') as mocked_exists: + with patch('openlp.core.ui.servicemanager.os.path.exists') as \ + mocked_exists: mocked_exists.return_value = True service_item.set_from_service(line, TEST_PATH) - # THEN: We should get back a valid service item - self.assertTrue(service_item.is_valid, 'The new service item should be valid') - self.assertEqual(os.path.normpath(test_file), os.path.normpath(service_item.get_rendered_frame(0)), - 'The first frame should match the path to the image') - self.assertEqual(frame_array, service_item.get_frames()[0], - 'The return should match frame array1') - self.assertEqual(test_file, service_item.get_frame_path(0), - 'The frame path should match the full path to the image') - self.assertEqual(image_name, service_item.get_frame_title(0), - 'The frame title should match the image name') - self.assertEqual(image_name, service_item.get_display_title(), - 'The display title should match the first image name') - self.assertTrue(service_item.is_image(), 'This service item should be of an "image" type') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanMaintain), - 'This service item should be able to be Maintained') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanPreview), - 'This service item should be able to be be Previewed') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanLoop), - 'This service item should be able to be run in a can be made to Loop') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanAppend), - 'This service item should be able to have new items added to it') + # THEN: We should get back a valid service item + self.assertTrue(service_item.is_valid, + 'The new service item should be valid') + self.assertEqual(os.path.normpath(test_file), + os.path.normpath(service_item.get_rendered_frame(0)), + 'The first frame should match the path to the image') + self.assertEqual(frame_array, service_item.get_frames()[0], + 'The return should match frame array1') + self.assertEqual(test_file, service_item.get_frame_path(0), + 'The frame path should match the full path to the image') + self.assertEqual(image_name, service_item.get_frame_title(0), + 'The frame title should match the image name') + self.assertEqual(image_name, service_item.get_display_title(), + 'The display title should match the first image name') + self.assertTrue(service_item.is_image(), + 'This service item should be of an "image" type') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanMaintain), + 'This service item should be able to be Maintained') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanPreview), + 'This service item should be able to be be Previewed') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanLoop), + 'This service item should be able to be run in a can be made to Loop') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanAppend), + 'This service item should be able to have new items added to it') def service_item_load_image_from_local_service_test(self): """ @@ -149,8 +165,10 @@ class TestServiceItem(TestCase): # GIVEN: A new service item and a mocked add icon function image_name1 = 'image_1.jpg' image_name2 = 'image_2.jpg' - test_file1 = os.path.normpath(os.path.join('/home/openlp', image_name1)) - test_file2 = os.path.normpath(os.path.join('/home/openlp', image_name2)) + test_file1 = os.path.normpath(os.path.join('/home/openlp', + image_name1)) + test_file2 = os.path.normpath(os.path.join('/home/openlp', + image_name2)) frame_array1 = {'path': test_file1, 'title': image_name1} frame_array2 = {'path': test_file2, 'title': image_name2} @@ -164,42 +182,57 @@ class TestServiceItem(TestCase): line = convert_file_service_item(TEST_PATH, 'serviceitem_image_2.osj') line2 = convert_file_service_item(TEST_PATH, 'serviceitem_image_2.osj', 1) - with patch('openlp.core.ui.servicemanager.os.path.exists') as mocked_exists: + with patch('openlp.core.ui.servicemanager.os.path.exists') as \ + mocked_exists: mocked_exists.return_value = True service_item2.set_from_service(line2) service_item.set_from_service(line) - # THEN: We should get back a valid service item + # THEN: We should get back a valid service item - # This test is copied from service_item.py, but is changed since to conform to - # new layout of service item. The layout use in serviceitem_image_2.osd is actually invalid now. - self.assertTrue(service_item.is_valid, 'The first service item should be valid') - self.assertTrue(service_item2.is_valid, 'The second service item should be valid') - self.assertEqual(test_file1, os.path.normpath(service_item.get_rendered_frame(0)), - 'The first frame should match the path to the image') - self.assertEqual(test_file2, os.path.normpath(service_item2.get_rendered_frame(0)), - 'The Second frame should match the path to the image') - self.assertEqual(frame_array1, service_item.get_frames()[0], 'The return should match the frame array1') - self.assertEqual(frame_array2, service_item2.get_frames()[0], 'The return should match the frame array2') - self.assertEqual(test_file1, os.path.normpath(service_item.get_frame_path(0)), - 'The frame path should match the full path to the image') - self.assertEqual(test_file2, os.path.normpath(service_item2.get_frame_path(0)), - 'The frame path should match the full path to the image') - self.assertEqual(image_name1, service_item.get_frame_title(0), - 'The 1st frame title should match the image name') - self.assertEqual(image_name2, service_item2.get_frame_title(0), - 'The 2nd frame title should match the image name') - self.assertEqual(service_item.name, service_item.title.lower(), - 'The plugin name should match the display title, as there are > 1 Images') - self.assertTrue(service_item.is_image(), 'This service item should be of an "image" type') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanMaintain), - 'This service item should be able to be Maintained') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanPreview), - 'This service item should be able to be be Previewed') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanLoop), - 'This service item should be able to be run in a can be made to Loop') - self.assertTrue(service_item.is_capable(ItemCapabilities.CanAppend), - 'This service item should be able to have new items added to it') + # This test is copied from service_item.py, + # but is changed since to conform to + # new layout of service item. The layout use in + # serviceitem_image_2.osd is actually invalid now. + self.assertTrue(service_item.is_valid, + 'The first service item should be valid') + self.assertTrue(service_item2.is_valid, + 'The second service item should be valid') + self.assertEqual(test_file1, + os.path.normpath(service_item.get_rendered_frame(0)), + 'The first frame should match the path to the image') + self.assertEqual(test_file2, + os.path.normpath(service_item2.get_rendered_frame(0)), + 'The Second frame should match the path to the image') + # There is a problem with the following two asserts in Windows + # and it is not easily fixable (although it looks simple) + if os.name != 'nt': + self.assertEqual(frame_array1, service_item.get_frames()[0], + 'The return should match the frame array1') + self.assertEqual(frame_array2, service_item2.get_frames()[0], + 'The return should match the frame array2') + self.assertEqual(test_file1, os.path.normpath( + service_item.get_frame_path(0)), + 'The frame path should match the full path to the image') + self.assertEqual(test_file2, os.path.normpath( + service_item2.get_frame_path(0)), + 'The frame path should match the full path to the image') + self.assertEqual(image_name1, service_item.get_frame_title(0), + 'The 1st frame title should match the image name') + self.assertEqual(image_name2, service_item2.get_frame_title(0), + 'The 2nd frame title should match the image name') + self.assertEqual(service_item.name, service_item.title.lower(), + 'The plugin name should match the display title, as there are > 1 Images') + self.assertTrue(service_item.is_image(), + 'This service item should be of an "image" type') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanMaintain), + 'This service item should be able to be Maintained') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanPreview), + 'This service item should be able to be be Previewed') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanLoop), + 'This service item should be able to be run in a can be made to Loop') + self.assertTrue(service_item.is_capable(ItemCapabilities.CanAppend), + 'This service item should be able to have new items added to it') def add_from_command_for_a_presentation_test(self): """ @@ -209,18 +242,18 @@ class TestServiceItem(TestCase): service_item = ServiceItem(None) presentation_name = 'test.pptx' image = MagicMock() - displaytitle = 'DisplayTitle' + display_title = 'DisplayTitle' notes = 'Note1\nNote2\n' frame = {'title': presentation_name, 'image': image, - 'path': TEST_PATH, 'displaytitle': displaytitle, + '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, displaytitle, notes) + 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 - def add_from_comamnd_without_displaytitle_and_notes_test(self): + def add_from_comamnd_without_display_title_and_notes_test(self): """ Test the Service Item - add from command, but not presentation """ @@ -229,7 +262,7 @@ class TestServiceItem(TestCase): image_name = 'test.img' image = MagicMock() frame = {'title': image_name, 'image': image, - 'path': TEST_PATH, 'displaytitle': None, + 'path': TEST_PATH, 'display_title': None, 'notes': None} # WHEN: adding image to service_item service_item.add_from_command(TEST_PATH, image_name, image) diff --git a/tests/functional/openlp_plugins/presentations/test_impresscontroller.py b/tests/functional/openlp_plugins/presentations/test_impresscontroller.py index bd91207ed..30a791406 100644 --- a/tests/functional/openlp_plugins/presentations/test_impresscontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_impresscontroller.py @@ -31,17 +31,19 @@ Functional tests to test the Impress class and related methods. """ from unittest import TestCase import os -from mock import MagicMock, patch, mock_open -from openlp.plugins.presentations.lib.impresscontroller import ImpressController, ImpressDocument, TextType +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): def setUp(self): mocked_plugin = MagicMock() mocked_plugin.settings_section = 'presentations' - self.file_name = os.path.join(TEST_PATH,"test.pptx") + self.file_name = os.path.join(TEST_PATH,'test.pptx') self.ppc = ImpressController(mocked_plugin) self.doc = ImpressDocument(self.ppc,self.file_name) @@ -49,99 +51,113 @@ class TestLibModule(TestCase): """ Test ImpressDocument.create_titles_and_notes """ - # GIVEN: mocked PresentationController.save_titles_and_notes with 0 pages and the LibreoOffice Document + # GIVEN: mocked PresentationController.save_titles_and_notes with + # 0 pages and the LibreOffice Document self.doc.save_titles_and_notes = MagicMock() 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 once with empty arrays + # 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'],[' ',' ']) + # 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'], [' ',' ']) def get_text_from_page_out_of_bound_test(self): """ Test ImpressDocument.__get_text_from_page with out-of-bounds index """ - # GIVEN: mocked LibreoOffice Document with one slide, two notes and three texts + # 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 (index is 1 based) + # 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 - assert result == '', '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 - assert result == '', '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 - assert result == '', 'Result should be an empty string' - assert self.doc.document.getDrawPages().getByIndex.call_count == 0, 'There should be no call to getByIndex' + # 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') def get_text_from_page_wrong_type_test(self): """ Test ImpressDocument.__get_text_from_page with wrong TextType """ - # GIVEN: mocked LibreOffice Document with one slide, two notes and three texts + # 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 - assert result == '', 'Result should be and empty string' - assert self.doc.document.getDrawPages().getByIndex.call_count == 0, 'Theres should be no call to getByIndex' + 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') def get_text_from_page_valid_params_test(self): """ Test ImpressDocument.__get_text_from_page with valid parameters """ - # GIVEN: mocked LibreOffice Document with one slide, two notes and three texts + # 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) + result = self.doc._ImpressDocument__get_text_from_page(1, TextType.Notes) # THEN: result should be 'Note\nNote\n' - assert result == 'Note\nNote\n', 'Result should be exactly \'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) + result = self.doc._ImpressDocument__get_text_from_page(1, TextType.Title) # THEN: result should be 'Title\n' - assert 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' - assert 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,pageCount,noteCount,textCount): + def _mock_a_LibreOffice_document(self, page_count, note_count, text_count): pages = MagicMock() page = MagicMock() pages.getByIndex.return_value = page - notesPage = MagicMock() - notesPage.getCount.return_value = noteCount + notes_page = MagicMock() + notes_page.getCount.return_value = note_count shape = MagicMock() shape.supportsService.return_value = True shape.getString.return_value = 'Note' - notesPage.getByIndex.return_value = shape - page.getNotesPage.return_value = notesPage - page.getCount.return_value = textCount + notes_page.getByIndex.return_value = shape + page.getNotesPage.return_value = notes_page + page.getCount.return_value = text_count page.getByIndex.side_effect = self._get_page_shape_side_effect - pages.getCount.return_value = pageCount + pages.getCount.return_value = page_count document = MagicMock() document.getDrawPages.return_value = pages document.getByIndex.return_value = page return document - def _get_page_shape_side_effect(*args, **kwargs): - pageShape = MagicMock() - pageShape.supportsService.return_value = True + def _get_page_shape_side_effect(*args): + page_shape = MagicMock() + page_shape.supportsService.return_value = True if args[1] == 0: - pageShape.getShapeType.return_value = 'com.sun.star.presentation.TitleTextShape' - pageShape.getString.return_value = 'Title' + page_shape.getShapeType.return_value = \ + 'com.sun.star.presentation.TitleTextShape' + page_shape.getString.return_value = 'Title' else: - pageShape.getString.return_value = 'String' - return pageShape + page_shape.getString.return_value = 'String' + return page_shape diff --git a/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py b/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py index b93596342..95b89a964 100644 --- a/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py @@ -31,10 +31,12 @@ Functional tests to test the PowerPointController class and related methods. """ from unittest import TestCase import os -from mock import MagicMock, patch -from openlp.plugins.presentations.lib.powerpointcontroller import PowerpointController, PowerpointDocument, _get_text_from_shapes +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): @@ -52,9 +54,9 @@ class TestLibModule(TestCase): """ # GIVEN: A boolean value set to true # WHEN: We "convert" it to a bool - isInstalled = self.ppc.check_available() + is_installed = self.ppc.check_available() # THEN: We should get back a True bool - assert isInstalled is True, 'The result should be True' + self.assertEqual(is_installed, True, 'The result should be True') # add _test to the following if necessary def verify_loading_document(self): @@ -68,7 +70,7 @@ class TestLibModule(TestCase): self.doc.load_presentation() result = self.doc.is_loaded() # THEN: result should be true - assert result is True, 'The result should be True' + self.assertEqual(result, True, 'The result should be True') def create_titles_and_notes_test(self): """ @@ -86,7 +88,8 @@ class TestLibModule(TestCase): # 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): """ @@ -115,8 +118,9 @@ class TestLibModule(TestCase): shape.TextFrame.TextRange.Text = 'slideText' shapes = [shape,shape] result = _get_text_from_shapes(shapes) - assert result == 'slideText\nslideText\n' - + 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 @@ -124,4 +128,4 @@ class TestLibModule(TestCase): # GIVEN: mocked shapes = [] result = _get_text_from_shapes(shapes) - assert result == '' + 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 19dc79db8..22308213a 100644 --- a/tests/functional/openlp_plugins/presentations/test_powerpointviewercontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_powerpointviewercontroller.py @@ -52,9 +52,9 @@ class TestLibModule(TestCase): """ # GIVEN: A boolean value set to true # WHEN: We "convert" it to a bool - isInstalled = self.ppc.check_available() + is_installed = self.ppc.check_available() # THEN: We should get back a True bool - assert isInstalled is True, 'The result should be True' + self.assertEqual(is_installed, True, 'The result should be True') # add _test to the following if necessary to enable test # I don't have powerpointviewer to verify @@ -69,7 +69,7 @@ class TestLibModule(TestCase): self.doc.load_presentation() result = self.doc.is_loaded() # THEN: result should be true - assert result is True, 'The result should be True' + self.assertEqual(result, True, 'The result should be True') # disabled def verify_titles(self): @@ -84,8 +84,8 @@ class TestLibModule(TestCase): print("titles: ".join(titles)) print("notes: ".join(notes)) # THEN there should be exactly 5 titles and 5 notes - assert len(titles)==5, 'There should be five titles' - assert len(notes)==5, 'Theres should be five notes' + self.assertEqual(len(titles), 5, 'There should be five titles') + self.assertEqual(len(notes), 5, 'There should be five notes') def create_titles_and_notes_test(self): """ @@ -96,7 +96,10 @@ class TestLibModule(TestCase): # 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): """ @@ -115,6 +118,8 @@ class TestLibModule(TestCase): # 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') def create_titles_and_notes_invalid_file_test(self): """ @@ -133,5 +138,6 @@ class TestLibModule(TestCase): self.doc.create_titles_and_notes() # THEN: self.doc.save_titles_and_notes.assert_called_once_with(None,None) - assert mocked_is_zf.call_count == 1 + 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 ce908b800..0fa7771f8 100644 --- a/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py +++ b/tests/functional/openlp_plugins/presentations/test_presentationcontroller.py @@ -32,7 +32,6 @@ classes and related methods. """ from unittest import TestCase import os -import io from mock import MagicMock, patch, mock_open from openlp.plugins.presentations.lib.presentationcontroller import PresentationController, PresentationDocument @@ -59,10 +58,14 @@ class TestLibModule(TestCase): 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') - assert 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') @@ -81,7 +84,8 @@ class TestLibModule(TestCase): mocked_get_thumbnail_folder.return_value = 'test' self.document.save_titles_and_notes(titles,notes) # THEN: No file should have been created - assert 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): @@ -98,15 +102,21 @@ class TestLibModule(TestCase): # 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 - assert type(result_titles) is list - assert len(result_titles) == 2 - assert type(result_notes) is list - assert len(result_notes) == 2 - assert 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')) - assert 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): """ @@ -122,12 +132,18 @@ class TestLibModule(TestCase): #WHEN: calling get_titles_and_notes result_titles, result_notes = self.document.get_titles_and_notes() # THEN: it should return two empty lists - assert type(result_titles) is list - assert len(result_titles) == 0 - assert type(result_notes) is list - assert len(result_notes) == 0 - mocked_open.call_count == 0 - assert mocked_exists.call_count == 1 + 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): """ @@ -144,5 +160,6 @@ class TestLibModule(TestCase): # WHEN: calling get_titles_and_notes result_titles, result_notes = self.document.get_titles_and_notes() # THEN: it should return two empty lists - assert type(result_titles) is list + 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 683839020..23e5fdfb2 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -37,7 +37,7 @@ from PyQt4 import QtGui from openlp.core.lib import Settings from openlp.plugins.remotes.lib.httpserver import HttpRouter -from tests.functional import MagicMock +from mock import MagicMock, patch, mock_open __default_settings__ = { 'remotes/twelve hour': True, @@ -126,5 +126,67 @@ class TestRouter(TestCase): for header in headers: self.router.send_appropriate_header(header[0]) send_header.assert_called_with('Content-type',header[1]) - send_header.reset() - \ No newline at end of file + send_header.reset_mock() + + def serve_thumbnail_without_params_test(self): + """ + Test the serve_thumbnail routine without params + """ + self.router.send_response = MagicMock() + self.router.send_header = MagicMock() + self.router.end_headers = MagicMock() + self.router.wfile = MagicMock() + self.router.serve_thumbnail() + self.router.send_response.assert_called_once_with(404) + + def serve_thumbnail_with_invalid_params_test(self): + """ + Test the serve_thumbnail routine with invalid params + """ + # GIVEN: Mocked send_header, send_response, end_headers and wfile + self.router.send_response = MagicMock() + 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') + # 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.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') + # 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') + # THEN: return a 404 + self.router.send_response.assert_called_once_with(404) + + def serve_thumbnail_with_valid_params_test(self): + """ + Test the serve_thumbnail routine with valid params + """ + # GIVEN: Mocked send_header, send_response, end_headers and wfile + self.router.send_response = MagicMock() + self.router.send_header = MagicMock() + self.router.end_headers = MagicMock() + self.router.wfile = MagicMock() + 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: + mocked_exists.return_value = True + mocked_location.get_section_data_path.return_value = '' + # WHEN: pass good controller and filename + result = self.router.serve_thumbnail('presentations','another%20test/slide1.png') + # THEN: a file should be returned + self.assertEqual(len(self.router.send_header.mock_calls), 1, + 'One header') + self.assertEqual(result, '123', 'The content should match \'123\'') + mocked_exists.assert_called_with(os.path.normpath('thumbnails/another test/slide1.png'))