Squish: Extract server into separate class

Change-Id: Iff370cd3571598ee32774e02f4f883d07e4c4685
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/squish/CMakeLists.txt

@@ -18,8 +18,10 @@ add_qtc_plugin(Squish
     squishoutputpane.cpp squishoutputpane.h
     squishperspective.cpp squishperspective.h
     squishplugin.cpp squishplugin.h
+    squishprocessbase.cpp squishprocessbase.h
     squishresultmodel.cpp squishresultmodel.h
     squishsettings.cpp squishsettings.h
+    squishserverprocess.cpp squishserverprocess.h
     squishtesttreemodel.cpp squishtesttreemodel.h
     squishtesttreeview.cpp squishtesttreeview.h
     squishtools.cpp squishtools.h

src/plugins/squish/squish.qbs

@@ -44,8 +44,12 @@ QtcPlugin {
         "squishplugin.cpp",
         "squishplugin.h",
         "squishplugin_global.h",
+        "squishprocessbase.cpp",
+        "squishprocessbase.h",
         "squishresultmodel.cpp",
         "squishresultmodel.h",
+        "squishserverprocess.cpp",
+        "squishserverprocess.h",
         "squishsettings.cpp",
         "squishsettings.h",
         "squishtesttreemodel.cpp",

src/plugins/squish/squishprocessbase.cpp

@@ -0,0 +1,25 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include "squishprocessbase.h"
+
+namespace Squish::Internal {
+
+SquishProcessBase::SquishProcessBase(QObject *parent)
+    : QObject(parent)
+{
+    connect(&m_process, &Utils::QtcProcess::readyReadStandardError,
+            this, &SquishProcessBase::onErrorOutput);
+    connect(&m_process, &Utils::QtcProcess::done,
+            this, &SquishProcessBase::onDone);
+}
+
+void SquishProcessBase::setState(SquishProcessState state)
+{
+    if (m_state == state)
+        return;
+    m_state = state;
+    emit stateChanged(state);
+}
+
+} // namespace Squish::Internal

src/plugins/squish/squishprocessbase.h

@@ -0,0 +1,44 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#pragma once
+
+#include <utils/qtcprocess.h>
+
+#include <QObject>
+
+namespace Squish::Internal {
+
+enum SquishProcessState { Idle, Starting, Started, StartFailed, Stopped, StopFailed };
+
+class SquishProcessBase : public QObject
+{
+    Q_OBJECT
+public:
+    explicit SquishProcessBase(QObject *parent = nullptr);
+    ~SquishProcessBase() = default;
+    SquishProcessState processState() const { return m_state; }
+
+    bool isRunning() const { return m_process.isRunning(); }
+    Utils::ProcessResult result() const { return m_process.result(); }
+    QProcess::ProcessError error() const { return m_process.error(); }
+    QProcess::ProcessState state() const { return m_process.state(); }
+
+    void closeProcess() { m_process.close(); }
+
+signals:
+    void logOutputReceived(const QString &output);
+    void stateChanged(SquishProcessState state);
+
+protected:
+    void setState(SquishProcessState state);
+    virtual void onDone() {}
+    virtual void onErrorOutput() {}
+
+    Utils::QtcProcess m_process;
+
+private:
+    SquishProcessState m_state = Idle;
+};
+
+} // namespace Squish::Internal

src/plugins/squish/squishserverprocess.cpp

@@ -0,0 +1,102 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include "squishserverprocess.h"
+
+#include <utils/qtcassert.h>
+
+namespace Squish::Internal {
+
+SquishServerProcess::SquishServerProcess(QObject *parent)
+    : SquishProcessBase(parent)
+{
+    connect(&m_process, &Utils::QtcProcess::readyReadStandardOutput,
+            this, &SquishServerProcess::onStandardOutput);
+}
+
+void SquishServerProcess::start(const Utils::CommandLine &commandLine,
+                                const Utils::Environment &environment)
+{
+    QTC_ASSERT(m_process.state() == QProcess::NotRunning, return);
+    // especially when writing server config we re-use the process fast and start the server
+    // several times and may crash as the process may not have been cleanly destructed yet
+    m_process.close();
+
+    m_serverPort = -1;
+    m_process.setCommand(commandLine);
+    m_process.setEnvironment(environment);
+
+    setState(Starting);
+    m_process.start();
+    if (!m_process.waitForStarted()) {
+        setState(StartFailed);
+        qWarning() << "squishserver did not start within 30s";
+    }
+}
+
+void SquishServerProcess::stop()
+{
+    if (m_process.state() != QProcess::NotRunning && m_serverPort > 0) {
+        Utils::QtcProcess serverKiller;
+        QStringList args;
+        args << "--stop" << "--port" << QString::number(m_serverPort);
+        serverKiller.setCommand({m_process.commandLine().executable(), args});
+        serverKiller.setEnvironment(m_process.environment());
+        serverKiller.start();
+        if (!serverKiller.waitForFinished()) {
+            qWarning() << "Could not shutdown server within 30s";
+            setState(StopFailed);
+        }
+    } else {
+        qWarning() << "either no process running or port < 1?"
+                   << m_process.state() << m_serverPort;
+        setState(StopFailed);
+    }
+}
+
+void SquishServerProcess::onStandardOutput()
+{
+    const QByteArray output = m_process.readAllRawStandardOutput();
+    const QList<QByteArray> lines = output.split('\n');
+    for (const QByteArray &line : lines) {
+        const QByteArray trimmed = line.trimmed();
+        if (trimmed.isEmpty())
+            continue;
+        if (trimmed.startsWith("Port:")) {
+            if (m_serverPort == -1) {
+                bool ok;
+                int port = trimmed.mid(6).toInt(&ok);
+                if (ok) {
+                    m_serverPort = port;
+                    setState(Started);
+                } else {
+                    qWarning() << "could not get port number" << trimmed.mid(6);
+                    setState(StartFailed);
+                }
+            } else {
+                qWarning() << "got a Port output - don't know why...";
+            }
+        }
+        emit logOutputReceived(QString("Server: ") + QLatin1String(trimmed));
+    }
+}
+
+void SquishServerProcess::onErrorOutput()
+{
+    // output that must be sent to the Runner/Server Log
+    const QByteArray output = m_process.readAllRawStandardError();
+    const QList<QByteArray> lines = output.split('\n');
+    for (const QByteArray &line : lines) {
+        const QByteArray trimmed = line.trimmed();
+        if (!trimmed.isEmpty())
+            emit logOutputReceived(QString("Server: ") + QLatin1String(trimmed));
+    }
+}
+
+void SquishServerProcess::onDone()
+{
+    m_serverPort = -1;
+    setState(Stopped);
+}
+
+} // namespace Squish::Internal

src/plugins/squish/squishserverprocess.h

@@ -0,0 +1,33 @@
+// Copyright (C) 2022 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#pragma once
+
+#include "squishprocessbase.h"
+
+#include <QObject>
+
+namespace Squish::Internal {
+
+class SquishServerProcess : public SquishProcessBase
+{
+    Q_OBJECT
+public:
+    explicit SquishServerProcess(QObject *parent = nullptr);
+    ~SquishServerProcess() = default;
+
+    int port() const { return m_serverPort; }
+
+    void start(const Utils::CommandLine &commandLine,
+               const Utils::Environment &environment);
+    void stop();
+
+private:
+    void onStandardOutput();
+    void onErrorOutput() override;
+    void onDone() override;
+
+    int m_serverPort = -1;
+};
+
+} // namespace Squish::Internal

src/plugins/squish/squishtools.cpp

@@ -127,12 +127,11 @@ SquishTools::SquishTools(QObject *parent)
     connect(&m_runnerProcess, &QtcProcess::done,
             this, &SquishTools::onRunnerFinished);
 
-    connect(&m_serverProcess, &QtcProcess::readyReadStandardOutput,
+    connect(&m_serverProcess, &SquishServerProcess::stateChanged,
-            this, &SquishTools::onServerOutput);
+            this, &SquishTools::onServerStateChanged);
-    connect(&m_serverProcess, &QtcProcess::readyReadStandardError,
+    connect(&m_serverProcess, &SquishServerProcess::logOutputReceived,
-            this, &SquishTools::onServerErrorOutput);
+            this, &SquishTools::logOutputReceived);
-    connect(&m_serverProcess, &QtcProcess::done,
+
-            this, &SquishTools::onServerFinished);
     s_instance = this;
     m_perspective.initPerspective();
     connect(&m_perspective, &SquishPerspective::interruptRequested,
@@ -338,6 +337,30 @@ void SquishTools::writeServerSettingsChanges(const QList<QStringList> &changes)
     startSquishServer(ServerConfigChangeRequested);
 }
 
+void SquishTools::onServerStateChanged(SquishProcessState state)
+{
+    switch (state) {
+    case Starting:
+        setState(SquishTools::ServerStarting);
+        break;
+    case Started:
+        setState(SquishTools::ServerStarted);
+        break;
+    case StartFailed:
+        setState(SquishTools::ServerStartFailed);
+        break;
+    case Stopped:
+        setState(SquishTools::ServerStopped);
+        break;
+    case StopFailed:
+        setState(SquishTools::ServerStopFailed);
+        break;
+    default:
+        // Idle currently unhandled / not needed?
+        break;
+    }
+}
+
 void SquishTools::setState(SquishTools::State state)
 {
     qCInfo(LOG) << "State change:" << toolsStateName(m_state) << ">" << toolsStateName(state);
@@ -405,7 +428,7 @@ void SquishTools::setState(SquishTools::State state)
         }
         break;
     case ServerStopFailed:
-        m_serverProcess.close();
+        m_serverProcess.closeProcess();
         if (toolsSettings.minimizeIDE)
             restoreQtCreatorWindows();
         m_perspective.destroyControlBar();
@@ -469,7 +492,7 @@ void SquishTools::handleSetStateStartAppRunner()
         }
         break;
     case ServerStopFailed:
-        m_serverProcess.close();
+        m_serverProcess.closeProcess();
         if (toolsSettings.minimizeIDE)
             restoreQtCreatorWindows();
         m_perspective.destroyControlBar();
@@ -553,7 +576,6 @@ void SquishTools::startSquishServer(Request request)
     }
 
     toolsSettings.setup();
