TestResult: Devirtualize the class - part 3 of 5

Step 3 - implement directParentHook.

Change-Id: I87518e700e9019ccd5b8a095b23971ae26eb776e
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/plugins/autotest/boost/boosttestresult.cpp

@@ -88,45 +88,55 @@ static ResultHooks::FindTestItemHook findTestItemHook(const FilePath &projectFil
     };
 }
 
+struct BoostTestData
+{
+    QString m_testCaseName;
+    QString m_testSuiteName;
+};
+
+static ResultHooks::DirectParentHook directParentHook(const QString &testCaseName,
+                                                      const QString &testSuiteName)
+{
+    return [=](const TestResult &result, const TestResult &other, bool *) -> bool {
+        if (!other.extraData().canConvert<BoostTestData>())
+            return false;
+        const BoostTestData otherData = other.extraData().value<BoostTestData>();
+
+        if (result.result() != ResultType::TestStart)
+            return false;
+
+        bool thisModule = (testCaseName.isEmpty() && testSuiteName.isEmpty());
+        bool thisSuite = (testCaseName.isEmpty() && !testSuiteName.isEmpty());
+        bool thisCase = (!testCaseName.isEmpty());
+
+        bool otherSuite = otherData.m_testCaseName.isEmpty() && !otherData.m_testSuiteName.isEmpty();
+        bool otherCase = !otherData.m_testCaseName.isEmpty();
+
+        if (otherSuite)
+            return thisSuite ? otherData.m_testSuiteName.startsWith(testSuiteName + '/') : thisModule;
+
+        if (otherCase) {
+            if (thisCase)
+                return otherData.m_testCaseName == testCaseName && otherData.m_testSuiteName == testSuiteName;
+            if (thisSuite)
+                return otherData.m_testSuiteName == testSuiteName;
+            if (thisModule)
+                return otherData.m_testSuiteName.isEmpty();
+        }
+        return false;
+    };
+}
+
 BoostTestResult::BoostTestResult(const QString &id, const QString &name,
                                  const FilePath &projectFile, const QString &testCaseName,
                                  const QString &testSuiteName)
     : TestResult(id, name, {outputStringHook(testCaseName),
-                            findTestItemHook(projectFile, testCaseName, testSuiteName)})
+                            findTestItemHook(projectFile, testCaseName, testSuiteName),
-    , m_projectFile(projectFile)
+                            directParentHook(testCaseName, testSuiteName),
-    , m_testCaseName(testCaseName)
+                            QVariant::fromValue(BoostTestData{testCaseName, testSuiteName})})
-    , m_testSuiteName(testSuiteName) {}
+{}
-
-bool BoostTestResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const
-{
-    if (!TestResult::isDirectParentOf(other, needsIntermediate))
-        return false;
-
-    if (result() != ResultType::TestStart)
-        return false;
-
-    bool weAreModule = (m_testCaseName.isEmpty() && m_testSuiteName.isEmpty());
-    bool weAreSuite = (m_testCaseName.isEmpty() && !m_testSuiteName.isEmpty());
-    bool weAreCase = (!m_testCaseName.isEmpty());
-
-    const BoostTestResult *boostOther = static_cast<const BoostTestResult *>(other);
-    bool otherIsSuite = boostOther->m_testCaseName.isEmpty() && !boostOther->m_testSuiteName.isEmpty();
-    bool otherIsCase = !boostOther->m_testCaseName.isEmpty();
-
-    if (otherIsSuite)
-        return weAreSuite ? boostOther->m_testSuiteName.startsWith(m_testSuiteName + '/') : weAreModule;
-
-    if (otherIsCase) {
-        if (weAreCase)
-            return boostOther->m_testCaseName == m_testCaseName && boostOther->m_testSuiteName == m_testSuiteName;
-        if (weAreSuite)
-            return boostOther->m_testSuiteName == m_testSuiteName;
-        if (weAreModule)
-            return boostOther->m_testSuiteName.isEmpty();
-    }
-    return false;
-}
 
 } // namespace Internal
 } // namespace Autotest
 
+Q_DECLARE_METATYPE(Autotest::Internal::BoostTestData);

src/plugins/autotest/boost/boosttestresult.h

@@ -15,13 +15,6 @@ class BoostTestResult : public TestResult
 public:
     BoostTestResult(const QString &id, const QString &name, const Utils::FilePath &projectFile,
                     const QString &testCaseName = {}, const QString &testSuiteName = {});
