Ensure a path set in PathEdit is a Path instance

openlp/core/widgets/dialogs.py

@@ -19,6 +19,8 @@
 # along with this program.  If not, see <https://www.gnu.org/licenses/>. #
 ##########################################################################
 """ Patch the QFileDialog so it accepts and returns Path objects"""
+from pathlib import Path
+
 from PyQt5 import QtCore, QtWidgets
 
 from openlp.core.common.path import path_to_str, replace_params, str_to_path
@@ -27,7 +29,7 @@ from openlp.core.common.i18n import UiStrings, translate
 
 class FileDialog(QtWidgets.QFileDialog):
     @classmethod
-    def getExistingDirectory(cls, *args, **kwargs):
+    def getExistingDirectory(cls, *args, **kwargs) -> Path:
         """
         Wraps `getExistingDirectory` so that it can be called with, and return Path objects
 
@@ -38,7 +40,6 @@ class FileDialog(QtWidgets.QFileDialog):
         :rtype: pathlib.Path
         """
         args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),))
-
         return_value = super().getExistingDirectory(*args, **kwargs)
 
         # getExistingDirectory returns a str that represents the path. The string is empty if the user cancels the
@@ -46,7 +47,7 @@ class FileDialog(QtWidgets.QFileDialog):
         return str_to_path(return_value)
 
     @classmethod
-    def getOpenFileName(cls, *args, **kwargs):
+    def getOpenFileName(cls, *args, **kwargs) -> tuple[Path, str]:
         """
         Wraps `getOpenFileName` so that it can be called with, and return Path objects
 
@@ -59,7 +60,6 @@ class FileDialog(QtWidgets.QFileDialog):
         :rtype: tuple[pathlib.Path, str]
         """
         args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),))
-
         file_name, selected_filter = super().getOpenFileName(*args, **kwargs)
 
         # getOpenFileName returns a tuple. The first item is a str that represents the path. The string is empty if
@@ -67,7 +67,7 @@ class FileDialog(QtWidgets.QFileDialog):
         return str_to_path(file_name), selected_filter
 
     @classmethod
-    def getOpenFileNames(cls, *args, **kwargs):
+    def getOpenFileNames(cls, *args, **kwargs) -> tuple[list[Path], str]:
         """
         Wraps `getOpenFileNames` so that it can be called with, and return Path objects
 
@@ -80,7 +80,6 @@ class FileDialog(QtWidgets.QFileDialog):
         :rtype: tuple[list[pathlib.Path], str]
         """
         args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),))
-
         file_names, selected_filter = super().getOpenFileNames(*args, **kwargs)
 
         # getSaveFileName returns a tuple. The first item is a list of str's that represents the path. The list is
@@ -89,7 +88,7 @@ class FileDialog(QtWidgets.QFileDialog):
         return paths, selected_filter
 
     @classmethod
-    def getSaveFileName(cls, *args, **kwargs):
+    def getSaveFileName(cls, *args, **kwargs) -> tuple[Path | None, str]:
         """
         Wraps `getSaveFileName` so that it can be called with, and return Path objects
 

openlp/core/widgets/edits.py

@@ -264,7 +264,7 @@ class PathEdit(QtWidgets.QWidget):
         :param Path path: The path to set the widget to
         :rtype: None
         """
-        self._path = path
+        self._path = Path(path)
         text = path_to_str(path)
         self.line_edit.setText(text)
         self.line_edit.setToolTip(text)

openlp/plugins/songs/lib/openlyricsexport.py

