Internal refactoring (ensure _children is always defined) and add sev…

…eral tests

sismic/model.py

@@ -302,7 +302,7 @@ def __init__(self, name: str, description: str=None, preamble: str=None):
 
         self._states = {}  # name -> State object
         self._parent = {}  # name -> parent.name
-        self._children = {}  # name -> list of names
+        self._children = {None: []}  # name -> list of names
         self._transitions = []  # list of Transition objects
 
     @property
@@ -365,9 +365,8 @@ def add_state(self, state: StateMixin, parent):
         # Save state
         self._states[state.name] = state
         self._parent[state.name] = parent
-
-        if parent:
-            self._children.setdefault(parent, []).append(state.name)
+        self._children[state.name] = []
+        self._children[parent].append(state.name)
 
     def remove_state(self, name: str):
         """
@@ -383,7 +382,7 @@ def remove_state(self, name: str):
         state = self.state_for(name)
 
         # Remove children
-        for child in self.children_for(state.name):
+        for child in list(self.children_for(state.name)):
             self.remove_state(child)
 
         # Remove transitions
@@ -399,16 +398,11 @@ def remove_state(self, name: str):
                 other_state.memory = None
 
         # Remove state
-        try:
-            self._children.pop(name)
-        except KeyError:
-            pass
-        try:
-            self._parent.pop(name)
-        except KeyError:
-            pass
-
         self._states.pop(name)
+        parent = self._parent.pop(name)
+        self._children.pop(name)
+
+        self._children[parent].remove(name)
 
     def rename_state(self, old_name: str, new_name: str):
         """
@@ -453,12 +447,10 @@ def rename_state(self, old_name: str, new_name: str):
 
         self._states[new_name] = self._states.pop(old_name)
         self._parent[new_name] = self._parent.pop(old_name)
-        self._children[new_name] = self._children.pop(old_name, [])
+        self._children[new_name] = self._children.pop(old_name)
 
-        parent_children = self._children.get(parent_name, [old_name])  # Default to avoid catching an exception
-        parent_children.remove(old_name)
-        parent_children.append(new_name)
-        self._children[parent_name] = parent_children
+        self._children[parent_name].remove(old_name)
+        self._children[parent_name].append(new_name)
 
     def move_state(self, name: str, new_parent: str):
         """
@@ -536,7 +528,7 @@ def children_for(self, name: str) -> list:
         """
         self.state_for(name)  # Raise StatechartError if state does not exist
 
-        return self._children.get(name, [])
+        return self._children[name]
 
     def ancestors_for(self, name: str) -> list:
         """

tests/test_model.py

@@ -284,6 +284,11 @@ def test_remove_existing_state(self):
         self.assertEqual(nb_transitions, 0)
         self.sc.validate()
 
+    def test_remove_unregister_parent_children(self):
+        self.sc.remove_state('s1')
+        self.assertFalse('s1' in self.sc.children_for('active'))
+        self.sc.validate()
+
     def test_remove_unexisting_state(self):
         with self.assertRaises(exceptions.StatechartError):
             self.sc.remove_state('unknown')
@@ -293,12 +298,18 @@ def test_remove_root_state(self):
         self.sc.remove_state('root')
         self.assertEqual(len(self.sc.transitions), 0)
         self.assertEqual(len(self.sc.states), 0)
+        self.assertEqual(self.sc.root, None)
         self.sc.validate()
 
-    def test_remove_appropriate_state(self):
-        self.sc.remove_state('active')
+    def test_remove_nested_states(self):
+        self.sc = io.import_from_yaml(open('tests/yaml/composite.yaml'))
         self.sc.remove_state('s1')
-        self.sc.remove_state('s2')
+        self.assertFalse('s1a' in self.sc.states)
+        self.sc.validate()
+
+        self.sc = io.import_from_yaml(open('tests/yaml/composite.yaml'))
+        self.sc.remove_state('s1a')
+        self.assertFalse('s1a' in self.sc.states)
         self.sc.validate()
 
 
@@ -313,24 +324,49 @@ def test_rename_unexisting_state(self):
 
     def test_do_not_change_name(self):
         self.sc.rename_state('s2', 's2')
+
+        self.assertTrue('s2' in self.sc.states)
+        self.assertEqual(self.sc.parent_for('s2'), 'root')
+        self.assertTrue('s2' in self.sc.children_for('root'))
+
         self.sc.validate()
 
     def test_rename_to_an_existing_state(self):
         with self.assertRaises(exceptions.StatechartError):
             self.sc.rename_state('s2', 's1')
         self.sc.validate()
 
-    def test_rename_simple(self):
-        self.sc.rename_state('active', 's3')
-        self.assertTrue('s3' in self.sc.states)
-        self.assertFalse('active' in self.sc.states)
+    def test_rename_old_disappears(self):
+        self.sc = io.import_from_yaml(open('tests/yaml/composite.yaml'))
+        self.sc.rename_state('s1', 'new s1')
+
+        self.assertFalse('s1' in self.sc.states)
+        self.assertNotEqual('s1', self.sc.parent_for('s1a'))
+        self.assertFalse('s1' in self.sc.children_for('root'))
+
+        with self.assertRaises(exceptions.StatechartError):
+            self.sc.state_for('s1')
+        with self.assertRaises(exceptions.StatechartError):
+            self.sc.children_for('s1')
+        with self.assertRaises(exceptions.StatechartError):
+            self.sc.parent_for('s1')
         self.sc.validate()
 
-    def test_rename_with_transitions(self):
-        self.sc.rename_state('s1', 's3')
+    def test_rename_new_appears(self):
+        self.sc = io.import_from_yaml(open('tests/yaml/composite.yaml'))
+        self.sc.rename_state('s1', 'new s1')
+
+        self.assertTrue('new s1' in self.sc.states)
+        self.assertEqual('new s1', self.sc.parent_for('s1a'))
+        self.assertTrue('new s1' in self.sc.children_for('root'))
 
-        self.assertTrue('s1' not in self.sc.states)
-        self.assertTrue('s3' in self.sc.states)
+        self.sc.state_for('new s1')
+        self.sc.children_for('new s1')
+        self.sc.parent_for('new s1')
+        self.sc.validate()
+
+    def test_rename_adapt_transitions(self):
+        self.sc.rename_state('s1', 's3')
 
         nb_transitions = 0
         for transition in self.sc.transitions:
@@ -353,6 +389,13 @@ def test_rename_root(self):
         self.assertEqual(self.sc.parent_for('new root'), None)
         self.assertEqual(self.sc.parent_for('s1'), 'new root')
         self.assertFalse('root' in self.sc.states)
+
+        self.assertTrue('new root' in self.sc.states)
+        self.assertFalse('root' in self.sc.states)
+        self.assertEqual(self.sc.parent_for('new root'), None)
+        self.assertEqual(self.sc.parent_for('active'), 'new root')
+        self.assertTrue('active' in self.sc.children_for('new root'))
+
         self.sc.validate()
 
     def test_rename_change_initial(self):