AutoTest: Do not pass pointers without owner between threads

Change-Id: I40e86716d1dd7c8a84e759e792042b84571fc2aa
Reviewed-by: Eike Ziller <eike.ziller@theqtcompany.com>

src/plugins/autotest/testcodeparser.cpp

@@ -83,8 +83,7 @@ TestCodeParser::TestCodeParser(TestTreeModel *parent)
             this, &TestCodeParser::onFinished);
     connect(&m_futureWatcher, &QFutureWatcher<TestParseResult>::resultReadyAt,
             this, [this] (int index) {
-        const TestParseResult result = m_futureWatcher.resultAt(index);
+        emit testParseResultReady(m_futureWatcher.resultAt(index));
-        emit testItemCreated(result.item, result.type);
     });
 }
 
@@ -427,65 +426,6 @@ static QMap<QString, TestCodeLocationList> checkForDataTags(const QString &fileN
     return QMap<QString, TestCodeLocationList>();
 }
 
-static TestTreeItem *constructTestTreeItem(const QString &fileName,
-                                           const QString &mainFile,  // used for Quick Tests only
-                                           const QString &testCaseName,
-                                           int line, int column,
-                                           const QMap<QString, TestCodeLocationAndType> &functions,
-                                           const QMap<QString, TestCodeLocationList> dataTags = QMap<QString, TestCodeLocationList>())
-{
-    TestTreeItem *treeItem = new TestTreeItem(testCaseName, fileName, TestTreeItem::TestClass);
-    treeItem->setMainFile(mainFile); // used for Quick Tests only
-    treeItem->setLine(line);
-    treeItem->setColumn(column);
-
-    foreach (const QString &functionName, functions.keys()) {
-        const TestCodeLocationAndType locationAndType = functions.value(functionName);
-        TestTreeItem *treeItemChild = new TestTreeItem(functionName, locationAndType.m_name,
-                                                       locationAndType.m_type);
-        treeItemChild->setLine(locationAndType.m_line);
-        treeItemChild->setColumn(locationAndType.m_column);
-        treeItemChild->setState(locationAndType.m_state);
-
-        // check for data tags and if there are any for this function add them
-        const QString qualifiedFunctionName = testCaseName + QLatin1String("::") + functionName;
-        if (dataTags.contains(qualifiedFunctionName)) {
-            const TestCodeLocationList &tags = dataTags.value(qualifiedFunctionName);
-            foreach (const TestCodeLocationAndType &tagLocation, tags) {
-                TestTreeItem *tagTreeItem = new TestTreeItem(tagLocation.m_name,
-                                                             locationAndType.m_name,
-                                                             tagLocation.m_type);
-                tagTreeItem->setLine(tagLocation.m_line);
-                tagTreeItem->setColumn(tagLocation.m_column);
-                tagTreeItem->setState(tagLocation.m_state);
-                treeItemChild->appendChild(tagTreeItem);
-            }
-        }
-
-        treeItem->appendChild(treeItemChild);
-    }
-    return treeItem;
-}
-
-static TestTreeItem *constructGTestTreeItem(const QString &filePath, const GTestCaseSpec &caseSpec,
-                                            const QString &proFile,
-                                            const TestCodeLocationList &testSets)
-{
-    TestTreeItem *item = new TestTreeItem(caseSpec.testCaseName, QString(),
-            caseSpec.parameterized ? TestTreeItem::GTestCaseParameterized
-                                   : TestTreeItem::GTestCase);
-    foreach (const TestCodeLocationAndType &locationAndType, testSets) {
-        TestTreeItem *treeItemChild = new TestTreeItem(locationAndType.m_name, filePath,
-                                                       locationAndType.m_type);
-        treeItemChild->setState(locationAndType.m_state);
-        treeItemChild->setLine(locationAndType.m_line);
-        treeItemChild->setColumn(locationAndType.m_column);
-        treeItemChild->setMainFile(proFile);
-        item->appendChild(treeItemChild);
-    }
-    return item;
-}
-
 /****** end of helpers ******/
 
 static void handleQtQuickTest(QFutureInterface<TestParseResult> futureInterface,
@@ -512,29 +452,16 @@ static void handleQtQuickTest(QFutureInterface<TestParseResult> futureInterface,
         const TestCodeLocationAndType tcLocationAndType = qmlVisitor.testCaseLocation();
         const QMap<QString, TestCodeLocationAndType> testFunctions = qmlVisitor.testFunctions();
 
-        TestTreeItem *testTreeItem;
+        TestParseResult parseResult(TestTreeModel::QuickTest);
-        if (testCaseName.isEmpty()) {
+        parseResult.referencingFile = cppFileName;
-            testTreeItem = new TestTreeItem(QString(), QString(), TestTreeItem::TestClass);
+        parseResult.functions = testFunctions;
-
+        if (!testCaseName.isEmpty()) {
-            foreach (const QString &functionName, testFunctions.keys()) {
+            parseResult.fileName = tcLocationAndType.m_name;
-                const TestCodeLocationAndType locationAndType = testFunctions.value(functionName);
+            parseResult.testCaseName = testCaseName;
-                TestTreeItem *testFunction = new TestTreeItem(functionName, locationAndType.m_name,
+            parseResult.line = tcLocationAndType.m_line;
-                                                              locationAndType.m_type);
+            parseResult.column = tcLocationAndType.m_column;
-                testFunction->setLine(locationAndType.m_line);
-                testFunction->setColumn(locationAndType.m_column);
-                testFunction->setMainFile(cppFileName);
-                testFunction->setReferencingFile(cppFileName);
-                testTreeItem->appendChild(testFunction);
         }
-
+        futureInterface.reportResult(parseResult);
-        } else {
-            testTreeItem = constructTestTreeItem(tcLocationAndType.m_name, cppFileName,
-                                                 testCaseName, tcLocationAndType.m_line,
-                                                 tcLocationAndType.m_column, testFunctions);
-            testTreeItem->setReferencingFile(cppFileName);
-        }
-
-        futureInterface.reportResult(TestParseResult(testTreeItem, TestTreeModel::QuickTest));
     }
 }
 
@@ -555,12 +482,18 @@ static void handleGTest(QFutureInterface<TestParseResult> futureInterface, const
     if (ppList.size())
         proFile = ppList.at(0)->projectFile;
 
+    QVector<TestParseResult> parseResults;
     foreach (const GTestCaseSpec &testSpec, result.keys()) {
-        TestTreeItem *item = constructGTestTreeItem(filePath, testSpec, proFile,
+        TestParseResult parseResult(TestTreeModel::GoogleTest);
-                                                    result.value(testSpec));
+        parseResult.fileName = filePath;
-
+        parseResult.testCaseName = testSpec.testCaseName;
-        futureInterface.reportResult(TestParseResult(item, TestTreeModel::GoogleTest));
+        parseResult.parameterized = testSpec.parameterized;
+        parseResult.referencingFile = proFile;
+        parseResult.dataTagsOrTestSets.insert(QString(), result.value(testSpec));
+        parseResults.append(parseResult);
     }
+    if (parseResults.size())
+        futureInterface.reportResults(parseResults);
 }
 
 static void checkDocumentForTestCode(QFutureInterface<TestParseResult> futureInterface,
@@ -603,13 +536,17 @@ static void checkDocumentForTestCode(QFutureInterface<TestParseResult> futureInt
             if (hasReferencingFile)
                 dataTags.unite(checkForDataTags(document->fileName(), testFunctions));
 
-            TestTreeItem *item = constructTestTreeItem(declaringDoc->fileName(), QString(),
+            TestParseResult parseResult(TestTreeModel::AutoTest);
-                                                       testCaseName, line, column, testFunctions,
+            parseResult.fileName = declaringDoc->fileName();
-                                                       dataTags);
+            parseResult.testCaseName = testCaseName;
+            parseResult.line = line;
+            parseResult.column = column;
+            parseResult.functions = testFunctions;
+            parseResult.dataTagsOrTestSets = dataTags;
             if (hasReferencingFile)
-                item->setReferencingFile(fileName);
+                parseResult.referencingFile = fileName;
 
-            futureInterface.reportResult(TestParseResult(item, TestTreeModel::AutoTest));
+            futureInterface.reportResult(parseResult);
             return;
         }
     }

src/plugins/autotest/testcodeparser.h

@@ -66,7 +66,7 @@ public:
 
 signals:
     void aboutToPerformFullParse();
-    void testItemCreated(TestTreeItem *item, TestTreeModel::Type type);
+    void testParseResultReady(TestParseResult result);
     void parsingStarted();
     void parsingFinished();
     void parsingFailed();

src/plugins/autotest/testtreeitem.h

@@ -124,6 +124,12 @@ struct TestCodeLocationAndType {
     TestTreeItem::TestStates m_state;
 };
 
+struct GTestCaseSpec
+{
+    QString testCaseName;
+    bool parameterized;
+};
+
 typedef QVector<TestCodeLocationAndType> TestCodeLocationList;
 
 } // namespace Internal

