AutoTest: Redo running tests

Removing the event loop and the costly internal
infinite loop to reduce CPU load.
We need an event loop for on-the-fly processing
of the results, but the main event loop is good
enough for this. There is no need to add another
one.
There is also no need to put all this into an
asynchronous job as all of this happens
asynchronously anyway by using signals and slots.

Task-number: QTCREATORBUG-20439
Change-Id: I126bf0c1be3e49fd0dd477e161e4fe7a10a080c9
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/autotest/testrunner.cpp

@@ -48,7 +48,6 @@
 #include <utils/hostosinfo.h>
 #include <utils/outputformat.h>
 #include <utils/qtcprocess.h>
-#include <utils/runextensions.h>
 
 #include <QComboBox>
 #include <QDialogButtonBox>
@@ -56,8 +55,9 @@
 #include <QFuture>
 #include <QFutureInterface>
 #include <QLabel>
+#include <QProcess>
 #include <QPushButton>
-#include <QTime>
+#include <QTimer>
 
 #include <debugger/debuggerkitinformation.h>
 #include <debugger/debuggerruncontrol.h>
@@ -88,9 +88,9 @@ TestRunner::TestRunner(QObject *parent) :
             &m_futureWatcher, &QFutureWatcher<TestResultPtr>::cancel);
     connect(&m_futureWatcher, &QFutureWatcher<TestResultPtr>::canceled,
             this, [this]() {
+        cancelCurrent(UserCanceled);
         emit testResultReady(TestResultPtr(new FaultyTestResult(
                 Result::MessageFatal, tr("Test run canceled by user."))));
-        m_executingTests = false; // avoid being stuck if finished() signal won't get emitted
     });
 }
 
@@ -103,13 +103,15 @@ TestRunner::~TestRunner()
 
 void TestRunner::setSelectedTests(const QList<TestConfiguration *> &selected)
 {
+    QTC_ASSERT(!m_executingTests, return);
     qDeleteAll(m_selectedTests);
     m_selectedTests.clear();
-     m_selectedTests = selected;
+    m_selectedTests.append(selected);
 }
 
 void TestRunner::runTest(TestRunMode mode, const TestTreeItem *item)
 {
+    QTC_ASSERT(!m_executingTests, return);
     TestConfiguration *configuration = item->asConfiguration(mode);
 
     if (configuration) {
@@ -118,15 +120,16 @@ void TestRunner::runTest(TestRunMode mode, const TestTreeItem *item)
     }
 }
 
-static QString processInformation(const QProcess &proc)
+static QString processInformation(const QProcess *proc)
 {
-    QString information("\nCommand line: " + proc.program() + ' ' + proc.arguments().join(' '));
+    QTC_ASSERT(proc, return QString());
+    QString information("\nCommand line: " + proc->program() + ' ' + proc->arguments().join(' '));
     QStringList important = { "PATH" };
     if (Utils::HostOsInfo::isLinuxHost())
         important.append("LD_LIBRARY_PATH");
     else if (Utils::HostOsInfo::isMacHost())
         important.append({ "DYLD_LIBRARY_PATH", "DYLD_FRAMEWORK_PATH" });
-    const QProcessEnvironment &environment = proc.processEnvironment();
+    const QProcessEnvironment &environment = proc->processEnvironment();
     for (const QString &var : important)
         information.append('\n' + var + ": " + environment.value(var));
     return information;
@@ -146,104 +149,126 @@ static QString constructOmittedDetailsString(const QStringList &omitted)
                           "configuration page for \"%1\":") + '\n' + omitted.join('\n');
 }
 
