From 69951133fe8d8973110092b8b0f7d174c90e5be5 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:47:57 +0200 Subject: [PATCH 01/14] Fix handeling of control chars and escaped chars in VideoPsalm import. Fixes bug 1594945. Fixes: https://launchpad.net/bugs/1594945 --- .../plugins/songs/lib/importers/videopsalm.py | 8 ++++ .../openlp_plugins/songs/test_videopsalm.py | 2 + .../videopsalm-as-safe-a-stronghold2.json | 47 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json diff --git a/openlp/plugins/songs/lib/importers/videopsalm.py b/openlp/plugins/songs/lib/importers/videopsalm.py index 25fd4d8eb..b536dd678 100644 --- a/openlp/plugins/songs/lib/importers/videopsalm.py +++ b/openlp/plugins/songs/lib/importers/videopsalm.py @@ -73,6 +73,14 @@ class VideoPsalmImport(SongImport): processed_content += c c = next(file_content_it) processed_content += '"' + c + # Remove control characters + elif (c < chr(32)): + processed_content += ' ' + # Handle escaped characters + elif c == '\\': + processed_content += c + c = next(file_content_it) + processed_content += c else: processed_content += c songbook = json.loads(processed_content.strip()) diff --git a/tests/functional/openlp_plugins/songs/test_videopsalm.py b/tests/functional/openlp_plugins/songs/test_videopsalm.py index 1bf13241d..ff1a81db5 100644 --- a/tests/functional/openlp_plugins/songs/test_videopsalm.py +++ b/tests/functional/openlp_plugins/songs/test_videopsalm.py @@ -43,3 +43,5 @@ class TestVideoPsalmFileImport(SongImportTestHelper): """ self.file_import(os.path.join(TEST_PATH, 'videopsalm-as-safe-a-stronghold.json'), self.load_external_result_data(os.path.join(TEST_PATH, 'as-safe-a-stronghold.json'))) + self.file_import(os.path.join(TEST_PATH, 'videopsalm-as-safe-a-stronghold2.json'), + self.load_external_result_data(os.path.join(TEST_PATH, 'as-safe-a-stronghold2.json'))) diff --git a/tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json b/tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json new file mode 100644 index 000000000..11bc082e6 --- /dev/null +++ b/tests/resources/videopsalmsongs/videopsalm-as-safe-a-stronghold2.json @@ -0,0 +1,47 @@ +{Abbreviation:"SB1",Copyright:"Public domain",Songs:[{ID:3,Composer:"Unknown",Author:"Martin Luther",Copyright:"Public +Domain",Theme:"tema1 +tema2",CCLI:"12345",Alias:"A safe stronghold",Memo1:"This is +the first comment +",Memo2:"This is +the second comment +",Memo3:"This is +the third comment +",Reference:"reference",Guid:"jtCkrJdPIUOmECjaQylg/g",Verses:[{ +Text:"As safe a stronghold our God is still, +A trusty shield and weapon; +He’ll help us clear from all the ill +That hath us now o’ertaken. +The ancient prince of hell +Hath risen with purpose fell; +Strong mail of craft and power +He weareth in this hour; +On earth is not His fellow."},{ID:2, +Text:"With \"force\" of arms we nothing can, +Full soon were we down-ridden; +But for us fights \\ the proper Man, +Whom God Himself hath bidden. +Ask ye: Who is this same? +Christ Jesus is His name, +The Lord Sabaoth’s Son; +He, and no other one, +Shall conquer in the battle."},{ID:3, +Text:"And were this world all devils o’er, +And watching to devour us, +We lay it not to heart so sore; +Not they can overpower us. +And let the prince of ill +Look grim as e’er he will, +He harms us not a whit; +For why? his doom is writ; +A word shall quickly slay him."},{ID:4, +Text:"God’s word, for all their craft and force, +One moment will not linger, +But, spite of hell, shall have its course; +’Tis written by His finger. +And though they take our life, +Goods, honour, children, wife, +Yet is their profit small: +These things shall vanish all; +The city of God remaineth."}],AudioFile:"282.mp3",IsAudioFileEnabled:1, +Text:"A Safe Stronghold Our God is Still"}],Guid:"khiHU2blX0Kb41dGdbDLhA",VersionDate:"20121012000000", +Text:"SongBook1"} From 8009bfef855192a36c6d225bdd5bcfef299a27d5 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:48:52 +0200 Subject: [PATCH 02/14] forgot test file --- .../as-safe-a-stronghold2.json | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/resources/videopsalmsongs/as-safe-a-stronghold2.json diff --git a/tests/resources/videopsalmsongs/as-safe-a-stronghold2.json b/tests/resources/videopsalmsongs/as-safe-a-stronghold2.json new file mode 100644 index 000000000..f6becc92a --- /dev/null +++ b/tests/resources/videopsalmsongs/as-safe-a-stronghold2.json @@ -0,0 +1,35 @@ +{ + "authors": [ + ["Martin Luther", "words"], + ["Unknown", "music"] + ], + "ccli_number": "12345", + "comments": "This is\nthe first comment\nThis is\nthe second comment\nThis is\nthe third comment\n", + "copyright": "Public Domain", + "song_book_name": "SongBook1", + "song_number": 0, + "title": "A Safe Stronghold Our God is Still", + "topics": [ + "tema1", + "tema2" + ], + "verse_order_list": [], + "verses": [ + [ + "As safe a stronghold our God is still,\nA trusty shield and weapon;\nHe’ll help us clear from all the ill\nThat hath us now o’ertaken.\nThe ancient prince of hell\nHath risen with purpose fell;\nStrong mail of craft and power\nHe weareth in this hour;\nOn earth is not His fellow.", + "v" + ], + [ + "With \"force\" of arms we nothing can,\nFull soon were we down-ridden;\nBut for us fights \\ the proper Man,\nWhom God Himself hath bidden.\nAsk ye: Who is this same?\nChrist Jesus is His name,\nThe Lord Sabaoth’s Son;\nHe, and no other one,\nShall conquer in the battle.", + "v" + ], + [ + "And were this world all devils o’er,\nAnd watching to devour us,\nWe lay it not to heart so sore;\nNot they can overpower us.\nAnd let the prince of ill\nLook grim as e’er he will,\nHe harms us not a whit;\nFor why? his doom is writ;\nA word shall quickly slay him.", + "v" + ], + [ + "God’s word, for all their craft and force,\nOne moment will not linger,\nBut, spite of hell, shall have its course;\n’Tis written by His finger.\nAnd though they take our life,\nGoods, honour, children, wife,\nYet is their profit small:\nThese things shall vanish all;\nThe city of God remaineth.", + "v" + ] + ] +} From f5da7e2a2b96b50bd6294accf1adf343be0695fc Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:51:25 +0200 Subject: [PATCH 03/14] Make easyslide importer try to recover when reading non-standard xml. Fixes bug 1588822. Fixes: https://launchpad.net/bugs/1588822 --- openlp/plugins/songs/lib/importers/easyslides.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/importers/easyslides.py b/openlp/plugins/songs/lib/importers/easyslides.py index 907a6c90f..2ae489208 100644 --- a/openlp/plugins/songs/lib/importers/easyslides.py +++ b/openlp/plugins/songs/lib/importers/easyslides.py @@ -46,7 +46,7 @@ class EasySlidesImport(SongImport): def do_import(self): log.info('Importing EasySlides XML file {source}'.format(source=self.import_source)) - parser = etree.XMLParser(remove_blank_text=True) + parser = etree.XMLParser(remove_blank_text=True, recover=True) parsed_file = etree.parse(self.import_source, parser) xml = etree.tostring(parsed_file).decode() song_xml = objectify.fromstring(xml) From ba80fe653ca89893d458139b5370b34f1d665baa Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:52:00 +0200 Subject: [PATCH 04/14] Fix format error --- openlp/plugins/bibles/lib/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index d1465475d..1c55222f2 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -370,7 +370,7 @@ class BibleManager(RegistryProperties): """ log.debug('save_meta data {bible}, {version}, {copyright}, {perms}'.format(bible=bible, version=version, - cr=copyright, + copyright=copyright, perms=permissions)) self.db_cache[bible].save_meta('name', version) self.db_cache[bible].save_meta('copyright', copyright) From 93fc6e014537b0959fd09e6dee1ce3fe24961958 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Jul 2016 22:56:50 +0200 Subject: [PATCH 05/14] Update Crosswalk webpage parser to match new layout. Fixes bug 1599999. Fixes: https://launchpad.net/bugs/1599999 --- openlp/plugins/bibles/lib/http.py | 40 +++++++++++-------- .../openlp_plugins/bibles/test_lib_http.py | 1 - 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index c50745c2f..fce5d3285 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -532,28 +532,26 @@ class CWExtract(RegistryProperties): returns a list in the form [(biblename, biblekey, language_code)] """ log.debug('CWExtract.get_bibles_from_http') - bible_url = 'http://www.biblestudytools.com/' + bible_url = 'http://www.biblestudytools.com/bible-versions/' soup = get_soup_for_bible_ref(bible_url) if not soup: return None - bible_select = soup.find('select') - if not bible_select: - log.debug('No select tags found - did site change?') - return None - option_tags = bible_select.find_all('option', {'class': 'log-translation'}) - if not option_tags: - log.debug('No option tags found - did site change?') + h4_tags = soup.find_all('h4', {'class': 'small-header'}) + if not h4_tags: + log.debug('No h4 tags found - did site change?') return None bibles = [] - for ot in option_tags: - tag_text = ot.get_text().strip() - try: - tag_value = ot['value'] - except KeyError: - log.exception('No value attribute found - did site change?') + for h4t in h4_tags: + short_name = None + if h4t.span: + short_name = h4t.span.get_text().strip().lower() + else: + log.error('No span tag found - did site change?') return None - if not tag_value: + if not short_name: continue + h4t.span.extract() + tag_text = h4t.get_text().strip() # The names of non-english bibles has their language in parentheses at the end if tag_text.endswith(')'): language = tag_text[tag_text.rfind('(') + 1:-1] @@ -561,12 +559,20 @@ class CWExtract(RegistryProperties): language_code = CROSSWALK_LANGUAGES[language] else: language_code = '' - # ... except for the latin vulgate + # ... except for those that don't... elif 'latin' in tag_text.lower(): language_code = 'la' + elif 'la biblia' in tag_text.lower() or 'nueva' in tag_text.lower(): + language_code = 'es' + elif 'chinese' in tag_text.lower(): + language_code = 'zh' + elif 'greek' in tag_text.lower(): + language_code = 'el' + elif 'nova' in tag_text.lower(): + language_code = 'pt' else: language_code = 'en' - bibles.append((tag_text, tag_value, language_code)) + bibles.append((tag_text, short_name, language_code)) return bibles diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py index 4a7fb4af3..4ca4a8b0f 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py @@ -146,7 +146,6 @@ class TestBibleHTTP(TestCase): self.assertIsNotNone(bibles) self.assertIn(('Holman Christian Standard Bible', 'HCSB', 'en'), bibles) - @skip("Waiting for Crosswalk to fix their server") def test_crosswalk_get_bibles(self): """ Test getting list of bibles from Crosswalk.com From ebefc03674ecb0c59d715b19cc78233c724aa325 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 23 Jul 2016 23:41:24 +0200 Subject: [PATCH 06/14] Fix the disappearance of a variable :-) --- openlp/core/lib/theme.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index fc20037e7..c27b08cd0 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -474,6 +474,7 @@ class ThemeXML(object): if element.startswith('shadow') or element.startswith('outline'): master = 'font_main' # fix bold font + ret_value = None if element == 'weight': element = 'bold' if value == 'Normal': @@ -482,7 +483,7 @@ class ThemeXML(object): ret_value = True if element == 'proportion': element = 'size' - return False, master, element, ret_value + return False, master, element, ret_value if ret_value is not None else value def _create_attr(self, master, element, value): """ From 031ae9ebc181765c1e70e47cda5a13da86d92b1b Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sun, 24 Jul 2016 21:49:29 +0200 Subject: [PATCH 07/14] Use BibleGateway standard site instead of the legacy site. Fixes bug 1562384. Fixes: https://launchpad.net/bugs/1562384 --- openlp/plugins/bibles/lib/http.py | 10 +++++----- .../openlp_plugins/bibles/test_lib_http.py | 18 +++++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/openlp/plugins/bibles/lib/http.py b/openlp/plugins/bibles/lib/http.py index fce5d3285..392cce05a 100644 --- a/openlp/plugins/bibles/lib/http.py +++ b/openlp/plugins/bibles/lib/http.py @@ -252,7 +252,7 @@ class BGExtract(RegistryProperties): chapter=chapter, version=version) soup = get_soup_for_bible_ref( - 'http://legacy.biblegateway.com/passage/?{url}'.format(url=url_params), + 'http://biblegateway.com/passage/?{url}'.format(url=url_params), pre_parse_regex=r'', pre_parse_substitute='') if not soup: return None @@ -281,7 +281,7 @@ class BGExtract(RegistryProperties): """ log.debug('BGExtract.get_books_from_http("{version}")'.format(version=version)) url_params = urllib.parse.urlencode({'action': 'getVersionInfo', 'vid': '{version}'.format(version=version)}) - reference_url = 'http://legacy.biblegateway.com/versions/?{url}#books'.format(url=url_params) + reference_url = 'http://biblegateway.com/versions/?{url}#books'.format(url=url_params) page = get_web_page(reference_url) if not page: send_error_message('download') @@ -312,7 +312,7 @@ class BGExtract(RegistryProperties): for book in content: book = book.find('td') if book: - books.append(book.contents[0]) + books.append(book.contents[1]) return books def get_bibles_from_http(self): @@ -322,11 +322,11 @@ class BGExtract(RegistryProperties): returns a list in the form [(biblename, biblekey, language_code)] """ log.debug('BGExtract.get_bibles_from_http') - bible_url = 'https://legacy.biblegateway.com/versions/' + bible_url = 'https://biblegateway.com/versions/' soup = get_soup_for_bible_ref(bible_url) if not soup: return None - bible_select = soup.find('select', {'class': 'translation-dropdown'}) + bible_select = soup.find('select', {'class': 'search-translation-select'}) if not bible_select: log.debug('No select tags found - did site change?') return None diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py index 4ca4a8b0f..de4d4bba8 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_http.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_http.py @@ -50,7 +50,8 @@ class TestBibleHTTP(TestCase): books = handler.get_books_from_http('NIV') # THEN: We should get back a valid service item - assert len(books) == 66, 'The bible should not have had any books added or removed' + self.assertEqual(len(books), 66, 'The bible should not have had any books added or removed') + self.assertEqual(books[0], 'Genesis', 'The first bible book should be Genesis') def test_bible_gateway_extract_books_support_redirect(self): """ @@ -63,7 +64,7 @@ class TestBibleHTTP(TestCase): books = handler.get_books_from_http('DN1933') # THEN: We should get back a valid service item - assert len(books) == 66, 'This bible should have 66 books' + self.assertEqual(len(books), 66, 'This bible should have 66 books') def test_bible_gateway_extract_verse(self): """ @@ -76,7 +77,8 @@ class TestBibleHTTP(TestCase): results = handler.get_bible_chapter('NIV', 'John', 3) # THEN: We should get back a valid service item - assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed' + self.assertEqual(len(results.verse_list), 36, + 'The book of John should not have had any verses added or removed') def test_bible_gateway_extract_verse_nkjv(self): """ @@ -89,7 +91,8 @@ class TestBibleHTTP(TestCase): results = handler.get_bible_chapter('NKJV', 'John', 3) # THEN: We should get back a valid service item - assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed' + self.assertEqual(len(results.verse_list), 36, + 'The book of John should not have had any verses added or removed') def test_crosswalk_extract_books(self): """ @@ -102,7 +105,7 @@ class TestBibleHTTP(TestCase): books = handler.get_books_from_http('niv') # THEN: We should get back a valid service item - assert len(books) == 66, 'The bible should not have had any books added or removed' + self.assertEqual(len(books), 66, 'The bible should not have had any books added or removed') def test_crosswalk_extract_verse(self): """ @@ -115,7 +118,8 @@ class TestBibleHTTP(TestCase): results = handler.get_bible_chapter('niv', 'john', 3) # THEN: We should get back a valid service item - assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed' + self.assertEqual(len(results.verse_list), 36, + 'The book of John should not have had any verses added or removed') def test_bibleserver_get_bibles(self): """ @@ -144,7 +148,7 @@ class TestBibleHTTP(TestCase): # THEN: The list should not be None, and some known bibles should be there self.assertIsNotNone(bibles) - self.assertIn(('Holman Christian Standard Bible', 'HCSB', 'en'), bibles) + self.assertIn(('Holman Christian Standard Bible (HCSB)', 'HCSB', 'en'), bibles) def test_crosswalk_get_bibles(self): """ From ea455d9b3201f485cbf3ff38bcf9877b1b58f810 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 24 Jul 2016 22:41:27 +0200 Subject: [PATCH 08/14] Write some tests --- .../functional/openlp_core_lib/test_theme.py | 95 +++++++++++++------ 1 file changed, 67 insertions(+), 28 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index c006abb78..abacface8 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -22,43 +22,82 @@ """ Package to test the openlp.core.lib.theme package. """ -from tests.functional import MagicMock, patch from unittest import TestCase +import os from openlp.core.lib.theme import ThemeXML -class TestTheme(TestCase): +class TestThemeXML(TestCase): """ - Test the functions in the Theme module + Test the ThemeXML class """ - def setUp(self): - """ - Create the UI - """ - pass - - def tearDown(self): - """ - Delete all the C++ objects at the end so that we don't have a segfault - """ - pass - def test_new_theme(self): """ - Test the theme creation - basic test + Test the ThemeXML constructor """ - # GIVEN: A new theme - - # WHEN: A theme is created + # GIVEN: The ThemeXML class + # WHEN: A theme object is created default_theme = ThemeXML() - # THEN: We should get some default behaviours - self.assertTrue(default_theme.background_border_color == '#000000', 'The theme should have a black border') - self.assertTrue(default_theme.background_type == 'solid', 'The theme should have a solid backgrounds') - self.assertTrue(default_theme.display_vertical_align == 0, - 'The theme should have a display_vertical_align of 0') - self.assertTrue(default_theme.font_footer_name == "Arial", - 'The theme should have a font_footer_name of Arial') - self.assertTrue(default_theme.font_main_bold is False, 'The theme should have a font_main_bold of false') - self.assertTrue(len(default_theme.__dict__) == 47, 'The theme should have 47 variables') + # THEN: The default values should be correct + self.assertEqual('#000000', default_theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"') + self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0') + self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"') + self.assertFalse(default_theme.font_main_bold, 'font_main_bold should be False') + self.assertEqual(47, len(default_theme.__dict__), 'The theme should have 47 attributes') + + def test_expand_json(self): + """ + Test the expand_json method + """ + # GIVEN: A ThemeXML object and some JSON to "expand" + theme = ThemeXML() + theme_json = { + 'background': { + 'border_color': '#000000', + 'type': 'solid' + }, + 'display': { + 'vertical_align': 0 + }, + 'font': { + 'footer': { + 'bold': False + }, + 'main': { + 'name': 'Arial' + } + } + } + + # WHEN: ThemeXML.expand_json() is run + theme.expand_json(theme_json) + + # THEN: The attributes should be set on the object + self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('solid', theme.background_type, 'background_type should be "solid"') + self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0') + self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False') + self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"') + + def test_extend_image_filename(self): + """ + Test the extend_image_filename method + """ + # GIVEN: A theme object + theme = ThemeXML() + theme.theme_name = 'MyBeautifulTheme ' + theme.background_filename = ' video.mp4' + theme.background_type = 'video' + path = os.path.expanduser('~') + + # WHEN: ThemeXML.extend_image_filename is run + theme.extend_image_filename(path) + + # THEN: The filename of the background should be correct + expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4') + self.assertEqual(expected_filename, theme.background_filename) + self.assertEqual('MyBeautifulTheme', theme.theme_name) + From 0dcae022bcf59f06bfb630605854153903bf0348 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 24 Jul 2016 22:49:00 +0200 Subject: [PATCH 09/14] some pep8 fixes --- tests/functional/openlp_core_lib/test_theme.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index abacface8..db6b78d02 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -41,7 +41,8 @@ class TestThemeXML(TestCase): default_theme = ThemeXML() # THEN: The default values should be correct - self.assertEqual('#000000', default_theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('#000000', default_theme.background_border_color, + 'background_border_color should be "#000000"') self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"') self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0') self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"') @@ -100,4 +101,3 @@ class TestThemeXML(TestCase): expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4') self.assertEqual(expected_filename, theme.background_filename) self.assertEqual('MyBeautifulTheme', theme.theme_name) - From 189b2dd400105a94b531ab321d7bac76dd27dd95 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 1 Aug 2016 19:42:29 +0200 Subject: [PATCH 10/14] Improve pylint testing, fixed a few issues. --- openlp/plugins/songs/lib/openlyricsxml.py | 4 ++-- tests/utils/test_pylint.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/openlyricsxml.py b/openlp/plugins/songs/lib/openlyricsxml.py index 5adffb300..e2964661d 100644 --- a/openlp/plugins/songs/lib/openlyricsxml.py +++ b/openlp/plugins/songs/lib/openlyricsxml.py @@ -458,7 +458,7 @@ class OpenLyrics(object): self._add_tag_to_formatting(tag, tags_element) # Replace end tags. for tag in end_tags: - text = text.replace('{/{tag}}}'.format(tag=tag), '') + text = text.replace('{{{tag}}}'.format(tag=tag), '') # Replace \n with
. text = text.replace('\n', '
') element = etree.XML('{text}'.format(text=text)) @@ -643,7 +643,7 @@ class OpenLyrics(object): # Append text from tail and add formatting end tag. # TODO: Verify format() with template variables if element.tag == NSMAP % 'tag' and use_endtag: - text += '{/{name}}}'.format(name=element.get('name')) + text += '{{{name}}}'.format(name=element.get('name')) # Append text from tail. if element.tail: text += element.tail diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index dc6c83909..1675a41ee 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -48,7 +48,7 @@ class TestPylint(TestCase): """ # GIVEN: Some checks to disable and enable, and the pylint script disabled_checks = 'import-error,no-member' - enabled_checks = 'missing-format-argument-key,unused-format-string-argument' + enabled_checks = 'missing-format-argument-key,unused-format-string-argument,bad-format-string' if is_win() or 'arch' in platform.dist()[0].lower(): pylint_script = 'pylint' else: From 76be31fee1f71221b4c38885404b61d354ec855f Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 1 Aug 2016 20:00:58 +0200 Subject: [PATCH 11/14] Only run pylint tests if specified. --- tests/utils/test_pylint.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index 1675a41ee..3d1333967 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -23,8 +23,8 @@ Package to test for proper bzr tags. """ import os -import logging import platform +import sys from unittest import TestCase, SkipTest try: @@ -35,6 +35,14 @@ except ImportError: from openlp.core.common import is_win +in_argv = False +for arg in sys.argv: + if arg.endswith('test_pylint.py'): + in_argv = True + break +if not in_argv: + raise SkipTest('test_pylint.py not specified in arguments - skipping tests using pylint.') + TOLERATED_ERRORS = {'registryproperties.py': ['access-member-before-definition'], 'opensong.py': ['no-name-in-module'], 'maindisplay.py': ['no-name-in-module']} From 7ca7ae9c074a596c9ab2143f342f031253181a30 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 1 Aug 2016 20:49:01 +0200 Subject: [PATCH 12/14] Ignore distutils errors --- tests/utils/test_pylint.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index 3d1333967..6b60fcf79 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -92,6 +92,9 @@ class TestPylint(TestCase): # Filter out PyQt related errors elif ('no-name-in-module' in line or 'no-member' in line) and 'PyQt5' in line: continue + # Filter out distutils related errors + elif 'distutils' in line: + continue elif self._is_line_tolerated(line): continue else: From c47321210770c4fec2edf9aada7cf401fc76c0db Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 2 Aug 2016 20:57:10 +0200 Subject: [PATCH 13/14] Added pylint test to jenkins script, and fixed a format error. --- openlp/core/lib/screen.py | 2 +- scripts/jenkins_script.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/openlp/core/lib/screen.py b/openlp/core/lib/screen.py index e7b4c0b97..31ff3d725 100644 --- a/openlp/core/lib/screen.py +++ b/openlp/core/lib/screen.py @@ -167,7 +167,7 @@ class ScreenList(object): :param number: The screen number (int). """ - log.info('remove_screen {number:d}'.forma(number=number)) + log.info('remove_screen {number:d}'.format(number=number)) for screen in self.screen_list: if screen['number'] == number: self.screen_list.remove(screen) diff --git a/scripts/jenkins_script.py b/scripts/jenkins_script.py index 61f74986a..0711d1257 100755 --- a/scripts/jenkins_script.py +++ b/scripts/jenkins_script.py @@ -63,9 +63,10 @@ class OpenLPJobs(object): Branch_Windows_Interface = 'Branch-04b-Windows_Interface_Tests' Branch_PEP = 'Branch-05a-Code_Analysis' Branch_Coverage = 'Branch-05b-Test_Coverage' + Branch_Pylint = 'Branch-05c-Code_Analysis2' Jobs = [Branch_Pull, Branch_Functional, Branch_Interface, Branch_Windows_Functional, Branch_Windows_Interface, - Branch_PEP, Branch_Coverage] + Branch_PEP, Branch_Coverage, Branch_Pylint] class Colour(object): From 82faa1da5e5e73fe81723d908038f23dc3d181ea Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 2 Aug 2016 21:32:36 +0200 Subject: [PATCH 14/14] Change pylint test so it works with nose2 --- tests/utils/test_pylint.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/utils/test_pylint.py b/tests/utils/test_pylint.py index 6b60fcf79..48c9e1393 100644 --- a/tests/utils/test_pylint.py +++ b/tests/utils/test_pylint.py @@ -35,14 +35,6 @@ except ImportError: from openlp.core.common import is_win -in_argv = False -for arg in sys.argv: - if arg.endswith('test_pylint.py'): - in_argv = True - break -if not in_argv: - raise SkipTest('test_pylint.py not specified in arguments - skipping tests using pylint.') - TOLERATED_ERRORS = {'registryproperties.py': ['access-member-before-definition'], 'opensong.py': ['no-name-in-module'], 'maindisplay.py': ['no-name-in-module']} @@ -54,6 +46,15 @@ class TestPylint(TestCase): """ Test for pylint errors """ + # Test if this file is specified in the arguments, if not skip the test. + in_argv = False + for arg in sys.argv: + if arg.endswith('test_pylint.py') or arg.endswith('test_pylint'): + in_argv = True + break + if not in_argv: + raise SkipTest('test_pylint.py not specified in arguments - skipping tests using pylint.') + # GIVEN: Some checks to disable and enable, and the pylint script disabled_checks = 'import-error,no-member' enabled_checks = 'missing-format-argument-key,unused-format-string-argument,bad-format-string'