@@ -23,6 +23,7 @@ The :mod:`openlyricsexport` module provides the functionality for exporting song
 format.
 """
 import logging
+from pathlib import Path
 
 from lxml import etree
 
@@ -51,7 +52,7 @@ class OpenLyricsExport(RegistryProperties):
         self.parent = parent
         self.manager = parent.plugin.manager
         self.songs = songs
-        self.save_path = save_path
+        self.save_path = Path(save_path)
         create_paths(self.save_path)
 
     def do_export(self):

tests/openlp_core/widgets/test_edits.py

@@ -48,7 +48,7 @@ SEARCH_TYPES = [(SearchTypes.First, QtGui.QIcon(), "First", "First Placeholder T
 
 
 @pytest.fixture()
-def search_edit(mock_settings):
+def search_edit(mock_settings: MagicMock) -> SearchEdit:
     main_window = QtWidgets.QMainWindow()
     Registry().register('main_window', main_window)
     Registry().remove('settings')
@@ -61,7 +61,7 @@ def search_edit(mock_settings):
 
 
 @pytest.fixture()
-def combo(mock_settings):
+def history_combo(mock_settings: MagicMock) -> HistoryComboBox:
     main_window = QtWidgets.QMainWindow()
     Registry().register('main_window', main_window)
     s_combo = HistoryComboBox(main_window)
@@ -69,120 +69,140 @@ def combo(mock_settings):
 
 
 @pytest.fixture()
-def widget():
+def path_edit() -> PathEdit:
     with patch('openlp.core.widgets.edits.PathEdit._setup'):
         return PathEdit()
 
 
-def test_path_getter(widget):
+def test_path_getter(path_edit: PathEdit):
     """
     Test the `path` property getter.
     """
     # GIVEN: An instance of PathEdit with the `_path` instance variable set
-    widget._path = Path('getter', 'test', 'pat.h')
+    path_edit._path = Path('getter', 'test', 'pat.h')
 
     # WHEN: Reading the `path` property
     # THEN: The value that we set should be returned
-    assert widget.path == Path('getter', 'test', 'pat.h')
+    assert path_edit.path == Path('getter', 'test', 'pat.h')
 
 
-def test_path_setter(widget):
+def test_path_setter(path_edit: PathEdit):
     """
     Test the `path` property setter.
     """
     # GIVEN: An instance of the PathEdit object and a mocked `line_edit`
-    widget.line_edit = MagicMock()
+    path_edit.line_edit = MagicMock()
 
     # WHEN: Writing to the `path` property
-    widget.path = Path('setter', 'test', 'pat.h')
+    path_edit.path = Path('setter', 'test', 'pat.h')
 
     # THEN: The `_path` instance variable should be set with the test data. The `line_edit` text and tooltip
     #       should have also been set.
-    assert widget._path == Path('setter', 'test', 'pat.h')
+    assert path_edit._path == Path('setter', 'test', 'pat.h')
-    widget.line_edit.setToolTip.assert_called_once_with(os.path.join('setter', 'test', 'pat.h'))
+    os_normalised_str = os.path.join('setter', 'test', 'pat.h')
-    widget.line_edit.setText.assert_called_once_with(os.path.join('setter', 'test', 'pat.h'))
+    path_edit.line_edit.setToolTip.assert_called_once_with(os_normalised_str)
+    path_edit.line_edit.setText.assert_called_once_with(os_normalised_str)
 
 
-def test_path_type_getter(widget):
+def test_path_setter_str(path_edit: PathEdit):
+    """
+    Test the `path` property setter with a string instead of a Path.
+    """
+    # GIVEN: An instance of the PathEdit object and a mocked `line_edit`
+    path_edit.line_edit = MagicMock()
+
+    # WHEN: Writing to the `path` property
+    path_edit.path = 'setter/str/test/pat.h'
+
+    # THEN: The `_path` instance variable should be set with the test data. The `line_edit` text and tooltip
+    #       should have also been set.
+    assert path_edit._path == Path('setter', 'str', 'test', 'pat.h')
+    os_normalised_str = os.path.join('setter', 'str', 'test', 'pat.h')
+    path_edit.line_edit.setToolTip.assert_called_once_with(os_normalised_str)
+    path_edit.line_edit.setText.assert_called_once_with(os_normalised_str)
+
+
+def test_path_type_getter(path_edit: PathEdit):
     """
     Test the `path_type` property getter.
     """
     # GIVEN: An instance of PathEdit
     # WHEN: Reading the `path` property
     # THEN: The default value should be returned
-    assert widget.path_type == PathEditType.Files
+    assert path_edit.path_type == PathEditType.Files
 
 
-def test_path_type_setter(widget):
+def test_path_type_setter(path_edit: PathEdit):
     """
     Test the `path_type` property setter.
     """
     # GIVEN: An instance of the PathEdit object and a mocked `update_button_tool_tips` method.
-    with patch.object(widget, 'update_button_tool_tips') as mocked_update_button_tool_tips:
+    with patch.object(path_edit, 'update_button_tool_tips') as mocked_update_button_tool_tips:
 
         # WHEN: Writing to a different value than default to the `path_type` property
-        widget.path_type = PathEditType.Directories
+        path_edit.path_type = PathEditType.Directories
 
         # THEN: The `_path_type` instance variable should be set with the test data and not the default. The
         #       update_button_tool_tips should have been called.
-        assert widget._path_type == PathEditType.Directories
+        assert path_edit._path_type == PathEditType.Directories
         mocked_update_button_tool_tips.assert_called_once_with()
 
 
-def test_update_button_tool_tips_directories(widget):
+def test_update_button_tool_tips_directories(path_edit: PathEdit):
     """
     Test the `update_button_tool_tips` method.
     """
     # GIVEN: An instance of PathEdit with the `path_type` set to `Directories`
-    widget.browse_button = MagicMock()
+    path_edit.browse_button = MagicMock()
-    widget.revert_button = MagicMock()
+    path_edit.revert_button = MagicMock()
-    widget._path_type = PathEditType.Directories
+    path_edit._path_type = PathEditType.Directories
 
     # WHEN: Calling update_button_tool_tips
-    widget.update_button_tool_tips()
+    path_edit.update_button_tool_tips()
 
-    widget.browse_button.setToolTip.assert_called_once_with('Browse for directory.')
+    path_edit.browse_button.setToolTip.assert_called_once_with('Browse for directory.')
-    widget.revert_button.setToolTip.assert_called_once_with('Revert to default directory.')
+    path_edit.revert_button.setToolTip.assert_called_once_with('Revert to default directory.')
 
 
-def test_update_button_tool_tips_files(widget):
+def test_update_button_tool_tips_files(path_edit: PathEdit):
     """
     Test the `update_button_tool_tips` method.
     """
     # GIVEN: An instance of PathEdit with the `path_type` set to `Files`
-    widget.browse_button = MagicMock()
+    path_edit.browse_button = MagicMock()
-    widget.revert_button = MagicMock()
+    path_edit.revert_button = MagicMock()
-    widget._path_type = PathEditType.Files
+    path_edit._path_type = PathEditType.Files
 
     # WHEN: Calling update_button_tool_tips
-    widget.update_button_tool_tips()
+    path_edit.update_button_tool_tips()
 
-    widget.browse_button.setToolTip.assert_called_once_with('Browse for file.')
+    path_edit.browse_button.setToolTip.assert_called_once_with('Browse for file.')
-    widget.revert_button.setToolTip.assert_called_once_with('Revert to default file.')
+    path_edit.revert_button.setToolTip.assert_called_once_with('Revert to default file.')
 
 
 @patch('openlp.core.widgets.edits.FileDialog.getExistingDirectory', return_value=None)
 @patch('openlp.core.widgets.edits.FileDialog.getOpenFileName')
-def test_on_browse_button_clicked_directory(mocked_get_open_file_name, mocked_get_existing_directory, widget):
+def test_on_browse_button_clicked_directory(mocked_get_open_file_name: MagicMock,
+                                            mocked_get_existing_directory: MagicMock, path_edit: PathEdit):
     """
     Test the `browse_button` `clicked` handler on_browse_button_clicked when the `path_type` is set to Directories.
     """
     # GIVEN: An instance of PathEdit with the `path_type` set to `Directories` and a mocked
     #        QFileDialog.getExistingDirectory
-    widget._path_type = PathEditType.Directories
+    path_edit._path_type = PathEditType.Directories
-    widget._path = Path('test', 'path')
+    path_edit._path = Path('test', 'path')
 
     # WHEN: Calling on_browse_button_clicked
-    widget.on_browse_button_clicked()
+    path_edit.on_browse_button_clicked()
 
     # THEN: The FileDialog.getExistingDirectory should have been called with the default caption
-    mocked_get_existing_directory.assert_called_once_with(widget, 'Select Directory',
+    mocked_get_existing_directory.assert_called_once_with(path_edit, 'Select Directory',
                                                           Path('test', 'path'),
                                                           FileDialog.ShowDirsOnly)
     assert mocked_get_open_file_name.called is False
 
 
-def test_on_browse_button_clicked_directory_custom_caption(widget):
+def test_on_browse_button_clicked_directory_custom_caption(path_edit: PathEdit):
     """
     Test the `browse_button` `clicked` handler on_browse_button_clicked when the `path_type` is set to Directories,
     and `dialog_caption` is set.
