[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    KDE/kdevplatform/veritas
From:       Manuel Breugelmans <mbr.nxi () gmail ! com>
Date:       2008-10-20 15:52:32
Message-ID: 1224517952.278513.19002.nullmailer () svn ! kde ! org
[Download RAW message or body]

SVN commit 874009 by mbreugel:

- add testexecutableinfo class
- cache test-result data, this appeared to be a bottleneck
- runner icon redo


 M  +2 -0      CMakeLists.txt  
 M  +5 -18     internal/resultsmodel.cpp  
 M  +62 -24    internal/runnermodel.cpp  
 M  +1 -1      internal/runnermodel.h  
 M  +1 -1      internal/runnerwindow.cpp  
 M  +13 -1     internal/test_p.cpp  
 M  +4 -0      internal/test_p.h  
 M  +6 -4      internal/utils.cpp  
 AM            resources/bug-small.png  
 AM            resources/go-next-bug.png  
 AM            resources/go-next-run.png  
 AM            resources/go-next-success.png  
 AM            resources/go-next2.png  
 M  +8 -0      resources/qxrunner.qrc  
 AM            resources/system-running-small.png  
 AM            resources/test-notrun-small.png  
 AM            resources/test-success-small.png  
 M  +2 -0      test.cpp  
 A             testexecutableinfo.cpp   [License: GPL (v2+)]
 A             testexecutableinfo.h   [License: GPL (v2+)]
 M  +46 -2     testresult.cpp  
 M  +3 -0      testresult.h  
 M  +4 -2      tests/runnermodeltest.cpp  


--- trunk/KDE/kdevplatform/veritas/CMakeLists.txt #874008:874009
@@ -11,6 +11,7 @@
     internal/testexecutor.cpp
     itestrunner.cpp
     itestframework.cpp
+    testexecutableinfo.cpp
     internal/utils.cpp
     internal/selectionstore.cpp
     testtoolviewfactory.cpp
@@ -62,5 +63,6 @@
     itestrunner.h
     itestframework.h
     testtoolviewfactory.h
+    testexecutableinfo.h
     tests/runnertesthelper.h
     DESTINATION ${INCLUDE_INSTALL_DIR}/kdevplatform/veritas )
--- trunk/KDE/kdevplatform/veritas/internal/resultsmodel.cpp #874008:874009
@@ -71,24 +71,11 @@
         return int(Qt::AlignLeft | Qt::AlignTop);
     case Qt::DisplayRole:
         Q_ASSERT(index.row() < m_results.size());
-        switch(index.column()) {
-        case 0:
-            ownerTest = m_results[index.row()]->owner();
-            if (!ownerTest) {
-                qWarning() << "Owner test not set for result.";
-                return QVariant();
-            }
-            return ownerTest->name();
-        case 1:
-            return m_results[index.row()]->message();
-        case 2:
-            return m_results[index.row()]->file().pathOrUrl();
-        case 3:
-            return QString::number(m_results[index.row()]->line());
-        default:
-            kDebug()<< "INVALID COLUMN" << index.column() <<"row " << index.column();
-            Q_ASSERT(0);
-        }
+        // col0 -> owner test name
+        // col1 -> descriptive message
+        // col2 -> file
+        // col3 -> linenumber
+        return m_results[index.row()]->cachedData(index.column());
     case Qt::DecorationRole:
         if (index.column() == 0) {
             return Utils::resultIcon(result(index.row()));
--- trunk/KDE/kdevplatform/veritas/internal/runnermodel.cpp #874008:874009
@@ -54,7 +54,8 @@
 QVariant g_failIcon;
 QVariant g_notRunIcon;
 QVariant g_successIcon;
-
+QVariant g_leafRunningIcon;
+QVariant g_aggregateRunnningIcon;
 }
 
 RunnerModel::RunnerModel(QObject* parent)
@@ -73,9 +74,11 @@
     setExpectedResults(Veritas::AllStates);
     //ModelTest* tm = new ModelTest(this);
 
-    g_failIcon = QIcon(":/icons/arrow-right-double-bordeaux-16.png");
-    g_notRunIcon = QIcon(":/icons/arrow-right-double-grey-16.png");
-    g_successIcon = KIcon("arrow-right-double");
+    g_aggregateRunnningIcon = QIcon(":/icons/go-next-run.png");
+    g_successIcon = QIcon(":/icons/go-next-success.png");
+    g_notRunIcon = QIcon(":/icons/go-next2.png");
+    g_failIcon = QIcon(":/icons/go-next-bug.png");
+    g_leafRunningIcon = QIcon(":/icons/system-running-small.png");
 }
 
 RunnerModel::~RunnerModel()