-    m_serverPort = -1;
 
     const FilePath squishServer = Environment::systemEnvironment().searchInPath(
         toolsSettings.serverPath.toString());
@@ -586,41 +608,13 @@ void SquishTools::startSquishServer(Request request)
     }
 
     const QStringList arguments = serverArgumentsFromSettings();
-    m_serverProcess.setCommand({toolsSettings.serverPath, arguments});
+    m_serverProcess.start({toolsSettings.serverPath, arguments}, squishEnvironment());
-
-    m_serverProcess.setEnvironment(squishEnvironment());
-
-    // especially when writing server config we re-use the process fast and start the server
-    // several times and may crash as the process may not have been cleanly destructed yet
-    m_serverProcess.close();
-    setState(ServerStarting);
-    qCDebug(LOG) << "Server starts:" << m_serverProcess.commandLine().toUserOutput();
-    m_serverProcess.start();
-    if (!m_serverProcess.waitForStarted()) {
-        setState(ServerStartFailed);
-        qWarning() << "squishserver did not start within 30s";
-    }
 }
 
 void SquishTools::stopSquishServer()
 {
     qCDebug(LOG) << "Stopping server";
-    if (m_serverProcess.state() != QProcess::NotRunning && m_serverPort > 0) {
+    m_serverProcess.stop();
-        QtcProcess serverKiller;
-        QStringList args;
-        args << "--stop" << "--port" << QString::number(m_serverPort);
-        serverKiller.setCommand({m_serverProcess.commandLine().executable(), args});
-        serverKiller.setEnvironment(m_serverProcess.environment());
-        serverKiller.start();
-        if (!serverKiller.waitForFinished()) {
-            qWarning() << "Could not shutdown server within 30s";
-            setState(ServerStopFailed);
-        }
-    } else {
-        qWarning() << "either no process running or port < 1?"
-                   << m_serverProcess.state() << m_serverPort;
-        setState(ServerStopFailed);
-    }
 }
 
 void SquishTools::startSquishRunner()
