Fix nexus save (#2405)

docs/release_notes/next/fix-2404-fix-nexus-save

@@ -0,0 +1 @@
+#2404: Fix nexus saving and make tests more robust

mantidimaging/gui/test/gui_system_loading_test.py

@@ -17,6 +17,7 @@
 from mantidimaging.gui.test.gui_system_base import GuiSystemBase, SHORT_DELAY, LOAD_SAMPLE, SHOW_DELAY
 from mantidimaging.gui.widgets.dataset_selector_dialog.dataset_selector_dialog import DatasetSelectorDialog
 from mantidimaging.gui.windows.main.image_save_dialog import ImageSaveDialog
+from mantidimaging.gui.windows.main.nexus_save_dialog import NexusSaveDialog
 from mantidimaging.test_helpers.qt_test_helpers import wait_until
 
 
@@ -175,7 +176,8 @@ def test_save_images(self):
 
         self.main_window.show_image_save_dialog()
 
-        with mock.patch("mantidimaging.gui.windows.main.MainWindowModel.do_images_saving") as mock_save:
+        with mock.patch("mantidimaging.core.io.saver.image_save") as mock_save:
+            mock_save.return_value = ["" for _ in range(100)]
             self._wait_for_widget_visible(ImageSaveDialog)
             QApplication.processEvents()
 
@@ -187,6 +189,24 @@ def test_save_images(self):
             # Confirm that save has been called only once
             mock_save.assert_called_once()
 
+    def test_save_nexus(self):
+        self._load_data_set()
+
+        self.main_window.show_nexus_save_dialog()
+
+        with mock.patch("mantidimaging.core.io.saver.nexus_save") as mock_save:
+            self._wait_for_widget_visible(NexusSaveDialog)
+            self.main_window.nexus_save_dialog.savePath.setText("/path/aaa.nxs")
+            self.main_window.nexus_save_dialog.sampleNameLineEdit.setText("aaa")
+            QApplication.processEvents()
+            ok_button = self.main_window.nexus_save_dialog.buttonBox.button(QDialogButtonBox.StandardButton.Save)
+            QTest.mouseClick(ok_button, Qt.LeftButton)
+
+            QApplication.processEvents()
+            wait_until(lambda: mock_save.call_count == 1)
+            # Confirm that save has been called only once
+            mock_save.assert_called_once()
+
     @parameterized.expand([
         (None, 100),
         ((0, 10, 1), 10),

mantidimaging/gui/windows/main/model.py

@@ -87,13 +87,14 @@ def do_images_saving(self, images_id: uuid.UUID, output_dir: str, name_prefix: s
         images.filenames = filenames
         return True
 
-    def do_nexus_saving(self, dataset_id: uuid.UUID, path: str, sample_name: str, save_as_float: bool) -> None:
+    def do_nexus_saving(self, dataset_id: uuid.UUID, path: str, sample_name: str, save_as_float: bool) -> bool:
         dataset = self.datasets.get(dataset_id)
         if not dataset:
             raise RuntimeError(f"Failed to get Dataset with ID {dataset_id}")
         if not dataset.sample:
             raise RuntimeError(f"Dataset with ID {dataset_id} does not have a sample")
         saver.nexus_save(dataset, path, sample_name, save_as_float)
+        return True
 
     def get_existing_180_id(self, dataset_id: uuid.UUID) -> uuid.UUID | None:
         """

mantidimaging/gui/windows/main/presenter.py

@@ -185,7 +185,7 @@ def _on_dataset_load_done(self, task: TaskWorkerThread) -> None:
             task.result = None
             self._open_window_if_not_open()
         else:
-            self._handle_task_error(self.LOAD_ERROR_STRING, task)
+            raise RuntimeError(self.LOAD_ERROR_STRING.format(task.error))
 
     def _add_dataset_to_view(self, dataset: Dataset) -> None:
         """
@@ -198,11 +198,6 @@ def _add_dataset_to_view(self, dataset: Dataset) -> None:
         if dataset.sample:
             self.add_alternative_180_if_required(dataset)
 
-    def _handle_task_error(self, base_message: str, task: TaskWorkerThread) -> None:
-        msg = base_message.format(task.error)
-        logger.error(msg)
-        self.show_error(msg, traceback.format_exc())
-
     def _create_and_tabify_stack_window(self, images: ImageStack, sample_dock: StackVisualiserView) -> None:
         """
         Creates a new stack window with a given ImageStack object then makes sure it is placed on top of a
@@ -345,7 +340,7 @@ def save_image_files(self) -> None:
     def _on_save_done(self, task: TaskWorkerThread) -> None:
 
         if not task.was_successful():
-            self._handle_task_error(self.SAVE_ERROR_STRING, task)
+            raise RuntimeError(self.SAVE_ERROR_STRING.format(task.error))
 
     @property
     def stack_visualiser_list(self) -> list[StackId]:

mantidimaging/gui/windows/main/test/presenter_test.py

@@ -63,10 +63,7 @@ def test_failed_attempt_to_load_shows_error(self):
         self.assertFalse(task.was_successful())
 
         # Call the callback with a task that failed
-        self.presenter._on_dataset_load_done(task)
-
-        # Expect error message
-        self.view.show_error_dialog.assert_called_once_with(self.presenter.LOAD_ERROR_STRING.format(task.error))
+        self.assertRaises(RuntimeError, self.presenter._on_dataset_load_done, task)
 
     def test_failed_attempt_to_save_shows_error(self):
         # Create a filed load async task
@@ -75,10 +72,7 @@ def test_failed_attempt_to_save_shows_error(self):
         self.assertFalse(task.was_successful())
 
         # Call the callback with a task that failed
-        self.presenter._on_save_done(task)
-
-        # Expect error message
-        self.view.show_error_dialog.assert_called_once_with(self.presenter.SAVE_ERROR_STRING.format(task.error))
+        self.assertRaises(RuntimeError, self.presenter._on_save_done, task)
 
     @mock.patch("mantidimaging.gui.windows.main.presenter.start_async_task_view")
     def test_dataset_stack(self, start_async_mock: mock.Mock):