Merge lp:~andrew-haigh-b/cdts/bug1218846 into lp:cdts

[Andrew Haigh]

This MR fixes the linked bug by removing unnecessary dbus calls when processing a job result.
The UI is now much more responsive.

Note that this eliminates the QML warning about missing "outcome" for the manual interaction dialog, and avoids a (small) memory leak when gathering the outcome.

Please review individual commits for details.

[Daniel Manrique (roadmr)]

I tested this by merging this branch on checkbox-ihv-ng rev 2278.

I had to change the interface name to com.canonical.certification.PlainBox1 on line 35 of PBNames.h.

Then I observed some weird behavior (against plainbox from checkbox trunk rev 2335).

I started plainbox service, then the gui client. I selected only the default whitelist.

I deselected all the tests, then selected all the camera tests. This contains at least two manual tests (camera/still and camera/display). Then I started the test run.

camera/display runs first and I get a live video window but the manual test window that pops up is for camera/still. Once the live video window closes, I get another capture window but for a static image (meaning, that's the command for camera/still). An outcome of "No" is selected once the static image closes. Then I click on "continue" and the client crashes with a message (following).

So first, it seems to be running the command for a manual job as soon as it hits the manual job (not waiting for me to press "test"). Then capturing the test outcome seems to be wrong. Finally, it shouldn't crash at the end :(

uiEngine::PrepareJobs()

Time for summary: 29
Start Testing
Running Job "package"
Real outcome "pass"
Running Job "device"
Real outcome "pass"
Running Job "camera/detect"
Real outcome "pass"
Running Job "camera/display"
GuiEngine::CatchallAskForOutcomeSignalsHandler
Real outcome ""
GuiEngine::CatchallAskForOutcomeSignalsHandler - Done
Real outcome "none"
Running Job "camera/still"
GuiEngine::CatchallAskForOutcomeSignalsHandler
Real outcome ""
updateManualInteractionDialog
GuiEngine::CatchallAskForOutcomeSignalsHandler - Done
Real outcome "none"
onJobsCompleted
Continue
GuiEngine::ResumeFromManualInteraction()
GuiEngine::SetOutcome
GuiEngine::SetOutcome - Done
GuiEngine::ResumeFromManualInteraction()
Real outcome "pass"
ASSERT failure in QList<T>::at: "index out of range", file /usr/include/qt5/QtCore/qlist.h, line 454
The program has unexpectedly finished.
/media/bulk/checkboxes/checkbox-ihv-ng/trunk/build-plainbox-gui-Desktop-Debug/driver-testing/driver-testing exited with code 0

I was able to reproduce this with stubbox leaving all three whitelists selected, it just blasts through all the manual test windows and then crashes when I click on "continue" on the manual test window, which shows stub/dependency/good (it shouldn't as that's an automated job).

Unmerged revisions

2280. By Andrew Haigh on 2013-08-30

Ensure that the outcome is signalled to the manual interaction screen and avoid the qml missing outcome error. Note that this could usefully be updated to read the assumed result but this is another piece of work.

2279. By Andrew Haigh on 2013-08-30

Borrowed wip updates from spineau

2278. By Daniel Manrique on 2013-08-23

Merged from lp:checkbox

2277. By Brendan Donegan on 2013-08-23

"automatic merge by tarmac [r=zkrynicki][bug=][author=brendan-donegan]"

2276. By Brendan Donegan on 2013-08-23

"automatic merge by tarmac [r=zkrynicki][bug=][author=brendan-donegan]"

2275. By Andrew Haigh on 2013-08-22

"automatic merge by tarmac [r=roadmr][bug=1214911][author=andrew-haigh-b]"

2274. By Daniel Manrique on 2013-08-21

Merged from lp:checkbox

2273. By Daniel Manrique on 2013-08-21

Merged Hiding any groups or tests not selected by the user when showing the run manager, by Andrew Haigh

2272. By Daniel Manrique on 2013-08-21

Merged activity spinner and better test counting, by Andrew Haigh

The ActivityIndicator spinner replaces the suite selection whilst generating local jobs

Added Ubuntu ActivityIndicator to the Manual Interaction Dialog

Added a count of the implicitly run tests to the Test Selection Screen. Note: This should also take care of bug #1211791 Plainbox runs test not selected in Test Selection screen.

2271. By Andrew Haigh on 2013-08-21

"automatic merge by tarmac [r=roadmr][bug=][author=andrew-haigh-b]"

=== modified file 'plainbox-gui/gui-engine/gui-engine.cpp'
--- plainbox-gui/gui-engine/gui-engine.cpp	2013-08-21 11:41:50 +0000
+++ plainbox-gui/gui-engine/gui-engine.cpp	2013-08-30 15:57:58 +0000
@@ -576,7 +576,7 @@
 * Selection screen as additional implicit jobs
 */
 
-int GuiEngine::PrepareJobs(void) 
+int GuiEngine::PrepareJobs(void)
 {
 
     qDebug("\n\nGuiEngine::PrepareJobs()\n");
@@ -1290,15 +1290,8 @@
     QString job_cmd = GetCommand(m_run_list.at(m_current_job_index));
 
 
-    // Get the interim Job Results
-    GetJobStateMap();
-
-    GetJobStates();
-
-    GetJobResults();
-
     // we should look up the prior job result if available
-    int outcome = GetOutcomeFromJobPath(m_run_list.at(m_current_job_index));
+    int outcome = GetOutcomeFromJobResultPath(m_run_list.at(m_current_job_index));
 
     // Open the GUI dialog -- TODO - Default the Yes/No/Skip icons
     if (!m_running_manual_job) {
@@ -1310,7 +1303,6 @@
         emit updateManualInteractionDialog(outcome);
     }
 
-
     qDebug("GuiEngine::CatchallAskForOutcomeSignalsHandler - Done");
 }
 
@@ -1350,6 +1342,9 @@
         case PBTreeNode::PBJobResult_Skip:
             return JobResult_OUTCOME_SKIP;
         break;
+
+        default:
+            return JobResult_OUTCOME_NONE;
     }
 }
 
