SquishTools: Keep processes on stack

Avoid manual termination and killing - use either
QtcProcess::close() or default QtcProcess d'tor.
Both will employ internal reaper for terminating
the running process asynchronously.

Change-Id: Ib1de1125da0c25b4f5c5b7ac0827336f99881679
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
This commit is contained in:
Jarek Kobus
2022-07-05 10:51:56 +02:00
parent d3786a895b
commit 1431452069
2 changed files with 50 additions and 100 deletions

View File

@@ -33,10 +33,8 @@
#include <coreplugin/icore.h> #include <coreplugin/icore.h>
#include <utils/environment.h>
#include <utils/hostosinfo.h> #include <utils/hostosinfo.h>
#include <utils/qtcassert.h> #include <utils/qtcassert.h>
#include <utils/qtcprocess.h>
#include <QApplication> #include <QApplication>
#include <QDateTime> #include <QDateTime>
@@ -60,43 +58,27 @@ SquishTools::SquishTools(QObject *parent)
: QObject(parent) : QObject(parent)
{ {
SquishOutputPane *outputPane = SquishOutputPane::instance(); SquishOutputPane *outputPane = SquishOutputPane::instance();
connect(this, connect(this, &SquishTools::logOutputReceived,
&SquishTools::logOutputReceived, outputPane, &SquishOutputPane::addLogOutput, Qt::QueuedConnection);
outputPane, connect(this, &SquishTools::squishTestRunStarted,
&SquishOutputPane::addLogOutput, outputPane, &SquishOutputPane::clearOldResults);
Qt::QueuedConnection); connect(this, &SquishTools::squishTestRunFinished,
connect(this, outputPane, &SquishOutputPane::onTestRunFinished);
&SquishTools::squishTestRunStarted,
outputPane, connect(&m_runnerProcess, &QtcProcess::readyReadStandardError,
&SquishOutputPane::clearOldResults); this, &SquishTools::onRunnerErrorOutput);
connect(this, connect(&m_runnerProcess, &QtcProcess::done,
&SquishTools::squishTestRunFinished, this, &SquishTools::onRunnerFinished);
outputPane,
&SquishOutputPane::onTestRunFinished); connect(&m_serverProcess, &QtcProcess::readyReadStandardOutput,
this, &SquishTools::onServerOutput);
connect(&m_serverProcess, &QtcProcess::readyReadStandardError,
this, &SquishTools::onServerErrorOutput);
connect(&m_serverProcess, &QtcProcess::done,
this, &SquishTools::onServerFinished);
} }
SquishTools::~SquishTools() SquishTools::~SquishTools() = default;
{
// TODO add confirmation dialog somewhere
// FIXME re-do to make it work
if (m_runnerProcess) {
m_runnerProcess->terminate();
if (!m_runnerProcess->waitForFinished(5000))
m_runnerProcess->kill();
delete m_runnerProcess;
m_runnerProcess = nullptr;
}
if (m_serverProcess) {
m_serverProcess->terminate();
if (!m_serverProcess->waitForFinished(5000))
m_serverProcess->kill();
delete m_serverProcess;
m_serverProcess = nullptr;
}
delete m_xmlOutputHandler;
}
struct SquishToolsSettings struct SquishToolsSettings
{ {
@@ -166,12 +148,9 @@ void SquishTools::runTestCases(const QString &suitePath,
m_additionalRunnerArguments << "--interactive" << "--resultdir" m_additionalRunnerArguments << "--interactive" << "--resultdir"
<< QDir::toNativeSeparators(m_currentResultsDirectory); << QDir::toNativeSeparators(m_currentResultsDirectory);
delete m_xmlOutputHandler; m_xmlOutputHandler.reset(new SquishXmlOutputHandler(this));
m_xmlOutputHandler = new SquishXmlOutputHandler(this); connect(this, &SquishTools::resultOutputCreated,
connect(this, m_xmlOutputHandler.get(), &SquishXmlOutputHandler::outputAvailable,
&SquishTools::resultOutputCreated,
m_xmlOutputHandler,
&SquishXmlOutputHandler::outputAvailable,
Qt::QueuedConnection); Qt::QueuedConnection);
m_testRunning = true; m_testRunning = true;
@@ -234,14 +213,7 @@ void SquishTools::setState(SquishTools::State state)
} }
break; break;
case ServerStopFailed: case ServerStopFailed:
if (m_serverProcess && m_serverProcess->state() != QProcess::NotRunning) { m_serverProcess.close();
m_serverProcess->terminate();
if (!m_serverProcess->waitForFinished(5000)) {
m_serverProcess->kill();
delete m_serverProcess;
m_serverProcess = nullptr;
}
}
m_state = Idle; m_state = Idle;
break; break;
case RunnerStartFailed: case RunnerStartFailed:
@@ -273,7 +245,7 @@ static SquishToolsSettings toolsSettings;
void SquishTools::startSquishServer(Request request) void SquishTools::startSquishServer(Request request)
{ {
m_request = request; m_request = request;
if (m_serverProcess) { if (m_serverProcess.state() != QProcess::NotRunning) {
if (QMessageBox::question(Core::ICore::dialogParent(), if (QMessageBox::question(Core::ICore::dialogParent(),
tr("Squish Server Already Running"), tr("Squish Server Already Running"),
tr("There is still an old Squish server instance running.\n" tr("There is still an old Squish server instance running.\n"
@@ -325,7 +297,6 @@ void SquishTools::startSquishServer(Request request)
else else
m_lastTopLevelWindows.clear(); m_lastTopLevelWindows.clear();
m_serverProcess = new QtcProcess;
QStringList arguments; QStringList arguments;
// TODO if isLocalServer is false we should start a squishserver on remote device // TODO if isLocalServer is false we should start a squishserver on remote device
if (toolsSettings.isLocalServer) if (toolsSettings.isLocalServer)
@@ -335,19 +306,12 @@ void SquishTools::startSquishServer(Request request)
if (toolsSettings.verboseLog) if (toolsSettings.verboseLog)
arguments << "--verbose"; arguments << "--verbose";
m_serverProcess->setCommand({toolsSettings.serverPath, arguments}); m_serverProcess.setCommand({toolsSettings.serverPath, arguments});
m_serverProcess->setEnvironment(squishEnvironment()); m_serverProcess.setEnvironment(squishEnvironment());
connect(m_serverProcess, &QtcProcess::readyReadStandardOutput,
this, &SquishTools::onServerOutput);
connect(m_serverProcess, &QtcProcess::readyReadStandardError,
this, &SquishTools::onServerErrorOutput);
connect(m_serverProcess, &QtcProcess::done,
this, &SquishTools::onServerFinished);
setState(ServerStarting); setState(ServerStarting);
m_serverProcess->start(); m_serverProcess.start();
if (!m_serverProcess->waitForStarted()) { if (!m_serverProcess.waitForStarted()) {
setState(ServerStartFailed); setState(ServerStartFailed);
qWarning() << "squishserver did not start within 30s"; qWarning() << "squishserver did not start within 30s";
} }
@@ -355,31 +319,27 @@ void SquishTools::startSquishServer(Request request)
void SquishTools::stopSquishServer() void SquishTools::stopSquishServer()
{ {
if (m_serverProcess && m_serverPort > 0) { if (m_serverProcess.state() != QProcess::NotRunning && m_serverPort > 0) {
QtcProcess serverKiller; QtcProcess serverKiller;
QStringList args; QStringList args;
args << "--stop" << "--port" << QString::number(m_serverPort); args << "--stop" << "--port" << QString::number(m_serverPort);
serverKiller.setCommand({m_serverProcess->commandLine().executable(), args}); serverKiller.setCommand({m_serverProcess.commandLine().executable(), args});
serverKiller.setEnvironment(m_serverProcess->environment()); serverKiller.setEnvironment(m_serverProcess.environment());
serverKiller.start(); serverKiller.start();
if (serverKiller.waitForStarted()) {
if (!serverKiller.waitForFinished()) { if (!serverKiller.waitForFinished()) {
qWarning() << "Could not shutdown server within 30s"; qWarning() << "Could not shutdown server within 30s";
setState(ServerStopFailed); setState(ServerStopFailed);
} }
} else { } else {
qWarning() << "Could not shutdown server within 30s"; qWarning() << "either no process running or port < 1?"
setState(ServerStopFailed); << m_serverProcess.state() << m_serverPort;
}
} else {
qWarning() << "either no process running or port < 1?" << m_serverProcess << m_serverPort;
setState(ServerStopFailed); setState(ServerStopFailed);
} }
} }
void SquishTools::startSquishRunner() void SquishTools::startSquishRunner()
{ {
if (!m_serverProcess) { if (!m_serverProcess.isRunning()) {
QMessageBox::critical(Core::ICore::dialogParent(), QMessageBox::critical(Core::ICore::dialogParent(),
tr("No Squish Server"), tr("No Squish Server"),
tr("Squish server does not seem to be running.\n" tr("Squish server does not seem to be running.\n"
@@ -403,7 +363,7 @@ void SquishTools::startSquishRunner()
return; return;
} }
if (m_runnerProcess) { if (m_runnerProcess.state() != QProcess::NotRunning) {
QMessageBox::critical(Core::ICore::dialogParent(), QMessageBox::critical(Core::ICore::dialogParent(),
tr("Squish Runner Running"), tr("Squish Runner Running"),
tr("Squish runner seems to be running already.\n" tr("Squish runner seems to be running already.\n"
@@ -427,8 +387,6 @@ void SquishTools::startSquishRunner()
} }
toolsSettings.runnerPath = squishRunner; toolsSettings.runnerPath = squishRunner;
m_runnerProcess = new QtcProcess;
QStringList args; QStringList args;
args << m_additionalServerArguments; args << m_additionalServerArguments;
if (!toolsSettings.isLocalServer) if (!toolsSettings.isLocalServer)
@@ -452,13 +410,8 @@ void SquishTools::startSquishRunner()
args << "--reportgen" args << "--reportgen"
<< QString::fromLatin1("xml2.2,%1").arg(caseReportFilePath); << QString::fromLatin1("xml2.2,%1").arg(caseReportFilePath);
m_runnerProcess->setCommand({toolsSettings.runnerPath, args}); m_runnerProcess.setCommand({toolsSettings.runnerPath, args});
m_runnerProcess->setEnvironment(squishEnvironment()); m_runnerProcess.setEnvironment(squishEnvironment());
connect(m_runnerProcess, &QtcProcess::readyReadStandardError,
this, &SquishTools::onRunnerErrorOutput);
connect(m_runnerProcess, &QtcProcess::done,
this, &SquishTools::onRunnerFinished);
setState(RunnerStarting); setState(RunnerStarting);
@@ -476,14 +429,15 @@ void SquishTools::startSquishRunner()
this, this,
&SquishTools::onResultsDirChanged); &SquishTools::onResultsDirChanged);
m_runnerProcess->start(); m_runnerProcess.start();
if (!m_runnerProcess->waitForStarted()) { if (!m_runnerProcess.waitForStarted()) {
QMessageBox::critical(Core::ICore::dialogParent(), QMessageBox::critical(Core::ICore::dialogParent(),
tr("Squish Runner Error"), tr("Squish Runner Error"),
tr("Squish runner failed to start within given timeframe.")); tr("Squish runner failed to start within given timeframe."));
delete m_resultsFileWatcher; delete m_resultsFileWatcher;
m_resultsFileWatcher = nullptr; m_resultsFileWatcher = nullptr;
setState(RunnerStartFailed); setState(RunnerStartFailed);
m_runnerProcess.close();
return; return;
} }
setState(RunnerStarted); setState(RunnerStarted);
@@ -501,17 +455,12 @@ Environment SquishTools::squishEnvironment()
void SquishTools::onServerFinished() void SquishTools::onServerFinished()
{ {
m_serverProcess->deleteLater();
m_serverProcess = nullptr;
m_serverPort = -1; m_serverPort = -1;
setState(ServerStopped); setState(ServerStopped);
} }
void SquishTools::onRunnerFinished() void SquishTools::onRunnerFinished()
{ {
m_runnerProcess->deleteLater();
m_runnerProcess = nullptr;
if (m_resultsFileWatcher) { if (m_resultsFileWatcher) {
delete m_resultsFileWatcher; delete m_resultsFileWatcher;
m_resultsFileWatcher = nullptr; m_resultsFileWatcher = nullptr;
@@ -531,7 +480,7 @@ void SquishTools::onRunnerFinished()
void SquishTools::onServerOutput() void SquishTools::onServerOutput()
{ {
// output used for getting the port information of the current squishserver // output used for getting the port information of the current squishserver
const QByteArray output = m_serverProcess->readAllStandardOutput(); const QByteArray output = m_serverProcess.readAllStandardOutput();
const QList<QByteArray> lines = output.split('\n'); const QList<QByteArray> lines = output.split('\n');
for (const QByteArray &line : lines) { for (const QByteArray &line : lines) {
const QByteArray trimmed = line.trimmed(); const QByteArray trimmed = line.trimmed();
@@ -559,7 +508,7 @@ void SquishTools::onServerOutput()
void SquishTools::onServerErrorOutput() void SquishTools::onServerErrorOutput()
{ {
// output that must be send to the Runner/Server Log // output that must be send to the Runner/Server Log
const QByteArray output = m_serverProcess->readAllStandardError(); const QByteArray output = m_serverProcess.readAllStandardError();
const QList<QByteArray> lines = output.split('\n'); const QList<QByteArray> lines = output.split('\n');
for (const QByteArray &line : lines) { for (const QByteArray &line : lines) {
const QByteArray trimmed = line.trimmed(); const QByteArray trimmed = line.trimmed();
@@ -648,7 +597,7 @@ void SquishTools::onRunnerOutput()
void SquishTools::onRunnerErrorOutput() void SquishTools::onRunnerErrorOutput()
{ {
// output that must be send to the Runner/Server Log // output that must be send to the Runner/Server Log
const QByteArray output = m_runnerProcess->readAllStandardError(); const QByteArray output = m_runnerProcess.readAllStandardError();
const QList<QByteArray> lines = output.split('\n'); const QList<QByteArray> lines = output.split('\n');
for (const QByteArray &line : lines) { for (const QByteArray &line : lines) {
const QByteArray trimmed = line.trimmed(); const QByteArray trimmed = line.trimmed();

View File

@@ -26,18 +26,19 @@
#pragma once #pragma once
#include <utils/environment.h> #include <utils/environment.h>
#include <utils/qtcprocess.h>
#include <QObject> #include <QObject>
#include <QStringList> #include <QStringList>
#include <QWindowList> #include <QWindowList>
#include <memory>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
class QFile; class QFile;
class QFileSystemWatcher; class QFileSystemWatcher;
QT_END_NAMESPACE QT_END_NAMESPACE
namespace Utils { class QtcProcess; }
namespace Squish { namespace Squish {
namespace Internal { namespace Internal {
@@ -103,8 +104,9 @@ private:
void minimizeQtCreatorWindows(); void minimizeQtCreatorWindows();
void restoreQtCreatorWindows(); void restoreQtCreatorWindows();
Utils::QtcProcess *m_serverProcess = nullptr; std::unique_ptr<SquishXmlOutputHandler> m_xmlOutputHandler;
Utils::QtcProcess *m_runnerProcess = nullptr; Utils::QtcProcess m_serverProcess;
Utils::QtcProcess m_runnerProcess;
int m_serverPort = -1; int m_serverPort = -1;
QString m_serverHost; QString m_serverHost;
Request m_request = None; Request m_request = None;
@@ -120,7 +122,6 @@ private:
QWindowList m_lastTopLevelWindows; QWindowList m_lastTopLevelWindows;
bool m_testRunning = false; bool m_testRunning = false;
qint64 m_readResultsCount; qint64 m_readResultsCount;
SquishXmlOutputHandler *m_xmlOutputHandler = nullptr;
}; };
} // namespace Internal } // namespace Internal