Improve test coverage of spectraselectordialog

qt/python/mantidqt/dialogs/spectraselectordialog.py

@@ -54,9 +54,18 @@ class SpectraSelection(object):
 
 class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
 
+    @staticmethod
+    def raise_error_if_workspaces_not_compatible(workspaces):
+        def value_error_if_not_compatible(x):
+            if not hasattr(x, 'getNumberHistograms'):
+                raise ValueError("Workspace '{}' is not a MatrixWorkspace".format(x.name()))
+        map(value_error_if_not_compatible, workspaces)
+
     def __init__(self, workspaces,
                  parent=None):
         super(SpectraSelectionDialog, self).__init__(parent)
+        self.raise_error_if_workspaces_not_compatible(workspaces)
+
         # attributes
         self._workspaces = workspaces
         self.spec_min, self.spec_max = None, None
@@ -78,6 +87,7 @@ class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
         self.accept()
 
     # ------------------- Private -------------------------
+
     def _init_ui(self):
         ui = SpectraSelectionDialogUI()
         ui.setupUi(self)
@@ -171,7 +181,9 @@ def get_spectra_selection(workspaces, parent_widget=None):
     :param parent_widget: An optional parent_widget to use for the input selection dialog
     :returns: Either a SpectraSelection object containing the details of workspaces to plot or None indicating
     the request was cancelled
