Fix for 1668: Given a large test suite, selecting all tests from the root of tree, freezes RIDE for some time (#2210)

* 1668: The Select All Tests command on the root of a deeply nested test folder, freezes RIDE for several minutes.
The problem is caused by Tree.SelectAllTests strategy: expanding and collapsing all the nodes. This causes CPU waste to expand/collapse (and maybe render) and wait for the creation of the tree node handlers.
The idea of the fix is to navigate the tests using the link between controllers. In order to avoid all the dancing of the tree nodes.
In this commit:

- Changed Tree.SelectAllTests() to extract all the TestCaseController(s) and invoke TestSelectionController to change the selection for these.
- Now both _ActionHandler OnDeselectAllTests() and OnSelectAllTests() invoke Tree.SelectAllTests(), with a parameter.
- When a new TestCaseHandler is created, is registered to be notified about selection changes. It uses this notification to update the graphical node with the check.
-  RideTestSelectedForRunningChanged now also holds the controller of the test which has changed selection and whether has been selected or unselected.

* Replace Tree DeselectAllTests(node) with SelectAllTests(node,False) as it's much more efficient.

* TestSelectionController: Keep a copy of the tests to propagate in the event, rather than rebuild it everytime.

* When selecting multiple tests, from the test search, do it efficiently.

TestSelectionController: add select_all() to select multiple tests efficiently. Rewrite unselect_all() in terms of select_all()

Tree: when selecting multiple tests, no need to navigate the full tree, just use TestSelectionController.

* When deselecting multiple tests, from the test search, do it efficiently.

Tree: when deselecting multiple tests, no need to navigate the full tree, just use TestSelectionController.

* Do not expand all the tree nodes when selecting failed/passed tests.

- Keep the information of failed/passed tests in TestCaseController
- Update the information on the test execution
- When selecting only failed/passed tests navigate the controller'tree rather than the graphical tree.
- removed method _for_all_tests() as it was causing all the nodes to be loaded/rendered.

* When selecting/deselecting multiple Tests, notify a single selection change event. Rather than one for each Test.

* Restore previous behavior with "Select Failed/Passed": select only those and de-select any other test.

* keep track of the selected tests using the TestCaseController(s), instead of the list of names.

This removes the need for listen of name changes.

* No need to generate the list of names, when determining if there's any.

* Fix TestSelectionController.add_tag()

* Handle RideTestSelectedForRunningChanged event on a new wx thread stack.
This is to avoid the "Recursionerror: maximum recursion depth exceeded"

* Remove unused variable.

* Propagate the RideTestSelectedForRunningChanged event only if the selection actually changed.

Co-authored-by: Valerio Bruno <[email protected]>

Author: valeriobruno

src/robotide/contrib/testrunner/testrunnerplugin.py

@@ -60,6 +60,7 @@
 from robotide.context import IS_WINDOWS, IS_MAC
 from robotide.contrib.testrunner import TestRunner
 from robotide.contrib.testrunner import runprofiles
+from robotide.controller.macrocontrollers import TestCaseController
 from robotide.publish import RideSettingsChanged, PUBLISHER
 from robotide.publish.messages import RideTestSelectedForRunningChanged
 from robotide.pluginapi import Plugin, ActionInfo
@@ -136,7 +137,11 @@ def __init__(self, application=None):
         self._test_runner = TestRunner(application.model)
         self._register_shortcuts()
         self._min_log_level_number = LOG_LEVELS['INFO']
-        self._names_to_run = set()
+        self._selected_tests: {TestCaseController} = set()
+
+    @property
+    def _names_to_run(self):
+        return list(map(lambda ctrl: (ctrl.datafile_controller.longname, ctrl.longname), self._selected_tests))
 
     def _register_shortcuts(self):
         self.register_shortcut('CtrlCmd-C', self._copy_from_out)
@@ -219,7 +224,7 @@ def OnSettingsChanged(self, data):
             self.save_setting(setting, data.new)
 
     def OnTestSelectedForRunningChanged(self, message):
-        self._names_to_run = message.tests
+        self._selected_tests = message.tests
 
     def disable(self):
         self._remove_from_notebook()
@@ -305,7 +310,7 @@ def OnRun(self, event):
         if not self._can_start_running_tests():
             return
         if self.__getattr__('confirm run'):
-            if not self.tests_selected():
+            if not self._tests_selected():
                 if not self.ask_user_to_run_anyway():
                     # In Linux NO runs dialog 4 times
                     return
@@ -375,8 +380,8 @@ def _ask_user_to_save_before_running(self):
                             wx.ICON_QUESTION | wx.YES_NO)
         return ret == wx.YES
 
