From 1ac69b69fce4c5a1aafbc4bd587bf797e5da0498 Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Wed, 23 Nov 2022 16:14:31 +0100 Subject: [PATCH] Squish: Extract server into separate class Change-Id: Iff370cd3571598ee32774e02f4f883d07e4c4685 Reviewed-by: Reviewed-by: David Schulz --- src/plugins/squish/CMakeLists.txt | 2 + src/plugins/squish/squish.qbs | 4 + src/plugins/squish/squishprocessbase.cpp | 25 ++++ src/plugins/squish/squishprocessbase.h | 44 +++++++ src/plugins/squish/squishserverprocess.cpp | 102 +++++++++++++++++ src/plugins/squish/squishserverprocess.h | 33 ++++++ src/plugins/squish/squishtools.cpp | 127 ++++++--------------- src/plugins/squish/squishtools.h | 8 +- 8 files changed, 250 insertions(+), 95 deletions(-) create mode 100644 src/plugins/squish/squishprocessbase.cpp create mode 100644 src/plugins/squish/squishprocessbase.h create mode 100644 src/plugins/squish/squishserverprocess.cpp create mode 100644 src/plugins/squish/squishserverprocess.h diff --git a/src/plugins/squish/CMakeLists.txt b/src/plugins/squish/CMakeLists.txt index 9ff534c19d8..5471e71559e 100644 --- a/src/plugins/squish/CMakeLists.txt +++ b/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 diff --git a/src/plugins/squish/squish.qbs b/src/plugins/squish/squish.qbs index 4c74c6466fc..1a6eecbb9ad 100644 --- a/src/plugins/squish/squish.qbs +++ b/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", diff --git a/src/plugins/squish/squishprocessbase.cpp b/src/plugins/squish/squishprocessbase.cpp new file mode 100644 index 00000000000..7b68f70a553 --- /dev/null +++ b/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 diff --git a/src/plugins/squish/squishprocessbase.h b/src/plugins/squish/squishprocessbase.h new file mode 100644 index 00000000000..5c863b55997 --- /dev/null +++ b/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 + +#include + +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 diff --git a/src/plugins/squish/squishserverprocess.cpp b/src/plugins/squish/squishserverprocess.cpp new file mode 100644 index 00000000000..006eeb594b6 --- /dev/null +++ b/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 + +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 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 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 diff --git a/src/plugins/squish/squishserverprocess.h b/src/plugins/squish/squishserverprocess.h new file mode 100644 index 00000000000..3503dfbd0cc --- /dev/null +++ b/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 + +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 diff --git a/src/plugins/squish/squishtools.cpp b/src/plugins/squish/squishtools.cpp index 2ea67ab94a8..fa23640a213 100644 --- a/src/plugins/squish/squishtools.cpp +++ b/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, - this, &SquishTools::onServerOutput); - connect(&m_serverProcess, &QtcProcess::readyReadStandardError, - this, &SquishTools::onServerErrorOutput); - connect(&m_serverProcess, &QtcProcess::done, - this, &SquishTools::onServerFinished); + connect(&m_serverProcess, &SquishServerProcess::stateChanged, + this, &SquishTools::onServerStateChanged); + connect(&m_serverProcess, &SquishServerProcess::logOutputReceived, + this, &SquishTools::logOutputReceived); + s_instance = this; m_perspective.initPerspective(); connect(&m_perspective, &SquishPerspective::interruptRequested, @@ -338,6 +337,30 @@ void SquishTools::writeServerSettingsChanges(const QList &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.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"; - } + m_serverProcess.start({toolsSettings.serverPath, arguments}, squishEnvironment()); } void SquishTools::stopSquishServer() { qCDebug(LOG) << "Stopping server"; - if (m_serverProcess.state() != QProcess::NotRunning && m_serverPort > 0) { - 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); - } + m_serverProcess.stop(); } 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 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 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" diff --git a/src/plugins/squish/squishtools.h b/src/plugins/squish/squishtools.h index 7777595f9c5..18cf4dbec12 100644 --- a/src/plugins/squish/squishtools.h +++ b/src/plugins/squish/squishtools.h @@ -4,6 +4,7 @@ #pragma once #include "squishperspective.h" +#include "squishserverprocess.h" #include "suiteconf.h" #include @@ -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 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;