@@ -646,7 +640,7 @@ void SquishTools::setupAndStartRecorder()
     QStringList args;
     if (!toolsSettings.isLocalServer)
         args << "--host" << toolsSettings.serverHost;
-    args << "--port" << QString::number(m_serverPort);
+    args << "--port" << QString::number(m_serverProcess.port());
     args << "--debugLog" << "alpw"; // TODO make this configurable?
     args << "--record";
     args << "--suitedir" << m_suitePath.toUserOutput();
@@ -686,7 +680,7 @@ void SquishTools::executeRunnerQuery()
     if (!isValidToStartRunner() || !setupRunnerPath())
         return;
 
-    QStringList arguments = { "--port", QString::number(m_serverPort) };
+    QStringList arguments = { "--port", QString::number(m_serverProcess.port()) };
     Utils::CommandLine cmdLine = {toolsSettings.runnerPath, arguments};
     switch (m_query) {
     case ServerInfo:
@@ -717,13 +711,6 @@ Environment SquishTools::squishEnvironment()
     return environment;
 }
 
-void SquishTools::onServerFinished()
-{
-    qCDebug(LOG) << "Server finished";
-    m_serverPort = -1;
-    setState(ServerStopped);
-}
-
 void SquishTools::onRunnerFinished()
 {
     qCDebug(LOG) << "Runner finished";
@@ -799,46 +786,6 @@ void SquishTools::onRecorderFinished()
     m_currentRecorderSnippetFile.clear();
 }
 