-
-    bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const override;
-private:
-
-    Utils::FilePath m_projectFile;
-    QString m_testCaseName;
-    QString m_testSuiteName;
 };
 
 } // namespace Internal

src/plugins/autotest/catch/catchresult.cpp

@@ -35,34 +35,44 @@ static ResultHooks::FindTestItemHook findTestItemHook()
     };
 }
 
-CatchResult::CatchResult(const QString &id, const QString &name, int depth)
+struct CatchData
-    : TestResult(id, name, {{}, findTestItemHook()})
-    , m_sectionDepth(depth) {}
-
-bool CatchResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const
 {
-    if (!TestResult::isDirectParentOf(other, needsIntermediate))
+    int m_sectionDepth = 0;
-        return false;
+};
-    const CatchResult *catchOther = static_cast<const CatchResult *>(other);
 
-    if (result() != ResultType::TestStart)
+static ResultHooks::DirectParentHook directParentHook(int depth)
-        return false;
+{
+    return [=](const TestResult &result, const TestResult &other, bool *) -> bool {
+        if (!other.extraData().canConvert<CatchData>())
+            return false;
+        const CatchData otherData = other.extraData().value<CatchData>();
 
-    if (catchOther->result() == ResultType::TestStart) {
+        if (result.result() != ResultType::TestStart)
-        if (fileName() != catchOther->fileName())
             return false;
 
-        return sectionDepth() + 1 == catchOther->sectionDepth();
+        if (other.result() == ResultType::TestStart) {
-    }
+            if (result.fileName() != other.fileName())
+                return false;
 
-    if (sectionDepth() <= catchOther->sectionDepth() && catchOther->result() == ResultType::Pass)
+            return depth + 1 == otherData.m_sectionDepth;
-        return true;
+        }
 
-    if (fileName() != catchOther->fileName() || sectionDepth() > catchOther->sectionDepth())
+        if (depth <= otherData.m_sectionDepth && other.result() == ResultType::Pass)
-        return false;
+            return true;
 
-    return name() == catchOther->name();
+        if (result.fileName() != other.fileName() || depth > otherData.m_sectionDepth)
+            return false;
+
+        return result.name() == other.name();
+    };
 }
 
+CatchResult::CatchResult(const QString &id, const QString &name, int depth)
+    : TestResult(id, name, {{}, findTestItemHook(), directParentHook(depth),
+                            QVariant::fromValue(CatchData{depth})})
+{}
+
 } // namespace Internal
 } // namespace Autotest
+
+Q_DECLARE_METATYPE(Autotest::Internal::CatchData);

src/plugins/autotest/catch/catchresult.h

@@ -12,12 +12,6 @@ class CatchResult : public TestResult
 {
 public:
     CatchResult(const QString &id, const QString &name, int depth);
-
-    bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const override;
-
-private:
-    int sectionDepth() const { return m_sectionDepth; }
-    int m_sectionDepth = 0;
 };
 
 } // namespace Internal

src/plugins/autotest/ctest/ctestoutputreader.cpp

@@ -37,19 +37,19 @@ static ResultHooks::FindTestItemHook findTestItemHook(const QString &testCaseNam
     };
 }
 
+static ResultHooks::DirectParentHook directParentHook()
+{
+    return [=](const TestResult &result, const TestResult &, bool *) -> bool {
+        return result.result() == ResultType::TestStart;
+    };
+}
+
 class CTestResult : public TestResult
 {
 public:
     CTestResult(const QString &id, const QString &project, const QString &testCaseName)
-        : TestResult(id, project, {{}, findTestItemHook(testCaseName)})
+        : TestResult(id, project, {{}, findTestItemHook(testCaseName), directParentHook(), {}})
     {}
-
-    bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const override
-    {
-        if (!TestResult::isDirectParentOf(other, needsIntermediate))
-            return false;
-        return result() == ResultType::TestStart;
-    }
 };
 
 CTestOutputReader::CTestOutputReader(const QFutureInterface<TestResultPtr> &futureInterface,

src/plugins/autotest/gtest/gtestresult.cpp

@@ -98,28 +98,41 @@ static ResultHooks::FindTestItemHook findTestItemHook(const FilePath &projectFil
     };
 }
 
