From b1ef25b2085531100643138dcfdcb531078857df Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Sat, 14 Jan 2023 11:31:15 +0100 Subject: [PATCH] TestResult: Devirtualize the class - part 3 of 5 Step 3 - implement directParentHook. Change-Id: I87518e700e9019ccd5b8a095b23971ae26eb776e Reviewed-by: Christian Stenger Reviewed-by: --- .../autotest/boost/boosttestresult.cpp | 78 +++++++++++-------- src/plugins/autotest/boost/boosttestresult.h | 7 -- src/plugins/autotest/catch/catchresult.cpp | 48 +++++++----- src/plugins/autotest/catch/catchresult.h | 6 -- .../autotest/ctest/ctestoutputreader.cpp | 16 ++-- src/plugins/autotest/gtest/gtestresult.cpp | 51 +++++++----- src/plugins/autotest/gtest/gtestresult.h | 9 --- src/plugins/autotest/qtest/qttestresult.cpp | 68 ++++++++++------ src/plugins/autotest/qtest/qttestresult.h | 1 - src/plugins/autotest/testresult.cpp | 9 ++- src/plugins/autotest/testresult.h | 7 +- 11 files changed, 169 insertions(+), 131 deletions(-) diff --git a/src/plugins/autotest/boost/boosttestresult.cpp b/src/plugins/autotest/boost/boosttestresult.cpp index 9e9b111df90..373493cc09a 100644 --- a/src/plugins/autotest/boost/boosttestresult.cpp +++ b/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()) + return false; + const BoostTestData otherData = other.extraData().value(); + + 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)}) - , m_projectFile(projectFile) - , m_testCaseName(testCaseName) - , 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(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; -} + findTestItemHook(projectFile, testCaseName, testSuiteName), + directParentHook(testCaseName, testSuiteName), + QVariant::fromValue(BoostTestData{testCaseName, testSuiteName})}) +{} } // namespace Internal } // namespace Autotest +Q_DECLARE_METATYPE(Autotest::Internal::BoostTestData); diff --git a/src/plugins/autotest/boost/boosttestresult.h b/src/plugins/autotest/boost/boosttestresult.h index b994b0c4646..228f070996b 100644 --- a/src/plugins/autotest/boost/boosttestresult.h +++ b/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 diff --git a/src/plugins/autotest/catch/catchresult.cpp b/src/plugins/autotest/catch/catchresult.cpp index 3020b6a9d1c..265700ca658 100644 --- a/src/plugins/autotest/catch/catchresult.cpp +++ b/src/plugins/autotest/catch/catchresult.cpp @@ -35,34 +35,44 @@ static ResultHooks::FindTestItemHook findTestItemHook() }; } -CatchResult::CatchResult(const QString &id, const QString &name, int depth) - : TestResult(id, name, {{}, findTestItemHook()}) - , m_sectionDepth(depth) {} - -bool CatchResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const +struct CatchData { - if (!TestResult::isDirectParentOf(other, needsIntermediate)) - return false; - const CatchResult *catchOther = static_cast(other); + int m_sectionDepth = 0; +}; - if (result() != ResultType::TestStart) - return false; +static ResultHooks::DirectParentHook directParentHook(int depth) +{ + return [=](const TestResult &result, const TestResult &other, bool *) -> bool { + if (!other.extraData().canConvert()) + return false; + const CatchData otherData = other.extraData().value(); - if (catchOther->result() == ResultType::TestStart) { - if (fileName() != catchOther->fileName()) + if (result.result() != ResultType::TestStart) 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 true; + return depth + 1 == otherData.m_sectionDepth; + } - if (fileName() != catchOther->fileName() || sectionDepth() > catchOther->sectionDepth()) - return false; + if (depth <= otherData.m_sectionDepth && other.result() == ResultType::Pass) + 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); diff --git a/src/plugins/autotest/catch/catchresult.h b/src/plugins/autotest/catch/catchresult.h index 19264e9e4cc..e76396270aa 100644 --- a/src/plugins/autotest/catch/catchresult.h +++ b/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 diff --git a/src/plugins/autotest/ctest/ctestoutputreader.cpp b/src/plugins/autotest/ctest/ctestoutputreader.cpp index 585fbd2b2b3..066b5e5680c 100644 --- a/src/plugins/autotest/ctest/ctestoutputreader.cpp +++ b/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 &futureInterface, diff --git a/src/plugins/autotest/gtest/gtestresult.cpp b/src/plugins/autotest/gtest/gtestresult.cpp index 90fbf78551d..902c28132e5 100644 --- a/src/plugins/autotest/gtest/gtestresult.cpp +++ b/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()) + return false; + const GTestData otherData = other.extraData().value(); + + 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)}) - , m_testCaseName(testCaseName) - , m_iteration(iteration) {} - -bool GTestResult::isDirectParentOf(const TestResult *other, bool *needsIntermediate) const -{ - if (!TestResult::isDirectParentOf(other, needsIntermediate)) - return false; - - const GTestResult *gtOther = static_cast(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(); -} + findTestItemHook(projectFile, testCaseName), + directParentHook(testCaseName, iteration), + QVariant::fromValue(GTestData{testCaseName, iteration})}) +{} } // namespace Internal } // namespace Autotest + +Q_DECLARE_METATYPE(Autotest::Internal::GTestData); + diff --git a/src/plugins/autotest/gtest/gtestresult.h b/src/plugins/autotest/gtest/gtestresult.h index 8b66db1dee6..acd617b2027 100644 --- a/src/plugins/autotest/gtest/gtestresult.h +++ b/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 diff --git a/src/plugins/autotest/qtest/qttestresult.cpp b/src/plugins/autotest/qtest/qttestresult.cpp index ad4b5ca6124..945acc05499 100644 --- a/src/plugins/autotest/qtest/qttestresult.cpp +++ b/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()) + return false; + const QtTestData otherData = other.extraData().value(); + + 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(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); diff --git a/src/plugins/autotest/qtest/qttestresult.h b/src/plugins/autotest/qtest/qttestresult.h index 382649b5d0c..66ff5dc4570 100644 --- a/src/plugins/autotest/qtest/qttestresult.h +++ b/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; diff --git a/src/plugins/autotest/testresult.cpp b/src/plugins/autotest/testresult.cpp index ae42e28231d..6a9478cbc6b 100644 --- a/src/plugins/autotest/testresult.cpp +++ b/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 diff --git a/src/plugins/autotest/testresult.h b/src/plugins/autotest/testresult.h index 556802da3c5..34d0b9d0466 100644 --- a/src/plugins/autotest/testresult.h +++ b/src/plugins/autotest/testresult.h @@ -64,8 +64,11 @@ struct ResultHooks { using OutputStringHook = std::function; using FindTestItemHook = std::function; + using DirectParentHook = std::function; 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;