From 1cd7d73d10b60ef5a0b058bcd5cbbc69a7e7d6bc Mon Sep 17 00:00:00 2001 From: Stefan Strasser Date: Fri, 25 Apr 2014 22:04:43 +0200 Subject: [PATCH 1/2] fixed bug #1312629 Error on entering invalid character in verse-order Fixes: https://launchpad.net/bugs/1312629 --- openlp/plugins/songs/lib/__init__.py | 6 ++-- .../openlp_plugins/songs/test_lib.py | 32 +++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index dc198d4b7..343bc710b 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -206,14 +206,14 @@ class VerseType(object): Return the VerseType for a given tag :param verse_tag: The string to return a VerseType for - :param default: Default return value if no matching tag is found + :param default: Default return value if no matching tag is found (a valid VerseType or None) :return: A VerseType of the tag """ verse_tag = verse_tag[0].lower() for num, tag in enumerate(VerseType.tags): if verse_tag == tag: return num - if len(VerseType.names) > default: + if default in range(0,len(VerseType.names)) or default is None: return default else: return VerseType.Other @@ -231,7 +231,7 @@ class VerseType(object): for num, tag in enumerate(VerseType.translated_tags): if verse_tag == tag: return num - if len(VerseType.names) > default: + if default in range(0,len(VerseType.names)) or default is None: return default else: return VerseType.Other diff --git a/tests/functional/openlp_plugins/songs/test_lib.py b/tests/functional/openlp_plugins/songs/test_lib.py index b67c1a4be..b06a18a7c 100644 --- a/tests/functional/openlp_plugins/songs/test_lib.py +++ b/tests/functional/openlp_plugins/songs/test_lib.py @@ -445,9 +445,9 @@ class TestVerseType(TestCase): # THEN: The result should be VerseType.Chorus self.assertEqual(result, VerseType.Chorus, 'The result should be VerseType.Chorus, but was "%s"' % result) - def from_tag_with_invalid_default_test(self): + def from_tag_with_invalid_intdefault_test(self): """ - Test that the from_tag() method returns a sane default when passed an invalid tag and an invalid default. + Test that the from_tag() method returns a sane default when passed an invalid tag and an invalid int default. """ # GIVEN: A mocked out translate() function that just returns what it was given with patch('openlp.plugins.songs.lib.translate') as mocked_translate: @@ -458,3 +458,31 @@ class TestVerseType(TestCase): # THEN: The result should be VerseType.Other self.assertEqual(result, VerseType.Other, 'The result should be VerseType.Other, but was "%s"' % result) + + def from_tag_with_invalid_default_test(self): + """ + Test that the from_tag() method returns a sane default when passed an invalid tag and an invalid default. + """ + # GIVEN: A mocked out translate() function that just returns what it was given + with patch('openlp.plugins.songs.lib.translate') as mocked_translate: + mocked_translate.side_effect = lambda x, y: y + + # WHEN: We run the from_tag() method with an invalid verse type, we get the specified default back + result = VerseType.from_tag('@', 'asdf') + + # THEN: The result should be VerseType.Other + self.assertEqual(result, VerseType.Other, 'The result should be VerseType.Other, but was "%s"' % result) + + def from_tag_with_none_default_test(self): + """ + Test that the from_tag() method returns a sane default when passed an invalid tag and None as default. + """ + # GIVEN: A mocked out translate() function that just returns what it was given + with patch('openlp.plugins.songs.lib.translate') as mocked_translate: + mocked_translate.side_effect = lambda x, y: y + + # WHEN: We run the from_tag() method with an invalid verse type, we get the specified default back + result = VerseType.from_tag('m', None) + + # THEN: The result should be VerseType.Other + self.assertIsNone(result, 'The result should be None, but was "%s"' % result) From c2d1589dcb2787891faed11d2b77f8be08a62762 Mon Sep 17 00:00:00 2001 From: Stefan Strasser Date: Sat, 26 Apr 2014 07:07:08 +0200 Subject: [PATCH 2/2] fixed a pep-issue in songs-lib and corrected a comment in a test --- openlp/plugins/songs/lib/__init__.py | 4 ++-- tests/functional/openlp_plugins/songs/test_lib.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 343bc710b..a0a8e94bd 100644 --- a/openlp/plugins/songs/lib/__init__.py +++ b/openlp/plugins/songs/lib/__init__.py @@ -213,7 +213,7 @@ class VerseType(object): for num, tag in enumerate(VerseType.tags): if verse_tag == tag: return num - if default in range(0,len(VerseType.names)) or default is None: + if default in range(0, len(VerseType.names)) or default is None: return default else: return VerseType.Other @@ -231,7 +231,7 @@ class VerseType(object): for num, tag in enumerate(VerseType.translated_tags): if verse_tag == tag: return num - if default in range(0,len(VerseType.names)) or default is None: + if default in range(0, len(VerseType.names)) or default is None: return default else: return VerseType.Other diff --git a/tests/functional/openlp_plugins/songs/test_lib.py b/tests/functional/openlp_plugins/songs/test_lib.py index b06a18a7c..140126f26 100644 --- a/tests/functional/openlp_plugins/songs/test_lib.py +++ b/tests/functional/openlp_plugins/songs/test_lib.py @@ -484,5 +484,5 @@ class TestVerseType(TestCase): # WHEN: We run the from_tag() method with an invalid verse type, we get the specified default back result = VerseType.from_tag('m', None) - # THEN: The result should be VerseType.Other + # THEN: The result should be None self.assertIsNone(result, 'The result should be None, but was "%s"' % result)