+    :raises ValueError: if the workspaces are not of type MatrixWorkspace
     """
+    SpectraSelectionDialog.raise_error_if_workspaces_not_compatible(workspaces)
     single_spectra_ws = [wksp.getNumberHistograms() for wksp in workspaces if wksp.getNumberHistograms() == 1]
     if len(single_spectra_ws) > 0:
         # At least 1 workspace contains only a single spectrum so this is all

qt/python/mantidqt/dialogs/test/test_spectraselectiondialog.py

@@ -16,51 +16,63 @@
 #  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 # std imports
-import time
 import unittest
+try:
+    from unittest import mock
+except ImportError:
+    import mock
 
 # 3rdparty imports
-from mantid.simpleapi import CreateSampleWorkspace, CropWorkspace
-from qtpy.QtWidgets import QDialogButtonBox
+from mantid.api import WorkspaceFactory
+from qtpy.QtWidgets import QDialog, QDialogButtonBox
 
 # local imports
 from mantidqt.utils.qt.test import requires_qapp
-from mantidqt.dialogs.spectraselectordialog import parse_selection_str, SpectraSelectionDialog
+from mantidqt.dialogs.spectraselectordialog import (get_spectra_selection, parse_selection_str, \
+                                                    SpectraSelectionDialog)
 
 
 @requires_qapp
 class SpectraSelectionDialogTest(unittest.TestCase):
 
+    _single_spec_ws = None
+    _multi_spec_ws = None
+
+    def setUp(self):
+        if self._single_spec_ws is None:
+            self.__class__._single_spec_ws = WorkspaceFactory.Instance().create("Workspace2D", NVectors=1,
+                                                                                XLength=1, YLength=1)
+            self.__class__._multi_spec_ws = WorkspaceFactory.Instance().create("Workspace2D", NVectors=200,
+                                                               XLength=1, YLength=1)
+
     def test_initial_dialog_setup(self):
-        workspaces = [CreateSampleWorkspace(OutputWorkspace='ws', StoreInADS=False)]
+        workspaces = [self._multi_spec_ws]
         dlg = SpectraSelectionDialog(workspaces)
         self.assertFalse(dlg._ui.buttonBox.button(QDialogButtonBox.Ok).isEnabled())
 
     def test_filling_workspace_details_single_workspace(self):
-        workspaces = [CreateSampleWorkspace(OutputWorkspace='ws', StoreInADS=False)]
+        workspaces = [self._multi_spec_ws]
         dlg = SpectraSelectionDialog(workspaces)
         self.assertEqual("valid range: 1-200", dlg._ui.specNums.placeholderText())
         self.assertEqual("valid range: 0-199", dlg._ui.wkspIndices.placeholderText())
 
     def test_filling_workspace_details_multiple_workspaces_of_same_size(self):
-        workspaces = [CreateSampleWorkspace(OutputWorkspace='ws', StoreInADS=False),
-                      CreateSampleWorkspace(OutputWorkspace='ws2', StoreInADS=False)]
+        workspaces = [self._multi_spec_ws,
+                      self._multi_spec_ws]
         dlg = SpectraSelectionDialog(workspaces)
         self.assertEqual("valid range: 1-200", dlg._ui.specNums.placeholderText())
         self.assertEqual("valid range: 0-199", dlg._ui.wkspIndices.placeholderText())
 
     def test_filling_workspace_details_multiple_workspaces_of_different_sizes(self):
-        ws1 = CreateSampleWorkspace(OutputWorkspace='ws', NumBanks=1, StoreInADS=False)
-        ws1 = CropWorkspace(ws1, StartWorkspaceIndex=50)
-        ws2 = CreateSampleWorkspace(OutputWorkspace='ws', StoreInADS=False)
-
-        dlg = SpectraSelectionDialog([ws1, ws2])
+        cropped_ws = WorkspaceFactory.Instance().create("Workspace2D", NVectors=50, XLength=1, YLength=1)
+        for i in range(cropped_ws.getNumberHistograms()):
+            cropped_ws.getSpectrum(i).setSpectrumNo(51 + i)
+        dlg = SpectraSelectionDialog([cropped_ws, self._multi_spec_ws])
         self.assertEqual("valid range: 51-100", dlg._ui.specNums.placeholderText())
         self.assertEqual("valid range: 0-49", dlg._ui.wkspIndices.placeholderText())
 
     def test_valid_text_in_boxes_activates_ok(self):
-        workspaces = [CreateSampleWorkspace(OutputWorkspace='ws', StoreInADS=False)]
-        dlg = SpectraSelectionDialog(workspaces)
+        dlg = SpectraSelectionDialog([self._multi_spec_ws])
 
         def do_test(input_box):
             input_box.setText("1")
@@ -72,13 +84,51 @@ class SpectraSelectionDialogTest(unittest.TestCase):
         do_test(dlg._ui.specNums)
 
     def test_plot_all_gives_only_workspaces_indices(self):
-        ws = CreateSampleWorkspace(OutputWorkspace='ws', StoreInADS=False)
-        dlg = SpectraSelectionDialog([ws])
+        dlg = SpectraSelectionDialog([self._multi_spec_ws])
         dlg._ui.buttonBox.button(QDialogButtonBox.YesToAll).click()
         self.assertTrue(dlg.selection is not None)
         self.assertTrue(dlg.selection.spectra is None)
         self.assertEqual(range(200), dlg.selection.wksp_indices)
 
+    def test_entered_workspace_indices_gives_correct_selection_back(self):
+        dlg = SpectraSelectionDialog([self._multi_spec_ws])
+        dlg._ui.wkspIndices.setText("1-3,5")
+        dlg._ui.buttonBox.button(QDialogButtonBox.Ok).click()
+
+        self.assertTrue(dlg.selection is not None)
+        self.assertTrue(dlg.selection.spectra is None)
+        self.assertEqual([1, 2, 3, 5], dlg.selection.wksp_indices)
+
+    def test_entered_spectra_gives_correct_selection_back(self):
+        dlg = SpectraSelectionDialog([self._multi_spec_ws])
+        dlg._ui.wkspIndices.setText("50-60")
+        dlg._ui.buttonBox.button(QDialogButtonBox.Ok).click()
+
+        self.assertTrue(dlg.selection is not None)
+        self.assertTrue(dlg.selection.spectra is None)
+        self.assertEqual(range(50, 61), dlg.selection.wksp_indices)
+
+    @mock.patch('mantidqt.dialogs.spectraselectordialog.SpectraSelectionDialog', autospec=True)
+    def test_get_spectra_selection_cancelled_returns_None(self, dialog_mock):
+        # a new instance of the mock created inside get_spectra_selection will return
+        # dialog_mock
+        dialog_mock.return_value = dialog_mock
+        dialog_mock.Rejected = QDialog.Rejected
+        dialog_mock.exec_.return_value = dialog_mock.Rejected
+
+        selection = get_spectra_selection([self._multi_spec_ws])
+
+        dialog_mock.exec_.assert_called_once_with()
+        self.assertTrue(selection is None)
+
+    @mock.patch('mantidqt.dialogs.spectraselectordialog.SpectraSelectionDialog')
+    def test_get_spectra_selection_does_not_use_dialog_for_single_spectrum(self, dialog_mock):
+        selection = get_spectra_selection([self._single_spec_ws])
+
+        dialog_mock.assert_not_called()
+        self.assertEqual([0], selection.wksp_indices)
+        self.assertEqual([self._single_spec_ws], selection.workspaces)
+
     def test_parse_selection_str_single_number(self):
         s = '1'
         self.assertEqual([1], parse_selection_str(s, 1, 3))
@@ -108,8 +158,15 @@ class SpectraSelectionDialogTest(unittest.TestCase):
         self.assertEqual([1, 2, 3, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19],
                          parse_selection_str(s, 1, 20))
 
+    # --------------- failure tests -----------
+    def test_construction_with_non_MatrixWorkspace_type_raises_exception(self):
+        table = WorkspaceFactory.Instance().createTable()
+        self.assertRaises(ValueError, SpectraSelectionDialog, [self._single_spec_ws, table])
+
+    def test_get_spectra_selection_raises_error_with_wrong_workspace_type(self):
+        table = WorkspaceFactory.Instance().createTable()
+        self.assertRaises(ValueError, get_spectra_selection, [self._single_spec_ws, table])
+
 
 if __name__ == '__main__':
     unittest.main()
-    # investigate why this is needed to avoid a segfault on Linux
-    time.sleep(0.5)