+struct GTestData
+{
+    QString m_testCaseName;
+    int m_iteration = 1;
+};
+
+static ResultHooks::DirectParentHook directParentHook(const QString &testCaseName, int iteration)
+{
+    return [=](const TestResult &result, const TestResult &other, bool *) -> bool {
+        if (!other.extraData().canConvert<GTestData>())
+            return false;
+        const GTestData otherData = other.extraData().value<GTestData>();
+
+        if (testCaseName == otherData.m_testCaseName) {
+            const ResultType thisResult = result.result();
+            const ResultType otherResult = other.result();
+            if (otherResult == ResultType::MessageInternal || otherResult == ResultType::MessageLocation)
+                return thisResult != ResultType::MessageInternal && thisResult != ResultType::MessageLocation;
+        }
+        if (iteration != otherData.m_iteration)
+            return false;
+        return testCaseName.isEmpty() && !otherData.m_testCaseName.isEmpty();
+    };
+}
+
 GTestResult::GTestResult(const QString &id, const QString &name, const FilePath &projectFile,
                          const QString &testCaseName, int iteration)
     : TestResult(id, name, {outputStringHook(testCaseName),
-                            findTestItemHook(projectFile, testCaseName)})
+                            findTestItemHook(projectFile, testCaseName),
-    , m_testCaseName(testCaseName)
+                            directParentHook(testCaseName, iteration),
-    , m_iteration(iteration) {}
+                            QVariant::fromValue(GTestData{testCaseName, iteration})})
-
+{}
-bool GTestResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const
-{
-    if (!TestResult::isDirectParentOf(other, needsIntermediate))
-        return false;
-
-    const GTestResult *gtOther = static_cast<const GTestResult *>(other);
-    if (m_testCaseName == gtOther->m_testCaseName) {
-        const ResultType otherResult = other->result();
-        if (otherResult == ResultType::MessageInternal || otherResult == ResultType::MessageLocation)
-            return result() != ResultType::MessageInternal && result() != ResultType::MessageLocation;
-    }
-    if (m_iteration != gtOther->m_iteration)
-        return false;
-    return isTestSuite() && gtOther->isTestCase();
-}
 
 } // namespace Internal
 } // namespace Autotest
+
+Q_DECLARE_METATYPE(Autotest::Internal::GTestData);
+

src/plugins/autotest/gtest/gtestresult.h

@@ -16,15 +16,6 @@ class GTestResult : public TestResult
 public:
     GTestResult(const QString &id, const QString &name, const Utils::FilePath &projectFile,
                 const QString &testCaseName = {}, int iteration = 1);
-
-    bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const override;
-
-private:
-    bool isTestSuite() const { return m_testCaseName.isEmpty(); }
-    bool isTestCase() const { return !m_testCaseName.isEmpty(); }
-
-    QString m_testCaseName;
-    int m_iteration = 1;
 };
 
 } // namespace Internal

src/plugins/autotest/qtest/qttestresult.cpp

@@ -145,39 +145,55 @@ static ResultHooks::FindTestItemHook findTestItemHook(const FilePath &projectFil
     };
 }
 
+struct QtTestData
+{
+    FilePath m_projectFile;
+    TestType m_type;
+    QString m_function;
+    QString m_dataTag;
+    bool isTestFunction() const { return !m_function.isEmpty() && m_dataTag.isEmpty(); };
+    bool isDataTag() const { return !m_function.isEmpty() && !m_dataTag.isEmpty(); };
+};
+
+static ResultHooks::DirectParentHook directParentHook(const QString &functionName,
+                                                      const QString &dataTag)
+{
+    return [=](const TestResult &result, const TestResult &other, bool *needsIntermediate) -> bool {
+        if (!other.extraData().canConvert<QtTestData>())
+            return false;
+        const QtTestData otherData = other.extraData().value<QtTestData>();
+
+        if (result.result() == ResultType::TestStart) {
+            if (otherData.isDataTag()) {
+                if (otherData.m_function == functionName) {
+                    if (dataTag.isEmpty()) {
+                        // avoid adding function's TestCaseEnd to the data tag
+                        *needsIntermediate = other.result() != ResultType::TestEnd;
+                        return true;
+                    }
+                    return otherData.m_dataTag == dataTag;
+                }
+            } else if (otherData.isTestFunction()) {
+                return (functionName.isEmpty() && dataTag.isEmpty())
+                        || (functionName == otherData.m_function
+                            && other.result() != ResultType::TestStart);
+            }
+        }
+        return false;
+    };
+}
+
 QtTestResult::QtTestResult(const QString &id, const QString &name, const FilePath &projectFile,
                            TestType type, const QString &functionName, const QString &dataTag)
     : TestResult(id, name, {outputStringHook(functionName, dataTag),
-                            findTestItemHook(projectFile, type, functionName, dataTag)})
+                            findTestItemHook(projectFile, type, functionName, dataTag),
+                            directParentHook(functionName, dataTag),
+                            QVariant::fromValue(QtTestData{projectFile, type, functionName, dataTag})})
     , m_projectFile(projectFile)
     , m_type(type)
     , m_function(functionName)
     , m_dataTag(dataTag) {}
 
