Remove common fixture and parameters from SortPlugins() tests

src/api/game/game.cpp

@@ -325,7 +325,41 @@ std::vector<const PluginInterface*> Game::GetLoadedPlugins() const {
 
 std::vector<std::string> Game::SortPlugins(
     const std::vector<std::string>& pluginFilenames) {
-  return loot::SortPlugins(*this, pluginFilenames);
+  std::vector<const Plugin*> plugins;
+  for (const auto& pluginFilename : pluginFilenames) {
+    const auto plugin = cache_.GetPlugin(pluginFilename);
+    if (plugin == nullptr) {
+      throw std::invalid_argument("The plugin \"" + pluginFilename +
+                                  "\" has not been loaded.");
+    }
+
+    plugins.push_back(plugin);
+  }
+
+  auto pluginsSortingData = GetPluginsSortingData(database_, plugins);
+
+  const auto logger = getLogger();
+  if (logger) {
+    logger->debug("Current load order:");
+    for (const auto& plugin : pluginFilenames) {
+      logger->debug("\t{}", plugin);
+    }
+  }
+
+  const auto newLoadOrder =
+      loot::SortPlugins(std::move(pluginsSortingData),
+                  database_.GetGroups(false),
+                  database_.GetUserGroups(),
+                  loadOrderHandler_.GetEarlyLoadingPlugins());
+
+  if (logger) {
+    logger->debug("Calculated order:");
+    for (const auto& name : newLoadOrder) {
+      logger->debug("\t{}", name);
+    }
+  }
+
+  return newLoadOrder;
 }
 
 void Game::LoadCurrentLoadOrderState() {

src/api/sorting/plugin_sort.cpp

@@ -369,44 +369,4 @@ std::vector<std::string> SortPlugins(
 
   return newMastersLoadOrder;
 }
-
-std::vector<std::string> SortPlugins(
-    const Game& game,
-    const std::vector<std::string>& loadOrder) {
-  std::vector<const Plugin*> plugins;
-  for (const auto& pluginFilename : loadOrder) {
-    const auto plugin = game.GetCache().GetPlugin(pluginFilename);
-    if (plugin == nullptr) {
-      throw std::invalid_argument("The plugin \"" + pluginFilename +
-                                  "\" has not been loaded.");
-    }
-
-    plugins.push_back(plugin);
-  }
-
-  auto pluginsSortingData = GetPluginsSortingData(game.GetDatabase(), plugins);
-
-  const auto logger = getLogger();
-  if (logger) {
-    logger->debug("Current load order:");
-    for (const auto& plugin : loadOrder) {
-      logger->debug("\t{}", plugin);
-    }
-  }
-
-  const auto newLoadOrder =
-      SortPlugins(std::move(pluginsSortingData),
-                  game.GetDatabase().GetGroups(false),
-                  game.GetDatabase().GetUserGroups(),
-                  game.GetLoadOrderHandler().GetEarlyLoadingPlugins());
-
-  if (logger) {
-    logger->debug("Calculated order:");
-    for (const auto& name : newLoadOrder) {
-      logger->debug("\t{}", name);
-    }
-  }
-
-  return newLoadOrder;
-}
 }

src/api/sorting/plugin_sort.h

@@ -32,14 +32,15 @@
 #include "api/sorting/plugin_sorting_data.h"
 
 namespace loot {
-std::vector<std::string> SortPlugins(
+std::vector<PluginSortingData> GetPluginsSortingData(
+    const DatabaseInterface& db,
+    const std::vector<const Plugin*>& loadOrder);
+
+    std::vector<std::string> SortPlugins(
     std::vector<PluginSortingData>&& pluginsSortingData,
     const std::vector<Group>& masterlistGroups,
     const std::vector<Group>& userGroups,
     const std::vector<std::string>& earlyLoadingPlugins);
-
-std::vector<std::string> SortPlugins(const Game& game,
-                                     const std::vector<std::string>& loadOrder);
 }
 
 #endif

src/tests/api/internals/game/game_test.h

@@ -231,6 +231,8 @@ TEST_P(GameTest,
   EXPECT_NO_THROW(loadInstalledPlugins(game, true));
   if (GetParam() == GameType::starfield) {
     EXPECT_EQ(10, game.GetCache().GetPlugins().size());
+  } else if (supportsLightPlugins(GetParam())) {
+    EXPECT_EQ(12, game.GetCache().GetPlugins().size());
   } else {
     EXPECT_EQ(11, game.GetCache().GetPlugins().size());
   }
@@ -279,6 +281,8 @@ TEST_P(GameTest,
   EXPECT_NO_THROW(loadInstalledPlugins(game, false));
   if (GetParam() == GameType::starfield) {
     EXPECT_EQ(10, game.GetCache().GetPlugins().size());
+  } else if (supportsLightPlugins(GetParam())) {
+    EXPECT_EQ(12, game.GetCache().GetPlugins().size());
   } else {
     EXPECT_EQ(11, game.GetCache().GetPlugins().size());
   }
@@ -441,6 +445,70 @@ TEST_P(
   }
 }
 
+TEST_P(GameTest, sortPluginsWithNoLoadedPluginsShouldReturnAnEmptyList) {
+  Game game = Game(GetParam(), gamePath, localPath);
+
+  const auto sorted = game.SortPlugins(game.GetLoadOrder());
+
+  EXPECT_TRUE(sorted.empty());
+}
+
+TEST_P(GameTest, sortPluginsShouldOnlySortTheGivenPlugins) {
+  Game game = Game(GetParam(), gamePath, localPath);
+  loadInstalledPlugins(game, false);
+
+  std::vector<std::string> plugins{blankEsp, blankDifferentEsp};
+  const auto sorted = game.SortPlugins(plugins);
+
+  EXPECT_EQ(plugins, sorted);
+}
+
+TEST_P(GameTest, sortingShouldNotMakeUnnecessaryChangesToAnExistingLoadOrder) {
+  Game game = Game(GetParam(), gamePath, localPath);
+  game.LoadCurrentLoadOrderState();
+
+  auto plugins = GetInstalledPlugins();
+  game.LoadPlugins({plugins.front()}, true);
+  plugins.erase(plugins.begin());
+  game.LoadPlugins(plugins, false);
+
+  std::vector<std::string> expectedSortedOrder;
+  if (GetParam() == GameType::openmw) {
+    // The existing load order for OpenMW doesn't have plugins loading after
+    // their masters, because the game doesn't enforce that, and the test
+    // setup cannot enforce the positions of inactive plugins.
+    expectedSortedOrder = {
+        blankDifferentEsm,
+        blankDifferentMasterDependentEsm,
+        blankDifferentEsp,
+        blankDifferentPluginDependentEsp,
+        blankEsm,
+        blankMasterDependentEsm,
+        blankMasterDependentEsp,
+        blankEsp,
+        blankPluginDependentEsp,
+        masterFile,
+        blankDifferentMasterDependentEsp,
+    };
+  } else {
+    expectedSortedOrder = getLoadOrder();
+  }
+
+  // Check stability by running the sort 100 times.
+  for (int i = 0; i < 100; i++) {
+    auto sorted = game.SortPlugins(game.GetLoadOrder());
+    ASSERT_EQ(expectedSortedOrder, sorted) << " for sort " << i;
+  }
+}
+
+TEST_P(GameTest, sortPluginsShouldThrowIfAGivenPluginIsNotLoaded) {
+  Game game = Game(GetParam(), gamePath, localPath);
+
+  std::vector<std::string> plugins{blankEsp, blankDifferentEsp};
+
+  EXPECT_THROW(game.SortPlugins(plugins), std::invalid_argument);
+}
+
 TEST_P(GameTest, clearLoadedPluginsShouldClearThePluginsCache) {
   Game game = Game(GetParam(), gamePath, localPath);
 
@@ -453,21 +521,21 @@ TEST_P(GameTest, clearLoadedPluginsShouldClearThePluginsCache) {
   EXPECT_EQ(nullptr, game.GetPlugin(blankEsm));
 }
 
-TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItHasNotBeenLoaded) {
+TEST_P(GameTest, isPluginActiveShouldActivePluginAsActiveEvenIfNotLoaded) {
   Game game = Game(GetParam(), gamePath, localPath);
   game.LoadCurrentLoadOrderState();
 
   EXPECT_TRUE(game.IsPluginActive(blankEsm));
 }
 
-TEST_P(GameTest, shouldShowBlankEspAsInactiveIfItHasNotBeenLoaded) {
+TEST_P(GameTest, isPluginActiveShouldInactivePluginAsInactiveEvenIfNotLoaded) {
   Game game = Game(GetParam(), gamePath, localPath);
   game.LoadCurrentLoadOrderState();
 
   EXPECT_FALSE(game.IsPluginActive(blankEsp));
 }
 
-TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItsHeaderHasBeenLoaded) {
+TEST_P(GameTest, isPluginActiveShouldActivePluginAsActiveWithHeaderLoaded) {
   Game game = Game(GetParam(), gamePath, localPath);
   game.LoadCurrentLoadOrderState();
 
@@ -476,7 +544,7 @@ TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItsHeaderHasBeenLoaded) {
   EXPECT_TRUE(game.IsPluginActive(blankEsm));
 }
 
-TEST_P(GameTest, shouldShowBlankEspAsInactiveIfItsHeaderHasBeenLoaded) {
+TEST_P(GameTest, isPluginActiveShouldInactivePluginAsInactiveWithHeaderLoaded) {
   Game game = Game(GetParam(), gamePath, localPath);
   game.LoadCurrentLoadOrderState();
 
@@ -485,7 +553,7 @@ TEST_P(GameTest, shouldShowBlankEspAsInactiveIfItsHeaderHasBeenLoaded) {
   EXPECT_FALSE(game.IsPluginActive(blankEsp));
 }
 
-TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItHasBeenFullyLoaded) {
+TEST_P(GameTest, isPluginActiveShouldActivePluginAsActiveWhenFullyLoaded) {
   Game game = Game(GetParam(), gamePath, localPath);
   game.LoadCurrentLoadOrderState();
 
@@ -494,7 +562,7 @@ TEST_P(GameTest, shouldShowBlankEsmAsActiveIfItHasBeenFullyLoaded) {
   EXPECT_TRUE(game.IsPluginActive(blankEsm));
 }
 
-TEST_P(GameTest, shouldShowBlankEspAsInactiveIfItHasBeenFullyLoaded) {
+TEST_P(GameTest, isPluginActiveShouldInactivePluginAsInactiveWhenFullyLoaded) {
   Game game = Game(GetParam(), gamePath, localPath);
   game.LoadCurrentLoadOrderState();
 

src/tests/api/internals/metadata/condition_evaluator_test.h

@@ -66,10 +66,6 @@ protected:
 
     plugins.push_back(std::filesystem::u8path(nonAsciiEsm));
 
-    if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se) {
-      plugins.push_back(blankEsl);
-    }
-
     game_.LoadCurrentLoadOrderState();
     game_.LoadPlugins(plugins, true);
   }

src/tests/common_game_test_fixture.h

@@ -344,6 +344,21 @@ protected:
           blankDifferentEsp,
           blankMasterDependentEsp,
       };
+    } else if (supportsLightPlugins(gameType_)) {
+      return {
+          masterFile,
+          blankEsm,
+          blankDifferentEsm,
+          blankMasterDependentEsm,
+          blankDifferentMasterDependentEsm,
+          blankEsl,
+          blankEsp,
+          blankDifferentEsp,
+          blankMasterDependentEsp,
+          blankDifferentMasterDependentEsp,
+          blankPluginDependentEsp,
+          blankDifferentPluginDependentEsp,
+      };
     } else {
       return {
           masterFile,