From f61d2c5d34a0ecc5912b448edbd9848d8b5d130c Mon Sep 17 00:00:00 2001
From: Matthew Andrew <matthew.andrew@stfc.ac.uk>
Date: Fri, 25 Jan 2019 11:29:38 +0000
Subject: [PATCH] Added unit tests for invalid dets Re #23642

---
 .../grouping_tab_widget_presenter.py          |  2 +-
 .../grouping_table_widget_presenter.py        |  2 --
 scripts/Muon/GUI/Common/muon_data_context.py  |  4 ++--
 .../grouping_tab_presenter_test.py            | 19 +++++++++++++++++++
 .../grouping_table_presenter_test.py          | 11 +++++++++++
 5 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py b/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py
index 50ac4a6f7a5..58f3fb458c7 100644
--- a/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py
+++ b/scripts/Muon/GUI/Common/grouping_tab_widget/grouping_tab_widget_presenter.py
@@ -110,7 +110,7 @@ class GroupingTabPresenter(object):
             for pair in pairs:
                 self._model.add_pair(pair)
         except ValueError as error:
-            self._view.display_warning_box(error)
+            self._view.display_warning_box(str(error))
 
         self.grouping_table_widget.update_view_from_model()
         self.pairing_table_widget.update_view_from_model()
diff --git a/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py b/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py
index f3212e7b4d1..bd10dde2469 100644
--- a/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py
+++ b/scripts/Muon/GUI/Common/grouping_table_widget/grouping_table_widget_presenter.py
@@ -50,7 +50,6 @@ class GroupingTablePresenter(object):
         return True
 
     def validate_detector_ids(self, text):
-        # detector_list = run_utils.run_string_to_list(text)
         if re.match(run_utils.run_string_regex, text) and max(run_utils.run_string_to_list(text)) <= self._model._data.num_detectors:
             return True
         self._view.warning_popup("Invalid detector list.")
@@ -120,7 +119,6 @@ class GroupingTablePresenter(object):
             try:
                 self.update_model_from_view()
             except ValueError as error:
-                self._model.remove_pairs_with_removed_name(self._view.get_table_item_text(row, 0))
                 self._view.warning_popup(error)
 
         self.update_view_from_model()
diff --git a/scripts/Muon/GUI/Common/muon_data_context.py b/scripts/Muon/GUI/Common/muon_data_context.py
index c8f79d7e2ab..aa8baa7316a 100644
--- a/scripts/Muon/GUI/Common/muon_data_context.py
+++ b/scripts/Muon/GUI/Common/muon_data_context.py
@@ -157,7 +157,7 @@ class MuonDataContext(object):
         if self.check_group_contains_valid_detectors(group):
             self._groups[group.name] = group
         else:
-            raise ValueError('Detectors in group {} not in instrument'.format(group.name))
+            raise ValueError('Invalid detectors in group {}'.format(group.name))
 
     def add_pair(self, pair):
         assert isinstance(pair, MuonPair)
@@ -303,7 +303,7 @@ class MuonDataContext(object):
             self.add_pair(pair)
 
     def check_group_contains_valid_detectors(self, group):
-        if max(group.detectors) > self.num_detectors:
+        if max(group.detectors) > self.num_detectors or min(group.detectors) < 1:
             return False
         else:
             return True
diff --git a/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py b/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py
index e0ee8a70876..29dfd74fd74 100644
--- a/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py
+++ b/scripts/test/Muon/grouping_tab/grouping_tab_presenter_test.py
@@ -45,6 +45,8 @@ class GroupingTabPresenterTest(unittest.TestCase):
                                               self.grouping_table_widget,
                                               self.pairing_table_widget)
 
+        self.view.display_warning_box = mock.MagicMock()
+
     def add_three_groups(self):
         testgroup1 = MuonGroup(group_name="fwd", detector_ids=[1, 2, 3, 4, 5])
         testgroup2 = MuonGroup(group_name="bwd", detector_ids=[6, 7, 8, 9, 10])
@@ -122,6 +124,23 @@ class GroupingTabPresenterTest(unittest.TestCase):
             self.assertEqual(self.pairing_table_view.pairing_table.cellWidget(0, 1).currentText(), "grp1")
             self.assertEqual(self.pairing_table_view.pairing_table.cellWidget(0, 2).currentText(), "grp2")
 
+    def test_loading_does_not_insert_invalid_groups(self):
+        self.view.show_file_browser_and_return_selection = mock.Mock(return_value="grouping.xml")
+        groups = [MuonGroup(group_name="grp1", detector_ids=[1, 2, 3, 4, 5]),
+                  MuonGroup(group_name="grp2", detector_ids=[6, 7, 8, 9, 1000])]
+        pairs = [MuonPair(pair_name="pair1", forward_group_name="grp1", backward_group_name="grp2")]
+        with mock.patch(
+                "Muon.GUI.Common.grouping_tab_widget.grouping_tab_widget_presenter.xml_utils.load_grouping_from_XML") as mock_load:
+            # mock the loading to return set groups/pairs
+            mock_load.return_value = (groups, pairs, 'description')
+            self.view.load_grouping_button.clicked.emit(True)
+
+            self.view.display_warning_box.assert_called_once_with('Invalid detectors in group grp2')
+            six.assertCountEqual(self, self.model.group_names, ["grp1"])
+            six.assertCountEqual(self, self.model.pair_names, [])
+            self.assertEqual(self.grouping_table_view.num_rows(), 1)
+            self.assertEqual(self.pairing_table_view.num_rows(), 0)
+
     def test_that_save_grouping_triggers_the_correct_function(self):
         # Save functionality is tested elsewhere
         self.view.show_file_save_browser_and_return_selection = mock.Mock(return_value="grouping.xml")
diff --git a/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py b/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py
index e999fb2fa2d..6a359d82cd6 100644
--- a/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py
+++ b/scripts/test/Muon/grouping_tab/grouping_table_presenter_test.py
@@ -295,6 +295,17 @@ class GroupingTablePresenterTest(unittest.TestCase):
 
         self.assertEqual(self.view.get_table_item_text(0, 2), "1")
 
+    def test_modifying_detector_ids_to_non_existent_detector_fails(self):
+        self.presenter.handle_add_group_button_clicked()
+        self.view.grouping_table.item(0, 1).setText("1000")
+
+        self.view.warning_popup.assert_called_once_with('Invalid detector list.')
+
+    def test_modifying_detector_ids_to_negative_detectors_fails(self):
+        self.presenter.handle_add_group_button_clicked()
+        self.view.grouping_table.item(0, 1).setText("-10-10")
+
+        self.view.warning_popup.assert_called_once_with('Invalid detector list.')
 
 if __name__ == '__main__':
     unittest.main(buffer=False, verbosity=2)
-- 
GitLab