Fix that plugins were wrongly indirectly enabled when testing

Since the disabling of all plugins except tested onces was implemented
as an afterthought, it did not update the indirectly enabled plugins.
Instead, update the list of enabled/disabled plugins in the
optionsparser like for the -(no)load options, and trigger the update of
indirectly enabled plugins afterwards. Also take test dependencies into
account when indirectly enabling plugins directly.

Change-Id: I59d6c05de69a3073576155f7bd6201f1cd44697c
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

6 files changed, 37 insertions, 39 deletions

diff --git a/src/libs/extensionsystem/optionsparser.cpp b/src/libs/extensionsystem/optionsparser.cpp
index 852df2863d..babb058713 100644
--- a/src/libs/extensionsystem/optionsparser.cpp
+++ b/src/libs/extensionsystem/optionsparser.cpp
@@ -91,9 +91,12 @@ bool OptionsParser::parse()
         // probably a file or something
         m_pmPrivate->arguments << m_currentArg;
     }
+    if (PluginManager::testRunRequested()) {
+        m_isDependencyRefreshNeeded = true;
+        forceDisableAllPluginsExceptTestedAndForceEnabled();
+    }
     if (m_isDependencyRefreshNeeded)
-        m_pmPrivate->resolveDependencies();
-    m_pmPrivate->enableOnlyTestedSpecs();
+        m_pmPrivate->enableDependenciesIndirectly();
     return !m_hasError;
 }
 
@@ -260,6 +263,16 @@ bool OptionsParser::checkForUnknownOption()
     return true;
 }
 