-bool QtTestResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const
-{
-    if (!TestResult::isDirectParentOf(other, needsIntermediate))
-        return false;
-    const QtTestResult *qtOther = static_cast<const QtTestResult *>(other);
-
-    if (result() == ResultType::TestStart) {
-        if (qtOther->isDataTag()) {
-            if (qtOther->m_function == m_function) {
-                if (m_dataTag.isEmpty()) {
-                    // avoid adding function's TestCaseEnd to the data tag
-                    *needsIntermediate = qtOther->result() != ResultType::TestEnd;
-                    return true;
-                }
-                return qtOther->m_dataTag == m_dataTag;
-            }
-        } else if (qtOther->isTestFunction()) {
-            return isTestCase() || (m_function == qtOther->m_function
-                                    && qtOther->result() != ResultType::TestStart);
-        }
-    }
-    return false;
-}
-
 bool QtTestResult::isIntermediateFor(const TestResult *other) const
 {
     QTC_ASSERT(other, return false);
@@ -207,3 +223,5 @@ TestResult *QtTestResult::createIntermediateResultFor(const TestResult *other) c
 
 } // namespace Internal
 } // namespace Autotest
+
+Q_DECLARE_METATYPE(Autotest::Internal::QtTestData);

src/plugins/autotest/qtest/qttestresult.h

@@ -18,7 +18,6 @@ public:
     QtTestResult(const QString &id, const QString &name, const Utils::FilePath &projectFile,
                  TestType type, const QString &functionName = {}, const QString &dataTag = {});
 
-    bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const override;
     bool isIntermediateFor(const TestResult *other) const override;
     TestResult *createIntermediateResultFor(const TestResult *other) const override;
 

src/plugins/autotest/testresult.cpp

@@ -156,10 +156,15 @@ QColor TestResult::colorForType(const ResultType type)
     }
 }
 
-bool TestResult::isDirectParentOf(const TestResult *other, bool * /*needsIntermediate*/) const
+bool TestResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const
 {
     QTC_ASSERT(other, return false);
-    return !m_id.isEmpty() && m_id == other->m_id && m_name == other->m_name;
+    const bool ret = !m_id.isEmpty() && m_id == other->m_id && m_name == other->m_name;
+    if (!ret)
+        return false;
+    if (m_hooks.directParent)
+        return m_hooks.directParent(*this, *other, needsIntermediate);
+    return true;
 }
 
 bool TestResult::isIntermediateFor(const TestResult *other) const

src/plugins/autotest/testresult.h

@@ -64,8 +64,11 @@ struct ResultHooks
 {
     using OutputStringHook = std::function<QString(const TestResult &, bool)>;
     using FindTestItemHook = std::function<ITestTreeItem *(const TestResult &)>;
+    using DirectParentHook = std::function<bool(const TestResult &, const TestResult &, bool *)>;
     OutputStringHook outputString;
     FindTestItemHook findTestItem;
+    DirectParentHook directParent;
+    QVariant extraData;
 };
 
 class TestResult
@@ -84,6 +87,7 @@ public:
     QString description() const { return m_description; }
     Utils::FilePath fileName() const { return m_file; }
     int line() const { return m_line; }
+    QVariant extraData() const { return m_hooks.extraData; }
 
     void setDescription(const QString &description) { m_description = description; }
     void setFileName(const Utils::FilePath &fileName) { m_file = fileName; }
@@ -95,9 +99,10 @@ public:
     static QString resultToString(const ResultType type);
     static QColor colorForType(const ResultType type);
 
-    virtual bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const;
+    bool isDirectParentOf(const TestResult *other, bool *needsIntermediate) const;
     virtual bool isIntermediateFor(const TestResult *other) const;
     virtual TestResult *createIntermediateResultFor(const TestResult *other) const;
+
 private:
     QString m_id;
     QString m_name;