@@ -192,21 +212,21 @@ def test_on_browse_button_clicked_directory_custom_caption(widget):
     with patch('openlp.core.widgets.edits.FileDialog.getExistingDirectory', return_value=None) as \
             mocked_get_existing_directory, \
             patch('openlp.core.widgets.edits.FileDialog.getOpenFileName') as mocked_get_open_file_name:
-        widget._path_type = PathEditType.Directories
+        path_edit._path_type = PathEditType.Directories
-        widget._path = Path('test', 'path')
+        path_edit._path = Path('test', 'path')
-        widget.dialog_caption = 'Directory Caption'
+        path_edit.dialog_caption = 'Directory Caption'
 
         # WHEN: Calling on_browse_button_clicked
-        widget.on_browse_button_clicked()
+        path_edit.on_browse_button_clicked()
 
         # THEN: The FileDialog.getExistingDirectory should have been called with the custom caption
-        mocked_get_existing_directory.assert_called_once_with(widget, 'Directory Caption',
+        mocked_get_existing_directory.assert_called_once_with(path_edit, 'Directory Caption',
                                                               Path('test', 'path'),
                                                               FileDialog.ShowDirsOnly)
         assert mocked_get_open_file_name.called is False
 
 
-def test_on_browse_button_clicked_file(widget):
+def test_on_browse_button_clicked_file(path_edit: PathEdit):
     """
     Test the `browse_button` `clicked` handler on_browse_button_clicked when the `path_type` is set to Files.
     """
@@ -214,19 +234,19 @@ def test_on_browse_button_clicked_file(widget):
     with patch('openlp.core.widgets.edits.FileDialog.getExistingDirectory') as mocked_get_existing_directory, \
             patch('openlp.core.widgets.edits.FileDialog.getOpenFileName', return_value=(None, '')) as \
             mocked_get_open_file_name:
-        widget._path_type = PathEditType.Files
+        path_edit._path_type = PathEditType.Files
-        widget._path = Path('test', 'pat.h')
+        path_edit._path = Path('test', 'pat.h')
 
         # WHEN: Calling on_browse_button_clicked
-        widget.on_browse_button_clicked()
+        path_edit.on_browse_button_clicked()
 
         # THEN: The FileDialog.getOpenFileName should have been called with the default caption
-        mocked_get_open_file_name.assert_called_once_with(widget, 'Select File', Path('test', 'pat.h'),
+        mocked_get_open_file_name.assert_called_once_with(path_edit, 'Select File', Path('test', 'pat.h'),
-                                                          widget.filters)
+                                                          path_edit.filters)
         assert mocked_get_existing_directory.called is False
 
 
-def test_on_browse_button_clicked_file_custom_caption(widget):
+def test_on_browse_button_clicked_file_custom_caption(path_edit: PathEdit):
     """
     Test the `browse_button` `clicked` handler on_browse_button_clicked when the `path_type` is set to Files and
     `dialog_caption` is set.
@@ -236,20 +256,20 @@ def test_on_browse_button_clicked_file_custom_caption(widget):
     with patch('openlp.core.widgets.edits.FileDialog.getExistingDirectory') as mocked_get_existing_directory, \
             patch('openlp.core.widgets.edits.FileDialog.getOpenFileName', return_value=(None, '')) as \
             mocked_get_open_file_name:
-        widget._path_type = PathEditType.Files
+        path_edit._path_type = PathEditType.Files
-        widget._path = Path('test', 'pat.h')
+        path_edit._path = Path('test', 'pat.h')
-        widget.dialog_caption = 'File Caption'
+        path_edit.dialog_caption = 'File Caption'
 
         # WHEN: Calling on_browse_button_clicked
-        widget.on_browse_button_clicked()
+        path_edit.on_browse_button_clicked()
 
         # THEN: The FileDialog.getOpenFileName should have been called with the custom caption
-        mocked_get_open_file_name.assert_called_once_with(widget, 'File Caption', Path('test', 'pat.h'),
+        mocked_get_open_file_name.assert_called_once_with(path_edit, 'File Caption', Path('test', 'pat.h'),
-                                                          widget.filters)
+                                                          path_edit.filters)
         assert mocked_get_existing_directory.called is False
 
 
-def test_on_browse_button_clicked_user_cancels(widget):
+def test_on_browse_button_clicked_user_cancels(path_edit: PathEdit):
     """
     Test the `browse_button` `clicked` handler on_browse_button_clicked when the user cancels the FileDialog (an
     empty str is returned)
@@ -260,13 +280,13 @@ def test_on_browse_button_clicked_user_cancels(widget):
             mocked_get_open_file_name:
 
         # WHEN: Calling on_browse_button_clicked
-        widget.on_browse_button_clicked()
+        path_edit.on_browse_button_clicked()
 
         # THEN: normpath should not have been called
         assert mocked_get_open_file_name.called is True
 
 
-def test_on_browse_button_clicked_user_accepts(widget):
+def test_on_browse_button_clicked_user_accepts(path_edit: PathEdit):
     """
     Test the `browse_button` `clicked` handler on_browse_button_clicked when the user accepts the FileDialog (a path
     is returned)
@@ -275,79 +295,79 @@ def test_on_browse_button_clicked_user_accepts(widget):
     #        path.
     with patch('openlp.core.widgets.edits.FileDialog.getOpenFileName',
                return_value=(Path('test', 'pat.h'), '')) as mocked_get_open_file_name, \
-            patch.object(widget, 'on_new_path'):
+            patch.object(path_edit, 'on_new_path'):
 
         # WHEN: Calling on_browse_button_clicked
-        widget.on_browse_button_clicked()
+        path_edit.on_browse_button_clicked()
 
         # THEN: normpath and `on_new_path` should have been called
         assert mocked_get_open_file_name.called is True
-        assert widget.on_new_path.called is True
+        assert path_edit.on_new_path.called is True
 
 
-def test_on_revert_button_clicked(widget):
+def test_on_revert_button_clicked(path_edit: PathEdit):
     """
     Test that the default path is set as the path when the `revert_button.clicked` handler is called.
     """
     # GIVEN: An instance of PathEdit with a mocked `on_new_path`, and the `default_path` set.
-    with patch.object(widget, 'on_new_path') as mocked_on_new_path:
+    with patch.object(path_edit, 'on_new_path') as mocked_on_new_path:
-        widget.default_path = Path('default', 'pat.h')
+        path_edit.default_path = Path('default', 'pat.h')
 
         # WHEN: Calling `on_revert_button_clicked`
-        widget.on_revert_button_clicked()
+        path_edit.on_revert_button_clicked()
 
         # THEN: on_new_path should have been called with the default path
         mocked_on_new_path.assert_called_once_with(Path('default', 'pat.h'))
 
 
-def test_on_line_edit_editing_finished(widget):
+def test_on_line_edit_editing_finished(path_edit: PathEdit):
     """
     Test that the new path is set as the path when the `line_edit.editingFinished` handler is called.
     """
     # GIVEN: An instance of PathEdit with a mocked `line_edit` and `on_new_path`.
-    with patch.object(widget, 'on_new_path') as mocked_on_new_path:
+    with patch.object(path_edit, 'on_new_path') as mocked_on_new_path:
-        widget.line_edit = MagicMock(**{'text.return_value': 'test/pat.h'})
+        path_edit.line_edit = MagicMock(**{'text.return_value': 'test/pat.h'})
 
         # WHEN: Calling `on_line_edit_editing_finished`
-        widget.on_line_edit_editing_finished()
+        path_edit.on_line_edit_editing_finished()
 
         # THEN: on_new_path should have been called with the path enetered in `line_edit`
         mocked_on_new_path.assert_called_once_with(Path('test', 'pat.h'))
 
 
-def test_on_new_path_no_change(widget):
+def test_on_new_path_no_change(path_edit: PathEdit):
     """
     Test `on_new_path` when called with a path that is the same as the existing path.
     """
     # GIVEN: An instance of PathEdit with a test path and mocked `pathChanged` signal
     with patch('openlp.core.widgets.edits.PathEdit.path', new_callable=PropertyMock):
-        widget._path = Path('/old', 'test', 'pat.h')
+        path_edit._path = Path('/old', 'test', 'pat.h')
-        widget.pathChanged = MagicMock()
+        path_edit.pathChanged = MagicMock()
 
         # WHEN: Calling `on_new_path` with the same path as the existing path
-        widget.on_new_path(Path('/old', 'test', 'pat.h'))
+        path_edit.on_new_path(Path('/old', 'test', 'pat.h'))
 
         # THEN: The `pathChanged` signal should not be emitted
-        assert widget.pathChanged.emit.called is False
+        assert path_edit.pathChanged.emit.called is False
 
 
-def test_on_new_path_change(widget):
+def test_on_new_path_change(path_edit: PathEdit):
     """
     Test `on_new_path` when called with a path that is the different to the existing path.
     """
     # GIVEN: An instance of PathEdit with a test path and mocked `pathChanged` signal
     with patch('openlp.core.widgets.edits.PathEdit.path', new_callable=PropertyMock):
-        widget._path = Path('/old', 'test', 'pat.h')
+        path_edit._path = Path('/old', 'test', 'pat.h')
-        widget.pathChanged = MagicMock()
+        path_edit.pathChanged = MagicMock()
 
         # WHEN: Calling `on_new_path` with the a new path
-        widget.on_new_path(Path('/new', 'test', 'pat.h'))
+        path_edit.on_new_path(Path('/new', 'test', 'pat.h'))
 
         # THEN: The `pathChanged` signal should be emitted
-        widget.pathChanged.emit.assert_called_once_with(Path('/new', 'test', 'pat.h'))
+        path_edit.pathChanged.emit.assert_called_once_with(Path('/new', 'test', 'pat.h'))
 
 
-def test_set_search_types(search_edit):
+def test_set_search_types(search_edit: SearchEdit):
     """
     Test setting the search types of the search edit.
     """
@@ -363,7 +383,7 @@ def test_set_search_types(search_edit):
     Registry().get('settings').setValue.assert_called_once_with('settings_section/last used search type', 0)
 
 
-def test_set_current_search_type(search_edit):
+def test_set_current_search_type(search_edit: SearchEdit):
     """
     Test if changing the search type works.
     """
@@ -381,7 +401,7 @@ def test_set_current_search_type(search_edit):
         [call('settings_section/last used search type', 0), call('settings_section/last used search type', 1)])
 
 
-def test_clear_button_visibility(search_edit):
+def test_clear_button_visibility(search_edit: SearchEdit):
     """
     Test if the clear button is hidden/shown correctly.
     """
@@ -396,7 +416,7 @@ def test_clear_button_visibility(search_edit):
     assert not search_edit.clear_button.isHidden(), "The clear button should be visible."
 
 
-def test_press_clear_button(search_edit):
+def test_press_clear_button(search_edit: SearchEdit):
     """
     Check if the search edit behaves correctly when pressing the clear button.
     """
@@ -412,15 +432,15 @@ def test_press_clear_button(search_edit):
     assert search_edit.clear_button.isHidden(), "The clear button should be hidden."
 
 
-def test_history_combo_get_items(combo):
+def test_history_combo_get_items(history_combo: HistoryComboBox):
     """
     Test the getItems() method
     """
-    # GIVEN: The combo.
+    # GIVEN: The history_combo.
 
     # WHEN: Add two items.
-    combo.addItem('test1')
+    history_combo.addItem('test1')
-    combo.addItem('test2')
+    history_combo.addItem('test2')
 
     # THEN: The list of items should contain both strings.
-    assert combo.getItems() == ['test1', 'test2']
+    assert history_combo.getItems() == ['test1', 'test2']