-static void performTestRun(QFutureInterface<TestResultPtr> &futureInterface,
+void TestRunner::scheduleNext()
-                           const QList<TestConfiguration *> selectedTests,
-                           const TestSettings &settings, int testCaseCount)
 {
-    const int timeout = settings.timeout;
+    QTC_ASSERT(!m_selectedTests.isEmpty(), onFinished(); return);
-    QEventLoop eventLoop;
+    QTC_ASSERT(!m_currentConfig && !m_currentProcess, resetInternalPointers());
-    QProcess testProcess;
+    QTC_ASSERT(m_fakeFutureInterface, onFinished(); return);
-    testProcess.setReadChannel(QProcess::StandardOutput);
 
-    futureInterface.setProgressRange(0, testCaseCount);
+    m_currentConfig = m_selectedTests.dequeue();
-    futureInterface.setProgressValue(0);
 
-    for (const TestConfiguration *testConfiguration : selectedTests) {
+    QString commandFilePath = m_currentConfig->executableFilePath();
-        QString commandFilePath = testConfiguration->executableFilePath();
     if (commandFilePath.isEmpty()) {
-            futureInterface.reportResult(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
+        emit testResultReady(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
-                TestRunner::tr("Executable path is empty. (%1)")
+            tr("Executable path is empty. (%1)").arg(m_currentConfig->displayName()))));
-                                                   .arg(testConfiguration->displayName()))));
+        delete m_currentConfig;
-            continue;
+        m_currentConfig = nullptr;
-        }
+        if (m_selectedTests.isEmpty())
-        testProcess.setProgram(commandFilePath);
+            onFinished();
-
+        else
-        QScopedPointer<TestOutputReader> outputReader;
+            onProcessFinished();
-        outputReader.reset(testConfiguration->outputReader(futureInterface, &testProcess));
-        QTC_ASSERT(outputReader, continue);
-        TestRunner::connect(outputReader.data(), &TestOutputReader::newOutputAvailable,
-                            TestResultsPane::instance(), &TestResultsPane::addOutput);
-        if (futureInterface.isCanceled())
-            break;
-
-        if (!testConfiguration->project())
-            continue;
-
-        QStringList omitted;
-        testProcess.setArguments(testConfiguration->argumentsForTestRunner(&omitted));
-        if (!omitted.isEmpty()) {
-            const QString &details = constructOmittedDetailsString(omitted);
-            futureInterface.reportResult(TestResultPtr(new FaultyTestResult(Result::MessageWarn,
-                details.arg(testConfiguration->displayName()))));
-        }
-        testProcess.setWorkingDirectory(testConfiguration->workingDirectory());
-        QProcessEnvironment environment = testConfiguration->environment().toProcessEnvironment();
-        if (Utils::HostOsInfo::isWindowsHost())
-            environment.insert("QT_LOGGING_TO_CONSOLE", "1");
-        testProcess.setProcessEnvironment(environment);
-        testProcess.start();
-
-        bool ok = testProcess.waitForStarted();
-        QTime executionTimer;
-        executionTimer.start();
-        bool canceledByTimeout = false;
-        if (ok) {
-            while (testProcess.state() == QProcess::Running) {
-                if (executionTimer.elapsed() >= timeout) {
-                    canceledByTimeout = true;
-                    break;
-                }
-                if (futureInterface.isCanceled()) {
-                    testProcess.kill();
-                    testProcess.waitForFinished();
         return;
     }
-                eventLoop.processEvents();
+    if (!m_currentConfig->project())
+        onProcessFinished();
+
+    m_currentProcess = new QProcess;
+    m_currentProcess->setReadChannel(QProcess::StandardOutput);
+    m_currentProcess->setProgram(commandFilePath);
+
+    QTC_ASSERT(!m_currentOutputReader, delete m_currentOutputReader);
+    m_currentOutputReader = m_currentConfig->outputReader(*m_fakeFutureInterface, m_currentProcess);
+    QTC_ASSERT(m_currentOutputReader, onProcessFinished();return);
+
+    connect(m_currentOutputReader, &TestOutputReader::newOutputAvailable,
+            TestResultsPane::instance(), &TestResultsPane::addOutput);
+
+
+    QStringList omitted;
+    m_currentProcess->setArguments(m_currentConfig->argumentsForTestRunner(&omitted));
+    if (!omitted.isEmpty()) {
+        const QString &details = constructOmittedDetailsString(omitted);
+        emit testResultReady(TestResultPtr(new FaultyTestResult(Result::MessageWarn,
+            details.arg(m_currentConfig->displayName()))));
     }
-        } else {
+    m_currentProcess->setWorkingDirectory(m_currentConfig->workingDirectory());
-            futureInterface.reportResult(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
+    QProcessEnvironment environment = m_currentConfig->environment().toProcessEnvironment();
-                TestRunner::tr("Failed to start test for project \"%1\".")
+    if (Utils::HostOsInfo::isWindowsHost())
-                    .arg(testConfiguration->displayName()) + processInformation(testProcess)
+        environment.insert("QT_LOGGING_TO_CONSOLE", "1");
-                                                           + rcInfo(testConfiguration))));
+    m_currentProcess->setProcessEnvironment(environment);
+
+    connect(m_currentProcess,
+            static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished),
+            this, &TestRunner::onProcessFinished);
+    QTimer::singleShot(AutotestPlugin::settings()->timeout, m_currentProcess, [this]() {
+        cancelCurrent(Timeout);
+    });
+
+    m_currentProcess->start();
+    if (!m_currentProcess->waitForStarted()) {
+        emit testResultReady(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
+            tr("Failed to start test for project \"%1\".").arg(m_currentConfig->displayName())
+                + processInformation(m_currentProcess) + rcInfo(m_currentConfig))));
     }
