AutoTest: Avoid unneeded duplication on rebuild

When the rebuild of the tree model has been triggered
due to switching between grouping by filter or directory
it could happen that some children did not get merged
into others due to (insignificant) differences.
Avoid this visual duplication by finding items similar
to the one to be added and if there is one re-use this
instead.

Change-Id: Ife49593638e0af23ffc7353e305be4ea25eb2180
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/autotest/gtest/gtesttreeitem.cpp

@@ -323,6 +323,37 @@ TestTreeItem *GTestTreeItem::find(const TestParseResult *result)
     }
 }
 
+TestTreeItem *GTestTreeItem::findChild(const TestTreeItem *other)
+{
+    QTC_ASSERT(other, return nullptr);
+    const Type otherType = other->type();
+    switch (type()) {
+    case Root: {
+        TestTreeItem *result = nullptr;
+        if (otherType == GroupNode) {
+            result = findChildByNameAndFile(other->name(), other->filePath());
+        } else if (otherType == TestCase) {
+            auto gtOther = static_cast<const GTestTreeItem *>(other);
+            result = findChildByNameStateAndFile(gtOther->name(), gtOther->state(),
+                                                 gtOther->proFile());
+        }
+        return (result && result->type() == otherType) ? result : nullptr;
+    }
+    case GroupNode: {
+        auto gtOther = static_cast<const GTestTreeItem *>(other);
+        return otherType == TestCase
+                ? findChildByNameStateAndFile(gtOther->name(), gtOther->state(), gtOther->proFile())
+                : nullptr;
+    }
+    case TestCase:
+        return otherType == TestFunctionOrSet
+                ? findChildByNameAndFile(other->name(), other->filePath())
+                : nullptr;
+    default:
+        return nullptr;
+    }
+}
+
 bool GTestTreeItem::modify(const TestParseResult *result)
 {
     QTC_ASSERT(result, return false);

src/plugins/autotest/gtest/gtesttreeitem.h

@@ -58,6 +58,7 @@ public:
     QList<TestConfiguration *> getAllTestConfigurations() const override;
     QList<TestConfiguration *> getSelectedTestConfigurations() const override;
     TestTreeItem *find(const TestParseResult *result) override;
+    TestTreeItem *findChild(const TestTreeItem *other) override;
     bool modify(const TestParseResult *result) override;
     TestTreeItem *createParentGroupNode() const override;
 

src/plugins/autotest/qtest/qttesttreeitem.cpp

@@ -275,6 +275,30 @@ TestTreeItem *QtTestTreeItem::find(const TestParseResult *result)
     }
 }
 
+TestTreeItem *QtTestTreeItem::findChild(const TestTreeItem *other)
+{
+    QTC_ASSERT(other, return nullptr);
+    const Type otherType = other->type();
+    switch (type()) {
+    case Root:
+        return findChildByFileAndType(other->filePath(), otherType);
+    case GroupNode:
+        return otherType == TestCase ? findChildByFile(other->filePath()) : nullptr;
+    case TestCase: {
+        if (otherType != TestFunctionOrSet && otherType != TestDataFunction && otherType != TestSpecialFunction)
+            return nullptr;
+        auto qtOther = static_cast<const QtTestTreeItem *>(other);
+        return findChildByNameAndInheritance(other->filePath(), qtOther->inherited());
+    }
+    case TestFunctionOrSet:
+    case TestDataFunction:
+    case TestSpecialFunction:
+        return otherType == TestDataTag ? findChildByName(other->name()) : nullptr;
+    default:
+        return nullptr;
+    }
+}
+
 bool QtTestTreeItem::modify(const TestParseResult *result)
 {
     QTC_ASSERT(result, return false);

src/plugins/autotest/qtest/qttesttreeitem.h

@@ -46,6 +46,7 @@ public:
     QList<TestConfiguration *> getAllTestConfigurations() const override;
     QList<TestConfiguration *> getSelectedTestConfigurations() const override;
     TestTreeItem *find(const TestParseResult *result) override;
+    TestTreeItem *findChild(const TestTreeItem *other) override;
     bool modify(const TestParseResult *result) override;
     void setInherited(bool inherited) { m_inherited = inherited; }
     bool inherited() const { return m_inherited; }

src/plugins/autotest/quick/quicktesttreeitem.cpp

@@ -320,6 +320,28 @@ TestTreeItem *QuickTestTreeItem::find(const TestParseResult *result)
     }
 }
 
