From 026d04d2808378617a192354e1d4b020e4dba1d3 Mon Sep 17 00:00:00 2001 From: Christian Stenger Date: Wed, 25 Jan 2023 11:52:04 +0100 Subject: [PATCH] Squish: Extract common message handling Change-Id: I078046e52fa2e3580bfa978ebe74303fb02f1513 Reviewed-by: David Schulz --- src/plugins/squish/CMakeLists.txt | 1 + src/plugins/squish/objectsmapeditorwidget.cpp | 8 +- src/plugins/squish/squish.qbs | 2 + src/plugins/squish/squishfilehandler.cpp | 36 ++--- src/plugins/squish/squishmessages.cpp | 35 +++++ src/plugins/squish/squishmessages.h | 15 ++ src/plugins/squish/squishnavigationwidget.cpp | 30 ++-- src/plugins/squish/squishsettings.cpp | 9 +- src/plugins/squish/squishtools.cpp | 142 +++++++----------- 9 files changed, 137 insertions(+), 141 deletions(-) create mode 100644 src/plugins/squish/squishmessages.cpp create mode 100644 src/plugins/squish/squishmessages.h diff --git a/src/plugins/squish/CMakeLists.txt b/src/plugins/squish/CMakeLists.txt index 54f60e10804..476a4af2366 100644 --- a/src/plugins/squish/CMakeLists.txt +++ b/src/plugins/squish/CMakeLists.txt @@ -14,6 +14,7 @@ add_qtc_plugin(Squish scripthelper.cpp scripthelper.h squish.qrc squishfilehandler.cpp squishfilehandler.h + squishmessages.cpp squishmessages.h squishnavigationwidget.cpp squishnavigationwidget.h squishoutputpane.cpp squishoutputpane.h squishperspective.cpp squishperspective.h diff --git a/src/plugins/squish/objectsmapeditorwidget.cpp b/src/plugins/squish/objectsmapeditorwidget.cpp index e26e6001390..7e215d0d77f 100644 --- a/src/plugins/squish/objectsmapeditorwidget.cpp +++ b/src/plugins/squish/objectsmapeditorwidget.cpp @@ -7,6 +7,7 @@ #include "objectsmapdocument.h" #include "objectsmaptreeitem.h" #include "propertyitemdelegate.h" +#include "squishmessages.h" #include "squishtr.h" #include "symbolnameitemdelegate.h" @@ -25,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -467,10 +467,8 @@ void ObjectsMapEditorWidget::onRemoveSymbolicNameTriggered() newReference = dialog.selectedSymbolicName(); } else { // Squish does not ask for removing objects without references, but we prefer to do it - if (QMessageBox::question(Core::ICore::dialogParent(), - Tr::tr("Remove Symbolic Name"), - Tr::tr("Do you really want to remove \"%1\"?").arg(symbolicName)) - != QMessageBox::Yes) + const QString detail = Tr::tr("Do you really want to remove \"%1\"?").arg(symbolicName); + if (SquishMessages::simpleQuestion(Tr::tr("Remove Symbolic Name"), detail) != QMessageBox::Yes) return; } diff --git a/src/plugins/squish/squish.qbs b/src/plugins/squish/squish.qbs index 19bfbd20455..29dd1da4042 100644 --- a/src/plugins/squish/squish.qbs +++ b/src/plugins/squish/squish.qbs @@ -35,6 +35,8 @@ QtcPlugin { "squishconstants.h", "squishfilehandler.cpp", "squishfilehandler.h", + "squishmessages.cpp", + "squishmessages.h", "squishnavigationwidget.cpp", "squishnavigationwidget.h", "squishoutputpane.cpp", diff --git a/src/plugins/squish/squishfilehandler.cpp b/src/plugins/squish/squishfilehandler.cpp index fdd7f16cae9..aa3843d1ad6 100644 --- a/src/plugins/squish/squishfilehandler.cpp +++ b/src/plugins/squish/squishfilehandler.cpp @@ -5,7 +5,7 @@ #include "opensquishsuitesdialog.h" #include "squishconstants.h" -#include "squishplugin.h" +#include "squishmessages.h" #include "squishsettings.h" #include "squishtesttreemodel.h" #include "squishtools.h" @@ -327,12 +327,10 @@ void SquishFileHandler::runTestCase(const QString &suiteName, const QString &tes const Utils::FilePath suitePath = m_suites.value(suiteName).parentDir(); if (!suitePath.exists() || !suitePath.isReadableDir()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Test Suite Path Not Accessible"), - Tr::tr("The path \"%1\" does not exist or is not accessible.\n" - "Refusing to run test case \"%2\".") - .arg(suitePath.toUserOutput()) - .arg(testCaseName)); + const QString detail = Tr::tr("The path \"%1\" does not exist or is not accessible.\n" + "Refusing to run test case \"%2\".") + .arg(suitePath.toUserOutput()).arg(testCaseName); + SquishMessages::criticalMessage(Tr::tr("Test Suite Path Not Accessible"), detail); return; } @@ -350,11 +348,9 @@ void SquishFileHandler::runTestSuite(const QString &suiteName) const Utils::FilePath suiteConf = m_suites.value(suiteName); const Utils::FilePath suitePath = suiteConf.parentDir(); if (!suitePath.exists() || !suitePath.isReadableDir()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Test Suite Path Not Accessible"), - Tr::tr("The path \"%1\" does not exist or is not accessible.\n" - "Refusing to run test cases.") - .arg(suitePath.toUserOutput())); + const QString detail = Tr::tr("The path \"%1\" does not exist or is not accessible.\n" + "Refusing to run test cases.").arg(suitePath.toUserOutput()); + SquishMessages::criticalMessage(Tr::tr("Test Suite Path Not Accessible"), detail); return; } @@ -379,12 +375,10 @@ void SquishFileHandler::recordTestCase(const QString &suiteName, const QString & const Utils::FilePath suitePath = m_suites.value(suiteName).parentDir(); if (!suitePath.exists() || !suitePath.isReadableDir()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Test Suite Path Not Accessible"), - Tr::tr("The path \"%1\" does not exist or is not accessible.\n" - "Refusing to record test case \"%2\".") - .arg(suitePath.toUserOutput()) - .arg(testCaseName)); + const QString detail = Tr::tr("The path \"%1\" does not exist or is not accessible.\n" + "Refusing to record test case \"%2\".") + .arg(suitePath.toUserOutput()).arg(testCaseName); + SquishMessages::criticalMessage(Tr::tr("Test Suite Path Not Accessible"), detail); return; } @@ -491,10 +485,8 @@ void SquishFileHandler::openObjectsMap(const QString &suiteName) QTC_ASSERT(conf.ensureObjectMapExists(), return); if (!Core::EditorManager::openEditor(objectsMapPath, Constants::OBJECTSMAP_EDITOR_ID)) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Failed to open objects.map file at \"%1\".") - .arg(objectsMapPath.toUserOutput())); + SquishMessages::criticalMessage(Tr::tr("Failed to open objects.map file at \"%1\".") + .arg(objectsMapPath.toUserOutput())); } } diff --git a/src/plugins/squish/squishmessages.cpp b/src/plugins/squish/squishmessages.cpp new file mode 100644 index 00000000000..ea98f629ac5 --- /dev/null +++ b/src/plugins/squish/squishmessages.cpp @@ -0,0 +1,35 @@ +// 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 "squishmessages.h" + +#include "squishtr.h" + +#include + +namespace Squish::Internal::SquishMessages { + +void criticalMessage(const QString &title, const QString &details) +{ + QMessageBox::critical(Core::ICore::dialogParent(), title, details); +} + +void criticalMessage(const QString &details) +{ + criticalMessage(Tr::tr("Error"), details); +} + +void toolsInUnexpectedState(int state, const QString &additionalDetails) +{ + QString details = Tr::tr("Squish Tools in unexpected state (%1).").arg(state); + if (!additionalDetails.isEmpty()) + details.append('\n').append(additionalDetails); + criticalMessage(details); +} + +QMessageBox::StandardButton simpleQuestion(const QString &title, const QString &detail) +{ + return QMessageBox::question(Core::ICore::dialogParent(), title, detail); +} + +} // Squish::Internal::SquishMessages diff --git a/src/plugins/squish/squishmessages.h b/src/plugins/squish/squishmessages.h new file mode 100644 index 00000000000..941949cbd56 --- /dev/null +++ b/src/plugins/squish/squishmessages.h @@ -0,0 +1,15 @@ +// 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 + +namespace Squish::Internal::SquishMessages { + +void criticalMessage(const QString &title, const QString &details); +void criticalMessage(const QString &details); +void toolsInUnexpectedState(int state, const QString &additionalDetails); +QMessageBox::StandardButton simpleQuestion(const QString &title, const QString &detail); + +} // Squish::Internal::SquishMessages diff --git a/src/plugins/squish/squishnavigationwidget.cpp b/src/plugins/squish/squishnavigationwidget.cpp index 753fffba09f..be9a1ccb99c 100644 --- a/src/plugins/squish/squishnavigationwidget.cpp +++ b/src/plugins/squish/squishnavigationwidget.cpp @@ -5,6 +5,7 @@ #include "squishconstants.h" #include "squishfilehandler.h" +#include "squishmessages.h" #include "squishplugin.h" #include "squishsettings.h" #include "squishtesttreemodel.h" @@ -22,7 +23,6 @@ #include #include -#include #include namespace Squish { @@ -185,10 +185,9 @@ void SquishNavigationWidget::contextMenuEvent(QContextMenuEvent *event) menu.addAction(closeAllSuites); connect(closeAllSuites, &QAction::triggered, this, [this] { - if (QMessageBox::question(this, - Tr::tr("Close All Test Suites"), - Tr::tr("Close all test suites?" - /*"\nThis will close all related files as well."*/)) + if (SquishMessages::simpleQuestion(Tr::tr("Close All Test Suites"), + Tr::tr("Close all test suites?" + /*"\nThis will close all related files as well."*/)) == QMessageBox::Yes) SquishFileHandler::instance()->closeAllTestSuites(); }); @@ -273,13 +272,10 @@ void SquishNavigationWidget::onRemoveSharedFolderTriggered(int row, const QModel const auto folder = Utils::FilePath::fromVariant(m_sortModel->index(row, 0, parent).data(LinkRole)); QTC_ASSERT(!folder.isEmpty(), return ); - if (QMessageBox::question(Core::ICore::dialogParent(), - Tr::tr("Remove Shared Folder"), - Tr::tr("Remove \"%1\" from the list of shared folders?") - .arg(folder.toUserOutput())) - != QMessageBox::Yes) { + const QString detail = Tr::tr("Remove \"%1\" from the list of shared folders?") + .arg(folder.toUserOutput()); + if (SquishMessages::simpleQuestion(Tr::tr("Remove Shared Folder"), detail) != QMessageBox::Yes) return; - } const QModelIndex &realIdx = m_sortModel->mapToSource(m_sortModel->index(row, 0, parent)); if (SquishFileHandler::instance()->removeSharedFolder(folder)) @@ -288,10 +284,8 @@ void SquishNavigationWidget::onRemoveSharedFolderTriggered(int row, const QModel void SquishNavigationWidget::onRemoveAllSharedFolderTriggered() { - if (QMessageBox::question(Core::ICore::dialogParent(), - Tr::tr("Remove All Shared Folders"), - Tr::tr("Remove all shared folders?")) - != QMessageBox::Yes) { + if (SquishMessages::simpleQuestion(Tr::tr("Remove All Shared Folders"), + Tr::tr("Remove all shared folders?")) != QMessageBox::Yes) { return; } @@ -320,10 +314,8 @@ void SquishNavigationWidget::onNewTestCaseTriggered(const QModelIndex &index) QTC_ASSERT(settings, return); if (!settings->squishPath.filePath().pathAppended("scriptmodules").exists()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Set up a valid Squish path to be able to create " - "a new test case.\n(Edit > Preferences > Squish)")); + SquishMessages::criticalMessage(Tr::tr("Set up a valid Squish path to be able to create " + "a new test case.\n(Edit > Preferences > Squish)")); return; } diff --git a/src/plugins/squish/squishsettings.cpp b/src/plugins/squish/squishsettings.cpp index 0a9e9503c11..144e3046083 100644 --- a/src/plugins/squish/squishsettings.cpp +++ b/src/plugins/squish/squishsettings.cpp @@ -4,6 +4,7 @@ #include "squishsettings.h" #include "squishconstants.h" +#include "squishmessages.h" #include "squishtools.h" #include "squishtr.h" @@ -20,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -691,10 +691,9 @@ SquishServerSettingsDialog::SquishServerSettingsDialog(QWidget *parent) void SquishServerSettingsDialog::configWriteFailed(QProcess::ProcessError error) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Failed to write configuration changes.\n" - "Squish server finished with process error %1.").arg(error)); + const QString detail = Tr::tr("Failed to write configuration changes.\n" + "Squish server finished with process error %1.").arg(error); + SquishMessages::criticalMessage(detail); } } // namespace Internal diff --git a/src/plugins/squish/squishtools.cpp b/src/plugins/squish/squishtools.cpp index 18cb12bc025..5aa6bc8bc36 100644 --- a/src/plugins/squish/squishtools.cpp +++ b/src/plugins/squish/squishtools.cpp @@ -4,6 +4,7 @@ #include "squishtools.h" #include "scripthelper.h" +#include "squishmessages.h" #include "squishoutputpane.h" #include "squishplugin.h" #include "squishsettings.h" @@ -31,7 +32,6 @@ #include #include #include -#include #include #include @@ -198,18 +198,13 @@ void SquishTools::runTestCases(const FilePath &suitePath, if (m_shutdownInitiated) return; if (m_state != Idle) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Squish Tools in unexpected state (%1).\n" - "Refusing to run a test case.") - .arg(m_state)); + SquishMessages::toolsInUnexpectedState(m_state, Tr::tr("Refusing to run a test case.")); return; } // create test results directory (if necessary) and return on fail if (!resultsDirectory.ensureWritableDir()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Could not create test results folder. Canceling test run.")); + SquishMessages::criticalMessage( + Tr::tr("Could not create test results folder. Canceling test run.")); return; } @@ -264,11 +259,7 @@ void SquishTools::queryServer(RunnerQuery query) return; if (m_state != Idle) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Squish Tools in unexpected state (%1).\n" - "Refusing to execute server query.") - .arg(m_state)); + SquishMessages::toolsInUnexpectedState(m_state, Tr::tr("Refusing to execute server query.")); return; } m_perspective.setPerspectiveMode(SquishPerspective::Querying); @@ -283,11 +274,7 @@ void SquishTools::recordTestCase(const FilePath &suitePath, const QString &testC if (m_shutdownInitiated) return; if (m_state != Idle) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Squish Tools in unexpected state (%1).\n" - "Refusing to record a test case.") - .arg(m_state)); + SquishMessages::toolsInUnexpectedState(m_state, Tr::tr("Refusing to record a test case.")); return; } @@ -306,10 +293,7 @@ void SquishTools::writeServerSettingsChanges(const QList &changes) if (m_shutdownInitiated) return; if (m_state != Idle) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Squish Tools in unexpected state (%1).\n" - "Refusing to write configuration changes.").arg(m_state)); + SquishMessages::toolsInUnexpectedState(m_state, Tr::tr("Refusing to write configuration changes.")); return; } m_serverConfigChanges = changes; @@ -435,9 +419,8 @@ void SquishTools::onRunnerStateChanged(SquishProcessState state) break; case StartFailed: logAndChangeToolsState(SquishTools::RunnerStartFailed); - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Squish Runner Error"), - Tr::tr("Squish runner failed to start within given timeframe.")); + SquishMessages::criticalMessage(Tr::tr("Squish Runner Error"), + Tr::tr("Squish runner failed to start within given timeframe.")); onRunnerStopped(); break; case Stopped: @@ -473,7 +456,7 @@ void SquishTools::onRunnerStopped() m_suitePath.fileName(), &error); if (!error.isEmpty()) - QMessageBox::critical(Core::ICore::dialogParent(), Tr::tr("Error"), error); + SquishMessages::criticalMessage(error); logrotateTestResults(); } else { m_xmlOutputHandler->clearForNextRun(); @@ -494,12 +477,10 @@ void SquishTools::onRunnerError(SquishRunnerProcess::RunnerError error) } break; case SquishRunnerProcess::MappedAutMissing: - QMessageBox::critical(Core::ICore::dialogParent(), Tr::tr("Error"), - Tr::tr("Squish could not find the AUT \"%1\" to start. " - "Make sure it has been added as a Mapped AUT in the " - "squishserver settings.\n" - "(Tools > Squish > Server Settings...)") - .arg(m_suiteConf.aut())); + SquishMessages::criticalMessage( + Tr::tr("Squish could not find the AUT \"%1\" to start. Make sure it has been " + "added as a Mapped AUT in the squishserver settings.\n" + "(Tools > Squish > Server Settings...)").arg(m_suiteConf.aut())); break; } } @@ -534,11 +515,9 @@ void SquishTools::startSquishServer(Request request) const FilePath squishServer = Environment::systemEnvironment().searchInPath( toolsSettings.serverPath.toString()); if (!squishServer.isExecutableFile()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Squish Server Error"), - Tr::tr("\"%1\" could not be found or is not executable.\n" - "Check the settings.") - .arg(toolsSettings.serverPath.toUserOutput())); + const QString detail = Tr::tr("\"%1\" could not be found or is not executable.\nCheck the " + "settings.").arg(toolsSettings.serverPath.toUserOutput()); + SquishMessages::criticalMessage(Tr::tr("Squish Server Error"), detail); setIdle(); return; } @@ -1070,33 +1049,29 @@ void SquishTools::terminateRunner() void SquishTools::handleSquishServerAlreadyRunning() { - if (QMessageBox::question(Core::ICore::dialogParent(), - Tr::tr("Squish Server Already Running"), - Tr::tr("There is still an old Squish server instance running.\n" - "This will cause problems later on.\n\n" - "If you continue, the old instance will be terminated.\n" - "Do you want to continue?")) - == QMessageBox::Yes) { - switch (m_request) { - case RunTestRequested: - m_request = KillOldBeforeRunRunner; - break; - case RecordTestRequested: - m_request = KillOldBeforeRecordRunner; - break; - case RunnerQueryRequested: - m_request = KillOldBeforeQueryRunner; - break; - default: - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Error"), - Tr::tr("Unexpected state or request while starting Squish " - "server. (state: %1, request: %2)") - .arg(m_state) - .arg(m_request)); - } - stopSquishServer(); + const QString detail = Tr::tr("There is still an old Squish server instance running.\n" + "This will cause problems later on.\n\n" + "If you continue, the old instance will be terminated.\n" + "Do you want to continue?"); + if (SquishMessages::simpleQuestion(Tr::tr("Squish Server Already Running"), detail) != QMessageBox::Yes) + return; + + switch (m_request) { + case RunTestRequested: + m_request = KillOldBeforeRunRunner; + break; + case RecordTestRequested: + m_request = KillOldBeforeRecordRunner; + break; + case RunnerQueryRequested: + m_request = KillOldBeforeQueryRunner; + break; + default: + const QString detail = Tr::tr("Unexpected state or request while starting Squish server. " + "(state: %1, request: %2)").arg(m_state).arg(m_request); + SquishMessages::criticalMessage(detail); } + stopSquishServer(); } QStringList SquishTools::serverArgumentsFromSettings() const @@ -1165,37 +1140,26 @@ QStringList SquishTools::runnerArgumentsFromSettings() bool SquishTools::isValidToStartRunner() { if (!m_serverProcess.isRunning()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("No Squish Server"), - Tr::tr("Squish server does not seem to be running.\n" - "(state: %1, request: %2)\n" - "Try again.") - .arg(m_state) - .arg(m_request)); + const QString detail = Tr::tr("Squish server does not seem to be running.\n(state: %1, " + "request: %2)\nTry again.").arg(m_state).arg(m_request); + SquishMessages::criticalMessage(Tr::tr("No Squish Server"), detail); setIdle(); return false; } 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" - "(state: %1, request: %2)\n" - "Try again.") - .arg(m_state) - .arg(m_request)); + const QString detail = Tr::tr("Failed to get the server port.\n(state: %1, request: %2)\n" + "Try again.").arg(m_state).arg(m_request); + SquishMessages::criticalMessage(Tr::tr("No Squish Server Port"), detail); // setting state to ServerStartFailed will terminate/kill the current unusable server onServerStateChanged(StartFailed); return false; } if (m_primaryRunner && m_primaryRunner->state() != QProcess::NotRunning) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Squish Runner Running"), - Tr::tr("Squish runner seems to be running already.\n" - "(state: %1, request: %2)\n" - "Wait until it has finished and try again.") - .arg(m_state) - .arg(m_request)); + const QString detail = Tr::tr("Squish runner seems to be running already.\n(state: %1, " + "request: %2)\nWait until it has finished and try again.") + .arg(m_state).arg(m_request); + SquishMessages::criticalMessage(Tr::tr("Squish Runner Running"), detail); return false; } return true; @@ -1206,11 +1170,9 @@ bool SquishTools::setupRunnerPath() const FilePath squishRunner = Environment::systemEnvironment().searchInPath( toolsSettings.runnerPath.toString()); if (!squishRunner.isExecutableFile()) { - QMessageBox::critical(Core::ICore::dialogParent(), - Tr::tr("Squish Runner Error"), - Tr::tr("\"%1\" could not be found or is not executable.\n" - "Check the settings.") - .arg(toolsSettings.runnerPath.toUserOutput())); + const QString detail = Tr::tr("\"%1\" could not be found or is not executable.\nCheck the " + "settings.").arg(toolsSettings.runnerPath.toUserOutput()); + SquishMessages::criticalMessage(Tr::tr("Squish Runner Error"), detail); onRunnerStateChanged(Stopped); return false; }