Merge pull request #205 from neutrons/try-catch-export

Wrap export xml in try-except

Author: glass-ships

src/refred/configuration/export_xml_config.py

@@ -1,7 +1,7 @@
 import datetime
 import logging
 import os
-from typing import Optional
+from typing import TYPE_CHECKING, List, Optional
 
 import lr_reduction
 import mantid
@@ -12,11 +12,14 @@
     GlobalReductionSettingsHandler,
 )
 
+if TYPE_CHECKING:
+    from refred.main import MainGui
+
 
 class ExportXMLConfig(object):
-    def __init__(self, parent=None):
+    def __init__(self, parent: "MainGui"):
         self.parent = parent
-        self.str_array = []
+        self.str_array: List[str] = []
 
     def header_part(self):
         str_array = self.str_array

src/refred/configuration/saving_configuration.py

@@ -1,4 +1,5 @@
 import os
+from typing import TYPE_CHECKING
 
 from qtpy import QtWidgets
 
@@ -7,11 +8,14 @@
 from refred.status_message_handler import StatusMessageHandler
 from refred.utilities import makeSureFileHasExtension
 
+if TYPE_CHECKING:
+    from refred.main import MainGui
+
 
 class SavingConfiguration(object):
-    parent = None
+    """Class to save the current configuration to an XML file."""
 
-    def __init__(self, parent=None, filename=""):
+    def __init__(self, parent: "MainGui", filename: str = ""):
         self.parent = parent
         self.filename = filename
 
@@ -41,7 +45,12 @@ def run(self):
 
         self.parent.path_config = os.path.dirname(self.filename)
         self.filename = makeSureFileHasExtension(self.filename)
-        ExportXMLConfig(parent=self.parent).save(self.filename)
+        export_config = ExportXMLConfig(parent=self.parent)
+        try:
+            export_config.save(self.filename)
+        except (PermissionError, IsADirectoryError) as e:
+            StatusMessageHandler(parent=self.parent, message=f"Error: {e}", is_threaded=True)
+            return
 
         StatusMessageHandler(parent=self.parent, message="Done!", is_threaded=True)
 

src/refred/interfaces/deadtime_settings.py

@@ -125,10 +125,10 @@ def __init__(self, parent: QWidget):
         self.ui = load_ui(ui_filename="deadtime_settings.ui", baseinstance=self)
         self.options = self.get_state_from_form()
 
+    # TODO: Should add default values for these parameters (Glass)
     def set_state(self, paralyzable, dead_time, tof_step, use_threshold_ratio, threshold_ratio):
         """
         Store options and populate the form
-        :param apply_correction: If True, dead time correction will be applied
         :param paralyzable: If True, a paralyzable correction will be used
         :param dead_time: Value of the dead time in micro second
         :param tof_step: TOF binning in micro second

src/refred/main.py

@@ -151,11 +151,13 @@ class MainGui(QtWidgets.QMainWindow):
 
     big_table_data = TableData(REDUCTIONTABLE_MAX_ROWCOUNT)
 
+    # TODO: `parent` argument is never used and can probably be removed (Glass)
     def __init__(self, parent=None):
         if parent is None:
             QtWidgets.QMainWindow.__init__(self)
         else:
             QtWidgets.QMainWindow.__init__(self, parent, QtCore.Qt.Window)
+
         self.ui = load_ui("refred_main_interface.ui", self)
 
         # Get default values for widgets

src/refred/sf_calculator/sf_calculator.py

@@ -1054,6 +1054,7 @@ def show_dead_time_dialog(self):
         Pop up dialog for dead time options
         """
         dt_settings = DeadTimeSettingsView(parent=self)
+        # TODO: Missing `use_threshold_ratio` and `threshold_ratio` arguments (Glass)
         dt_settings.set_state(self.paralyzable_deadtime, self.deadtime_value, self.deadtime_tof_step)
         dt_settings.exec_()
 

test/unit/refred/test_saving_configuration.py

