openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

Review: Needs Fixing

Just a few to fix up

Diff comments:

> 
> === modified file 'tests/functional/openlp_plugins/bibles/test_lib.py'
> --- tests/functional/openlp_plugins/bibles/test_lib.py	2017-06-05 02:58:38 +0000
> +++ tests/functional/openlp_plugins/bibles/test_lib.py	2017-12-23 09:39:35 +0000
> @@ -116,13 +116,13 @@
>  
>                                  # THEN: A match should be returned, and the book and reference should match the
>                                  #       expected result
> -                                self.assertIsNotNone(match, '{text} should provide a match'.format(text=reference_text))
> -                                self.assertEqual(book_result, match.group('book'),
> -                                                 '{text} does not provide the expected result for the book group.'
> -                                                 .format(text=reference_text))
> -                                self.assertEqual(ranges_result, match.group('ranges'),
> -                                                 '{text} does not provide the expected result for the ranges group.'
> -                                                 .format(text=reference_text))
> +                                assert match is not None, '{text} should provide a match'.format(text=reference_text)
> +                                assert book_result, match.group('book') == \

should be:
assert book_result == match.group('book'), \

> +                                    '{text} does not provide the expected result for the book group.'\
> +                                    .format(text=reference_text)
> +                                assert ranges_result, match.group('ranges') == \

again:
 assert ranges_result == match.group('ranges'), \

> +                                    '{text} does not provide the expected result for the ranges group.' \
> +                                    .format(text=reference_text)
>  
>      def test_reference_matched_range(self):
>          """
> 
> === modified file 'tests/functional/openlp_plugins/bibles/test_mediaitem.py'
> --- tests/functional/openlp_plugins/bibles/test_mediaitem.py	2017-10-07 07:05:07 +0000
> +++ tests/functional/openlp_plugins/bibles/test_mediaitem.py	2017-12-23 09:39:35 +0000
> @@ -746,8 +746,8 @@
>          # WHEN: Calling on_style_combo_box_index_changed
>          self.media_item.on_style_combo_box_index_changed(2)
>  
> -        # THEN: The layput_style settimg should have been set
> -        self.assertEqual(self.media_item.settings.layout_style, 2)
> +        # THEN: The layout_style setting should have been set
> +        assert self.media_item.settings.layout_style, 2

should be:
assert self.media_item.settings.layout_style == 2

>          self.media_item.settings.layout_style_combo_box.setCurrentIndex.assert_called_once_with(2)
>          self.mocked_settings_instance.setValue.assert_called_once_with('bibles/verse layout style', 2)
>  
> 
> === modified file 'tests/functional/openlp_plugins/presentations/test_pdfcontroller.py'
> --- tests/functional/openlp_plugins/presentations/test_pdfcontroller.py	2017-11-18 11:23:15 +0000
> +++ tests/functional/openlp_plugins/presentations/test_pdfcontroller.py	2017-12-23 09:39:35 +0000
> @@ -92,7 +92,7 @@
>          controller = PdfController(plugin=self.mock_plugin)
>  
>          # THEN: The name of the presentation controller should be correct
> -        self.assertEqual('Pdf', controller.name, 'The name of the presentation controller should be correct')
> +        assert'Pdf' == controller.name, 'The name of the presentation controller should be correct'

space between assert and 'pdf'

>  
>      def test_load_pdf(self):
>          """
> 
> === modified file 'tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py'
> --- tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py	2017-10-07 07:05:07 +0000
> +++ tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py	2017-12-23 09:39:35 +0000
> @@ -250,7 +249,7 @@
>          doc.goto_slide(1)
>  
>          # THEN: next_step() should be call to try to advance to the next effect.
> -        self.assertTrue(doc.next_step.called, 'next_step() should have been called!')
> +        assert doc.next_step.called, 'next_step() should have been called!' is True

should be:
assert doc.next_step.called is True, 'next_step() should have been called!'

>  
>      def test_blank_screen(self):
>          """
> 
> === modified file 'tests/functional/openlp_plugins/presentations/test_pptviewcontroller.py'
> --- tests/functional/openlp_plugins/presentations/test_pptviewcontroller.py	2017-10-07 07:05:07 +0000
> +++ tests/functional/openlp_plugins/presentations/test_pptviewcontroller.py	2017-12-23 09:39:35 +0000
> @@ -67,8 +67,7 @@
>          controller = PptviewController(plugin=self.mock_plugin)
>  
>          # THEN: The name of the presentation controller should be correct
> -        self.assertEqual('Powerpoint Viewer', controller.name,
> -                         'The name of the presentation controller should be correct')
> +        assert'Powerpoint Viewer' == controller.name, 'The name of the presentation controller should be correct'

assert 'Powerpoint Viewer' == controller.name, 'The name of the presentation controller should be correct'

>  
>      def test_check_available(self):
>          """
> 
> === modified file 'tests/functional/openlp_plugins/songs/test_ewimport.py'
> --- tests/functional/openlp_plugins/songs/test_ewimport.py	2017-12-22 10:45:39 +0000
> +++ tests/functional/openlp_plugins/songs/test_ewimport.py	2017-12-23 09:39:35 +0000
> @@ -165,11 +165,10 @@
>          field_desc_entry = FieldDescEntry(name, field_type, size)
>  
>          # THEN:
> -        self.assertIsNotNone(field_desc_entry, 'Import should not be none')
> -        self.assertEqual(field_desc_entry.name, name, 'FieldDescEntry.name should be the same as the name argument')
> -        self.assertEqual(field_desc_entry.field_type, field_type,
> -                         'FieldDescEntry.type should be the same as the type argument')
> -        self.assertEqual(field_desc_entry.size, size, 'FieldDescEntry.size should be the same as the size argument')
> +        assert field_desc_entry is not None, 'Import should not be none'
> +        assert field_desc_entry.name, name == 'FieldDescEntry.name should be the same as the name argument'

assert field_desc_entry.name == name, 'FieldDescEntry.name should be the same as the name argument'

> +        assert field_desc_entry.field_type == field_type, 'FieldDescEntry.type should be the same as the type argument'
> +        assert field_desc_entry.size == size, 'FieldDescEntry.size should be the same as the size argument'
>  
>      def test_create_importer(self):
>          """
> 
> === modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
> --- tests/functional/openlp_plugins/songs/test_songselect.py	2017-10-29 06:01:25 +0000
> +++ tests/functional/openlp_plugins/songs/test_songselect.py	2017-12-23 09:39:35 +0000
> @@ -384,19 +384,18 @@
>          result = importer.get_song(fake_song, callback=mocked_callback)
>  
>          # THEN: The callback should have been called three times and the song should be returned
> -        self.assertEqual(3, mocked_callback.call_count, 'The callback should have been called twice')
> -        self.assertIsNotNone(result, 'The get_song() method should have returned a song dictionary')
> -        self.assertEqual(2, mocked_lyrics_page.find.call_count, 'The find() method should have been called twice')
> -        self.assertEqual(2, mocked_find_all.call_count, 'The find_all() method should have been called twice')
> -        self.assertEqual([call('div', 'song-viewer lyrics'), call('div', 'song-viewer lyrics')],
> -                         mocked_lyrics_page.find.call_args_list,
> -                         'The find() method should have been called with the right arguments')
> -        self.assertEqual([call('p'), call('h3')], mocked_find_all.call_args_list,
> -                         'The find_all() method should have been called with the right arguments')
> -        self.assertIn('copyright', result, 'The returned song should have a copyright')
> -        self.assertIn('ccli_number', result, 'The returned song should have a CCLI number')
> -        self.assertIn('verses', result, 'The returned song should have verses')
> -        self.assertEqual(3, len(result['verses']), 'Three verses should have been returned')
> +        assert 3 == mocked_callback.call_count, 'The callback should have been called twice'
> +        assert result is not None, 'The get_song() method should have returned a song dictionary'
> +        assert 2 == mocked_lyrics_page.find.call_count, 'The find() method should have been called twice'
> +        assert 2 == mocked_find_all.call_count, 'The find_all() method should have been called twice'
> +        assert [call('div', 'song-viewer lyrics'), call('div', 'song-viewer lyrics')] == \
> +            mocked_lyrics_page.find.call_args_list, 'The find() method should have been called with the right arguments'
> +        assert [call('p'), call('h3')], mocked_find_all.call_args_list == \

assert [call('p'), call('h3')] == mocked_find_all.call_args_list, \

> +            'The find_all() method should have been called with the right arguments'
> +        assert 'copyright' in result, 'The returned song should have a copyright'
> +        assert 'ccli_number' in result, 'The returned song should have a CCLI number'
> +        assert 'verses' in result, 'The returned song should have verses'
> +        assert 3 == len(result['verses']), 'Three verses should have been returned'
>  
>      @patch('openlp.plugins.songs.lib.songselect.clean_song')
>      @patch('openlp.plugins.songs.lib.songselect.Topic')


-- 
https://code.launchpad.net/~trb143/openlp/asserts2/+merge/335571
Your team OpenLP Core is subscribed to branch lp:openlp.