-    def tests_selected(self):
-        return len(self._names_to_run) != 0
+    def _tests_selected(self):
+        return len(self._selected_tests) != 0
 
     def ask_user_to_run_anyway(self):
         ret = wx.MessageBox('No tests selected. \n'

src/robotide/controller/filecontrollers.py

@@ -626,6 +626,13 @@ def exclude(self):
         self.parent.children[index] = result
         return result
 
+    def retrieve_test_controllers(self):
+        controllers: list[TestCaseController] = []
+        for child in self.children:
+            if isinstance(child,TestCaseFileController) or isinstance(child,TestDataDirectoryController):
+                controllers += child.retrieve_test_controllers()
+        return controllers
+
 
 class DirtyRobotDataException(Exception):
     """
@@ -705,6 +712,11 @@ def reload(self):
     def get_template(self):
         return self.data.setting_table.test_template
 
+    def retrieve_test_controllers(self) :
+        controllers = []
+        for test_ctrl in iter(self.tests):
+            controllers.append(test_ctrl)
+        return controllers
 
 class ResourceFileControllerFactory(object):
 

src/robotide/controller/macrocontrollers.py

@@ -312,6 +312,7 @@ class TestCaseController(_WithStepsController):
 
     def _init(self, test):
         self._test = test
+        self._run_passed = None
 
     def __eq__(self, other):
         if self is other:
@@ -389,6 +390,18 @@ def _get_template(self):
             return template
         return self.datafile_controller.get_template()
 
+    @property
+    def run_passed(self):
+        return self._run_passed
+
+    @run_passed.setter
+    def run_passed(self,value):
+        if value == True:
+            self._run_passed = True # Test execution passed
+        elif value == False:
+            self._run_passed = False # Test execution failed
+        else:
+            self._run_passed = None # Test did not run
 
 class UserKeywordController(_WithStepsController):
     _populator = robotapi.UserKeywordPopulator

src/robotide/controller/ui/treecontroller.py

@@ -18,6 +18,7 @@
 from robotide.action.actioninfo import ActionInfoCollection, ActionInfo
 from robotide.context import IS_WINDOWS, ctrl_or_cmd, bind_keys_to_evt_menu
 from robotide.controller.ctrlcommands import ChangeTag
+from robotide.controller.macrocontrollers import TestCaseController
 from robotide.controller.tags import Tag, DefaultTag
 from robotide.controller.filecontrollers import TestCaseFileController
 from robotide.publish import PUBLISHER, RideTestSelectedForRunningChanged, RideItemNameChanged, RideFileNameChanged, RideNewProject, RideOpenSuite
@@ -60,7 +61,7 @@ def OnAddTagToSelected(self, event):
             self._test_selection.add_tag(name)
 
     def OnClearSelected(self, event):
-        self._tree.DeselectAllTests(self._tree._root)
+        self._test_selection.clear_all()
 
     def OnGoForward(self, event):
         node = self._history.forward()
@@ -175,57 +176,43 @@ def top(self):
 class TestSelectionController(object):
 
     def __init__(self):
-        self._tests = {}
-        self._subscribe()
-
-    def _subscribe(self):
-        PUBLISHER.subscribe(self._test_name_changed, RideItemNameChanged)
-        PUBLISHER.subscribe(self._suite_name_changed, RideFileNameChanged)
-
-    def _test_name_changed(self, message):
-        longname = message.item.longname
-        path, new_name = longname.rsplit('.', 1)
-        if message.old_name:
-            old_name = path + '.' + message.old_name
-            self._tests[longname] = new_name
-            del self._tests[old_name]
-
-    def _suite_name_changed(self, message):
-        df = message.datafile
-        if isinstance(df, TestCaseFileController):
-            filename = os.path.splitext(os.path.basename(message.old_filename))[0]
-            old_name = df.longname[:-len(df.name)] + filename
-            for test in self._tests:
-                if test.lower().startswith(old_name.lower()):
-                    del self._tests[test]
+        self._tests: {TestCaseController} = set()
 
     def is_empty(self):
         return not bool(self._tests)
 
     def is_test_selected(self, test):
-        return test.longname in self._tests.keys()
+        return test in self._tests
 
-    def clear_all(self, message=None):
-        self._tests = {}
-        self.send_selection_changed_message()
+    def clear_all(self):
+        self._tests = set()
+        self._send_selection_changed_message()
 
     def unselect_all(self, tests):
-        for test in tests:
-            self.select(test, False)
-
-    def select(self, test, selected=True):
-        if selected:
-            self._tests[test.longname] = test
-        elif self.is_test_selected(test):
-            del self._tests[test.longname]
-        self.send_selection_changed_message()
+        self.select_all(tests, selected=False)
 
-    def send_selection_changed_message(self):
-        RideTestSelectedForRunningChanged(tests=set([(t.datafile_controller.longname, t.longname)
-                                                     for t in self._tests.values()])).publish()
+    def select_all(self, tests, selected=True):
+        for test in tests:
+            self.select(test, selected, notifySelection=False)
+        self._send_selection_changed_message()
+
+    def select(self, test: TestCaseController, doSelect=True, notifySelection=True):
+        changed = False
+        if doSelect and not self.is_test_selected(test):
+            self._tests.add(test)
+            changed = True
+        elif not doSelect and self.is_test_selected(test):
+            self._tests.remove(test)
+            changed = True
+        if notifySelection and changed:
+            self._send_selection_changed_message()
+
+    def _send_selection_changed_message(self):
+        message = RideTestSelectedForRunningChanged(tests=self._tests)
+        wx.CallAfter(message.publish)
 
     def add_tag(self, name):
-        for test in self._tests.values():
+        for test in self._tests:
             self._add_tag_to_test(name, test)
 
     def _add_tag_to_test(self, name, test):

src/robotide/ui/treenodehandlers.py

@@ -30,7 +30,7 @@
 from robotide.editor.editordialogs import (
     TestCaseNameDialog, UserKeywordNameDialog, ScalarVariableDialog,
     ListVariableDialog, CopyUserKeywordDialog, DictionaryVariableDialog)
-from robotide.publish import RideOpenVariableDialog
+from robotide.publish import RideOpenVariableDialog, RideTestSelectedForRunningChanged, PUBLISHER
 from robotide.ui.progress import LoadProgressObserver
 from robotide.usages.UsageRunner import Usages, ResourceFileUsages
 from .filedialogs import (
@@ -155,7 +155,7 @@ def OnSelectAllTests(self, event):
         self._tree.SelectAllTests(self._node)
 
     def OnDeselectAllTests(self, event):
-        self._tree.DeselectAllTests(self._node)
+        self._tree.SelectAllTests(self._node, False)
 
     def OnSelectOnlyFailedTests(self, event):
         self._tree.SelectFailedTests(self._node)
@@ -488,7 +488,7 @@ def _rename_command(self, label):
 
     @overrides(_FileHandlerThanCanBeRenamed)
     def _rename_ok_handler(self):
-        self._tree.DeselectAllTests(self._node)
+        self._tree.SelectAllTests(self._node,False)
 
 
 class _TestOrUserKeywordHandler(_CanBeRenamed, _ActionHandler):
@@ -524,6 +524,11 @@ def OnDelete(self, event):
 
 
 class TestCaseHandler(_TestOrUserKeywordHandler):
+    def __init__(self, controller, tree, node, settings):
+        _TestOrUserKeywordHandler.__init__(self, controller, tree, node, settings)
+        PUBLISHER.subscribe(self.test_selection_changed, RideTestSelectedForRunningChanged,
+                            key=self)  # TODO: unsubscribe when the object is destroyed!
+
     _datalist = property(lambda self: self.item.datalist)
     _copy_name_dialog_class = TestCaseNameDialog
 
@@ -533,6 +538,13 @@ def _add_copy_to_tree(self, parent_node, copied):
     def _create_rename_command(self, new_name):
         return RenameTest(new_name)
 
+    def test_selection_changed(self, message: RideTestSelectedForRunningChanged):
+        if self.controller in message.tests:
+            if not self.node.GetValue():
+                self._tree.CheckItem(self.node, checked=True)
+        else:
+            if self.node.GetValue():
+                self._tree.CheckItem(self.node, checked=False)
 
 class UserKeywordHandler(_TestOrUserKeywordHandler):
     is_user_keyword = True