From 1b6a54c55d64ee1f47d3276c15450cbba3ff838d Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Fri, 15 Feb 2013 19:57:05 +0000 Subject: [PATCH 01/15] started on tests for SongShowPlusImport --- openlp/plugins/songs/lib/__init__.py | 2 +- .../openlp_plugins_songs_lib/__init__.py | 0 .../test_songshowplusimport.py | 43 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 tests/functional/openlp_plugins_songs_lib/__init__.py create mode 100644 tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index c7c24533b..4041bb12e 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -167,7 +167,7 @@ class VerseType(object): translate('SongsPlugin.VerseType', 'Intro'), translate('SongsPlugin.VerseType', 'Ending'), translate('SongsPlugin.VerseType', 'Other')] - TranslatedTags = [name[0].lower() for name in TranslatedNames] + TranslatedTags = [unicode(name[0]).lower() for name in TranslatedNames] @staticmethod def translated_tag(verse_tag, default=Other): diff --git a/tests/functional/openlp_plugins_songs_lib/__init__.py b/tests/functional/openlp_plugins_songs_lib/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py new file mode 100644 index 000000000..6189c6a7f --- /dev/null +++ b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py @@ -0,0 +1,43 @@ +""" + Package to test the openlp.plugins.songs.lib package. +""" +import os + +from unittest import TestCase +from mock import MagicMock, patch +from openlp.plugins.songs.lib import songshowplusimport + +TESTPATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..', u'..', u'resources')) + + +class TestSongShowPlusImport(TestCase): + + def default_test(self): + """ + Test the defaults of songshowplusimport + """ + # Given: The songshowplusimport module as imported + + # When: Imported the module should have defaults set + constants = {u'TITLE' : 1, u'AUTHOR' : 2, u'COPYRIGHT' : 3, u'CCLI_NO' : 5, u'VERSE' : 12, u'CHORUS' : 20, + u'BRIDGE' : 24, u'TOPIC' : 29, u'COMMENTS' : 30, u'VERSE_ORDER' : 31, u'SONG_BOOK' : 35, + u'SONG_NUMBER' : 36, u'CUSTOM_VERSE' : 37, u'SongShowPlusImport.otherList' : {}, + u'SongShowPlusImport.otherCount' : 0} + + # Then: The constants should not have changed. + for constant in constants: + value = constants[constant] + self.assertEquals(eval(u'songshowplusimport.%s' % constant), value, + u'%s should be set as %s' % (constant, value)) + + + def do_import_test(self): + mocked_manager = MagicMock() + songshowplusimport.SongImport = MagicMock() + + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport') as mocked_song_import: + ssp_import_class = songshowplusimport.SongShowPlusImport(mocked_manager) + + songshowplusimport.SongShowPlusImport.importSource = '' + + self.assertEquals(ssp_import_class.SongShowPlusImport().doImport(), False) From ca2b2db6402fbe94e55e8698a00dc193714de7e2 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 17 Feb 2013 19:37:59 +0000 Subject: [PATCH 02/15] fixed duplicates in verse order when adding verses with the same tag. fixed handling of part verses eg (1a, 1b, 1.5, etc) in SongShowPlus importer --- openlp/plugins/songs/lib/songimport.py | 3 ++- .../plugins/songs/lib/songshowplusimport.py | 16 +++++++++----- .../test_songshowplusimport.py | 22 +++++-------------- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py index f6a84945c..0d563935f 100644 --- a/openlp/plugins/songs/lib/songimport.py +++ b/openlp/plugins/songs/lib/songimport.py @@ -260,7 +260,8 @@ class SongImport(QtCore.QObject): elif int(verse_def[1:]) > self.verseCounts[verse_def[0]]: self.verseCounts[verse_def[0]] = int(verse_def[1:]) self.verses.append([verse_def, verse_text.rstrip(), lang]) - self.verseOrderListGenerated.append(verse_def) + if verse_def not in self.verseOrderListGenerated: + self.verseOrderListGenerated.append(verse_def) def repeatVerse(self): """ diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index c5bb8832d..8e4957c71 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -32,6 +32,7 @@ SongShow Plus songs into the OpenLP database. """ import os import logging +import re import struct from openlp.core.ui.wizard import WizardStrings @@ -44,13 +45,13 @@ COPYRIGHT = 3 CCLI_NO = 5 VERSE = 12 CHORUS = 20 +BRIDGE = 24 TOPIC = 29 COMMENTS = 30 VERSE_ORDER = 31 SONG_BOOK = 35 SONG_NUMBER = 36 CUSTOM_VERSE = 37 -BRIDGE = 24 log = logging.getLogger(__name__) @@ -183,13 +184,16 @@ class SongShowPlusImport(SongImport): self.logError(file) def toOpenLPVerseTag(self, verse_name, ignore_unique=False): - if verse_name.find(" ") != -1: - verse_parts = verse_name.split(" ") - verse_type = verse_parts[0] - verse_number = verse_parts[1] + # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have + # concept of part verses, so just ignore any non integers on the end (including floats)) + match = re.match(u'(\D*)(\d+)', verse_name) + if match is not None: + verse_type = match.group(1).strip() + verse_number = match.group(2) else: + # otherwise we assume number 1 and take the whole prefix as the verse tag verse_type = verse_name - verse_number = "1" + verse_number = u'1' verse_type = verse_type.lower() if verse_type == "verse": verse_tag = VerseType.Tags[VerseType.Verse] diff --git a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py index 6189c6a7f..77733a1ba 100644 --- a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py @@ -12,28 +12,16 @@ TESTPATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..', u'..', class TestSongShowPlusImport(TestCase): - def default_test(self): - """ - Test the defaults of songshowplusimport - """ - # Given: The songshowplusimport module as imported +#test do import + # set self.import source to non list type. Do import should return None or False? + # set self.import source to a list of files + # importWizard.progressBar should be set to the number of files in the list + # set self.stop_import_flag to true. Do import should return None or False? - # When: Imported the module should have defaults set - constants = {u'TITLE' : 1, u'AUTHOR' : 2, u'COPYRIGHT' : 3, u'CCLI_NO' : 5, u'VERSE' : 12, u'CHORUS' : 20, - u'BRIDGE' : 24, u'TOPIC' : 29, u'COMMENTS' : 30, u'VERSE_ORDER' : 31, u'SONG_BOOK' : 35, - u'SONG_NUMBER' : 36, u'CUSTOM_VERSE' : 37, u'SongShowPlusImport.otherList' : {}, - u'SongShowPlusImport.otherCount' : 0} - - # Then: The constants should not have changed. - for constant in constants: - value = constants[constant] - self.assertEquals(eval(u'songshowplusimport.%s' % constant), value, - u'%s should be set as %s' % (constant, value)) def do_import_test(self): mocked_manager = MagicMock() - songshowplusimport.SongImport = MagicMock() with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport') as mocked_song_import: ssp_import_class = songshowplusimport.SongShowPlusImport(mocked_manager) From ea86ce905c382cc674a439f3a5e76bf57b679524 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Mon, 18 Feb 2013 17:15:07 +0000 Subject: [PATCH 03/15] Simplified if statment --- openlp/plugins/songs/lib/songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 8e4957c71..c9f9617df 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -187,7 +187,7 @@ class SongShowPlusImport(SongImport): # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have # concept of part verses, so just ignore any non integers on the end (including floats)) match = re.match(u'(\D*)(\d+)', verse_name) - if match is not None: + if match: verse_type = match.group(1).strip() verse_number = match.group(2) else: From 86fb3437599062d4972d6a0c7815ebdee1b7ca2f Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 24 Mar 2013 11:32:03 +0000 Subject: [PATCH 04/15] Added test for SongShowPlus --- .../plugins/songs/lib/songshowplusimport.py | 17 +- .../songs/test_songshowplusimport.py | 169 ++++++++++++++++++ .../openlp_plugins_songs_lib/__init__.py | 0 .../test_songshowplusimport.py | 31 ---- tests/resources/Amazing Grace.sbsong | Bin 0 -> 1018 bytes 5 files changed, 178 insertions(+), 39 deletions(-) create mode 100644 tests/functional/openlp_plugins/songs/test_songshowplusimport.py delete mode 100644 tests/functional/openlp_plugins_songs_lib/__init__.py delete mode 100644 tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py create mode 100644 tests/resources/Amazing Grace.sbsong diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 84e888856..57a0f3236 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -89,8 +89,9 @@ class SongShowPlusImport(SongImport): * .sbsong """ - otherList = {} - otherCount = 0 + + other_count = 0 + other_list = {} def __init__(self, manager, **kwargs): """ @@ -109,8 +110,8 @@ class SongShowPlusImport(SongImport): if self.stop_import_flag: return self.sspVerseOrderList = [] - other_count = 0 - other_list = {} + self.other_count = 0 + self.other_list = {} file_name = os.path.split(file)[1] self.import_wizard.increment_progress_bar(WizardStrings.ImportingType % file_name, 0) song_data = open(file, 'rb') @@ -204,11 +205,11 @@ class SongShowPlusImport(SongImport): elif verse_type == "pre-chorus": verse_tag = VerseType.tags[VerseType.PreChorus] else: - if verse_name not in self.otherList: + if verse_name not in self.other_list: if ignore_unique: return None - self.otherCount += 1 - self.otherList[verse_name] = str(self.otherCount) + self.other_count += 1 + self.other_list[verse_name] = str(self.other_count) verse_tag = VerseType.tags[VerseType.Other] - verse_number = self.otherList[verse_name] + verse_number = self.other_list[verse_name] return verse_tag + verse_number diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py new file mode 100644 index 000000000..38962b79d --- /dev/null +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -0,0 +1,169 @@ +""" +This module contains tests for the OpenLP song importer. +""" + +import os +from unittest import TestCase +from mock import call, patch, MagicMock + +from openlp.plugins.songs.lib import VerseType +from openlp.plugins.songs.lib.songshowplusimport import SongShowPlusImport + +TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'../../../resources')) + +class TestSongShowPlusFileImport(TestCase): + """ + Test the functions in the :mod:`lib` module. + """ + def create_importer_test(self): + """ + Test creating an instance of the SongShow Plus file importer + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + + # WHEN: An importer object is created + importer = SongShowPlusImport(mocked_manager) + + # THEN: The importer object should not be None + self.assertIsNotNone(importer, u'Import should not be none') + + def toOpenLPVerseTag_test(self): + """ + Test toOpenLPVerseTag method + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + importer = SongShowPlusImport(mocked_manager) + + # WHEN: Supplied with the following arguments replicating verses being added + test_values = [(u'Verse 1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse 2', VerseType.tags[VerseType.Verse] + u'2'), + (u'verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'chorus 1', VerseType.tags[VerseType.Chorus] + u'1'), + (u'bridge 1', VerseType.tags[VerseType.Bridge] + u'1'), + (u'pre-chorus 1', VerseType.tags[VerseType.PreChorus] + u'1'), + (u'different 1', VerseType.tags[VerseType.Other] + u'1'), + (u'random 1', VerseType.tags[VerseType.Other] + u'2')] + + # THEN: The returned value should should correlate with the input arguments + for original_tag, openlp_tag in test_values: + self.assertEquals(importer.toOpenLPVerseTag(original_tag), openlp_tag, + u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + % (openlp_tag, original_tag)) + + # WHEN: Supplied with the following arguments replicating a verse order being added + test_values = [(u'Verse 1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse 2', VerseType.tags[VerseType.Verse] + u'2'), + (u'verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse', VerseType.tags[VerseType.Verse] + u'1'), + (u'Verse1', VerseType.tags[VerseType.Verse] + u'1'), + (u'chorus 1', VerseType.tags[VerseType.Chorus] + u'1'), + (u'bridge 1', VerseType.tags[VerseType.Bridge] + u'1'), + (u'pre-chorus 1', VerseType.tags[VerseType.PreChorus] + u'1'), + (u'different 1', VerseType.tags[VerseType.Other] + u'1'), + (u'random 1', VerseType.tags[VerseType.Other] + u'2'), + (u'unused 2', None)] + + # THEN: The returned value should should correlate with the input arguments + for original_tag, openlp_tag in test_values: + self.assertEquals(importer.toOpenLPVerseTag(original_tag, ignore_unique=True), openlp_tag, + u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + % (openlp_tag, original_tag)) + + + + + def import_source_test(self): + """ + Test creating an instance of the SongShow Plus file importer + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True + + # WHEN: Import source is a string + importer.import_source = u'not a list' + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is an int + importer.import_source = 0 + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is a list + importer.import_source = [u'List', u'of', u'files'] + + # THEN: doImport should return none and the progress bar maximum should be set. + self.assertIsNone(importer.doImport(), + u'doImport should return None when import_source is a list and stop_import_flag is True') + mocked_import_wizard.progress_bar.setMaximum.assert_called_with( + len(importer.import_source)) + + def file_import_test(self): + """ + Test creating an instance of the SongShow Plus file importer + """ + + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + mocked_parse_author = MagicMock() + mocked_add_copyright = MagicMock() + mocked_add_verse = MagicMock() + mocked_finish = MagicMock() + mocked_finish.return_value = True + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = False + importer.parse_author = mocked_parse_author + importer.addCopyright = mocked_add_copyright + importer.addVerse = mocked_add_verse + importer.finish = mocked_finish + importer.topics = [] + + # WHEN: Import source is a string + importer.import_source = [os.path.join(TEST_PATH, u'Amazing Grace.sbsong')] + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(importer.title, u'Amazing Grace (Demonstration)', + u'Title for Amazing Grace.sbsong should be "Amazing Grace (Demonstration)"') + calls = [call(u'John Newton'), call(u'Edwin Excell'), call(u'John P. Rees')] + mocked_parse_author.assert_has_calls(calls) + mocked_add_copyright.assert_called_with(u'Public Domain ') + self.assertEquals(importer.ccliNumber, 22025, u'ccliNumber should be set as 22025 for Amazing Grace.sbsong') + calls = [call(u'Amazing grace! How sweet the sound!\r\nThat saved a wretch like me!\r\n' + u'I once was lost, but now am found;\r\nWas blind, but now I see.', u'v1'), + call(u"'Twas grace that taught my heart to fear,\r\nAnd grace my fears relieved.\r\n" + u"How precious did that grace appear,\r\nThe hour I first believed.", u'v2'), + call(u'The Lord has promised good to me,\r\nHis Word my hope secures.\r\n' + u'He will my shield and portion be\r\nAs long as life endures.', u'v3'), + call(u"Thro' many dangers, toils and snares\r\nI have already come.\r\n" + u"'Tis grace that brought me safe thus far,\r\nAnd grace will lead me home.", u'v4'), + call(u"When we've been there ten thousand years,\r\nBright shining as the sun,\r\n" + u"We've no less days to sing God's praise,\r\nThan when we first begun.", u'v5')] + mocked_add_verse.assert_has_calls(calls) + self.assertEquals(importer.topics, [u'Assurance', u'Grace', u'Praise', u'Salvation']) + self.assertEquals(importer.comments, u'\n\n\n', u'comments should be "\\n\\n\\n" Amazing Grace.sbsong') + self.assertEquals(importer.songBookName, u'Demonstration Songs', u'songBookName should be ' + u'"Demonstration Songs"') + self.assertEquals(importer.songNumber, 0, u'songNumber should be 0') + self.assertEquals(importer.verseOrderList, [], u'verseOrderList should be empty') + mocked_finish.assert_called_with() \ No newline at end of file diff --git a/tests/functional/openlp_plugins_songs_lib/__init__.py b/tests/functional/openlp_plugins_songs_lib/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py b/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py deleted file mode 100644 index 77733a1ba..000000000 --- a/tests/functional/openlp_plugins_songs_lib/test_songshowplusimport.py +++ /dev/null @@ -1,31 +0,0 @@ -""" - Package to test the openlp.plugins.songs.lib package. -""" -import os - -from unittest import TestCase -from mock import MagicMock, patch -from openlp.plugins.songs.lib import songshowplusimport - -TESTPATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'..', u'..', u'resources')) - - -class TestSongShowPlusImport(TestCase): - -#test do import - # set self.import source to non list type. Do import should return None or False? - # set self.import source to a list of files - # importWizard.progressBar should be set to the number of files in the list - # set self.stop_import_flag to true. Do import should return None or False? - - - - def do_import_test(self): - mocked_manager = MagicMock() - - with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport') as mocked_song_import: - ssp_import_class = songshowplusimport.SongShowPlusImport(mocked_manager) - - songshowplusimport.SongShowPlusImport.importSource = '' - - self.assertEquals(ssp_import_class.SongShowPlusImport().doImport(), False) diff --git a/tests/resources/Amazing Grace.sbsong b/tests/resources/Amazing Grace.sbsong new file mode 100644 index 0000000000000000000000000000000000000000..14b7c35971ad673c98ca9ad8e128e0ad3922444c GIT binary patch literal 1018 zcmZ8g-)j>=5KdZq+S7<2K31nFL?J{8_~cWhwT*}r8_4tKZgRJ{w;Oi%dX4xXC@3QS z&3<#&AL_#;vpcijeDlprzt`(M!k6q#EA>g+f{wh(n4TVR#*FANZ?v30Foz50$#@$t?#ZjWC-u_kj1F9- z5cULjg1FV&!S79p*qKaTOkO51`}lU{u8X*JW!;7)U$Xm#=!j^#q&ql%lYoBm<+8X! zirc4S*HCDfBgK*_xZ39XgLGc1NI{)(PKp}OF)PXFk4zQAJ0oWyOrruB7vhMPbtDTQ zRnbZiUJcR(oT$a-*WMWg=CN@3C0w?WAH%s|v`mm5DWj^3GE%jnl9k8V(F(?BkWOuW z5eTQ;1@de(gW`CQN)>C*nRa!cT<0BH2dviX4q}c1OILfE(MtOeX?Y1CoIVSu?c`jd z-Z`IB32JNaDjlFg;T%96>Iau&9cUpT!qcrG8)voWAVeUGHby+5)NG(1h_9WO*+D`S zBBEiqfNu1PiEZA#6%OBp!;R$Yy!38Jm9iVkl`Ys~R-)4;v}nO9B$GCj=nyI6S>+qb zT*Y88oP*t8k}kdLGzCqCe6fT?tN%1@IUB&BK$HX^q4Qhl>?A)IC0lBEh-6EKiAnJQ zYApyZ6>g*>kmj}5(m>R1WrI*;J65%YZ_y%HM}`Bsq&9Fm3hk!3d?;!wh>b{$9};$1 zuX5fJlwC-YlNRmz#i=r9?Fv7HTNWzWPSX_sy+1Rg B8W{ip literal 0 HcmV?d00001 From c3492d6aedd1ce06f3cb680f8eb3c72ae61b2968 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 24 Mar 2013 13:25:31 +0000 Subject: [PATCH 05/15] Added an extra test song, and cleaned up the test some what --- .../songs/test_songshowplusimport.py | 228 +++++++++++------- .../Beautiful Garden Of Prayer.sbsong | Bin 0 -> 964 bytes 2 files changed, 140 insertions(+), 88 deletions(-) create mode 100644 tests/resources/Beautiful Garden Of Prayer.sbsong diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 38962b79d..4fb1e2479 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -1,5 +1,5 @@ """ -This module contains tests for the OpenLP song importer. +This module contains tests for the SongShow Plus song importer. """ import os @@ -10,10 +10,51 @@ from openlp.plugins.songs.lib import VerseType from openlp.plugins.songs.lib.songshowplusimport import SongShowPlusImport TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'../../../resources')) +SONG_TEST_DATA = {u'Amazing Grace.sbsong': + {u'title': u'Amazing Grace (Demonstration)', + u'authors': [u'John Newton', u'Edwin Excell', u'John P. Rees'], + u'copyright': u'Public Domain ', + u'ccli_number': 22025, + u'verses': + [(u'Amazing grace! How sweet the sound!\r\nThat saved a wretch like me!\r\n' + u'I once was lost, but now am found;\r\nWas blind, but now I see.', u'v1'), + (u'\'Twas grace that taught my heart to fear,\r\nAnd grace my fears relieved.\r\n' + u'How precious did that grace appear,\r\nThe hour I first believed.', u'v2'), + (u'The Lord has promised good to me,\r\nHis Word my hope secures.\r\n' + u'He will my shield and portion be\r\nAs long as life endures.', u'v3'), + (u'Thro\' many dangers, toils and snares\r\nI have already come.\r\n' + u'\'Tis grace that brought me safe thus far,\r\nAnd grace will lead me home.', u'v4'), + (u'When we\'ve been there ten thousand years,\r\nBright shining as the sun,\r\n' + u'We\'ve no less days to sing God\'s praise,\r\nThan when we first begun.', u'v5')], + u'topics': [u'Assurance', u'Grace', u'Praise', u'Salvation'], + u'comments': u'\n\n\n', + u'song_book_name': u'Demonstration Songs', + u'song_number': 0, + u'verse_order_list': []}, + u'Beautiful Garden Of Prayer.sbsong': + {u'title': u'Beautiful Garden Of Prayer (Demonstration)', + u'authors': [u'Eleanor Allen Schroll', u'James H. Fillmore'], + u'copyright': u'Public Domain ', + u'ccli_number': 60252, + u'verses': + [(u'There\'s a garden where Jesus is waiting,\r\nThere\'s a place that is wondrously fair.\r\n' + u'For it glows with the light of His presence,\r\n\'Tis the beautiful garden of prayer.', u'v1'), + (u'There\'s a garden where Jesus is waiting,\r\nAnd I go with my burden and care.\r\n' + u'Just to learn from His lips, words of comfort,\r\nIn the beautiful garden of prayer.', u'v2'), + (u'There\'s a garden where Jesus is waiting,\r\nAnd He bids you to come meet Him there,\r\n' + u'Just to bow and receive a new blessing,\r\nIn the beautiful garden of prayer.', u'v3'), + (u'O the beautiful garden, the garden of prayer,\r\nO the beautiful garden of prayer.\r\n' + u'There my Savior awaits, and He opens the gates\r\nTo the beautiful garden of prayer.', u'c1')], + u'topics': [u'Devotion', u'Prayer'], + u'comments': u'', + u'song_book_name': u'', + u'song_number': 0, + u'verse_order_list': []}} -class TestSongShowPlusFileImport(TestCase): + +class TestSongShowPlusImport(TestCase): """ - Test the functions in the :mod:`lib` module. + Test the functions in the :mod:`songshowplusimport` module. """ def create_importer_test(self): """ @@ -29,6 +70,43 @@ class TestSongShowPlusFileImport(TestCase): # THEN: The importer object should not be None self.assertIsNotNone(importer, u'Import should not be none') + def import_source_test(self): + """ + Test SongShowPlusImport.doImport handles different import_source values + """ + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True + + # WHEN: Import source is a string + importer.import_source = u'not a list' + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is an int + importer.import_source = 0 + + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + + # WHEN: Import source is a list + importer.import_source = [u'List', u'of', u'files'] + + # THEN: doImport should return none and the progress bar setMaximum should be called with the length of + # import_source. + self.assertIsNone(importer.doImport(), + u'doImport should return None when import_source is a list and stop_import_flag is True') + mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(importer.import_source)) + def toOpenLPVerseTag_test(self): """ Test toOpenLPVerseTag method @@ -75,95 +153,69 @@ class TestSongShowPlusFileImport(TestCase): u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) - - - - def import_source_test(self): - """ - Test creating an instance of the SongShow Plus file importer - """ - # GIVEN: A mocked out SongImport class, and a mocked out "manager" - with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = SongShowPlusImport(mocked_manager) - importer.import_wizard = mocked_import_wizard - importer.stop_import_flag = True - - # WHEN: Import source is a string - importer.import_source = u'not a list' - - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') - - # WHEN: Import source is an int - importer.import_source = 0 - - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') - - # WHEN: Import source is a list - importer.import_source = [u'List', u'of', u'files'] - - # THEN: doImport should return none and the progress bar maximum should be set. - self.assertIsNone(importer.doImport(), - u'doImport should return None when import_source is a list and stop_import_flag is True') - mocked_import_wizard.progress_bar.setMaximum.assert_called_with( - len(importer.import_source)) - def file_import_test(self): """ - Test creating an instance of the SongShow Plus file importer + Test the actual import of real song files and check that the imported data is correct. """ - # GIVEN: A mocked out SongImport class, and a mocked out "manager" + # GIVEN: Test files with a mocked out SongImport class, a mocked out "manager", a mocked out "import_wizard", + # and mocked out "author", "add_copyright", "add_verse", "finish" methods. with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - mocked_parse_author = MagicMock() - mocked_add_copyright = MagicMock() - mocked_add_verse = MagicMock() - mocked_finish = MagicMock() - mocked_finish.return_value = True - importer = SongShowPlusImport(mocked_manager) - importer.import_wizard = mocked_import_wizard - importer.stop_import_flag = False - importer.parse_author = mocked_parse_author - importer.addCopyright = mocked_add_copyright - importer.addVerse = mocked_add_verse - importer.finish = mocked_finish - importer.topics = [] + for song_file in SONG_TEST_DATA: + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + mocked_parse_author = MagicMock() + mocked_add_copyright = MagicMock() + mocked_add_verse = MagicMock() + mocked_finish = MagicMock() + mocked_finish.return_value = True + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = False + importer.parse_author = mocked_parse_author + importer.addCopyright = mocked_add_copyright + importer.addVerse = mocked_add_verse + importer.finish = mocked_finish + importer.topics = [] - # WHEN: Import source is a string - importer.import_source = [os.path.join(TEST_PATH, u'Amazing Grace.sbsong')] + # WHEN: Importing each file + importer.import_source = [os.path.join(TEST_PATH, song_file)] + title = SONG_TEST_DATA[song_file][u'title'] + parse_author_calls = [call(author) for author in SONG_TEST_DATA[song_file][u'authors']] + song_copyright = SONG_TEST_DATA[song_file][u'copyright'] + ccli_number = SONG_TEST_DATA[song_file][u'ccli_number'] + add_verse_calls = \ + [call(verse_text, verse_tag) for verse_text, verse_tag in SONG_TEST_DATA[song_file][u'verses']] + topics = SONG_TEST_DATA[song_file][u'topics'] + comments = SONG_TEST_DATA[song_file][u'comments'] + song_book_name = SONG_TEST_DATA[song_file][u'song_book_name'] + song_number = SONG_TEST_DATA[song_file][u'song_number'] + verse_order_list = SONG_TEST_DATA[song_file][u'verse_order_list'] - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(importer.title, u'Amazing Grace (Demonstration)', - u'Title for Amazing Grace.sbsong should be "Amazing Grace (Demonstration)"') - calls = [call(u'John Newton'), call(u'Edwin Excell'), call(u'John P. Rees')] - mocked_parse_author.assert_has_calls(calls) - mocked_add_copyright.assert_called_with(u'Public Domain ') - self.assertEquals(importer.ccliNumber, 22025, u'ccliNumber should be set as 22025 for Amazing Grace.sbsong') - calls = [call(u'Amazing grace! How sweet the sound!\r\nThat saved a wretch like me!\r\n' - u'I once was lost, but now am found;\r\nWas blind, but now I see.', u'v1'), - call(u"'Twas grace that taught my heart to fear,\r\nAnd grace my fears relieved.\r\n" - u"How precious did that grace appear,\r\nThe hour I first believed.", u'v2'), - call(u'The Lord has promised good to me,\r\nHis Word my hope secures.\r\n' - u'He will my shield and portion be\r\nAs long as life endures.', u'v3'), - call(u"Thro' many dangers, toils and snares\r\nI have already come.\r\n" - u"'Tis grace that brought me safe thus far,\r\nAnd grace will lead me home.", u'v4'), - call(u"When we've been there ten thousand years,\r\nBright shining as the sun,\r\n" - u"We've no less days to sing God's praise,\r\nThan when we first begun.", u'v5')] - mocked_add_verse.assert_has_calls(calls) - self.assertEquals(importer.topics, [u'Assurance', u'Grace', u'Praise', u'Salvation']) - self.assertEquals(importer.comments, u'\n\n\n', u'comments should be "\\n\\n\\n" Amazing Grace.sbsong') - self.assertEquals(importer.songBookName, u'Demonstration Songs', u'songBookName should be ' - u'"Demonstration Songs"') - self.assertEquals(importer.songNumber, 0, u'songNumber should be 0') - self.assertEquals(importer.verseOrderList, [], u'verseOrderList should be empty') - mocked_finish.assert_called_with() \ No newline at end of file + # THEN: doImport should return none, the song data should be as expected, and finish should have been + # called. + self.assertIsNone(importer.doImport(), u'doImport should return None when it has completed') + self.assertEquals(importer.title, title, u'title for %s should be "%s"' % (song_file, title)) + mocked_parse_author.assert_has_calls(parse_author_calls) + if song_copyright: + mocked_add_copyright.assert_called_with(song_copyright) + if ccli_number: + self.assertEquals(importer.ccliNumber, ccli_number, u'ccliNumber for %s should be %s' + % (song_file, ccli_number)) + mocked_add_verse.assert_has_calls(add_verse_calls) + if topics: + self.assertEquals(importer.topics, topics, u'topics for %s should be %s' % (song_file, topics)) + if comments: + self.assertEquals(importer.comments, comments, u'comments for %s should be "%s"' + % (song_file, comments)) + if song_book_name: + self.assertEquals(importer.songBookName, song_book_name, u'songBookName for %s should be "%s"' + % (song_file, song_book_name)) + if song_number: + self.assertEquals(importer.songNumber, song_number, u'songNumber for %s should be %s' + % (song_file, song_number)) + if verse_order_list: + self.assertEquals(importer.verseOrderList, [], u'verseOrderList for %s should be %s' + % (song_file, verse_order_list)) + mocked_finish.assert_called_with() + \ No newline at end of file diff --git a/tests/resources/Beautiful Garden Of Prayer.sbsong b/tests/resources/Beautiful Garden Of Prayer.sbsong new file mode 100644 index 0000000000000000000000000000000000000000..c227d48097fb3f8722a874447c39476a80e35828 GIT binary patch literal 964 zcmb7@ZEMs(5XW;@8r#!~puQ9a6zW4frJ(Ptv0m>@3)e5O$xV`l%}&_eT(~czpd#X@ zbS61^DhJgU8)hf}o&C+jj-n_^c*W1(p=yIJS_2=ITcU-F0xq3eql2d@)?|HfDrmCL z9<%!kmMHe$zCL+#60n0Q7yOfjBJ53>Ok^vn4?}H_s?PnnL+P|cHK<+ zfGK|n=_vkou`{it_x{}trr0f)Qf(Tx85iK9FVsr&Kq#3cE~-FtrqrMv+Fq)Fp7pd7 c;kTHNn03M{K1gvz^7zCflOD&z+hjBP1!=b$FaQ7m literal 0 HcmV?d00001 From 401da98d7cf4d3357038a50035e36f57bf89a584 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 24 Mar 2013 14:56:22 +0000 Subject: [PATCH 06/15] increased line lengths to 120 renamed SongShowPlusImport vairables and methods to standards --- .../plugins/songs/lib/songshowplusimport.py | 35 ++++++++----------- .../songs/test_songshowplusimport.py | 11 +++--- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 57a0f3236..14631ebc2 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -57,31 +57,24 @@ log = logging.getLogger(__name__) class SongShowPlusImport(SongImport): """ - The :class:`SongShowPlusImport` class provides the ability to import song - files from SongShow Plus. + The :class:`SongShowPlusImport` class provides the ability to import song files from SongShow Plus. **SongShow Plus Song File Format:** The SongShow Plus song file format is as follows: - * Each piece of data in the song file has some information that precedes - it. + * Each piece of data in the song file has some information that precedes it. * The general format of this data is as follows: - 4 Bytes, forming a 32 bit number, a key if you will, this describes what - the data is (see blockKey below) - 4 Bytes, forming a 32 bit number, which is the number of bytes until the - next block starts + 4 Bytes, forming a 32 bit number, a key if you will, this describes what the data is (see blockKey below) + 4 Bytes, forming a 32 bit number, which is the number of bytes until the next block starts 1 Byte, which tells how many bytes follows - 1 or 4 Bytes, describes how long the string is, if its 1 byte, the string - is less than 255 + 1 or 4 Bytes, describes how long the string is, if its 1 byte, the string is less than 255 The next bytes are the actual data. The next block of data follows on. - This description does differ for verses. Which includes extra bytes - stating the verse type or number. In some cases a "custom" verse is used, - in that case, this block will in include 2 strings, with the associated - string length descriptors. The first string is the name of the verse, the - second is the verse content. + This description does differ for verses. Which includes extra bytes stating the verse type or number. In some cases + a "custom" verse is used, in that case, this block will in include 2 strings, with the associated string length + descriptors. The first string is the name of the verse, the second is the verse content. The file is ended with four null bytes. @@ -109,7 +102,7 @@ class SongShowPlusImport(SongImport): for file in self.import_source: if self.stop_import_flag: return - self.sspVerseOrderList = [] + self.ssp_verse_order_list = [] self.other_count = 0 self.other_list = {} file_name = os.path.split(file)[1] @@ -164,27 +157,27 @@ class SongShowPlusImport(SongImport): elif block_key == COMMENTS: self.comments = unicode(data, u'cp1252') elif block_key == VERSE_ORDER: - verse_tag = self.toOpenLPVerseTag(data, True) + verse_tag = self.to_openlp_verse_tag(data, True) if verse_tag: if not isinstance(verse_tag, unicode): verse_tag = unicode(verse_tag, u'cp1252') - self.sspVerseOrderList.append(verse_tag) + self.ssp_verse_order_list.append(verse_tag) elif block_key == SONG_BOOK: self.songBookName = unicode(data, u'cp1252') elif block_key == SONG_NUMBER: self.songNumber = ord(data) elif block_key == CUSTOM_VERSE: - verse_tag = self.toOpenLPVerseTag(verse_name) + verse_tag = self.to_openlp_verse_tag(verse_name) self.addVerse(unicode(data, u'cp1252'), verse_tag) else: log.debug("Unrecognised blockKey: %s, data: %s" % (block_key, data)) song_data.seek(next_block_starts) - self.verseOrderList = self.sspVerseOrderList + self.verseOrderList = self.ssp_verse_order_list song_data.close() if not self.finish(): self.logError(file) - def toOpenLPVerseTag(self, verse_name, ignore_unique=False): + def to_openlp_verse_tag(self, verse_name, ignore_unique=False): # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have # concept of part verses, so just ignore any non integers on the end (including floats)) match = re.match(u'(\D*)(\d+)', verse_name) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 4fb1e2479..9db0ee421 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -109,7 +109,7 @@ class TestSongShowPlusImport(TestCase): def toOpenLPVerseTag_test(self): """ - Test toOpenLPVerseTag method + Test to_openlp_verse_tag method """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): @@ -130,8 +130,8 @@ class TestSongShowPlusImport(TestCase): # THEN: The returned value should should correlate with the input arguments for original_tag, openlp_tag in test_values: - self.assertEquals(importer.toOpenLPVerseTag(original_tag), openlp_tag, - u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + self.assertEquals(importer.to_openlp_verse_tag(original_tag), openlp_tag, + u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) # WHEN: Supplied with the following arguments replicating a verse order being added @@ -149,8 +149,8 @@ class TestSongShowPlusImport(TestCase): # THEN: The returned value should should correlate with the input arguments for original_tag, openlp_tag in test_values: - self.assertEquals(importer.toOpenLPVerseTag(original_tag, ignore_unique=True), openlp_tag, - u'SongShowPlusImport.toOpenLPVerseTag should return "%s" when called with "%s"' + self.assertEquals(importer.to_openlp_verse_tag(original_tag, ignore_unique=True), openlp_tag, + u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) def file_import_test(self): @@ -218,4 +218,3 @@ class TestSongShowPlusImport(TestCase): self.assertEquals(importer.verseOrderList, [], u'verseOrderList for %s should be %s' % (song_file, verse_order_list)) mocked_finish.assert_called_with() - \ No newline at end of file From 4d0f9f3f914221852870b509323bbe792caf2954 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 19:50:06 +0000 Subject: [PATCH 07/15] Tided up the test somewhat --- .../songs/test_songshowplusimport.py | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 9db0ee421..0fae3fd56 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -88,7 +88,15 @@ class TestSongShowPlusImport(TestCase): # THEN: doImport should return none and the progress bar maximum should not be set. self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') + u'setMaxium on import_wizard.progress_bar should not have been called') + + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True # WHEN: Import source is an int importer.import_source = 0 @@ -96,7 +104,15 @@ class TestSongShowPlusImport(TestCase): # THEN: doImport should return none and the progress bar maximum should not be set. self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') + u'setMaxium on import_wizard.progress_bar should not have been called') + + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + mocked_import_wizard = MagicMock() + importer = SongShowPlusImport(mocked_manager) + importer.import_wizard = mocked_import_wizard + importer.stop_import_flag = True # WHEN: Import source is a list importer.import_source = [u'List', u'of', u'files'] @@ -104,10 +120,10 @@ class TestSongShowPlusImport(TestCase): # THEN: doImport should return none and the progress bar setMaximum should be called with the length of # import_source. self.assertIsNone(importer.doImport(), - u'doImport should return None when import_source is a list and stop_import_flag is True') + u'doImport should return None when import_source is a list and stop_import_flag is True') mocked_import_wizard.progress_bar.setMaximum.assert_called_with(len(importer.import_source)) - def toOpenLPVerseTag_test(self): + def to_openlp_verse_tag_test(self): """ Test to_openlp_verse_tag method """ @@ -134,6 +150,11 @@ class TestSongShowPlusImport(TestCase): u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) + # GIVEN: A mocked out SongImport class, and a mocked out "manager" + with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): + mocked_manager = MagicMock() + importer = SongShowPlusImport(mocked_manager) + # WHEN: Supplied with the following arguments replicating a verse order being added test_values = [(u'Verse 1', VerseType.tags[VerseType.Verse] + u'1'), (u'Verse 2', VerseType.tags[VerseType.Verse] + u'2'), From bc032eb7e247cc18ff39c4481060dbf9518a69d1 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 19:56:50 +0000 Subject: [PATCH 08/15] fixed indentation --- .../functional/openlp_plugins/songs/test_songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 0fae3fd56..aa2753485 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -106,7 +106,7 @@ class TestSongShowPlusImport(TestCase): self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, u'setMaxium on import_wizard.progress_bar should not have been called') - # GIVEN: A mocked out SongImport class, and a mocked out "manager" + # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): mocked_manager = MagicMock() mocked_import_wizard = MagicMock() From 2920eecd314298c4450d52741afd455620119801 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 21:09:38 +0000 Subject: [PATCH 09/15] Moved sample song files to their own directory --- .../{ => songshowplussongs}/Amazing Grace.sbsong | Bin .../Beautiful Garden Of Prayer.sbsong | Bin 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/resources/{ => songshowplussongs}/Amazing Grace.sbsong (100%) rename tests/resources/{ => songshowplussongs}/Beautiful Garden Of Prayer.sbsong (100%) diff --git a/tests/resources/Amazing Grace.sbsong b/tests/resources/songshowplussongs/Amazing Grace.sbsong similarity index 100% rename from tests/resources/Amazing Grace.sbsong rename to tests/resources/songshowplussongs/Amazing Grace.sbsong diff --git a/tests/resources/Beautiful Garden Of Prayer.sbsong b/tests/resources/songshowplussongs/Beautiful Garden Of Prayer.sbsong similarity index 100% rename from tests/resources/Beautiful Garden Of Prayer.sbsong rename to tests/resources/songshowplussongs/Beautiful Garden Of Prayer.sbsong From 86f294e8707c59875eb4875b4aad4b9c22b7884a Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 28 Mar 2013 21:17:07 +0000 Subject: [PATCH 10/15] Changed path in test file --- .../functional/openlp_plugins/songs/test_songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index aa2753485..24c77d0f3 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -9,7 +9,7 @@ from mock import call, patch, MagicMock from openlp.plugins.songs.lib import VerseType from openlp.plugins.songs.lib.songshowplusimport import SongShowPlusImport -TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'../../../resources')) +TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), u'../../../resources/songshowplussongs')) SONG_TEST_DATA = {u'Amazing Grace.sbsong': {u'title': u'Amazing Grace (Demonstration)', u'authors': [u'John Newton', u'Edwin Excell', u'John P. Rees'], From 3686da6a2eea5e1803094d70064bc0d3dd6714b6 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 31 Mar 2013 11:13:56 +0100 Subject: [PATCH 11/15] Changed regex string type to raw --- openlp/plugins/songs/lib/songshowplusimport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/songshowplusimport.py b/openlp/plugins/songs/lib/songshowplusimport.py index 14631ebc2..a72f83c4f 100644 --- a/openlp/plugins/songs/lib/songshowplusimport.py +++ b/openlp/plugins/songs/lib/songshowplusimport.py @@ -180,7 +180,7 @@ class SongShowPlusImport(SongImport): def to_openlp_verse_tag(self, verse_name, ignore_unique=False): # Have we got any digits? If so, verse number is everything from the digits to the end (OpenLP does not have # concept of part verses, so just ignore any non integers on the end (including floats)) - match = re.match(u'(\D*)(\d+)', verse_name) + match = re.match(r'(\D*)(\d+)', verse_name) if match: verse_type = match.group(1).strip() verse_number = match.group(2) From 7bae770458e471c05acd94fd27de78d3b8ad151f Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 31 Mar 2013 11:47:31 +0100 Subject: [PATCH 12/15] Some test had more than one GIVEN, WHEN, THEN. Fixed that --- .../openlp_plugins/songs/test_songshowplusimport.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 24c77d0f3..236c8bcc2 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -125,7 +125,7 @@ class TestSongShowPlusImport(TestCase): def to_openlp_verse_tag_test(self): """ - Test to_openlp_verse_tag method + Test to_openlp_verse_tag method by simulating adding a verse """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): @@ -150,6 +150,10 @@ class TestSongShowPlusImport(TestCase): u'SongShowPlusImport.to_openlp_verse_tag should return "%s" when called with "%s"' % (openlp_tag, original_tag)) + def to_openlp_verse_tag_verse_order_test(self): + """ + Test to_openlp_verse_tag method by simulating adding a verse to the verse order + """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): mocked_manager = MagicMock() From 8864e1e51b911f1ec2ecafe9dcf8c3c966a2ea6e Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Sun, 31 Mar 2013 11:55:58 +0100 Subject: [PATCH 13/15] Missed a few GIVEN, WHEN, THENS --- .../songs/test_songshowplusimport.py | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 236c8bcc2..1ae1e16f9 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -70,9 +70,9 @@ class TestSongShowPlusImport(TestCase): # THEN: The importer object should not be None self.assertIsNotNone(importer, u'Import should not be none') - def import_source_test(self): + def invalid_import_source_test(self): """ - Test SongShowPlusImport.doImport handles different import_source values + Test SongShowPlusImport.doImport handles different invalid import_source values """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): @@ -82,30 +82,19 @@ class TestSongShowPlusImport(TestCase): importer.import_wizard = mocked_import_wizard importer.stop_import_flag = True - # WHEN: Import source is a string - importer.import_source = u'not a list' + # WHEN: Import source is not a list + for source in [u'not a list', 0]: + importer.import_source = source - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') - - # GIVEN: A mocked out SongImport class, and a mocked out "manager" - with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): - mocked_manager = MagicMock() - mocked_import_wizard = MagicMock() - importer = SongShowPlusImport(mocked_manager) - importer.import_wizard = mocked_import_wizard - importer.stop_import_flag = True - - # WHEN: Import source is an int - importer.import_source = 0 - - # THEN: doImport should return none and the progress bar maximum should not be set. - self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') - self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, - u'setMaxium on import_wizard.progress_bar should not have been called') + # THEN: doImport should return none and the progress bar maximum should not be set. + self.assertIsNone(importer.doImport(), u'doImport should return None when import_source is not a list') + self.assertEquals(mocked_import_wizard.progress_bar.setMaximum.called, False, + u'setMaxium on import_wizard.progress_bar should not have been called') + def valid_import_source_test(self): + """ + Test SongShowPlusImport.doImport handles different invalid import_source values + """ # GIVEN: A mocked out SongImport class, and a mocked out "manager" with patch(u'openlp.plugins.songs.lib.songshowplusimport.SongImport'): mocked_manager = MagicMock() From c3583bd21dc7b684d25f69b8df2dec164ac122e3 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Thu, 4 Apr 2013 17:42:22 +0100 Subject: [PATCH 14/15] Added comment --- openlp/plugins/songs/lib/songimport.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openlp/plugins/songs/lib/songimport.py b/openlp/plugins/songs/lib/songimport.py index a9383c0cb..8886e2884 100644 --- a/openlp/plugins/songs/lib/songimport.py +++ b/openlp/plugins/songs/lib/songimport.py @@ -260,6 +260,8 @@ class SongImport(QtCore.QObject): elif int(verse_def[1:]) > self.verseCounts[verse_def[0]]: self.verseCounts[verse_def[0]] = int(verse_def[1:]) self.verses.append([verse_def, verse_text.rstrip(), lang]) + # A verse_def refers to all verses with that name, adding it once adds every instance, so do not add if already + # used. if verse_def not in self.verseOrderListGenerated: self.verseOrderListGenerated.append(verse_def) From 8a88cec855f42403ad9d15070bd3bda2cfa08f60 Mon Sep 17 00:00:00 2001 From: phill-ridout Date: Fri, 5 Apr 2013 19:26:39 +0100 Subject: [PATCH 15/15] Removed the use of the mock's call helper object --- .../openlp_plugins/songs/test_songshowplusimport.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py index 1ae1e16f9..86d77bbdc 100644 --- a/tests/functional/openlp_plugins/songs/test_songshowplusimport.py +++ b/tests/functional/openlp_plugins/songs/test_songshowplusimport.py @@ -4,7 +4,7 @@ This module contains tests for the SongShow Plus song importer. import os from unittest import TestCase -from mock import call, patch, MagicMock +from mock import patch, MagicMock from openlp.plugins.songs.lib import VerseType from openlp.plugins.songs.lib.songshowplusimport import SongShowPlusImport @@ -195,11 +195,10 @@ class TestSongShowPlusImport(TestCase): # WHEN: Importing each file importer.import_source = [os.path.join(TEST_PATH, song_file)] title = SONG_TEST_DATA[song_file][u'title'] - parse_author_calls = [call(author) for author in SONG_TEST_DATA[song_file][u'authors']] + author_calls = SONG_TEST_DATA[song_file][u'authors'] song_copyright = SONG_TEST_DATA[song_file][u'copyright'] ccli_number = SONG_TEST_DATA[song_file][u'ccli_number'] - add_verse_calls = \ - [call(verse_text, verse_tag) for verse_text, verse_tag in SONG_TEST_DATA[song_file][u'verses']] + add_verse_calls = SONG_TEST_DATA[song_file][u'verses'] topics = SONG_TEST_DATA[song_file][u'topics'] comments = SONG_TEST_DATA[song_file][u'comments'] song_book_name = SONG_TEST_DATA[song_file][u'song_book_name'] @@ -210,13 +209,15 @@ class TestSongShowPlusImport(TestCase): # called. self.assertIsNone(importer.doImport(), u'doImport should return None when it has completed') self.assertEquals(importer.title, title, u'title for %s should be "%s"' % (song_file, title)) - mocked_parse_author.assert_has_calls(parse_author_calls) + for author in author_calls: + mocked_parse_author.assert_any_call(author) if song_copyright: mocked_add_copyright.assert_called_with(song_copyright) if ccli_number: self.assertEquals(importer.ccliNumber, ccli_number, u'ccliNumber for %s should be %s' % (song_file, ccli_number)) - mocked_add_verse.assert_has_calls(add_verse_calls) + for verse_text, verse_tag in add_verse_calls: + mocked_add_verse.assert_any_call(verse_text, verse_tag) if topics: self.assertEquals(importer.topics, topics, u'topics for %s should be %s' % (song_file, topics)) if comments: