openlp-core team mailing list archive

[Phill <phill.ridout@xxxxxxxxx>]

See my inline comments about your test doc stings and comments.

And also see my inline comments with regard to the Unicode literals. They're not required, and seem only to be implemented to ease porting from py2! https://www.python.org/dev/peps/pep-0414/#proposal

Are there any tests for the new module/method you've addded:
reporting.py
on_tools_report_song_list_triggered

Diff comments:

> 
> === modified file 'tests/functional/openlp_core_ui/test_servicemanager.py'
> --- tests/functional/openlp_core_ui/test_servicemanager.py	2016-07-17 19:46:06 +0000
> +++ tests/functional/openlp_core_ui/test_servicemanager.py	2016-09-08 05:12:22 +0000
> @@ -679,3 +680,72 @@
>          # THEN: The "save_as" method is called to save the service
>          self.assertTrue(result)
>          mocked_save_file_as.assert_called_with()
> +
> +    @patch(u'openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')

Do you really need the u' here? Isn't u' and ' the same?

> +    def test_theme_change_global(self, mocked_regenerate_service_items):
> +        """
> +        Test that when a Toolbar theme combobox displays correctly

Test what?

> +        """
> +
> +        # GIVEN: A service manager, a service to save with a theme level of the renderer
> +        mocked_renderer = MagicMock()
> +        service_manager = ServiceManager(None)
> +        Registry().register('renderer', mocked_renderer)
> +        service_manager.toolbar = OpenLPToolbar(None)
> +        service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
> +        service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
> +
> +        # WHEN: The service manager has a Global theme
> +        mocked_renderer.theme_level = ThemeLevel.Global
> +        result = service_manager.theme_change()
> +
> +        # THEN: The the theme toolbar should not be visible
> +
> +        self.assertFalse(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
> +                         'The visibility should be False')
> +
> +    @patch(u'openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')

And u' here?

> +    def test_theme_change_service(self, mocked_regenerate_service_items):
> +        """
> +        Test that when a Toolbar theme combobox displays correctly

Again test what?

> +        """
> +
> +        # GIVEN: A service manager, a service to save with a theme level of the renderer
> +        mocked_renderer = MagicMock()
> +        service_manager = ServiceManager(None)
> +        Registry().register('renderer', mocked_renderer)
> +        service_manager.toolbar = OpenLPToolbar(None)
> +        service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
> +        service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
> +
> +        # WHEN: The service manager has a Global theme

Service level theme

> +        mocked_renderer.theme_level = ThemeLevel.Service
> +        result = service_manager.theme_change()
> +
> +        # THEN: The the theme toolbar should not be visible

Should be visible

> +
> +        self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
> +                        'The visibility should be True')
> +
> +    @patch(u'openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
> +    def test_theme_change_song(self, mocked_regenerate_service_items):
> +        """

And u' again?

> +        Test that when a Toolbar theme combobox displays correctly

Again test what?

> +        """
> +
> +        # GIVEN: A service manager, a service to save with a theme level of the renderer
> +        mocked_renderer = MagicMock()
> +        service_manager = ServiceManager(None)
> +        Registry().register('renderer', mocked_renderer)
> +        service_manager.toolbar = OpenLPToolbar(None)
> +        service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
> +        service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
> +
> +        # WHEN: The service manager has a Global theme

Should be song level theme

> +        mocked_renderer.theme_level = ThemeLevel.Song
> +        result = service_manager.theme_change()
> +
> +        # THEN: The the theme toolbar should not be visible

Should be visible

> +
> +        self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
> +                        'The visibility should be True')


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