From 96ccb95e6fd8d198aa962e96320dbdeb942f158e Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Tue, 13 Mar 2018 13:38:43 +0100 Subject: [PATCH] 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 Reviewed-by: David Schulz --- src/plugins/autotest/gtest/gtesttreeitem.cpp | 31 +++++++++++++++++++ src/plugins/autotest/gtest/gtesttreeitem.h | 1 + src/plugins/autotest/qtest/qttesttreeitem.cpp | 24 ++++++++++++++ src/plugins/autotest/qtest/qttesttreeitem.h | 1 + .../autotest/quick/quicktesttreeitem.cpp | 22 +++++++++++++ .../autotest/quick/quicktesttreeitem.h | 1 + src/plugins/autotest/testtreeitem.cpp | 7 +++++ src/plugins/autotest/testtreeitem.h | 2 ++ src/plugins/autotest/testtreemodel.cpp | 20 +++++++++++- 9 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/plugins/autotest/gtest/gtesttreeitem.cpp b/src/plugins/autotest/gtest/gtesttreeitem.cpp index a85e09605cf..f05dcae9b7d 100644 --- a/src/plugins/autotest/gtest/gtesttreeitem.cpp +++ b/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(other); + result = findChildByNameStateAndFile(gtOther->name(), gtOther->state(), + gtOther->proFile()); + } + return (result && result->type() == otherType) ? result : nullptr; + } + case GroupNode: { + auto gtOther = static_cast(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); diff --git a/src/plugins/autotest/gtest/gtesttreeitem.h b/src/plugins/autotest/gtest/gtesttreeitem.h index eac6c9696c6..c3ff8dba71f 100644 --- a/src/plugins/autotest/gtest/gtesttreeitem.h +++ b/src/plugins/autotest/gtest/gtesttreeitem.h @@ -58,6 +58,7 @@ public: QList getAllTestConfigurations() const override; QList 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; diff --git a/src/plugins/autotest/qtest/qttesttreeitem.cpp b/src/plugins/autotest/qtest/qttesttreeitem.cpp index 8035c934e50..04c0d9c129a 100644 --- a/src/plugins/autotest/qtest/qttesttreeitem.cpp +++ b/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(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); diff --git a/src/plugins/autotest/qtest/qttesttreeitem.h b/src/plugins/autotest/qtest/qttesttreeitem.h index 2d6d443db5e..9b70b735fad 100644 --- a/src/plugins/autotest/qtest/qttesttreeitem.h +++ b/src/plugins/autotest/qtest/qttesttreeitem.h @@ -46,6 +46,7 @@ public: QList getAllTestConfigurations() const override; QList 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; } diff --git a/src/plugins/autotest/quick/quicktesttreeitem.cpp b/src/plugins/autotest/quick/quicktesttreeitem.cpp index 68a844fbbca..69f3cdc8447 100644 --- a/src/plugins/autotest/quick/quicktesttreeitem.cpp +++ b/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); diff --git a/src/plugins/autotest/quick/quicktesttreeitem.h b/src/plugins/autotest/quick/quicktesttreeitem.h index 88d21bf637c..48c2467de98 100644 --- a/src/plugins/autotest/quick/quicktesttreeitem.h +++ b/src/plugins/autotest/quick/quicktesttreeitem.h @@ -46,6 +46,7 @@ public: QList getAllTestConfigurations() const override; QList 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; diff --git a/src/plugins/autotest/testtreeitem.cpp b/src/plugins/autotest/testtreeitem.cpp index 6d75efb4408..9d42858a99c 100644 --- a/src/plugins/autotest/testtreeitem.cpp +++ b/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 { diff --git a/src/plugins/autotest/testtreeitem.h b/src/plugins/autotest/testtreeitem.h index 44a91a1682b..d47097efba2 100644 --- a/src/plugins/autotest/testtreeitem.h +++ b/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 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; diff --git a/src/plugins/autotest/testtreemodel.cpp b/src/plugins/autotest/testtreemodel.cpp index d525c717a2c..7260b272ca7 100644 --- a/src/plugins/autotest/testtreemodel.cpp +++ b/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); }