-        if (testProcess.exitStatus() == QProcess::CrashExit) {
-            outputReader->reportCrash();
-            futureInterface.reportResult(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
-                TestRunner::tr("Test for project \"%1\" crashed.")
-                    .arg(testConfiguration->displayName()) + processInformation(testProcess)
-                                                           + rcInfo(testConfiguration))));
-        } else if (!outputReader->hadValidOutput()) {
-            futureInterface.reportResult(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
-                TestRunner::tr("Test for project \"%1\" did not produce any expected output.")
-                    .arg(testConfiguration->displayName()) + processInformation(testProcess)
-                                                           + rcInfo(testConfiguration))));
 }
 
-        if (canceledByTimeout) {
+void TestRunner::cancelCurrent(TestRunner::CancelReason reason)
-            if (testProcess.state() != QProcess::NotRunning) {
+{
-                testProcess.kill();
+    if (reason == UserCanceled) {
-                testProcess.waitForFinished();
+        if (!m_fakeFutureInterface->isCanceled()) // depends on using the button / progress bar
+            m_fakeFutureInterface->reportCanceled();
     }
-            futureInterface.reportResult(TestResultPtr(
+    if (m_currentProcess && m_currentProcess->state() != QProcess::NotRunning) {
-                    new FaultyTestResult(Result::MessageFatal, TestRunner::tr(
+        m_currentProcess->kill();
-                    "Test case canceled due to timeout.\nMaybe raise the timeout?"))));
+        m_currentProcess->waitForFinished();
+    }
+    if (reason == Timeout) {
+        emit testResultReady(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
+                tr("Test case canceled due to timeout.\nMaybe raise the timeout?"))));
     }
 }