src/plugins/autotest/testtreemodel.cpp

@@ -90,8 +90,8 @@ TestTreeModel::TestTreeModel(QObject *parent) :
 
     connect(m_parser, &TestCodeParser::aboutToPerformFullParse, this,
             &TestTreeModel::removeAllTestItems, Qt::QueuedConnection);
-    connect(m_parser, &TestCodeParser::testItemCreated,
+    connect(m_parser, &TestCodeParser::testParseResultReady,
-            this, &TestTreeModel::addTestTreeItem, Qt::QueuedConnection);
+            this, &TestTreeModel::onParseResultReady, Qt::QueuedConnection);
     connect(m_parser, &TestCodeParser::parsingFinished,
             this, &TestTreeModel::sweep, Qt::QueuedConnection);
     connect(m_parser, &TestCodeParser::parsingFailed,
@@ -676,6 +676,8 @@ TestTreeItem *TestTreeModel::findTestTreeItemByContent(TestTreeItem *item, TestT
             if (current->type() == item->type())
                 return current;
             break;
+        case Invalid:
+            break;
         }
     }
     return 0;
@@ -710,6 +712,105 @@ void TestTreeModel::addTestTreeItem(TestTreeItem *item, Type type)
     emit testTreeModelChanged();
 }
 
+static TestTreeItem *constructTestTreeItem(const QString &fileName,
+                                           const QString &referencingFile,
+                                           const QString &testCaseName,
+                                           int line, int column,
+                                           const QMap<QString, TestCodeLocationAndType> &functions,
+                                           const QMap<QString, TestCodeLocationList> dataTags = QMap<QString, TestCodeLocationList>())
+{
+    TestTreeItem *treeItem;
+    if (testCaseName.isEmpty()) { // unnamed Quick Test
+        treeItem = new TestTreeItem(QString(), QString(), TestTreeItem::TestClass);
+        foreach (const QString &functionName, functions.keys()) {
+            const TestCodeLocationAndType locationAndType = functions.value(functionName);
+            TestTreeItem *testFunction = new TestTreeItem(functionName, locationAndType.m_name,
+                                                          locationAndType.m_type);
+            testFunction->setLine(locationAndType.m_line);
+            testFunction->setColumn(locationAndType.m_column);
+            testFunction->setMainFile(referencingFile); // FIXME: can be handled by referencingFile
+            testFunction->setReferencingFile(referencingFile);
+            treeItem->appendChild(testFunction);
+        }
+    } else {
+        treeItem = new TestTreeItem(testCaseName, fileName, TestTreeItem::TestClass);
+        treeItem->setMainFile(referencingFile); // FIXME: can be handled by referencingFile
+        treeItem->setReferencingFile(referencingFile);
+        treeItem->setLine(line);
+        treeItem->setColumn(column);
+
+        foreach (const QString &functionName, functions.keys()) {
+            const TestCodeLocationAndType locationAndType = functions.value(functionName);
+            TestTreeItem *treeItemChild = new TestTreeItem(functionName, locationAndType.m_name,
+                                                           locationAndType.m_type);
+            treeItemChild->setLine(locationAndType.m_line);
+            treeItemChild->setColumn(locationAndType.m_column);
+            treeItemChild->setState(locationAndType.m_state);
+
+            // check for data tags and if there are any for this function add them
+            const QString qualifiedFunctionName = testCaseName + QLatin1String("::") + functionName;
+            if (dataTags.contains(qualifiedFunctionName)) {
+                const TestCodeLocationList &tags = dataTags.value(qualifiedFunctionName);
+                foreach (const TestCodeLocationAndType &tagLocation, tags) {
+                    TestTreeItem *tagTreeItem = new TestTreeItem(tagLocation.m_name,
+                                                                 locationAndType.m_name,
+                                                                 tagLocation.m_type);
+                    tagTreeItem->setLine(tagLocation.m_line);
+                    tagTreeItem->setColumn(tagLocation.m_column);
+                    tagTreeItem->setState(tagLocation.m_state);
+                    treeItemChild->appendChild(tagTreeItem);
+                }
+            }
+            treeItem->appendChild(treeItemChild);
+        }
+    }
+    return treeItem;
+}
+
+static TestTreeItem *constructGTestTreeItem(const QString &filePath, const QString &testCaseName,
+                                            const bool parameterized, const QString &proFile,
+                                            const QString &referencingFile,
+                                            const TestCodeLocationList &testSets)
+{
+    TestTreeItem *item = new TestTreeItem(testCaseName, QString(),
+            parameterized ? TestTreeItem::GTestCaseParameterized
+                          : TestTreeItem::GTestCase);
+    foreach (const TestCodeLocationAndType &locationAndType, testSets) {
+        TestTreeItem *treeItemChild = new TestTreeItem(locationAndType.m_name, filePath,
+                                                       locationAndType.m_type);
+        treeItemChild->setState(locationAndType.m_state);
+        treeItemChild->setLine(locationAndType.m_line);
+        treeItemChild->setColumn(locationAndType.m_column);
+        treeItemChild->setMainFile(proFile);
+        item->appendChild(treeItemChild);
+    }
+    item->setReferencingFile(referencingFile);
+    return item;
+}
+
+void TestTreeModel::onParseResultReady(TestParseResult result)
+{
+    switch (result.type) {
+    case AutoTest:
+    case QuickTest:
+        addTestTreeItem(constructTestTreeItem(result.fileName, result.referencingFile,
+                                              result.testCaseName, result.line, result.column,
+                                              result.functions, result.dataTagsOrTestSets),
+                        result.type);
+        break;
+    case GoogleTest:
+        QTC_ASSERT(result.dataTagsOrTestSets.size() == 1, return);
+        addTestTreeItem(constructGTestTreeItem(result.fileName, result.testCaseName,
+                                               result.parameterized, result.proFile,
+                                               result.referencingFile,
+                                               result.dataTagsOrTestSets.first()), result.type);
+        break;
+    case Invalid:
+        QTC_ASSERT(false, qWarning("TestParseResult of type Invalid unexpected."));
+        break;
+    }
+}
+
 void TestTreeModel::removeAllTestItems()
 {
     m_autoTestRootItem->removeChildren();
@@ -727,6 +828,8 @@ TestTreeItem *TestTreeModel::rootItemForType(TestTreeModel::Type type)
         return m_quickTestRootItem;
     case GoogleTest:
         return m_googleTestRootItem;
+    case Invalid:
+        break;
     }
     QTC_ASSERT(false, return 0);
 }

