Autotest: Slim down TestFrameworkManager interface

- sort frameworks at insertion time
- apply the 'static' pattern
- inline code from convenience functions that are only used once on
  the caller side

Change-Id: Ic4a46ad9487c58b4dfd61658a6fccebb88a428a9
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/autotest/testframeworkmanager.cpp

@@ -27,21 +27,13 @@
 #include "autotestconstants.h"
 #include "autotestplugin.h"
 #include "iframeworksettings.h"
-#include "itestparser.h"
 #include "testsettings.h"
-#include "testtreeitem.h"
-#include "testtreemodel.h"
-
-#include <coreplugin/dialogs/ioptionspage.h>
 
 #include <utils/algorithm.h>
 #include <utils/qtcassert.h>
 
-#include <QLoggingCategory>
 #include <QSettings>
 
-static Q_LOGGING_CATEGORY(LOG, "qtc.autotest.frameworkmanager", QtWarningMsg)
-
 using namespace Core;
 
 namespace Autotest {
@@ -53,11 +45,6 @@ TestFrameworkManager::TestFrameworkManager()
     s_instance = this;
 }
 
-TestFrameworkManager *TestFrameworkManager::instance()
-{
-    return s_instance;
-}
-
 TestFrameworkManager::~TestFrameworkManager()
 {
     qDeleteAll(m_registeredFrameworks);
@@ -70,46 +57,21 @@ bool TestFrameworkManager::registerTestFramework(ITestFramework *framework)
     QTC_ASSERT(!m_registeredFrameworks.contains(framework), return false);
     // TODO check for unique priority before registering
     m_registeredFrameworks.append(framework);
+    Utils::sort(m_registeredFrameworks, &ITestFramework::priority);
     return true;
 }
 
 void TestFrameworkManager::activateFrameworksFromSettings(const Internal::TestSettings *settings)
 {
-    for (ITestFramework *framework : qAsConst(m_registeredFrameworks)) {
+    for (ITestFramework *framework : qAsConst(s_instance->m_registeredFrameworks)) {
         framework->setActive(settings->frameworks.value(framework->id(), false));
         framework->setGrouping(settings->frameworksGrouping.value(framework->id(), false));
     }
 }
 
-TestFrameworks TestFrameworkManager::registeredFrameworks() const
+TestFrameworks TestFrameworkManager::registeredFrameworks()
 {
-    return m_registeredFrameworks;
+    return s_instance->m_registeredFrameworks;
-}
-
-TestFrameworks TestFrameworkManager::sortedRegisteredFrameworks() const
-{
-    TestFrameworks registered = m_registeredFrameworks;
-    Utils::sort(registered, &ITestFramework::priority);
-    qCDebug(LOG) << "Registered frameworks sorted by priority" << registered;
-    return registered;
-}
-
-TestFrameworks TestFrameworkManager::activeFrameworks() const
-{
-    TestFrameworks active;
-    for (ITestFramework *framework : qAsConst(m_registeredFrameworks)) {
-        if (framework->active())
-            active.append(framework);
-    }
-    return active;
-}
-
-TestFrameworks TestFrameworkManager::sortedActiveFrameworks() const
-{
-    TestFrameworks active = activeFrameworks();
-    Utils::sort(active, &ITestFramework::priority);
-    qCDebug(LOG) << "Active frameworks sorted by priority" << active;
-    return active;
 }
 
 ITestFramework *TestFrameworkManager::frameworkForId(Id frameworkId)
@@ -129,11 +91,6 @@ void TestFrameworkManager::synchronizeSettings(QSettings *s)
     }
 }
 
