From 7e559239c1f4c0ba63d9ee22273921169a6916de Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 15 Jan 2016 20:41:30 +0000 Subject: [PATCH 1/3] Remove unnecessary include --- src/tests/gui/test_loot_settings.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tests/gui/test_loot_settings.h b/src/tests/gui/test_loot_settings.h index f96722ba..24bb3916 100644 --- a/src/tests/gui/test_loot_settings.h +++ b/src/tests/gui/test_loot_settings.h @@ -28,8 +28,6 @@ along with LOOT. If not, see #include "gui/loot_settings.h" #include "backend/globals.h" -#include "tests/fixtures.h" - namespace loot { namespace test { class LootSettings : public ::testing::Test { From 5e9566d8c0ba891fe0fee922ade207c7be7a7ee2 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 16 Jan 2016 09:36:07 +0000 Subject: [PATCH 2/3] Add tests for LootState's unapplied changes counter --- CMakeLists.txt | 4 +- src/gui/loot_state.cpp | 7 +-- src/tests/gui/test_loot_state.h | 77 +++++++++++++++++++++++++++++++++ src/tests/main.cpp | 1 + 4 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 src/tests/gui/test_loot_state.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 10fb2d9c..5529b25f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -165,6 +165,7 @@ set (LOOT_VALIDATOR_HEADERS ${LOOT_HEADERS}) set (LOOT_TESTS_SRC ${LOOT_SRC} "${CMAKE_SOURCE_DIR}/src/api/loot_db.cpp" "${CMAKE_SOURCE_DIR}/src/gui/loot_settings.cpp" + "${CMAKE_SOURCE_DIR}/src/gui/loot_state.cpp" "${CMAKE_SOURCE_DIR}/src/tests/main.cpp") set (LOOT_TESTS_HEADERS "${CMAKE_SOURCE_DIR}/src/tests/fixtures.h" @@ -193,7 +194,8 @@ set (LOOT_TESTS_HEADERS "${CMAKE_SOURCE_DIR}/src/tests/fixtures.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/test_metadata_list.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/test_masterlist.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/test_plugin_sorter.h" - "${CMAKE_SOURCE_DIR}/src/tests/gui/test_loot_settings.h") + "${CMAKE_SOURCE_DIR}/src/tests/gui/test_loot_settings.h" + "${CMAKE_SOURCE_DIR}/src/tests/gui/test_loot_state.h") source_group("Header Files" FILES ${LOOT_HEADERS} ${LOOT_GUI_HEADERS} ${LOOT_API_HEADERS} ${LOOT_TESTS_HEADERS}) diff --git a/src/gui/loot_state.cpp b/src/gui/loot_state.cpp index d2ac57fa..8f8ff875 100644 --- a/src/gui/loot_state.cpp +++ b/src/gui/loot_state.cpp @@ -239,7 +239,8 @@ namespace loot { } void LootState::decrementUnappliedChangeCounter() { - --unappliedChangeCounter; + if (unappliedChangeCounter > 0) + --unappliedChangeCounter; } void LootState::SelectGame(std::string preferredGame) { @@ -252,12 +253,12 @@ namespace loot { } // Get iterator to preferred game. - _currentGame = find_if(begin(_games), end(_games), [&](auto& game) { + _currentGame = find_if(begin(_games), end(_games), [&](Game& game) { return (preferredGame.empty() || preferredGame == game.FolderName()) && game.IsInstalled(); }); // If the preferred game cannot be found, get the first installed game. if (_currentGame == end(_games)) { - _currentGame = find_if(begin(_games), end(_games), [](auto& game) { + _currentGame = find_if(begin(_games), end(_games), [](Game& game) { return game.IsInstalled(); }); } diff --git a/src/tests/gui/test_loot_state.h b/src/tests/gui/test_loot_state.h new file mode 100644 index 00000000..6799cd56 --- /dev/null +++ b/src/tests/gui/test_loot_state.h @@ -0,0 +1,77 @@ +/* LOOT + +A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and +Fallout: New Vegas. + +Copyright (C) 2014-2015 WrinklyNinja + +This file is part of LOOT. + +LOOT is free software: you can redistribute +it and/or modify it under the terms of the GNU General Public License +as published by the Free Software Foundation, either version 3 of +the License, or (at your option) any later version. + +LOOT is distributed in the hope that it will +be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with LOOT. If not, see +. +*/ + +#ifndef LOOT_TEST_GUI_LOOT_STATE +#define LOOT_TEST_GUI_LOOT_STATE + +#include "gui/loot_state.h" + +#include "tests/fixtures.h" + +namespace loot { + namespace test { + class LootState : public ::testing::Test { + protected: + loot::LootState lootState; + }; + + TEST_F(LootState, hasUnappliedChangesShouldBeFalseByDefault) { + EXPECT_FALSE(lootState.hasUnappliedChanges()); + } + + TEST_F(LootState, shouldNotHaveUnappliedChangesIfCounterIsDeccremented) { + lootState.decrementUnappliedChangeCounter(); + EXPECT_FALSE(lootState.hasUnappliedChanges()); + } + + TEST_F(LootState, shouldHaveUnappliedChangesIfCounterIsIncremented) { + lootState.incrementUnappliedChangeCounter(); + EXPECT_TRUE(lootState.hasUnappliedChanges()); + } + + TEST_F(LootState, incrementingTheChangeCounterMoreThanItIsDecrementedShouldLeaveUnappliedChanges) { + lootState.incrementUnappliedChangeCounter(); + lootState.incrementUnappliedChangeCounter(); + lootState.decrementUnappliedChangeCounter(); + EXPECT_TRUE(lootState.hasUnappliedChanges()); + } + + TEST_F(LootState, incrementingTheChangeCounterLessThanItIsDecrementedShouldLeaveNoUnappliedChanges) { + lootState.incrementUnappliedChangeCounter(); + lootState.decrementUnappliedChangeCounter(); + lootState.decrementUnappliedChangeCounter(); + EXPECT_FALSE(lootState.hasUnappliedChanges()); + } + + TEST_F(LootState, incrementingTheChangeCounterThenDecrementingItEquallyShouldLeaveNoUnappliedChanges) { + lootState.incrementUnappliedChangeCounter(); + lootState.incrementUnappliedChangeCounter(); + lootState.decrementUnappliedChangeCounter(); + lootState.decrementUnappliedChangeCounter(); + EXPECT_FALSE(lootState.hasUnappliedChanges()); + } + } +} + +#endif diff --git a/src/tests/main.cpp b/src/tests/main.cpp index 78023cc1..fca2ac5b 100644 --- a/src/tests/main.cpp +++ b/src/tests/main.cpp @@ -52,6 +52,7 @@ #include "backend/test_masterlist.h" #include "backend/test_plugin_sorter.h" #include "gui/test_loot_settings.h" +#include "gui/test_loot_state.h" #include From 49db4fb7e150fb0c314f1048bfac5b27860f5502 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 18 Jan 2016 19:54:57 +0000 Subject: [PATCH 3/3] Replace LootState::UpdateGamesFromSettings() With an load() that calls LootSettings::load() and enables/disables debug logging and includes the functionality of UpdateGamesFromSettings(). There isn't a test for this because it can only be tested for games detected as installed, and none can be without significant changes to the testing files/folders that would break most other tests. --- src/gui/handler.cpp | 13 ++---- src/gui/loot_state.cpp | 94 ++++++++++++++++++++++-------------------- src/gui/loot_state.h | 2 +- 3 files changed, 54 insertions(+), 55 deletions(-) diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index b185dd6b..4d80422a 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -275,16 +275,11 @@ namespace loot { else if (requestName == "closeSettings") { BOOST_LOG_TRIVIAL(trace) << "Settings dialog closed and changes accepted, updating settings object."; - // Update the settings. - _lootState.load(request["args"][0]); - // If the user has deleted a default game, we don't want to restore it now. - // It will be restored when LOOT is next loaded. try { - BOOST_LOG_TRIVIAL(trace) << "Updating games object."; - _lootState.UpdateGamesFromSettings(); - - // Also enable/disable debug logging as required. - boost::log::core::get()->set_logging_enabled(_lootState.isDebugLoggingEnabled()); + // Update the settings. + // If the user has deleted a default game, we don't want to restore it now. + // It will be restored when LOOT is next loaded. + _lootState.load(request["args"][0]); // Now send back the new list of installed games to the UI. BOOST_LOG_TRIVIAL(trace) << "Getting new list of installed games."; diff --git a/src/gui/loot_state.cpp b/src/gui/loot_state.cpp index 8f8ff875..693f7af3 100644 --- a/src/gui/loot_state.cpp +++ b/src/gui/loot_state.cpp @@ -52,6 +52,54 @@ namespace fs = boost::filesystem; namespace loot { LootState::LootState() : unappliedChangeCounter(0), _currentGame(_games.end()) {} + void LootState::load(YAML::Node& settings) { + std::lock_guard guard(mutex); + + LootSettings::load(settings); + + // Enable/disable debug logging in case it has changed. + boost::log::core::get()->set_logging_enabled(isDebugLoggingEnabled()); + + // Update existing games, add new games. + unordered_set newGameFolders; + BOOST_LOG_TRIVIAL(trace) << "Updating existing games and adding new games."; + for (const auto &game : getGameSettings()) { + auto pos = find(_games.begin(), _games.end(), game); + + if (pos != _games.end()) { + pos->SetName(game.Name()) + .SetMaster(game.Master()) + .SetRepoURL(game.RepoURL()) + .SetRepoBranch(game.RepoBranch()) + .SetGamePath(game.GamePath()) + .SetRegistryKey(game.RegistryKey()); + } + else { + BOOST_LOG_TRIVIAL(trace) << "Adding new game entry for: " << game.FolderName(); + _games.push_back(game); + } + + newGameFolders.insert(game.FolderName()); + } + + // Remove deleted games. As the current game is stored using its index, + // removing an earlier game may invalidate it. + BOOST_LOG_TRIVIAL(trace) << "Removing deleted games."; + for (auto it = _games.begin(); it != _games.end();) { + if (newGameFolders.find(it->FolderName()) == newGameFolders.end()) { + BOOST_LOG_TRIVIAL(trace) << "Removing game: " << it->FolderName(); + it = _games.erase(it); + } + else + ++it; + } + + // Re-initialise the current game in case the game path setting was changed. + _currentGame->Init(true); + // Update game path in settings object. + storeGameSettings(ToGameSettings(_games)); + } + void LootState::Init(const std::string& cmdLineGame) { // Do some preliminary locale / UTF-8 support setup here, in case the settings file reading requires it. //Boost.Locale initialisation: Specify location of language dictionaries. @@ -75,7 +123,7 @@ namespace loot { } if (fs::exists(g_path_settings)) { try { - load(g_path_settings); + LootSettings::load(g_path_settings); } catch (exception& e) { _initErrors.push_back((format(translate("Error: Settings parsing failed. %1%")) % e.what()).str()); @@ -159,50 +207,6 @@ namespace loot { LootSettings::save(file); } - void LootState::UpdateGamesFromSettings() { - std::lock_guard guard(mutex); - - unordered_set newGameFolders; - - // Update existing games, add new games. - BOOST_LOG_TRIVIAL(trace) << "Updating existing games and adding new games."; - for (const auto &game : getGameSettings()) { - auto pos = find(_games.begin(), _games.end(), game); - - if (pos != _games.end()) { - pos->SetName(game.Name()) - .SetMaster(game.Master()) - .SetRepoURL(game.RepoURL()) - .SetRepoBranch(game.RepoBranch()) - .SetGamePath(game.GamePath()) - .SetRegistryKey(game.RegistryKey()); - } - else { - BOOST_LOG_TRIVIAL(trace) << "Adding new game entry for: " << game.FolderName(); - _games.push_back(game); - } - - newGameFolders.insert(game.FolderName()); - } - - // Remove deleted games. As the current game is stored using its index, - // removing an earlier game may invalidate it. - BOOST_LOG_TRIVIAL(trace) << "Removing deleted games."; - for (auto it = _games.begin(); it != _games.end();) { - if (newGameFolders.find(it->FolderName()) == newGameFolders.end()) { - BOOST_LOG_TRIVIAL(trace) << "Removing game: " << it->FolderName(); - it = _games.erase(it); - } - else - ++it; - } - - // Re-initialise the current game in case the game path setting was changed. - _currentGame->Init(true); - // Update game path in settings object. - storeGameSettings(ToGameSettings(_games)); - } - void LootState::ChangeGame(const std::string& newGameFolder) { std::lock_guard guard(mutex); diff --git a/src/gui/loot_state.h b/src/gui/loot_state.h index b43242ca..b7d7934b 100644 --- a/src/gui/loot_state.h +++ b/src/gui/loot_state.h @@ -33,6 +33,7 @@ namespace loot { public: LootState(); + void load(YAML::Node& settings); void Init(const std::string& cmdLineGame); const std::vector& InitErrors() const; @@ -40,7 +41,6 @@ namespace loot { Game& CurrentGame(); void ChangeGame(const std::string& newGameFolder); - void UpdateGamesFromSettings(); // Get the folder names of the installed games. std::vector InstalledGames();