-    futureInterface.setProgressValue(testCaseCount);
+
+void TestRunner::onProcessFinished()
+{
+    m_fakeFutureInterface->setProgressValue(m_fakeFutureInterface->progressValue()
+                                            + m_currentConfig->testCaseCount());
+    if (!m_fakeFutureInterface->isCanceled()) {
+        if (m_currentProcess->exitStatus() == QProcess::CrashExit) {
+            m_currentOutputReader->reportCrash();
+            emit testResultReady(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
+                tr("Test for project \"%1\" crashed.").arg(m_currentConfig->displayName())
+                    + processInformation(m_currentProcess) + rcInfo(m_currentConfig))));
+        } else if (!m_currentOutputReader->hadValidOutput()) {
+            emit testResultReady(TestResultPtr(new FaultyTestResult(Result::MessageFatal,
+                tr("Test for project \"%1\" did not produce any expected output.")
+                    .arg(m_currentConfig->displayName()) + processInformation(m_currentProcess)
+                                                         + rcInfo(m_currentConfig))));
+        }
+    }
+
+    resetInternalPointers();
+
+    if (!m_selectedTests.isEmpty() && !m_fakeFutureInterface->isCanceled()) {
+        scheduleNext();
+    } else {
+        m_fakeFutureInterface->reportFinished();
+        onFinished();
+    }
+}
+
+void TestRunner::resetInternalPointers()
+{
+    delete m_currentOutputReader;
+    delete m_currentProcess;
+    delete m_currentConfig;
+    m_currentOutputReader = nullptr;
+    m_currentProcess = nullptr;
+    m_currentConfig = nullptr;
 }
 
 void TestRunner::prepareToRunTests(TestRunMode mode)
 {
+    QTC_ASSERT(!m_executingTests, return);
     m_runMode = mode;
     ProjectExplorer::Internal::ProjectExplorerSettings projectExplorerSettings =
         ProjectExplorer::ProjectExplorerPlugin::projectExplorerSettings();
@@ -384,10 +409,15 @@ void TestRunner::runTests()
 
     int testCaseCount = precheckTestConfigurations();
 
-    QFuture<TestResultPtr> future = Utils::runAsync(&performTestRun, m_selectedTests,
+    // Fake future interface - destruction will be handled by QFuture/QFutureWatcher
-                                                    *AutotestPlugin::settings(), testCaseCount);
+    m_fakeFutureInterface = new QFutureInterface<TestResultPtr>(QFutureInterfaceBase::Running);
+    QFuture<TestResultPtr> future = m_fakeFutureInterface->future();
+    m_fakeFutureInterface->setProgressRange(0, testCaseCount);
+    m_fakeFutureInterface->setProgressValue(0);
     m_futureWatcher.setFuture(future);
+
     Core::ProgressManager::addTask(future, tr("Running Tests"), Autotest::Constants::TASK_INDEX);
+    scheduleNext();
 }
 
 static void processOutput(TestOutputReader *outputreader, const QString &msg,
@@ -555,6 +585,11 @@ void TestRunner::buildFinished(bool success)
 
 void TestRunner::onFinished()
 {
+    // if we've been canceled and we still have test configurations queued just throw them away
+    qDeleteAll(m_selectedTests);
+    m_selectedTests.clear();
+
+    m_fakeFutureInterface = nullptr;
     m_executingTests = false;
     emit testRunFinished();
 }

src/plugins/autotest/testrunner.h

@@ -31,12 +31,13 @@
 #include <QDialog>
 #include <QFutureWatcher>
 #include <QObject>
-#include <QProcess>
+#include <QQueue>
 
 QT_BEGIN_NAMESPACE
 class QComboBox;
 class QDialogButtonBox;
 class QLabel;
+class QProcess;
 QT_END_NAMESPACE
 
 namespace ProjectExplorer {
@@ -51,6 +52,8 @@ class TestRunner : public QObject
     Q_OBJECT
 
 public:
+    enum CancelReason { UserCanceled, Timeout };
+
     static TestRunner* instance();
     ~TestRunner();
 
@@ -72,6 +75,10 @@ private:
     void onFinished();
 
     int precheckTestConfigurations();
+    void scheduleNext();
+    void cancelCurrent(CancelReason reason);
+    void onProcessFinished();
+    void resetInternalPointers();
 
     void runTests();
     void debugTests();
@@ -79,8 +86,12 @@ private:
     explicit TestRunner(QObject *parent = 0);
 
     QFutureWatcher<TestResultPtr> m_futureWatcher;
-    QList<TestConfiguration *> m_selectedTests;
+    QFutureInterface<TestResultPtr> *m_fakeFutureInterface = nullptr;
-    bool m_executingTests;
+    QQueue<TestConfiguration *> m_selectedTests;
+    bool m_executingTests = false;
+    TestConfiguration *m_currentConfig = nullptr;
+    QProcess *m_currentProcess = nullptr;
+    TestOutputReader *m_currentOutputReader = nullptr;
     TestRunMode m_runMode = TestRunMode::Run;
 
     // temporarily used if building before running is necessary