-bool TestFrameworkManager::hasActiveFrameworks() const
-{
-    return Utils::anyOf(m_registeredFrameworks, &ITestFramework::active);
-}
-
 Id ITestFramework::settingsId() const
 {
     return Core::Id(Constants::SETTINGSPAGE_PREFIX)

src/plugins/autotest/testframeworkmanager.h

@@ -27,8 +27,6 @@
 
 #include "itestframework.h"
 
-#include <QHash>
-
 QT_BEGIN_NAMESPACE
 class QSettings;
 QT_END_NAMESPACE
@@ -40,9 +38,6 @@ namespace Internal {
 struct TestSettings;
 }
 
-class IFrameworkSettings;
-class ITestParser;
-
 class TestFrameworkManager final
 {
 
@@ -50,23 +45,14 @@ public:
     TestFrameworkManager();
     ~TestFrameworkManager();
 
-    static TestFrameworkManager *instance();
+    bool registerTestFramework(ITestFramework *framework);
+    void synchronizeSettings(QSettings *s);
 
     static ITestFramework *frameworkForId(Core::Id frameworkId);
-
+    static void activateFrameworksFromSettings(const Internal::TestSettings *settings);
-    bool registerTestFramework(ITestFramework *framework);
+    static TestFrameworks registeredFrameworks();
-
-    void activateFrameworksFromSettings(const Internal::TestSettings *settings);
-    TestFrameworks registeredFrameworks() const;
-    TestFrameworks sortedRegisteredFrameworks() const;
-    TestFrameworks sortedActiveFrameworks() const;
-
-    void synchronizeSettings(QSettings *s);
-    bool hasActiveFrameworks() const;
 
 private:
-    TestFrameworks activeFrameworks() const;
-
     TestFrameworks m_registeredFrameworks;
 };
 

src/plugins/autotest/testnavigationwidget.cpp

@@ -76,7 +76,9 @@ TestNavigationWidget::TestNavigationWidget(QWidget *parent) :
     QHBoxLayout *hLayout = new QHBoxLayout;
     m_missingFrameworksWidget->setLayout(hLayout);
     hLayout->addWidget(new QLabel(tr("No active test frameworks.")));
-    m_missingFrameworksWidget->setVisible(!TestFrameworkManager::instance()->hasActiveFrameworks());
+    const bool hasActiveFrameworks = Utils::anyOf(TestFrameworkManager::registeredFrameworks(),
+                                                  &ITestFramework::active);
+    m_missingFrameworksWidget->setVisible(!hasActiveFrameworks);
     QVBoxLayout *layout = new QVBoxLayout;
     layout->setContentsMargins(0, 0, 0, 0);
     layout->setSpacing(0);

src/plugins/autotest/testprojectsettings.cpp

@@ -36,6 +36,8 @@ namespace Internal {
 static const char SK_ACTIVE_FRAMEWORKS[]        = "AutoTest.ActiveFrameworks";
 static const char SK_RUN_AFTER_BUILD[]          = "AutoTest.RunAfterBuild";
 
+static Q_LOGGING_CATEGORY(LOG, "qtc.autotest.frameworkmanager", QtWarningMsg)
+
 TestProjectSettings::TestProjectSettings(ProjectExplorer::Project *project)
     : m_project(project)
 {
@@ -60,7 +62,7 @@ void TestProjectSettings::setUseGlobalSettings(bool useGlobal)
 
 void TestProjectSettings::activateFramework(const Core::Id &id, bool activate)
 {
-    ITestFramework *framework = TestFrameworkManager::instance()->frameworkForId(id);
+    ITestFramework *framework = TestFrameworkManager::frameworkForId(id);
     m_activeTestFrameworks[framework] = activate;
     if (!activate)
         framework->resetRootNode();
@@ -71,8 +73,8 @@ void TestProjectSettings::load()
     const QVariant useGlobal = m_project->namedSettings(Constants::SK_USE_GLOBAL);
     m_useGlobalSettings = useGlobal.isValid() ? useGlobal.toBool() : true;
 
-    TestFrameworkManager *frameworkManager = TestFrameworkManager::instance();
+    const TestFrameworks registered = TestFrameworkManager::registeredFrameworks();
-    const TestFrameworks registered = frameworkManager->sortedRegisteredFrameworks();
+    qCDebug(LOG) << "Registered frameworks sorted by priority" << registered;
     const QVariant activeFrameworks = m_project->namedSettings(SK_ACTIVE_FRAMEWORKS);
 
     m_activeTestFrameworks.clear();

src/plugins/autotest/testsettings.cpp

@@ -92,8 +92,7 @@ void TestSettings::fromSettings(QSettings *s)
     runAfterBuild = RunAfterBuildMode(s->value(runAfterBuildKey,
                                                int(RunAfterBuildMode::None)).toInt());
     // try to get settings for registered frameworks
-    TestFrameworkManager *frameworkManager = TestFrameworkManager::instance();
+    const TestFrameworks &registered = TestFrameworkManager::registeredFrameworks();
-    const TestFrameworks &registered = frameworkManager->registeredFrameworks();
     frameworks.clear();
     frameworksGrouping.clear();
     for (const ITestFramework *framework : registered) {

src/plugins/autotest/testsettingspage.cpp

@@ -93,8 +93,7 @@ TestSettings TestSettingsWidget::settings() const
 
 void TestSettingsWidget::populateFrameworksListWidget(const QHash<Core::Id, bool> &frameworks)
 {
-    TestFrameworkManager *frameworkManager = TestFrameworkManager::instance();
+    const TestFrameworks &registered = TestFrameworkManager::registeredFrameworks();
-    const TestFrameworks &registered = frameworkManager->sortedRegisteredFrameworks();
     m_ui.frameworkTreeWidget->clear();
     for (const ITestFramework *framework : registered) {
         const Core::Id id = framework->id();
@@ -169,8 +168,12 @@ void TestSettingsPage::apply()
     });
     *m_settings = newSettings;
     m_settings->toSettings(Core::ICore::settings());
-    TestFrameworkManager *frameworkManager = TestFrameworkManager::instance();
+
-    frameworkManager->activateFrameworksFromSettings(m_settings);
+    for (ITestFramework *framework : TestFrameworkManager::registeredFrameworks()) {
+        framework->setActive(m_settings->frameworks.value(framework->id(), false));
+        framework->setGrouping(m_settings->frameworksGrouping.value(framework->id(), false));
+    }
+
     TestTreeModel::instance()->synchronizeTestFrameworks();
     if (!changedIds.isEmpty())
         TestTreeModel::instance()->rebuild(changedIds);

src/plugins/autotest/testtreemodel.cpp

@@ -44,6 +44,8 @@ using namespace Autotest::Internal;
 
 namespace Autotest {
 
+static Q_LOGGING_CATEGORY(LOG, "qtc.autotest.frameworkmanager", QtWarningMsg)
+
 static TestTreeModel *s_instance = nullptr;
 
 TestTreeModel::TestTreeModel(TestCodeParser *parser) :
@@ -211,11 +213,12 @@ void TestTreeModel::synchronizeTestFrameworks()
 {
     ProjectExplorer::Project *project = ProjectExplorer::SessionManager::startupProject();
     TestFrameworks sorted;
-    TestFrameworkManager *manager = TestFrameworkManager::instance();
     const QVariant useGlobal = project ? project->namedSettings(Constants::SK_USE_GLOBAL)
                                        : QVariant();
     if (!useGlobal.isValid() || AutotestPlugin::projectSettings(project)->useGlobalSettings()) {
-        sorted = manager->sortedActiveFrameworks();
+        sorted = Utils::filtered(TestFrameworkManager::registeredFrameworks(),
+                                 &ITestFramework::active);
+        qCDebug(LOG) << "Active frameworks sorted by priority" << sorted;
     } else { // we've got custom project settings
         const TestProjectSettings *settings = AutotestPlugin::projectSettings(project);
         const QMap<ITestFramework *, bool> active = settings->activeFrameworks();