From b4ca318383d0de704f9053cb979a1b35bebe3324 Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Mon, 10 Jul 2017 15:02:24 +0200 Subject: [PATCH] AutoTest: Improve test results summary items display Instead of letting warn prevail over pass and fail over warn just add warn as additional visual marker at the icons and keep the original test result. Task-number: QTCREATORBUG-18311 Change-Id: Ia67288fa84598b02c20fc1019799b1bb9282d63e Reviewed-by: David Schulz --- src/plugins/autotest/autotesticons.h | 8 +++++ src/plugins/autotest/testresult.cpp | 7 ++-- src/plugins/autotest/testresult.h | 3 +- src/plugins/autotest/testresultdelegate.cpp | 4 +-- src/plugins/autotest/testresultmodel.cpp | 37 +++++++++++++++------ 5 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/plugins/autotest/autotesticons.h b/src/plugins/autotest/autotesticons.h index b278509dbbc..1420be31ad8 100644 --- a/src/plugins/autotest/autotesticons.h +++ b/src/plugins/autotest/autotesticons.h @@ -68,6 +68,14 @@ const Utils::Icon RESULT_MESSAGEDEBUG({ const Utils::Icon RESULT_MESSAGEWARN({ {":/utils/images/filledcircle.png", Utils::Theme::OutputPanes_TestWarnTextColor}}, Utils::Icon::Tint); +const Utils::Icon RESULT_MESSAGEPASSWARN({ + {":/utils/images/filledcircle.png", Utils::Theme::OutputPanes_TestPassTextColor}, + {":/utils/images/iconoverlay_warning.png", Utils::Theme::OutputPanes_TestWarnTextColor}}, + Utils::Icon::Tint | Utils::Icon::PunchEdges); +const Utils::Icon RESULT_MESSAGEFAILWARN({ + {":/utils/images/filledcircle.png", Utils::Theme::OutputPanes_TestFailTextColor}, + {":/utils/images/iconoverlay_warning.png", Utils::Theme::OutputPanes_TestWarnTextColor}}, + Utils::Icon::Tint | Utils::Icon::PunchEdges); const Utils::Icon RESULT_MESSAGEFATAL({ {":/utils/images/filledcircle.png", Utils::Theme::OutputPanes_TestFatalTextColor}}, Utils::Icon::Tint); diff --git a/src/plugins/autotest/testresult.cpp b/src/plugins/autotest/testresult.cpp index fce6f05c856..2dcd9dda70a 100644 --- a/src/plugins/autotest/testresult.cpp +++ b/src/plugins/autotest/testresult.cpp @@ -103,9 +103,11 @@ QString TestResult::resultToString(const Result::Type type) switch (type) { case Result::Pass: case Result::MessageTestCaseSuccess: + case Result::MessageTestCaseSuccessWarn: return QString("PASS"); case Result::Fail: case Result::MessageTestCaseFail: + case Result::MessageTestCaseFailWarn: return QString("FAIL"); case Result::ExpectedFail: return QString("XFAIL"); @@ -120,7 +122,6 @@ QString TestResult::resultToString(const Result::Type type) case Result::MessageInfo: return QString("INFO"); case Result::MessageWarn: - case Result::MessageTestCaseWarn: return QString("WARN"); case Result::MessageFatal: return QString("FATAL"); @@ -172,8 +173,8 @@ QColor TestResult::colorForType(const Result::Type type) bool TestResult::isMessageCaseStart(const Result::Type type) { return type == Result::MessageTestCaseStart || type == Result::MessageTestCaseSuccess - || type == Result::MessageTestCaseFail || type == Result::MessageTestCaseWarn - || type == Result::MessageIntermediate; + || type == Result::MessageTestCaseFail || type == Result::MessageTestCaseSuccessWarn + || type == Result::MessageTestCaseFailWarn || type == Result::MessageIntermediate; } bool TestResult::isDirectParentOf(const TestResult *other, bool * /*needsIntermediate*/) const diff --git a/src/plugins/autotest/testresult.h b/src/plugins/autotest/testresult.h index 267b7be4196..a7414f85d36 100644 --- a/src/plugins/autotest/testresult.h +++ b/src/plugins/autotest/testresult.h @@ -55,8 +55,9 @@ enum Type { MessageDisabledTests, MessageTestCaseStart, MessageTestCaseSuccess, - MessageTestCaseWarn, + MessageTestCaseSuccessWarn, MessageTestCaseFail, + MessageTestCaseFailWarn, MessageTestCaseEnd, MessageIntermediate, MessageCurrentTest, INTERNAL_MESSAGES_END = MessageCurrentTest, diff --git a/src/plugins/autotest/testresultdelegate.cpp b/src/plugins/autotest/testresultdelegate.cpp index fb273384401..9bd01925848 100644 --- a/src/plugins/autotest/testresultdelegate.cpp +++ b/src/plugins/autotest/testresultdelegate.cpp @@ -41,8 +41,8 @@ const static int outputLimit = 100000; static bool isSummaryItem(Result::Type type) { - return type == Result::MessageTestCaseSuccess || type == Result::MessageTestCaseFail - || type == Result::MessageTestCaseWarn; + return type == Result::MessageTestCaseSuccess || type == Result::MessageTestCaseSuccessWarn + || type == Result::MessageTestCaseFail || type == Result::MessageTestCaseFailWarn; } TestResultDelegate::TestResultDelegate(QObject *parent) diff --git a/src/plugins/autotest/testresultmodel.cpp b/src/plugins/autotest/testresultmodel.cpp index ec996f1b353..a41fc64b011 100644 --- a/src/plugins/autotest/testresultmodel.cpp +++ b/src/plugins/autotest/testresultmodel.cpp @@ -61,6 +61,8 @@ static QIcon testResultIcon(Result::Type result) { Icons::RESULT_MESSAGEWARN.icon(), Icons::RESULT_MESSAGEFATAL.icon(), Icons::RESULT_MESSAGEFATAL.icon(), // System gets same handling as Fatal for now + Icons::RESULT_MESSAGEPASSWARN.icon(), + Icons::RESULT_MESSAGEFAILWARN.icon(), }; // provide an icon for unknown?? if (result < 0 || result >= Result::MessageInternal) { @@ -69,8 +71,10 @@ static QIcon testResultIcon(Result::Type result) { return icons[Result::Pass]; case Result::MessageTestCaseFail: return icons[Result::Fail]; - case Result::MessageTestCaseWarn: - return icons[Result::MessageWarn]; + case Result::MessageTestCaseSuccessWarn: + return icons[13]; + case Result::MessageTestCaseFailWarn: + return icons[14]; default: return QIcon(); } @@ -103,6 +107,7 @@ void TestResultItem::updateResult() return; Result::Type newResult = Result::MessageTestCaseSuccess; + bool withWarning = false; for (Utils::TreeItem *child : *this) { const TestResult *current = static_cast(child)->testResult(); if (current) { @@ -111,21 +116,28 @@ void TestResultItem::updateResult() case Result::MessageFatal: case Result::UnexpectedPass: case Result::MessageTestCaseFail: - m_testResult->setResult(Result::MessageTestCaseFail); - return; + newResult = Result::MessageTestCaseFail; + break; case Result::ExpectedFail: case Result::MessageWarn: case Result::Skip: case Result::BlacklistedFail: case Result::BlacklistedPass: - case Result::MessageTestCaseWarn: - newResult = Result::MessageTestCaseWarn; + case Result::MessageTestCaseSuccessWarn: + case Result::MessageTestCaseFailWarn: + withWarning = true; break; default: {} } } } - m_testResult->setResult(newResult); + if (withWarning) { + m_testResult->setResult(newResult == Result::MessageTestCaseSuccess + ? Result::MessageTestCaseSuccessWarn + : Result::MessageTestCaseFailWarn); + } else { + m_testResult->setResult(newResult); + } } void TestResultItem::updateIntermediateChildren() @@ -346,8 +358,9 @@ void TestResultFilterModel::enableAllResultTypes() << Result::MessageFatal << Result::Invalid << Result::BlacklistedPass << Result::BlacklistedFail << Result::Benchmark << Result::MessageIntermediate << Result::MessageCurrentTest << Result::MessageTestCaseStart - << Result::MessageTestCaseSuccess << Result::MessageTestCaseWarn - << Result::MessageTestCaseFail << Result::MessageTestCaseEnd + << Result::MessageTestCaseSuccess << Result::MessageTestCaseSuccessWarn + << Result::MessageTestCaseFail << Result::MessageTestCaseFailWarn + << Result::MessageTestCaseEnd << Result::MessageInfo << Result::MessageSystem; invalidateFilter(); } @@ -395,7 +408,8 @@ bool TestResultFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex &s case Result::MessageTestCaseSuccess: return m_enabled.contains(Result::Pass); case Result::MessageTestCaseFail: - case Result::MessageTestCaseWarn: + case Result::MessageTestCaseSuccessWarn: + case Result::MessageTestCaseFailWarn: return acceptTestCaseResult(index); default: return m_enabled.contains(resultType); @@ -409,7 +423,8 @@ bool TestResultFilterModel::acceptTestCaseResult(const QModelIndex &srcIndex) co Result::Type type = m_sourceModel->testResult(child)->result(); if (type == Result::MessageTestCaseSuccess) type = Result::Pass; - if (type == Result::MessageTestCaseFail || type == Result::MessageTestCaseWarn) { + if (type == Result::MessageTestCaseFail || type == Result::MessageTestCaseFailWarn + || type == Result::MessageTestCaseSuccessWarn) { if (acceptTestCaseResult(child)) return true; } else if (m_enabled.contains(type)) {