From b37fd8faca9cc5c38cb18c350b5eccba14482ce9 Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Tue, 7 Sep 2021 22:00:11 -0700 Subject: [PATCH] Fix #896 and write tests for the form --- openlp/core/ui/serviceitemeditform.py | 7 +- .../ui/test_serviceitemeditform.py | 273 ++++++++++++++++++ .../openlp_core/ui/test_servicenotedialog.py | 8 +- 3 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 tests/openlp_core/ui/test_serviceitemeditform.py diff --git a/openlp/core/ui/serviceitemeditform.py b/openlp/core/ui/serviceitemeditform.py index 680acc63f..a515596e7 100644 --- a/openlp/core/ui/serviceitemeditform.py +++ b/openlp/core/ui/serviceitemeditform.py @@ -59,11 +59,10 @@ class ServiceItemEditForm(QtWidgets.QDialog, Ui_ServiceItemEditDialog, RegistryP Get the modified service item. """ if self.data: + # self.data is only true for images self.item.slides = [] - if self.item.is_image(): - for item in self.item_list: - self.item.add_from_image(item['path'], item['title']) - self.item.render() + for item in self.item_list: + self.item.add_from_image(item['path'], item['title']) return self.item def load_data(self): diff --git a/tests/openlp_core/ui/test_serviceitemeditform.py b/tests/openlp_core/ui/test_serviceitemeditform.py new file mode 100644 index 000000000..4cb11a284 --- /dev/null +++ b/tests/openlp_core/ui/test_serviceitemeditform.py @@ -0,0 +1,273 @@ +# -*- coding: utf-8 -*- + +########################################################################## +# OpenLP - Open Source Lyrics Projection # +# ---------------------------------------------------------------------- # +# Copyright (c) 2008-2021 OpenLP Developers # +# ---------------------------------------------------------------------- # +# This program is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# This program is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with this program. If not, see . # +########################################################################## +""" +Package to test the :mod:`~openlp.core.ui.serviceitemeditform` package. +""" +import pytest +from unittest.mock import MagicMock, call, patch + +from PyQt5 import QtCore, QtTest + +from openlp.core.ui.serviceitemeditform import ServiceItemEditForm + + +@pytest.fixture +def form(settings): + frm = ServiceItemEditForm() + return frm + + +def test_basic_display(form): + """Test that the ServiceItemEditForm is displayed""" + # GIVEN: A dialog + # WHEN: Displaying the UI + with patch('openlp.core.ui.serviceitemeditform.QtWidgets.QDialog.exec'): + form.exec() + ok_widget = form.button_box.button(form.button_box.Save) + QtTest.QTest.mouseClick(ok_widget, QtCore.Qt.LeftButton) + + # THEN: Everything should be fine + assert form.item_list == [], 'The item list should be empty' + + +def test_set_service_item(form): + """Test that the ServiceItemEditForm.set_service_item() method works correctly""" + # GIVEN: A form with some methods mocked out + mocked_item = MagicMock(**{'is_image.return_value': True, 'slides': ['slide1.jpg']}) + form.load_data = MagicMock() + form.list_widget.setCurrentItem = MagicMock() + + # WHEN: set_service_item() is called + form.set_service_item(mocked_item) + + # THEN: The item should be set and the correct methods called + assert form.item is mocked_item, 'form.item should be mocked_item' + mocked_item.is_image.assert_called_once_with() + assert form.data is True, 'The data attribute should be true' + assert form.item_list == ['slide1.jpg'], 'form.item_list should be a list of 1 item' + form.load_data.assert_called_once_with() + form.list_widget.setCurrentItem.assert_called_once_with(None) + + +def test_get_service_item(form): + """Test that the ServiceItemEditForm.get_service_item() works correctly""" + # GIVEN: A form with some methods mocked out + form.data = True + form.item = MagicMock(slides=['slide1.jpg', 'slide2.jpg']) + form.item_list = [ + {'path': '../slide1.jpg', 'title': 'slide1.jpg'}, + {'path': '../slide2.jpg', 'title': 'slide2.jpg'} + ] + + # WHEN: get_service_item() is called + item = form.get_service_item() + + # THEN: The item should be returned and the correct methods called + assert item is form.item, 'The returned item should be form.item' + # FYI: list should be empty because it was cleared but never filled again due to the mock + assert item.slides == [], 'The list of slides should have been cleared' + assert item.add_from_image.call_args_list == [call('../slide1.jpg', 'slide1.jpg'), + call('../slide2.jpg', 'slide2.jpg')] + + +@patch('openlp.core.ui.serviceitemeditform.QtWidgets.QListWidgetItem') +def test_load_data(MockQListWidgetItem, form): + """Test that the ServiceItemEditForm.load_data() works correctly""" + # GIVEN: A form with some methods mocked out + MockQListWidgetItem.side_effect = lambda x: x + form.list_widget.clear = MagicMock() + form.list_widget.addItem = MagicMock() + form.item_list = [ + {'path': '../slide1.jpg', 'title': 'slide1.jpg'}, + {'path': '../slide2.jpg', 'title': 'slide2.jpg'} + ] + + # WHEN: load_data() is called + form.load_data() + + # THEN: The list widget should have the correct items added + form.list_widget.clear.assert_called_once_with() + assert form.list_widget.addItem.call_args_list == [call('slide1.jpg'), call('slide2.jpg')] + + +def test_on_delete_button_clicked_no_item(form): + """Test that nothing happens when the delete button is clicked but no item is selected""" + # GIVEN: A form with the currentItem() method mocked + form.list_widget.currentItem = MagicMock(return_value=None) + form.list_widget.row = MagicMock() + + # WHEN: on_delete_button_clicked() is called but no items are selected + form.on_delete_button_clicked() + + # THEN: Nothing should have happened + assert form.list_widget.row.call_count == 0, 'The row method should not have been called' + + +def test_on_delete_button_clicked(form): + """Test that clicking the delete button removes a row from the list""" + # GIVEN: A form with some items mocked out + mocked_list_item = MagicMock() + form.item_list = ['slide1', 'slide2'] + form.list_widget.currentItem = MagicMock(return_value=mocked_list_item) + form.list_widget.row = MagicMock(return_value=1) + form.load_data = MagicMock() + form.list_widget.count = MagicMock(return_value=1) + form.list_widget.setCurrentRow = MagicMock() + + # WHEN: on_delete_button_clicked() is called + form.on_delete_button_clicked() + + # THEN: The correct methods should have been called + form.list_widget.currentItem.assert_called_once_with() + form.list_widget.row.assert_called_once_with(mocked_list_item) + assert form.item_list == ['slide1'], 'There should be only 1 slide in the list' + form.load_data.assert_called_once_with() + form.list_widget.count.assert_called_once_with() + form.list_widget.setCurrentRow.assert_called_once_with(0) + + +def test_on_up_button_clicked(form): + """Test that the the up button click calls the correct method""" + # GIVEN: A form with a mocked out method + form._ServiceItemEditForm__move_item = MagicMock() + + # WHEN: The event handler is called + form.on_up_button_clicked() + + # THEN: The method should be called correctly + form._ServiceItemEditForm__move_item.assert_called_once_with('up') + + +def test_on_down_button_clicked(form): + """Test that the the down button click calls the correct method""" + # GIVEN: A form with a mocked out method + form._ServiceItemEditForm__move_item = MagicMock() + + # WHEN: The event handler is called + form.on_down_button_clicked() + + # THEN: The method should be called correctly + form._ServiceItemEditForm__move_item.assert_called_once_with('down') + + +def test_move_item_no_direction(form): + """Test that the __move_item() method exits early if no direction is specified""" + # GIVEN: A form with am ocked out method + form.list_widget.currentItem = MagicMock() + + # WHEN: __move_item() is called without a direction + form._ServiceItemEditForm__move_item() + + # THEN: The method should exit early, thus currentItem() should not have been called + assert form.list_widget.currentItem.call_count == 0, 'currentItem() should not have been called' + + +def test_move_item_no_item(form): + """Test that the __move_item() method exits early if no item in the list is selected""" + # GIVEN: A form with am ocked out method + form.list_widget.currentItem = MagicMock(return_value=None) + form.list_widget.row = MagicMock() + + # WHEN: __move_item() is called with a direction + form._ServiceItemEditForm__move_item('up') + + # THEN: The method should exit early, thus row() should not have been called + assert form.list_widget.row.call_count == 0, 'row() should not have been called' + + +def test_move_item_up(form): + """Test that the __move_item() method moves an item up""" + # GIVEN: A form with am ocked out method + mocked_list_item = MagicMock() + form.item_list = ['slide1', 'slide2'] + form.list_widget.currentItem = MagicMock(return_value=mocked_list_item) + form.list_widget.row = MagicMock(return_value=1) + form.load_data = MagicMock() + form.list_widget.setCurrentRow = MagicMock() + + # WHEN: __move_item() is called with the direction set to "up" + form._ServiceItemEditForm__move_item('up') + + # THEN: The internal list should be swapped around, and other methods should have been called + assert form.item_list == ['slide2', 'slide1'], 'The list should be swapped around' + form.list_widget.setCurrentRow.assert_called_once_with(0) + + +def test_move_item_down(form): + """Test that the __move_item() method moves an item down""" + # GIVEN: A form with am ocked out method + mocked_list_item = MagicMock() + form.item_list = ['slide1', 'slide2'] + form.list_widget.currentItem = MagicMock(return_value=mocked_list_item) + form.list_widget.row = MagicMock(return_value=0) + form.load_data = MagicMock() + form.list_widget.setCurrentRow = MagicMock() + + # WHEN: __move_item() is called with the direction set to "up" + form._ServiceItemEditForm__move_item('down') + + # THEN: The internal list should be swapped around, and other methods should have been called + assert form.item_list == ['slide2', 'slide1'], 'The list should be swapped around' + form.list_widget.setCurrentRow.assert_called_once_with(1) + + +def test_on_current_row_changed_no_selection(form): + """Test that all buttons are disabled when there is no selected item""" + # GIVEN: A form and a row of -1 + row = -1 + + # WHEN: on_current_row_changed() is called with the row + form.on_current_row_changed(row) + + # THEN: All the buttons should be disabled + assert form.down_button.isEnabled() is False, 'Down button should be disabled' + assert form.up_button.isEnabled() is False, 'Up button should be disabled' + assert form.delete_button.isEnabled() is False, 'Delete button should be disabled' + + +def test_on_current_row_changed_end_of_list(form): + """Test that the down button is disabled at the end of the list""" + # GIVEN: A form with a couple of mocked methods and a row of 5 + row = 5 + form.list_widget.count = MagicMock(return_value=6) + + # WHEN: on_current_row_changed() is called with the row + form.on_current_row_changed(row) + + # THEN: Down button should be disabled, up and delete should be enabled + assert form.down_button.isEnabled() is False, 'Down button should be disabled' + assert form.up_button.isEnabled() is True, 'Up button should be enabled' + assert form.delete_button.isEnabled() is True, 'Delete button should be enabled' + + +def test_on_current_row_changed_beginning_of_list(form): + """Test that the up button is disabled at the top of the list""" + # GIVEN: A form with a couple of mocked methods and a row of 0 + row = 0 + form.list_widget.count = MagicMock(return_value=6) + + # WHEN: on_current_row_changed() is called with the row + form.on_current_row_changed(row) + + # THEN: Up button should be disabled, down and delete should be enabled + assert form.down_button.isEnabled() is True, 'Down button should be enabled' + assert form.up_button.isEnabled() is False, 'Up button should be disabled' + assert form.delete_button.isEnabled() is True, 'Delete button should be enabled' diff --git a/tests/openlp_core/ui/test_servicenotedialog.py b/tests/openlp_core/ui/test_servicenotedialog.py index e08e7dd6e..fea1b1b2f 100644 --- a/tests/openlp_core/ui/test_servicenotedialog.py +++ b/tests/openlp_core/ui/test_servicenotedialog.py @@ -29,7 +29,7 @@ from PyQt5 import QtCore, QtTest from openlp.core.ui.servicenoteform import ServiceNoteForm -@pytest.fixture() +@pytest.fixture def form(settings): frm = ServiceNoteForm() return frm @@ -43,7 +43,7 @@ def test_basic_display(form): form.text_edit.setPlainText('') # WHEN displaying the UI and pressing enter - with patch('PyQt5.QtWidgets.QDialog.exec'): + with patch('openlp.core.ui.servicenoteform.QtWidgets.QDialog.exec'): form.exec() ok_widget = form.button_box.button(form.button_box.Save) QtTest.QTest.mouseClick(ok_widget, QtCore.Qt.LeftButton) @@ -54,7 +54,7 @@ def test_basic_display(form): # WHEN displaying the UI, having set the text and pressing enter text = 'OpenLP is the best worship software' form.text_edit.setPlainText(text) - with patch('PyQt5.QtWidgets.QDialog.exec'): + with patch('openlp.core.ui.servicenoteform.QtWidgets.QDialog.exec'): form.exec() ok_widget = form.button_box.button(form.button_box.Save) QtTest.QTest.mouseClick(ok_widget, QtCore.Qt.LeftButton) @@ -64,7 +64,7 @@ def test_basic_display(form): # WHEN displaying the UI, having set the text and pressing enter form.text_edit.setPlainText('') - with patch('PyQt5.QtWidgets.QDialog.exec'): + with patch('openlp.core.ui.servicenoteform.QtWidgets.QDialog.exec'): form.exec() form.text_edit.setPlainText(text) ok_widget = form.button_box.button(form.button_box.Save)