[tests] Add tests and more protection for the rename in core/graph

Author: Alxiice

meshroom/core/graph.py

@@ -514,14 +514,23 @@ def addNode(self, node, uniqueName=None):
         return node
 
     def renameNode(self, node: Node, newName: str):
-        """ Rename a node in the Node Graph
+        """ Rename a node in the Node Graph.
+        If the proposed name is already assigned to a node then it will create a unique name
 
         Args:
             node (Node): Node to rename.
             newName (str): New name of the node. Must be unique in the graph.
         """
+        # Handle empty string
+        if not newName:
+            return
+        usedNames = {n._name for n in self._nodes if n != node}
+        # Make sure we rename to an available name
+        if newName in usedNames:
+            newName = self._createUniqueNodeName(newName, usedNames)
+        # Rename in the dict model
         self._nodes.rename(node._name, newName)
-        # Finally rename
+        # Finally rename the node name property and notify Qt
         node._name = newName   
         node.nodeNameChanged.emit()
 

meshroom/ui/commands.py

@@ -162,13 +162,9 @@ def __init__(self, graph, node, name, parent=None):
         self.name = name
 
     def redoImpl(self):
-        newName = self.name
-        usedNames = {n._name for n in self.graph._nodes}
-        if newName in usedNames:
-            newName = self.graph._createUniqueNodeName(self.name, usedNames)
-        self.setText(f"Rename Node {self.oldName} to {newName}")
+        self.setText(f"Rename Node {self.oldName} to {self.name}")
         self.graph.renameNode(self.node, self.name)
-        return newName
+        return self.node._name
 
     def undoImpl(self):
         self.graph.renameNode(self.node, self.oldName)

tests/test_graph.py

@@ -285,3 +285,27 @@ def test_cyclic_connection_should_raise_error():
     # Then
     with pytest.raises(CyclicDependencyError):
         tC.output.connectTo(tB.input)
+
+
+def test_rename_nodes():
+    """
+    Test renaming nodes.
+    """
+
+    graph = Graph("")
+    ls0 = graph.addNewNode("Ls")
+    ls1 = graph.addNewNode("Ls")
+    ls2 = graph.addNewNode("Ls")
+    
+    # Test with empty string
+    assert ls0.name == "Ls_1"
+    graph.renameNode(ls0, "")
+    assert ls0.name == "Ls_1"
+    
+    # Rename
+    graph.renameNode(ls0, "nodels")
+    assert ls0.name == "nodels"
+    graph.renameNode(ls1, "nodels")
+    assert ls1.name == "nodels_1"
+    graph.renameNode(ls2, "nodels")
+    assert ls2.name == "nodels_2"