@@ -0,0 +1,149 @@
+from dataclasses import dataclass
+from pathlib import Path
+from typing import Callable
+
+import pytest
+
+from refred.configuration import saving_configuration
+
+
+### Helpers to fake the MainGui
+@dataclass
+class DummyParent:
+    path_config: str
+    config_saved: bool = False
+
+
+def fake_extension_factory(expected_input: Path, sanitized_output: str) -> Callable[[str], str]:
+    def _fake_extension(path: str) -> str:
+        assert path == str(expected_input)
+        return sanitized_output
+
+    return _fake_extension
+
+
+def fake_status_handler_factory(call_collector: list[tuple[str, bool]]):
+    class _FakeStatusMessageHandler:
+        def __init__(self, parent: DummyParent, message: str, is_threaded: bool):
+            call_collector.append((message, is_threaded))
+
+    return _FakeStatusMessageHandler
+
+
+def fake_export_config_factory() -> type:
+    class _FakeExportXMLConfig:
+        def __init__(self, parent: DummyParent):
+            self.parent: DummyParent = parent
+
+        def save(self, filename: str):
+            path = Path(filename)
+            path.parent.mkdir(parents=True, exist_ok=True)
+            _ = path.write_text("<Reduction />", encoding="utf-8")
+
+    return _FakeExportXMLConfig
+
+
+def fake_gui_utility_factory(sanitized_path: str) -> type:
+    class _FakeGuiUtility:
+        def __init__(self, parent: DummyParent):
+            self.parent: DummyParent = parent
+
+        def new_config_file_loaded(self, config_file_name: str):
+            assert config_file_name == sanitized_path
+            self.parent.config_saved = True
+
+        def gui_not_modified(self):
+            pass
+
+    return _FakeGuiUtility
+
+
+### Tests
+
+
+def test_saving_configuration_good_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
+    parent = DummyParent(path_config=str(tmp_path))
+    provided_path = tmp_path / "test_config"
+    sanitized_path = f"{provided_path}.xml"
+
+    monkeypatch.setattr(
+        saving_configuration,
+        "makeSureFileHasExtension",
+        fake_extension_factory(provided_path, sanitized_path),
+    )
+
+    status_messages: list[tuple[str, bool]] = []
+    monkeypatch.setattr(
+        saving_configuration,
+        "StatusMessageHandler",
+        fake_status_handler_factory(status_messages),
+    )
+
+    monkeypatch.setattr(
+        saving_configuration,
+        "ExportXMLConfig",
+        fake_export_config_factory(),
+    )
+
+    monkeypatch.setattr(
+        saving_configuration,
+        "GuiUtility",
+        fake_gui_utility_factory(sanitized_path),
+    )
+
+    saver = saving_configuration.SavingConfiguration(parent=parent, filename=str(provided_path))  # type: ignore[arg-type]
+    saver.run()
+
+    saved_file = Path(sanitized_path)
+    assert saved_file.exists()
+    assert parent.path_config == str(provided_path.parent)
+    assert parent.config_saved is True
+    assert status_messages == [("Saving config ...", False), ("Done!", True)]
+
+
+def test_saving_configuration_permission_error(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
+    parent = DummyParent(path_config=str(tmp_path))
+    provided_path = tmp_path / "forbidden" / "config"
+    sanitized_path = f"{provided_path}.xml"
+
+    monkeypatch.setattr(
+        saving_configuration,
+        "makeSureFileHasExtension",
+        fake_extension_factory(provided_path, sanitized_path),
+    )
+
+    status_messages: list[tuple[str, bool]] = []
+    monkeypatch.setattr(
+        saving_configuration,
+        "StatusMessageHandler",
+        fake_status_handler_factory(status_messages),
+    )
+
+    class _PermissionErrorExportXMLConfig:
+        def __init__(self, parent: DummyParent):
+            self.parent: DummyParent = parent
+
+        def save(self, filename: str):
+            raise PermissionError("No permission to write file.")
+
+    monkeypatch.setattr(
+        saving_configuration,
+        "ExportXMLConfig",
+        _PermissionErrorExportXMLConfig,
+    )
+
+    monkeypatch.setattr(
+        saving_configuration,
+        "GuiUtility",
+        fake_gui_utility_factory(sanitized_path),
+    )
+
+    saver = saving_configuration.SavingConfiguration(parent=parent, filename=str(provided_path))  # type: ignore[arg-type]
+    saver.run()
+
+    assert parent.path_config == str(provided_path.parent)
+    assert parent.config_saved is False
+    assert status_messages == [
+        ("Saving config ...", False),
+        ("Error: No permission to write file.", True),
+    ]