+TestTreeItem *QuickTestTreeItem::findChild(const TestTreeItem *other)
+{
+    QTC_ASSERT(other, return nullptr);
+    const Type otherType = other->type();
+
+    switch (type()) {
+    case Root:
+        if (otherType == TestCase && other->name().isEmpty())
+            return unnamedQuickTests();
+        return findChildByFileAndType(other->filePath(), otherType);
+    case GroupNode:
+        return findChildByFileAndType(other->filePath(), otherType);
+    case TestCase:
+        if (otherType != TestFunctionOrSet && otherType != TestDataFunction && otherType != TestSpecialFunction)
+            return nullptr;
+        return name().isEmpty() ? findChildByNameAndFile(other->name(), other->filePath())
+                                : findChildByName(other->name());
+    default:
+        return nullptr;
+    }
+}
+
 bool QuickTestTreeItem::modify(const TestParseResult *result)
 {
     QTC_ASSERT(result, return false);

src/plugins/autotest/quick/quicktesttreeitem.h

@@ -46,6 +46,7 @@ public:
     QList<TestConfiguration *> getAllTestConfigurations() const override;
     QList<TestConfiguration *> getSelectedTestConfigurations() const override;
     TestTreeItem *find(const TestParseResult *result) override;
+    TestTreeItem *findChild(const TestTreeItem *other) override;
     bool modify(const TestParseResult *result) override;
     bool lessThan(const TestTreeItem *other, SortMode mode) const override;
     bool isGroupNodeFor(const TestTreeItem *other) const override;

src/plugins/autotest/testtreeitem.cpp

@@ -229,6 +229,13 @@ TestTreeItem *TestTreeItem::findChildByFile(const QString &filePath)
     });
 }
 
+TestTreeItem *TestTreeItem::findChildByFileAndType(const QString &filePath, Type tType)
+{
+    return findChildBy([filePath, tType](const TestTreeItem *other) {
+        return other->type() == tType && other->filePath() == filePath;
+    });
+}
+
 TestTreeItem *TestTreeItem::findChildByNameAndFile(const QString &name, const QString &filePath)
 {
     return findChildBy([name, filePath](const TestTreeItem *other) -> bool {

src/plugins/autotest/testtreeitem.h

@@ -104,6 +104,7 @@ public:
 
     TestTreeItem *findChildByName(const QString &name);
     TestTreeItem *findChildByFile(const QString &filePath);
+    TestTreeItem *findChildByFileAndType(const QString &filePath, Type type);
     TestTreeItem *findChildByNameAndFile(const QString &name, const QString &filePath);
 
     virtual bool canProvideTestConfiguration() const { return false; }
@@ -115,6 +116,7 @@ public:
     virtual QList<TestConfiguration *> getSelectedTestConfigurations() const;
     virtual bool lessThan(const TestTreeItem *other, SortMode mode) const;
     virtual TestTreeItem *find(const TestParseResult *result) = 0;
+    virtual TestTreeItem *findChild(const TestTreeItem *other) = 0;
     virtual bool modify(const TestParseResult *result) = 0;
     virtual bool isGroupNodeFor(const TestTreeItem *other) const;
     virtual TestTreeItem *createParentGroupNode() const = 0;

src/plugins/autotest/testtreemodel.cpp

@@ -317,6 +317,15 @@ bool TestTreeModel::sweepChildren(TestTreeItem *item)
     return hasChanged;
 }
 
+static TestTreeItem *fullCopyOf(TestTreeItem *other)
+{
+    QTC_ASSERT(other, return nullptr);
+    TestTreeItem *result = other->copyWithoutChildren();
+    for (int row = 0, count = other->childCount(); row < count; ++row)
+        result->appendChild(fullCopyOf(other->childItem(row)));
+    return result;
+}
+
 void TestTreeModel::insertItemInParent(TestTreeItem *item, TestTreeItem *root, bool groupingEnabled)
 {
     TestTreeItem *parentNode = root;
@@ -332,7 +341,16 @@ void TestTreeModel::insertItemInParent(TestTreeItem *item, TestTreeItem *root, b
                 root->appendChild(parentNode);
         }
     }
-    parentNode->appendChild(item);
+    // check if a similar item is already present (can happen for rebuild())
+    if (auto otherChild = parentNode->findChild(item)) {
+        // only handle item's children and add them to the already present one
+        for (int row = 0, count = item->childCount(); row < count; ++row)
+            otherChild->appendChild(fullCopyOf(item->childItem(row)));
+        delete item;
+        item = otherChild; // TODO needed? or early return?
+    } else {
+        parentNode->appendChild(item);
+    }
     if (item->checked() != parentNode->checked())
         revalidateCheckState(parentNode);
 }