AutoTest: Fix and unify handling of group nodes

QtTest had been forgotten to handle correctly as well. So, it
was possible to have several levels of grouping which had not
been handled correctly.
By (current) definition we handle only one level of grouping,
so prohibit more for QtTest tree items as well.
Basically move the check into a separate function and use this
beforehand instead of creating a nullptr.

Change-Id: Icbf02eae67e89464f371eb349eecf2976636d05f
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/autotest/gtest/gtesttreeitem.cpp

@@ -403,8 +403,6 @@ bool GTestTreeItem::modify(const TestParseResult *result)
 
 TestTreeItem *GTestTreeItem::createParentGroupNode() const
 {
-    if (type() != TestCase)
-        return nullptr;
     if (GTestFramework::groupMode() == GTest::Constants::Directory) {
         const QFileInfo fileInfo(filePath());
         const QFileInfo base(fileInfo.absolutePath());
@@ -511,6 +509,11 @@ bool GTestTreeItem::isGroupNodeFor(const TestTreeItem *other) const
     }
 }
 
+bool GTestTreeItem::isGroupable() const
+{
+    return type() == TestCase;
+}
+
 TestTreeItem *GTestTreeItem::applyFilters()
 {
     if (type() != TestCase)

src/plugins/autotest/gtest/gtesttreeitem.h

@@ -72,6 +72,7 @@ public:
     QString nameSuffix() const;
     QSet<QString> internalTargets() const override;
     bool isGroupNodeFor(const TestTreeItem *other) const override;
+    bool isGroupable() const override;
     TestTreeItem *applyFilters() override;
 private:
     bool modifyTestSetContent(const GTestParseResult *result);

src/plugins/autotest/qtest/qttesttreeitem.cpp

@@ -346,6 +346,11 @@ TestTreeItem *QtTestTreeItem::createParentGroupNode() const
     return new QtTestTreeItem(base.baseName(), fileInfo.absolutePath(), TestTreeItem::GroupNode);
 }
 
+bool QtTestTreeItem::isGroupable() const
+{
+    return type() == TestCase;
+}
+
 TestTreeItem *QtTestTreeItem::findChildByNameAndInheritance(const QString &name, bool inherited) const
 {
     return findFirstLevelChild([name, inherited](const TestTreeItem *other) {

src/plugins/autotest/qtest/qttesttreeitem.h

@@ -52,6 +52,7 @@ public:
     void setInherited(bool inherited) { m_inherited = inherited; }
     bool inherited() const { return m_inherited; }
     TestTreeItem *createParentGroupNode() const override;
+    bool isGroupable() const override;
 private:
     TestTreeItem *findChildByNameAndInheritance(const QString &name, bool inherited) const;
     QString nameSuffix() const;

src/plugins/autotest/quick/quicktesttreeitem.cpp

@@ -401,16 +401,16 @@ bool QuickTestTreeItem::removeOnSweepIfEmpty() const
 
 TestTreeItem *QuickTestTreeItem::createParentGroupNode() const
 {
-    if (filePath().isEmpty() || name().isEmpty())
-        return nullptr;
-    if (type() == TestFunctionOrSet)
-        return nullptr;
-
     const QFileInfo fileInfo(filePath());
     const QFileInfo base(fileInfo.absolutePath());
     return new QuickTestTreeItem(base.baseName(), fileInfo.absolutePath(), TestTreeItem::GroupNode);
 }
 
+bool QuickTestTreeItem::isGroupable() const
+{
+    return type() == TestCase && !name().isEmpty() && !filePath().isEmpty();
+}
+
 QSet<QString> QuickTestTreeItem::internalTargets() const
 {
     QSet<QString> result;

src/plugins/autotest/quick/quicktesttreeitem.h

@@ -53,6 +53,7 @@ public:
     bool isGroupNodeFor(const TestTreeItem *other) const override;
     bool removeOnSweepIfEmpty() const override;
     TestTreeItem *createParentGroupNode() const override;
+    bool isGroupable() const override;
     QSet<QString> internalTargets() const override;
     void markForRemovalRecursively(const QString &filePath) override;
 private:

src/plugins/autotest/testtreeitem.cpp

@@ -302,6 +302,11 @@ bool TestTreeItem::isGroupNodeFor(const TestTreeItem *other) const
     return QFileInfo(other->filePath()).absolutePath() == filePath();
 }
 
+bool TestTreeItem::isGroupable() const
+{
+    return true;
+}
+
 QSet<QString> TestTreeItem::internalTargets() const
 {
     auto cppMM = CppTools::CppModelManager::instance();

src/plugins/autotest/testtreeitem.h

@@ -120,6 +120,7 @@ public:
     virtual TestTreeItem *findChild(const TestTreeItem *other) = 0;
     virtual bool modify(const TestParseResult *result) = 0;
     virtual bool isGroupNodeFor(const TestTreeItem *other) const;
+    virtual bool isGroupable() const;
     virtual TestTreeItem *createParentGroupNode() const = 0;
     // based on (internal) filters this will be used to filter out sub items (and remove them)
     // returns a copy of the item that contains the filtered out children or nullptr

src/plugins/autotest/testtreemodel.cpp

@@ -346,13 +346,13 @@ static void applyParentCheckState(TestTreeItem *parent, TestTreeItem *newItem)
 void TestTreeModel::insertItemInParent(TestTreeItem *item, TestTreeItem *root, bool groupingEnabled)
 {
     TestTreeItem *parentNode = root;
-    if (groupingEnabled) {
+    if (groupingEnabled && item->isGroupable()) {
         parentNode = root->findFirstLevelChild([item] (const TestTreeItem *it) {
             return it->isGroupNodeFor(item);
         });
         if (!parentNode) {
             parentNode = item->createParentGroupNode();
-            if (!parentNode) // we might not get a group node at all
+            if (!QTC_GUARD(parentNode)) // we might not get a group node at all
                 parentNode = root;
             else
                 root->appendChild(parentNode);