From 05427e075642a40e0fea21013b32ece485f09b17 Mon Sep 17 00:00:00 2001 From: Martyn Gigg <martyn.gigg@stfc.ac.uk> Date: Wed, 15 Aug 2018 12:10:39 +0100 Subject: [PATCH] Improve test coverage of spectraselectordialog --- .../mantidqt/dialogs/spectraselectordialog.py | 12 +++ .../test/test_spectraselectiondialog.py | 95 +++++++++++++++---- 2 files changed, 88 insertions(+), 19 deletions(-) diff --git a/qt/python/mantidqt/dialogs/spectraselectordialog.py b/qt/python/mantidqt/dialogs/spectraselectordialog.py index 98544861c41..03b985b99fe 100644 --- a/qt/python/mantidqt/dialogs/spectraselectordialog.py +++ b/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 diff --git a/qt/python/mantidqt/dialogs/test/test_spectraselectiondialog.py b/qt/python/mantidqt/dialogs/test/test_spectraselectiondialog.py index fe899f3841b..fbcd7f50e4d 100644 --- a/qt/python/mantidqt/dialogs/test/test_spectraselectiondialog.py +++ b/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) -- GitLab