@@ -1638,15 +1633,8 @@
 
     UpdateJobResult(m_session,job,result);
 
-    // Get the interim Job Results
-    GetJobStateMap();
-
-    GetJobStates();
-
-    GetJobResults();
-
-      // How did this job turn out?
-    int outcome = GetOutcomeFromJobPath(m_run_list.at(m_current_job_index));
+    // How did this job turn out?
+    int outcome = GetOutcomeFromJobResultPath(result);
 
     // Update the GUI so it knows what the job outcome was
     emit updateGuiObjects(m_run_list.at(m_current_job_index).path(), \
@@ -1769,33 +1757,20 @@
     return empty;
 }
 
-const int GuiEngine::GetOutcomeFromJobPath(const QDBusObjectPath &opath)
+const int GuiEngine::GetOutcomeFromJobResultPath(const QDBusObjectPath &opath)
 {
     QString outcome;
-    int result = 0;
 
     /* first, we need to go through the m_job_state_list to find the
      * relevant job to result mapping. then we go through m_job_state_results
      * to obtain the actual result.
      */
 
-    QDBusObjectPath resultpath;
-
-    for(int i=0; i < m_job_state_list.count(); i++) {
-        if (m_job_state_list.at(i)->job().path().compare(opath.path()) == 0) {
-            // ok, we found the right statelist entry
-            resultpath = m_job_state_list.at(i)->result();
-            break;
-        }
-    }
-
-    // Now to find the right result object
-    for(int i=0;i<m_job_state_results.count();i++) {
-        if (m_job_state_results.at(i)->object_path.path().compare(resultpath.path()) == 0) {
-            outcome = m_job_state_results.at(i)->outcome();
-            break;
-        }
-    }
+    PBTreeNode* result_node = new PBTreeNode();
+    result_node->AddNode(result_node, opath);
+    outcome = result_node->outcome();
+
+	delete result_node;
 
     qDebug() << "Real outcome " << outcome;
 

=== modified file 'plainbox-gui/gui-engine/gui-engine.h'
--- plainbox-gui/gui-engine/gui-engine.h	2013-08-21 11:41:50 +0000
+++ plainbox-gui/gui-engine/gui-engine.h	2013-08-30 15:57:58 +0000
@@ -214,7 +214,7 @@
         void RunJob(const QDBusObjectPath session, const QDBusObjectPath opath);
 
         // Convenience functions
-        const int GetOutcomeFromJobPath(const QDBusObjectPath &opath);
+        const int GetOutcomeFromJobResultPath(const QDBusObjectPath &opath);
 
         const QString ConvertOutcome(const int outcome);