@@ -97,7 +100,7 @@
 {
     if (!index.isValid()) return QVariant();
     Q_ASSERT(index.column() == 0);
-
+    Test* t = 0;
     switch(role) {
     case Qt::TextAlignmentRole :
         return int(Qt::AlignLeft | Qt::AlignTop);
@@ -111,7 +114,12 @@
         if (index.child(0, 0).isValid()) { // not a leaf test
             return computeIconFromChildState(testFromIndex(index));
         } else { // a leaf test
-            return Utils::resultIcon(testFromIndex(index)->state());
+            t = testFromIndex(index);
+            if (t->internal()->isRunning()) {
+                return g_leafRunningIcon;
+            } else {
+                return Utils::resultIcon(testFromIndex(index)->state());
+            }
         }
     default: {}
     }
@@ -127,25 +135,32 @@
 class ParentStateResolver
 {
 public:
-    ParentStateResolver() : done(false) { state = Veritas::RunSuccess; }
+    ParentStateResolver() : done(false), isRunning(false) {
+        icon = g_successIcon;
+    }
     void operator()(Test* current) {
         if (done) return;
         switch(current->state()) {
         case Veritas::RunFatal:
         case Veritas::RunError:
-            state = Veritas::RunError;
+            icon = g_failIcon;
             done = true;
             break;
         case Veritas::NoResult:
-            if (current->childCount()==0) {
-                state = Veritas::NoResult;
+            if (current->childCount() != 0) break;
+            if (current->internal()->isRunning()) {
+                icon = g_aggregateRunnningIcon;
+                isRunning = true;
+            } else if (!isRunning) {
+                icon = g_notRunIcon;
             }
             break;
         default: {}
         }
     }
+    bool isRunning;
     bool done;
-    Veritas::TestState state;
+    QVariant icon;
 };
 }
 
@@ -153,18 +168,7 @@
 {
     ParentStateResolver psr;
     traverseTree(test, psr);
-
-    switch(psr.state) {
-    case Veritas::RunSuccess:
-        return g_successIcon;
-    case Veritas::RunFatal:
-    case Veritas::RunError:
-        return g_failIcon;
-    case Veritas::NoResult:
-        return g_notRunIcon;
-    default: {}
-    }
-    return QVariant();
+    return psr.icon;
 }
 
 bool RunnerModel::setData(const QModelIndex& index, const QVariant& value, int role)
@@ -384,6 +388,14 @@
     traverseTree(m_rootItem, ct);
 }
 
+namespace {
+/*! Returns true when @p test is last in it's parents childlist */
+bool isLastSibling(Test* test)
+{
+    return test && test->parent() && (test->row() == test->parent()->childCount()-1);
+}
+}
+
 void RunnerModel::postItemCompleted(QModelIndex index)
 {
     Test* item = testFromIndex(index);
@@ -414,6 +426,24 @@
 
     emit numCompletedChanged(m_numStarted);
     emit itemCompleted(index);
+    emit dataChanged(index, index);
+    index = index.parent();
+    if ((item->state() != Veritas::RunSuccess) && (item->state() != Veritas::RunInfo)) {
+        emitParentsChanged(index);
+    } else {
+        // only emit dataChanged singals for the last siblings
+        // purpose of this is to get rid of icon flickering for 
+        // the parents from running->ready->running->ready->running
+        while (index.isValid() && isLastSibling(item)) {
+            emit dataChanged(index, index);
+            index = index.parent();
+            item = item->parent();
+        }
+    }
+}
+
+void RunnerModel::emitParentsChanged(QModelIndex index)
+{
     while (index.isValid()) {
         emit dataChanged(index, index);
         index = index.parent(); // the parent changed as well, since parent state is deduced 
@@ -423,7 +453,15 @@
 
 void RunnerModel::postItemStarted(QModelIndex index)
 {
-    emit itemStarted(index);
+    //emit itemStarted(index);
     m_numStarted++;
+    emit dataChanged(index, index);
+    Test* item = testFromIndex(index);
+    index = index.parent();
+    while (index.isValid() && item && item->row() == 0) { // to reduce icon flickering
+        emit dataChanged(index, index);
+        index = index.parent();
+        item = item->parent();
+    }
     emit numStartedChanged(m_numStarted);
 }
--- trunk/KDE/kdevplatform/veritas/internal/runnermodel.h #874008:874009
@@ -195,6 +195,7 @@
 
 private:  // Operations
     void initItemConnect(QModelIndex current);
+    void emitParentsChanged(QModelIndex index);
 
     // Copy and assignment not supported.
     RunnerModel(const RunnerModel&);
@@ -205,7 +206,6 @@
 
 private:  // Attributes
     Test* m_rootItem;
-    QModelIndex m_startedItemIndex;
     int m_expectedResults;
 
     int m_numSelected;
--- trunk/KDE/kdevplatform/veritas/internal/runnerwindow.cpp #874008:874009
@@ -119,7 +119,7 @@
 
     QPixmap refresh = KIconLoader::global()->loadIcon("view-refresh", KIconLoader::Small);
     m_ui->actionReload->setIcon(refresh);
-    QPixmap run = KIconLoader::global()->loadIcon("arrow-right", KIconLoader::Small);
+    QPixmap run = KIconLoader::global()->loadIcon("system-run", KIconLoader::Small);
     m_ui->actionStart->setIcon(run);
     QPixmap stop = KIconLoader::global()->loadIcon("window-close", KIconLoader::Small);
     m_ui->actionStop->setIcon(stop);
--- trunk/KDE/kdevplatform/veritas/internal/test_p.cpp #874008:874009
@@ -26,7 +26,9 @@
 
 Test::Internal::Internal(Veritas::Test* self)
   : self(self)
-{}
+{
+    m_isRunning = false;
+}
 
 void Test::Internal::setIndex(const QModelIndex& index)
 {
@@ -63,6 +65,7 @@
     }
     if (result) delete result;
     result = new TestResult;
+    m_isRunning = false;
 }
 
 bool Test::Internal::isChecked() const
@@ -86,4 +89,13 @@
     }
 }
 
