From 74d71f24ca16ba1dd34d684a49ff90116aa7f935 Mon Sep 17 00:00:00 2001
From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com>
Date: Sat, 12 Nov 2022 18:22:30 +0100
Subject: [PATCH] Improve the setOrder method in the NWTree class, and improve
 tests

---
 novelwriter/core/tree.py          | 23 ++++++---------
 tests/test_core/test_core_tree.py | 49 ++++++++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/novelwriter/core/tree.py b/novelwriter/core/tree.py
index 9d499aaf..1f9960de 100644
--- a/novelwriter/core/tree.py
+++ b/novelwriter/core/tree.py
@@ -331,20 +331,15 @@ class NWTree:
     def setOrder(self, newOrder):
         """Reorders the tree based on a list of items.
         """
-        tmpOrder = []
-
-        # Add all known elements to a new temp list
-        for tHandle in newOrder:
-            if tHandle in self._projTree:
-                tmpOrder.append(tHandle)
-            else:
-                logger.error("Handle '%s' in new tree order is not in project tree", tHandle)
-
-        # Do a reverse lookup to check for items that will be lost
-        # This is mainly for debugging purposes
-        for tHandle in self._treeOrder:
-            if tHandle not in tmpOrder:
-                logger.warning("Handle '%s' in old tree order is not in new tree order", tHandle)
+        tmpOrder = [tHandle for tHandle in newOrder if tHandle in self._projTree]
+        if not (len(tmpOrder) == len(newOrder) == len(self._treeOrder)):
+            # Something is wrong, so let's debug it
+            for tHandle in newOrder:
+                if tHandle not in self._projTree:
+                    logger.error("Handle '%s' in new tree order is not in old order", tHandle)
+            for tHandle in self._treeOrder:
+                if tHandle not in tmpOrder:
+                    logger.warning("Handle '%s' in old tree order is not in new order", tHandle)
 
         # Save the temp list
         self._treeOrder = tmpOrder
diff --git a/tests/test_core/test_core_tree.py b/tests/test_core/test_core_tree.py
index 9c3d0961..731e11aa 100644
--- a/tests/test_core/test_core_tree.py
+++ b/tests/test_core/test_core_tree.py
@@ -24,6 +24,7 @@ import random
 
 from pathlib import Path
 
+from mock import causeOSError
 from tools import readFile
 
 from novelwriter.enum import nwItemClass, nwItemType, nwItemLayout
@@ -230,6 +231,36 @@ def testCoreTree_BuildTree(mockGUI, mockItems):
 # END Test testCoreTree_BuildTree
 
 
+@pytest.mark.core
+def testCoreTree_PackUnpack(mockGUI, mockItems):
+    """Test packing and unpacking data.
+    """
+    theProject = NWProject(mockGUI)
+    theTree = NWTree(theProject)
+
+    aHandles = []
+    for tHandle, pHandle, nwItem in mockItems:
+        aHandles.append(tHandle)
+        theTree.append(tHandle, pHandle, nwItem)
+        theTree.updateItemData(tHandle)
+
+    assert len(theTree) == len(mockItems)
+
+    # Pack
+    tree = theTree.pack()
+    for i, (tHandle, pHandle, nwItem) in enumerate(mockItems):
+        assert tree[i]["itemAttr"]["handle"] == tHandle
+
+    # Unpack
+    theTree.clear()
+    assert len(theTree) == 0
+    assert theTree.handles() == []
+    assert theTree.unpack(tree) is True
+    assert theTree.handles() == aHandles
+
+# END Test testCoreTree_PackUnpack
+
+
 @pytest.mark.core
 def testCoreTree_Methods(mockGUI, mockItems):
     """Test various class methods.
@@ -272,6 +303,13 @@ def testCoreTree_Methods(mockGUI, mockItems):
     assert theTree.findRoot(nwItemClass.NOVEL) == "a000000000001"
     assert theTree.findRoot(nwItemClass.CHARACTER) == "a000000000004"
 
+    # Iter roots
+    roots = list(theTree.iterRoots(None))
+    assert roots[0][0] == "a000000000001"
+    assert roots[1][0] == "a000000000002"
+    assert roots[2][0] == "a000000000003"
+    assert roots[3][0] == "a000000000004"
+
     # Add a fake item to root and check that it can handle it
     theTree._treeRoots["0000000000000"] = NWItem(theProject)
     assert theTree.findRoot(nwItemClass.WORLD) is None
@@ -363,7 +401,7 @@ def testCoreTree_Stats(mockGUI, mockItems):
 
 
 @pytest.mark.core
-def testCoreTree_Reorder(mockGUI, mockItems):
+def testCoreTree_Reorder(caplog, mockGUI, mockItems):
     """Test changing tree order.
     """
     theProject = NWProject(mockGUI)
@@ -384,12 +422,16 @@ def testCoreTree_Reorder(mockGUI, mockItems):
     theTree.setOrder(bHandle)
     assert theTree.handles() == bHandle
 
+    caplog.clear()
     theTree.setOrder(bHandle + ["stuff"])
     assert theTree.handles() == bHandle
+    assert "Handle 'stuff' in new tree order is not in old order" in caplog.text
 
+    caplog.clear()
     theTree._treeOrder.append("stuff")
     theTree.setOrder(bHandle)
     assert theTree.handles() == bHandle
+    assert "Handle 'stuff' in old tree order is not in new order" in caplog.text
 
 # END Test testCoreTree_Reorder
 
@@ -421,6 +463,11 @@ def testCoreTree_ToCFile(monkeypatch, mockGUI, mockItems, tmpPath):
     theProject._storage._runtimePath = None
     assert theTree.writeToCFile() is False
 
+    theProject._storage._runtimePath = tmpPath
+    with monkeypatch.context() as mp:
+        mp.setattr("builtins.open", causeOSError)
+        assert theTree.writeToCFile() is False
+
     theProject._storage._runtimePath = tmpPath
     assert theTree.writeToCFile() is True