+void OptionsParser::forceDisableAllPluginsExceptTestedAndForceEnabled()
+{
+    for (const PluginManagerPrivate::TestSpec &testSpec : m_pmPrivate->testSpecs)
+        testSpec.pluginSpec->d->setForceEnabled(true);
+    for (PluginSpec *spec : m_pmPrivate->pluginSpecs) {
+        if (!spec->isForceEnabled())
+            spec->d->setForceDisabled(true);
+    }
+}
+
 bool OptionsParser::nextToken(OptionsParser::TokenType type)
 {
     if (m_it == m_end) {
diff --git a/src/libs/extensionsystem/optionsparser.h b/src/libs/extensionsystem/optionsparser.h
index 7b85d9e62a..a33486e951 100644
--- a/src/libs/extensionsystem/optionsparser.h
+++ b/src/libs/extensionsystem/optionsparser.h
@@ -60,6 +60,7 @@ private:
     bool checkForPluginOption();
     bool checkForProfilingOption();
     bool checkForUnknownOption();
+    void forceDisableAllPluginsExceptTestedAndForceEnabled();
 
     enum TokenType { OptionalToken, RequiredToken };
     bool nextToken(TokenType type = OptionalToken);
diff --git a/src/libs/extensionsystem/pluginmanager.cpp b/src/libs/extensionsystem/pluginmanager.cpp
index ad1109ff0f..5bd975ceac 100644
--- a/src/libs/extensionsystem/pluginmanager.cpp
+++ b/src/libs/extensionsystem/pluginmanager.cpp
@@ -1488,6 +1488,7 @@ void PluginManagerPrivate::readPluginPaths()
         pluginSpecs.append(spec);
     }
     resolveDependencies();
+    enableDependenciesIndirectly();
     // ensure deterministic plugin load order by sorting
     Utils::sort(pluginSpecs, &PluginSpec::name);
     emit q->pluginsChanged();
@@ -1495,42 +1496,19 @@ void PluginManagerPrivate::readPluginPaths()
 
 void PluginManagerPrivate::resolveDependencies()
 {
-    foreach (PluginSpec *spec, pluginSpecs) {
-        spec->d->enabledIndirectly = false; // reset, is recalculated below
+    foreach (PluginSpec *spec, pluginSpecs)
         spec->d->resolveDependencies(pluginSpecs);
-    }
-
-    Utils::reverseForeach(loadQueue(), [](PluginSpec *spec) {
-        spec->d->enableDependenciesIndirectly();
-    });
 }
 
-void PluginManagerPrivate::enableOnlyTestedSpecs()
+void PluginManagerPrivate::enableDependenciesIndirectly()
 {
-    if (testSpecs.isEmpty())
-        return;
-
-    QList<PluginSpec *> specsForTests;
-    foreach (const TestSpec &testSpec, testSpecs) {
-        QList<PluginSpec *> circularityCheckQueue;
-        loadQueue(testSpec.pluginSpec, specsForTests, circularityCheckQueue);
-        // add plugins that must be force loaded when running tests for the plugin
-        // (aka "test dependencies")
-        QHashIterator<PluginDependency, PluginSpec *> it(testSpec.pluginSpec->dependencySpecs());
-        while (it.hasNext()) {
-            it.next();
-            if (it.key().type != PluginDependency::Test)
-                continue;
-            PluginSpec *depSpec = it.value();
-            circularityCheckQueue.clear();
-            loadQueue(depSpec, specsForTests, circularityCheckQueue);
-        }
-    }
     foreach (PluginSpec *spec, pluginSpecs)
-        spec->d->setForceDisabled(true);
-    foreach (PluginSpec *spec, specsForTests) {
-        spec->d->setForceDisabled(false);
-        spec->d->setForceEnabled(true);
+        spec->d->enabledIndirectly = false;
+    // cannot use reverse loadQueue here, because test dependencies can introduce circles
+    QList<PluginSpec *> queue = Utils::filtered(pluginSpecs, &PluginSpec::isEffectivelyEnabled);
+    while (!queue.isEmpty()) {
+        PluginSpec *spec = queue.takeFirst();
+        queue += spec->d->enableDependenciesIndirectly(containsTestSpec(spec));
     }
 }
 
diff --git a/src/libs/extensionsystem/pluginmanager_p.h b/src/libs/extensionsystem/pluginmanager_p.h
index f6b1bced36..6b7a2cc75b 100644
--- a/src/libs/extensionsystem/pluginmanager_p.h
+++ b/src/libs/extensionsystem/pluginmanager_p.h
@@ -68,7 +68,7 @@ public:
     QList<PluginSpec *> loadQueue();
     void loadPlugin(PluginSpec *spec, PluginSpec::State destState);
     void resolveDependencies();
-    void enableOnlyTestedSpecs();
+    void enableDependenciesIndirectly();
     void initProfiling();
     void profilingSummary() const;
     void profilingReport(const char *what, const PluginSpec *spec = 0);
diff --git a/src/libs/extensionsystem/pluginspec.cpp b/src/libs/extensionsystem/pluginspec.cpp
index 8fd64007a2..98f1be8892 100644
--- a/src/libs/extensionsystem/pluginspec.cpp
+++ b/src/libs/extensionsystem/pluginspec.cpp
@@ -917,19 +917,25 @@ bool PluginSpecPrivate::resolveDependencies(const QList<PluginSpec *> &specs)
     return true;
 }
 
-void PluginSpecPrivate::enableDependenciesIndirectly()
+// returns the plugins that it actually indirectly enabled
+QList<PluginSpec *> PluginSpecPrivate::enableDependenciesIndirectly(bool enableTestDependencies)
 {
     if (!q->isEffectivelyEnabled()) // plugin not enabled, nothing to do
-        return;
+        return {};
+    QList<PluginSpec *> enabled;
     QHashIterator<PluginDependency, PluginSpec *> it(dependencySpecs);
     while (it.hasNext()) {
         it.next();
-        if (it.key().type != PluginDependency::Required)
+        if (it.key().type != PluginDependency::Required
+                && (!enableTestDependencies || it.key().type != PluginDependency::Test))
             continue;
         PluginSpec *dependencySpec = it.value();
-        if (!dependencySpec->isEffectivelyEnabled())
+        if (!dependencySpec->isEffectivelyEnabled()) {
             dependencySpec->d->enabledIndirectly = true;
+            enabled << dependencySpec;
+        }
     }
+    return enabled;
 }
 
 /*!
diff --git a/src/libs/extensionsystem/pluginspec_p.h b/src/libs/extensionsystem/pluginspec_p.h
index e0ac647d14..ee6944bf76 100644
--- a/src/libs/extensionsystem/pluginspec_p.h
+++ b/src/libs/extensionsystem/pluginspec_p.h
@@ -103,7 +103,7 @@ public:
     static bool isValidVersion(const QString &version);
     static int versionCompare(const QString &version1, const QString &version2);
 
-    void enableDependenciesIndirectly();
+    QList<PluginSpec *> enableDependenciesIndirectly(bool enableTestDependencies = false);
 
     bool readMetaData(const QJsonObject &pluginMetaData);