+bool Test::Internal::isRunning() const
+{
+    return m_isRunning;
+}
 
+void Test::Internal::setIsRunning(bool value)
+{
+    m_isRunning = value;
+}
+
--- trunk/KDE/kdevplatform/veritas/internal/test_p.h #874008:874009
@@ -54,6 +54,9 @@
     /*! Recursively lift check state */
     void unCheck();
 
+    bool isRunning() const;
+    void setIsRunning(bool);
+
 private:
     friend class Test;
 
@@ -67,6 +70,7 @@
     QList<QVariant> itemData;
     bool needVerboseToggle;
     bool needSelectionToggle;
+    bool m_isRunning;
 
     static const int columnCount;
 };
--- trunk/KDE/kdevplatform/veritas/internal/utils.cpp #874008:874009
@@ -34,16 +34,18 @@
 namespace {
 bool g_init = false;
 QVariant g_failIcon;
+QVariant g_runningIcon;
+QVariant g_successIcon;
 QVariant g_notRunIcon;
-QVariant g_successIcon;
 }
 
 QVariant Utils::resultIcon(int result)
 {
     if (!g_init) {
-        g_failIcon = QIcon(":/icons/arrow-right-bordeaux-16.png");
-        g_notRunIcon = QIcon(":/icons/arrow-right-grey-16.png");
-        g_successIcon = KIcon("arrow-right");
+        g_failIcon = QIcon(":/icons/bug-small.png");
+        g_successIcon = QIcon(":/icons/test-success-small.png");
+        g_runningIcon = QIcon(":/icons/system-running-small.png");
+        g_notRunIcon = QIcon(":/icons/test-notrun-small.png");
         g_init = true;
     }
     switch (result) {
** trunk/KDE/kdevplatform/veritas/resources/bug-small.png #property svn:mime-type
   + application/octet-stream
** trunk/KDE/kdevplatform/veritas/resources/go-next-bug.png #property svn:mime-type
   + application/octet-stream
** trunk/KDE/kdevplatform/veritas/resources/go-next-run.png #property svn:mime-type
   + application/octet-stream
** trunk/KDE/kdevplatform/veritas/resources/go-next-success.png #property svn:mime-type
   + application/octet-stream
** trunk/KDE/kdevplatform/veritas/resources/go-next2.png #property svn:mime-type
   + application/octet-stream
--- trunk/KDE/kdevplatform/veritas/resources/qxrunner.qrc #874008:874009
@@ -7,5 +7,13 @@
         <file>arrow-right-double-bordeaux-16.png</file>
         <file>arrow-right-grey-16.png</file>
         <file>arrow-right-double-grey-16.png</file>
+        <file>go-next-run.png</file>
+        <file>go-next-bug.png</file>
+	<file>go-next2.png</file>
+        <file>bug-small.png</file>
+        <file>test-success-small.png</file>
+        <file>system-running-small.png</file>
+        <file>test-notrun-small.png</file>
+        <file>go-next-success.png</file>
     </qresource>
 </RCC>
** trunk/KDE/kdevplatform/veritas/resources/system-running-small.png #property svn:mime-type
   + application/octet-stream
** trunk/KDE/kdevplatform/veritas/resources/test-notrun-small.png #property svn:mime-type
   + application/octet-stream
** trunk/KDE/kdevplatform/veritas/resources/test-success-small.png #property svn:mime-type
   + application/octet-stream
--- trunk/KDE/kdevplatform/veritas/test.cpp #874008:874009
@@ -188,11 +188,13 @@
 
 void Test::signalStarted()
 {
+    d->setIsRunning(true);
     emit started(d->index());
 }
 
 void Test::signalFinished()
 {
+    d->setIsRunning(false);
     emit finished(d->index());
 }
 
--- trunk/KDE/kdevplatform/veritas/testresult.cpp #874008:874009
@@ -20,6 +20,7 @@
 
 #include "testresult.h"
 #include "test.h"
+#include <KDebug>
 
 using Veritas::TestResult;
 using Veritas::TestState;
@@ -30,7 +31,18 @@
 {
 public:
     TestResultPrivate(TestState state, const QString& msg, int line, const KUrl& file) :
-        state(state), message(msg), line(line), file(file), owner(0) {}
+        state(state), message(msg), line(line), file(file), owner(0) {
+        cachedMessage = msg;
+        cachedFile = file.pathOrUrl();
+        cachedLine = QString::number(line);
+    }
+    void resetCache(){
+        cachedOwnerName = QVariant();
+        cachedFile = QVariant();
+        cachedLine = QVariant();
+        cachedMessage = QVariant();
+    }
+
     ~TestResultPrivate() {}
     TestState state;
     QString message;
@@ -38,6 +50,11 @@
     KUrl file;
     QList<TestResult*> children;
     Test* owner;
+
+    QVariant cachedOwnerName;
+    QVariant cachedMessage;
+    QVariant cachedFile;
+    QVariant cachedLine;
 };
 }
 
@@ -45,8 +62,30 @@
 
 TestResult::TestResult(TestState state, const QString& message, int line, const KUrl& file)
         : d(new TestResultPrivate(state, message, line, file))
-{}
+{
+}
 
+QVariant TestResult::cachedData(int item)
+{
+    switch(item) {
+    case 0:
+        if (!d->owner) {
+            qWarning() << "Owner test not set for result.";
+        }
+        return d->cachedOwnerName;
+    case 1:
+        return d->cachedMessage;
+    case 2:
+        return d->cachedFile;
+    case 3:
+        return d->cachedLine;
+    default:
+        Q_ASSERT(0);
+    }
+    return QVariant();
+}
+
+
 TestResult::~TestResult()
 {
     qDeleteAll(d->children);
@@ -81,16 +120,19 @@
 void TestResult::setMessage(const QString& message)
 {
     d->message = message;
+    d->cachedMessage = message;
 }
 
 void TestResult::setLine(int line)
 {
     d->line = line;
+    d->cachedLine = QString::number(line);
 }
 
 void TestResult::setFile(const KUrl& file)
 {
     d->file = file;
+    d->cachedFile = file.pathOrUrl();
 }
 
 void TestResult::clear()
@@ -101,6 +143,7 @@
     d->file = KUrl();
     d->children.clear();
     d->owner = 0;
+    d->resetCache();
 }
 
 int TestResult::childCount()
@@ -129,6 +172,7 @@
 {
     Q_ASSERT(owner);
     d->owner = owner;
+    d->cachedOwnerName = owner->name();
     foreach(TestResult* child, d->children) {
         child->setOwner(d->owner);
     }
--- trunk/KDE/kdevplatform/veritas/testresult.h #874008:874009
@@ -75,6 +75,9 @@
     void setOwner(Test* t);
     Test* owner() const;
 
+    /*! Owner test-name, message, file & line as precomputed QVariants.*/
+    QVariant cachedData(int item);
+
 private:
     TestResult& operator=(const TestResult&);
     TestResult(const TestResult&);
--- trunk/KDE/kdevplatform/veritas/tests/runnermodeltest.cpp #874008:874009
@@ -164,10 +164,12 @@
     executeItems(model);
 
     // dataChanged must have been emitted with both the parent's and the child's index
-    // the child since it has been run
+    // the child since it has been run.
     // the parent since it's state will have changed after the child has been run.
+    // dataChanged signals are being emitted both for started + finished on both parent + child
+    //    => 4 dataChanged signals
 
-    KOMPARE(2, dataChanged->count());
+    KOMPARE(4, dataChanged->count());
 
     Test* emitted1 = takeTestFromSpy(dataChanged);
     Test* emitted2 = takeTestFromSpy(dataChanged);
[prev in list] [next in list] [prev in thread] [next in thread]