-void SquishTools::onServerOutput()
-{
-    // output used for getting the port information of the current squishserver
-    const QByteArray output = m_serverProcess.readAllRawStandardOutput();
-    const QList<QByteArray> lines = output.split('\n');
-    for (const QByteArray &line : lines) {
-        const QByteArray trimmed = line.trimmed();
-        if (trimmed.isEmpty())
-            continue;
-        if (trimmed.startsWith("Port:")) {
-            if (m_serverPort == -1) {
-                bool ok;
-                int port = trimmed.mid(6).toInt(&ok);
-                if (ok) {
-                    m_serverPort = port;
-                    setState(ServerStarted);
-                } else {
-                    qWarning() << "could not get port number" << trimmed.mid(6);
-                    setState(ServerStartFailed);
-                }
-            } else {
-                qWarning() << "got a Port output - don't know why...";
-            }
-        }
-        emit logOutputReceived(QString("Server: ") + QLatin1String(trimmed));
-    }
-}
-
-void SquishTools::onServerErrorOutput()
-{
-    // output that must be send to the Runner/Server Log
-    const QByteArray output = m_serverProcess.readAllRawStandardError();
-    const QList<QByteArray> lines = output.split('\n');
-    for (const QByteArray &line : lines) {
-        const QByteArray trimmed = line.trimmed();
-        if (!trimmed.isEmpty())
-            emit logOutputReceived(QString("Server: ") + QLatin1String(trimmed));
-    }
-}
-
 static char firstNonWhitespace(const QByteArray &text)
 {
     for (int i = 0, limit = text.size(); i < limit; ++i)
@@ -1344,7 +1291,7 @@ QStringList SquishTools::runnerArgumentsFromSettings()
     QStringList arguments;
     if (!toolsSettings.isLocalServer)
         arguments << "--host" << toolsSettings.serverHost;
-    arguments << "--port" << QString::number(m_serverPort);
+    arguments << "--port" << QString::number(m_serverProcess.port());
     arguments << "--debugLog" << "alpw"; // TODO make this configurable?
 
     QTC_ASSERT(!m_testCases.isEmpty(), m_testCases.append(""));
@@ -1395,7 +1342,7 @@ bool SquishTools::isValidToStartRunner()
         setState(Idle);
         return false;
     }
-    if (m_serverPort == -1) {
+    if (m_serverProcess.port() == -1) {
         QMessageBox::critical(Core::ICore::dialogParent(),
                               Tr::tr("No Squish Server Port"),
                               Tr::tr("Failed to get the server port.\n"

src/plugins/squish/squishtools.h

@@ -4,6 +4,7 @@
 #pragma once
 
 #include "squishperspective.h"
+#include "squishserverprocess.h"
 #include "suiteconf.h"
 
 #include <utils/environment.h>
@@ -102,6 +103,7 @@ private:
 
     enum RunnerQuery { ServerInfo, GetGlobalScriptDirs, SetGlobalScriptDirs };
 
+    void onServerStateChanged(SquishProcessState state);
     void setState(State state);
     void handleSetStateStartAppRunner();
     void handleSetStateQueryRunner();
@@ -114,11 +116,8 @@ private:
     void queryServer(RunnerQuery query);
     void executeRunnerQuery();
     static Utils::Environment squishEnvironment();
-    void onServerFinished();
     void onRunnerFinished();
     void onRecorderFinished();
-    void onServerOutput();
-    void onServerErrorOutput();
     void onRunnerOutput();                              // runner's results file
     void onRunnerErrorOutput();                         // runner's error stream
     void onRunnerStdOutput(const QString &line);        // runner's output stream
@@ -142,10 +141,9 @@ private:
 
     SquishPerspective m_perspective;
     std::unique_ptr<SquishXmlOutputHandler> m_xmlOutputHandler;
-    Utils::QtcProcess m_serverProcess;
+    SquishServerProcess m_serverProcess;
     Utils::QtcProcess m_runnerProcess;
     Utils::QtcProcess m_recorderProcess;
-    int m_serverPort = -1;
     QString m_serverHost;
     Request m_request = None;
     State m_state = Idle;