openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing



Diff comments:

> === modified file 'openlp/plugins/images/lib/mediaitem.py'
> --- openlp/plugins/images/lib/mediaitem.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/images/lib/mediaitem.py	2015-01-19 23:03:54 +0000
> @@ -371,7 +371,7 @@
>          """
>          self.application.set_normal_cursor()
>          self.load_list(files, target_group)
> -        last_dir = os.path.split(str(files[0]))[0]
> +        last_dir = os.path.split(files[0])[0]
>          Settings().setValue(self.settings_section + '/last directory', last_dir)
>  
>      def load_list(self, images, target_group=None, initial_load=False):
> @@ -535,7 +535,7 @@
>              if not items:
>                  return False
>          # Determine service item title
> -        if isinstance(items[0].data(0, QtCore.Qt.UserRole), ImageGroups):
> +        if isinstance(items[0].data(0, QtCore.Qt.UserRole), ImageGroups) or len(items) == 1:
>              service_item.title = items[0].text(0)
>          else:
>              service_item.title = str(self.plugin.name_strings['plural'])
> 
> === modified file 'tests/functional/__init__.py'
> --- tests/functional/__init__.py	2015-01-18 13:39:21 +0000
> +++ tests/functional/__init__.py	2015-01-19 23:03:54 +0000
> @@ -35,9 +35,9 @@
>  from PyQt4 import QtGui
>  
>  if sys.version_info[1] >= 3:
> -    from unittest.mock import MagicMock, patch, mock_open, call
> +    from unittest.mock import ANY, MagicMock, patch, mock_open, call
>  else:
> -    from mock import MagicMock, patch, mock_open, call
> +    from mock import ANY, MagicMock, patch, mock_open, call
>  
>  # Only one QApplication can be created. Use QtGui.QApplication.instance() when you need to "create" a  QApplication.
>  application = QtGui.QApplication([])
> 
> === modified file 'tests/functional/openlp_plugins/images/test_lib.py'
> --- tests/functional/openlp_plugins/images/test_lib.py	2015-01-18 13:39:21 +0000
> +++ tests/functional/openlp_plugins/images/test_lib.py	2015-01-19 23:03:54 +0000
> @@ -27,7 +27,7 @@
>  from openlp.core.common import Registry
>  from openlp.plugins.images.lib.db import ImageFilenames, ImageGroups
>  from openlp.plugins.images.lib.mediaitem import ImageMediaItem
> -from tests.functional import MagicMock, patch
> +from tests.functional import ANY, MagicMock, patch
>  
>  
>  class TestImageMediaItem(TestCase):
> @@ -38,6 +38,7 @@
>      def setUp(self):
>          self.mocked_main_window = MagicMock()
>          Registry.create()
> +        Registry().register('application', MagicMock())
>          Registry().register('service_list', MagicMock())
>          Registry().register('main_window', self.mocked_main_window)
>          Registry().register('live_controller', MagicMock())
> @@ -45,6 +46,43 @@
>          with patch('openlp.plugins.images.lib.mediaitem.MediaManagerItem._setup'), \
>                  patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.setup_item'):
>              self.media_item = ImageMediaItem(None, mocked_plugin)
> +            self.media_item.settings_section = 'images'
> +
> +    def validate_and_load_test(self):
> +        """
> +        Test that the validate_and_load_test() method
> +        """
> +        # GIVEN: A list of files
> +        file_list = ['/path1/image1.jpg', '/path2/image2.jpg']
> +
> +        with patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list') as mocked_load_list, \
> +                patch('openlp.plugins.images.lib.mediaitem.Settings') as mocked_settings:
> +
> +            # WHEN: Calling validate_and_load with the list of files
> +            self.media_item.validate_and_load(file_list)
> +
> +            # THEN: load_list should have been called with the file list and None,
> +            #       the dectory should have been saved to the settings

dectory?

> +            mocked_load_list.assert_called_once_with(file_list, None)
> +            mocked_settings().setValue.assert_called_once_with(ANY, '/path1')
> +
> +    def validate_and_load_group_test(self):
> +        """
> +        Test that the validate_and_load_test() method

Needs a different name as this is the text from the log and is identical to the above test

> +        """
> +        # GIVEN: A list of files
> +        file_list = ['/path1/image1.jpg', '/path2/image2.jpg']
> +
> +        with patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list') as mocked_load_list, \
> +                patch('openlp.plugins.images.lib.mediaitem.Settings') as mocked_settings:
> +
> +            # WHEN: Calling validate_and_load with the list of files and a group
> +            self.media_item.validate_and_load(file_list, 'group')
> +
> +            # THEN: load_list should have been called with the file list and the group name,
> +            #       the dectory should have been saved to the settings

and again

> +            mocked_load_list.assert_called_once_with(file_list, 'group')
> +            mocked_settings().setValue.assert_called_once_with(ANY, '/path1')
>  
>      def save_new_images_list_empty_list_test(self):
>          """
> 


-- 
https://code.launchpad.net/~phill-ridout/openlp/bug1410456/+merge/246958
Your team OpenLP Core is subscribed to branch lp:openlp.