diff --git a/openlp/plugins/songs/lib/__init__.py b/openlp/plugins/songs/lib/__init__.py index 95868ffa7..ec14175ac 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..140126f26 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 None + self.assertIsNone(result, 'The result should be None, but was "%s"' % result)