src/plugins/autotest/testtreemodel.h

@@ -27,6 +27,7 @@
 #define TESTTREEMODEL_H
 
 #include "testconfiguration.h"
+#include "testtreeitem.h"
 
 #include <cplusplus/CppDocument.h>
 
@@ -37,15 +38,15 @@
 namespace Autotest {
 namespace Internal {
 
-struct TestCodeLocationAndType;
 class TestCodeParser;
-class TestTreeItem;
+struct TestParseResult;
 
 class TestTreeModel : public Utils::TreeModel
 {
     Q_OBJECT
 public:
     enum Type {
+        Invalid,
         AutoTest,
         QuickTest,
         GoogleTest
@@ -89,6 +90,7 @@ public slots:
 
 private:
     void addTestTreeItem(TestTreeItem *item, Type type);
+    void onParseResultReady(TestParseResult result);
     void removeAllTestItems();
     void removeFiles(const QStringList &files);
     void markForRemoval(const QString &filePath, Type type);
@@ -147,14 +149,24 @@ private:
 
 struct TestParseResult
 {
-    TestParseResult(TestTreeItem *it, TestTreeModel::Type t) : item(it), type(t) {}
+    TestParseResult(TestTreeModel::Type t = TestTreeModel::Invalid) : type(t) {}
-    TestTreeItem *item;
+
     TestTreeModel::Type type;
+    QString fileName;
+    QString proFile;
+    QString referencingFile;
+    QString testCaseName;
+    int line = 0;
+    int column = 0;
+    bool parameterized = false;
+    QMap<QString, TestCodeLocationAndType> functions;
+    QMap<QString, TestCodeLocationList> dataTagsOrTestSets;
 };
 
 } // namespace Internal
 } // namespace Autotest
 
 Q_DECLARE_METATYPE(Autotest::Internal::TestTreeModel::Type)
+Q_DECLARE_METATYPE(Autotest::Internal::TestParseResult)
 
 #endif // TESTTREEMODEL_H

src/plugins/autotest/testvisitor.h

@@ -129,12 +129,6 @@ private:
 
 };
 
-struct GTestCaseSpec
-{
-    QString testCaseName;
-    bool parameterized;
-};
-
 inline bool operator<(const GTestCaseSpec &spec1, const GTestCaseSpec &spec2)
 {
     if (spec1.testCaseName != spec2.testCaseName)