From 1bec0e652f98c2463fa36a9ddfbdaeb25fa6f100 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 16 Aug 2016 20:29:37 +0100 Subject: [PATCH] Simplify test fixtures CommonGameTestFixture used an unsigned int parameter, because the C API exposed game types as unsigned ints. Now that the C++ API exposes the GameType enum itself, it can be used for all game test fixtures. --- CMakeLists.txt | 1 - src/tests/api/api_game_operations_test.h | 9 ++-- src/tests/api/create_database_test.h | 12 ++--- src/tests/backend/base_game_test.h | 51 ------------------- src/tests/backend/game/game_cache_test.h | 4 +- src/tests/backend/game/game_settings_test.h | 4 +- src/tests/backend/game/game_test.h | 4 +- .../backend/game/load_order_handler_test.h | 4 +- src/tests/backend/helpers/helpers_test.h | 4 +- src/tests/backend/masterlist_test.h | 8 +-- .../backend/metadata/condition_grammar_test.h | 8 +-- .../metadata/conditional_metadata_test.h | 4 +- src/tests/backend/metadata/message_test.h | 4 +- .../metadata/plugin_cleaning_data_test.h | 4 +- .../backend/metadata/plugin_metadata_test.h | 4 +- src/tests/backend/metadata_list_test.h | 8 +-- src/tests/backend/plugin/plugin_sorter_test.h | 6 +-- src/tests/backend/plugin/plugin_test.h | 8 +-- src/tests/common_game_test_fixture.h | 51 ++++++++----------- 19 files changed, 65 insertions(+), 133 deletions(-) delete mode 100644 src/tests/backend/base_game_test.h diff --git a/CMakeLists.txt b/CMakeLists.txt index eb084cf7..f557f2c1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -263,7 +263,6 @@ set (LOOT_TESTS_HEADERS # Testing this here rather than as part of the API tests "${CMAKE_SOURCE_DIR}/src/tests/backend/app/loot_paths_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/app/loot_settings_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/app/loot_state_test.h" - "${CMAKE_SOURCE_DIR}/src/tests/backend/base_game_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/game/game_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/game/game_cache_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/game/game_settings_test.h" diff --git a/src/tests/api/api_game_operations_test.h b/src/tests/api/api_game_operations_test.h index dada5b36..01f0ee8f 100644 --- a/src/tests/api/api_game_operations_test.h +++ b/src/tests/api/api_game_operations_test.h @@ -31,12 +31,9 @@ along with LOOT. If not, see namespace loot { namespace test { -class ApiGameOperationsTest : - public ::testing::TestWithParam, - public CommonGameTestFixture { +class ApiGameOperationsTest : public CommonGameTestFixture { protected: ApiGameOperationsTest() : - CommonGameTestFixture(static_cast(GetParam())), db_(nullptr), masterlistPath(localPath / "masterlist.yaml"), noteMessage("Do not clean ITM records, they are intentional and required for the mod to function."), @@ -44,7 +41,7 @@ protected: errorMessage("Obsolete. Remove this and install Enhanced Weather.") {} virtual void SetUp() { - setUp(); + CommonGameTestFixture::SetUp(); ASSERT_FALSE(boost::filesystem::exists(masterlistPath)); @@ -52,7 +49,7 @@ protected: } virtual void TearDown() { - tearDown(); + CommonGameTestFixture::TearDown(); // The masterlist may have been created during the test, so delete it. ASSERT_NO_THROW(boost::filesystem::remove(masterlistPath)); diff --git a/src/tests/api/create_database_test.h b/src/tests/api/create_database_test.h index 1f512bef..b1a3fdf9 100644 --- a/src/tests/api/create_database_test.h +++ b/src/tests/api/create_database_test.h @@ -33,20 +33,16 @@ along with LOOT. If not, see namespace loot { namespace test { -class CreateDatabaseTest : - public ::testing::TestWithParam, - public CommonGameTestFixture { +class CreateDatabaseTest : public CommonGameTestFixture { protected: - CreateDatabaseTest() : - CommonGameTestFixture(static_cast(GetParam())), - db_(nullptr) {} + CreateDatabaseTest() : db_(nullptr) {} void SetUp() { - setUp(); + CommonGameTestFixture::SetUp(); } void TearDown() { - tearDown(); + CommonGameTestFixture::TearDown(); } std::shared_ptr db_; diff --git a/src/tests/backend/base_game_test.h b/src/tests/backend/base_game_test.h deleted file mode 100644 index 7d03112f..00000000 --- a/src/tests/backend/base_game_test.h +++ /dev/null @@ -1,51 +0,0 @@ -/* LOOT - -A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and -Fallout: New Vegas. - -Copyright (C) 2013-2016 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_TESTS_BACKEND_BASE_GAME_TEST -#define LOOT_TESTS_BACKEND_BASE_GAME_TEST - -#include "loot/game_type.h" -#include "tests/common_game_test_fixture.h" - -namespace loot { -namespace test { -class BaseGameTest : - public ::testing::TestWithParam, - public CommonGameTestFixture { -protected: - BaseGameTest() : - CommonGameTestFixture(static_cast(GetParam())) {} - - inline virtual void SetUp() { - setUp(); - } - - inline virtual void TearDown() { - tearDown(); - } -}; -} -} - -#endif diff --git a/src/tests/backend/game/game_cache_test.h b/src/tests/backend/game/game_cache_test.h index e30be89e..7036d5e7 100644 --- a/src/tests/backend/game/game_cache_test.h +++ b/src/tests/backend/game/game_cache_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/game/game_cache.h" #include "backend/game/game.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class GameCacheTest : public BaseGameTest { +class GameCacheTest : public CommonGameTestFixture { protected: GameCacheTest() : condition("Condition"), diff --git a/src/tests/backend/game/game_settings_test.h b/src/tests/backend/game/game_settings_test.h index fec00182..2b75fb13 100644 --- a/src/tests/backend/game/game_settings_test.h +++ b/src/tests/backend/game/game_settings_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/game/game_settings.h" #include "backend/app/loot_paths.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class GameSettingsTest : public BaseGameTest { +class GameSettingsTest : public CommonGameTestFixture { protected: GameSettings settings_; }; diff --git a/src/tests/backend/game/game_test.h b/src/tests/backend/game/game_test.h index bef3d7ce..b0d55c43 100644 --- a/src/tests/backend/game/game_test.h +++ b/src/tests/backend/game/game_test.h @@ -33,11 +33,11 @@ along with LOOT. If not, see namespace loot { namespace test { -class GameTest : public BaseGameTest { +class GameTest : public CommonGameTestFixture { protected: #ifndef _WIN32 void TearDown() { - BaseGameTest::TearDown(); + CommonGameTestFixture::TearDown(); ASSERT_NO_THROW(boost::filesystem::remove_all(LootPaths::getLootDataPath())); } diff --git a/src/tests/backend/game/load_order_handler_test.h b/src/tests/backend/game/load_order_handler_test.h index 5ab5aff9..6c432879 100644 --- a/src/tests/backend/game/load_order_handler_test.h +++ b/src/tests/backend/game/load_order_handler_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/game/load_order_handler.h" #include "loot/error.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class LoadOrderHandlerTest : public BaseGameTest { +class LoadOrderHandlerTest : public CommonGameTestFixture { protected: LoadOrderHandler loadOrderHandler_; }; diff --git a/src/tests/backend/helpers/helpers_test.h b/src/tests/backend/helpers/helpers_test.h index 76ead90f..a85ba53f 100644 --- a/src/tests/backend/helpers/helpers_test.h +++ b/src/tests/backend/helpers/helpers_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/helpers/helpers.h" #include "loot/error.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class GetCrc32Test : public BaseGameTest {}; +class GetCrc32Test : public CommonGameTestFixture {}; // Pass an empty first argument, as it's a prefix for the test instantation, // but we only have the one so no prefix is necessary. diff --git a/src/tests/backend/masterlist_test.h b/src/tests/backend/masterlist_test.h index 0e5021ff..f3e4c1a3 100644 --- a/src/tests/backend/masterlist_test.h +++ b/src/tests/backend/masterlist_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/masterlist.h" #include "backend/app/loot_paths.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class MasterlistTest : public BaseGameTest { +class MasterlistTest : public CommonGameTestFixture { protected: MasterlistTest() : repoBranch("2.x"), @@ -40,7 +40,7 @@ protected: masterlistPath(localPath / "masterlist.yaml") {} void SetUp() { - BaseGameTest::SetUp(); + CommonGameTestFixture::SetUp(); ASSERT_FALSE(boost::filesystem::exists(masterlistPath)); ASSERT_FALSE(boost::filesystem::exists(localPath / ".git")); @@ -49,7 +49,7 @@ protected: } void TearDown() { - BaseGameTest::TearDown(); + CommonGameTestFixture::TearDown(); ASSERT_NO_THROW(boost::filesystem::remove(masterlistPath)); ASSERT_NO_THROW(boost::filesystem::remove_all(localPath / ".git")); diff --git a/src/tests/backend/metadata/condition_grammar_test.h b/src/tests/backend/metadata/condition_grammar_test.h index f5300f46..bda4de17 100644 --- a/src/tests/backend/metadata/condition_grammar_test.h +++ b/src/tests/backend/metadata/condition_grammar_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/metadata/condition_grammar.h" #include "loot/error.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class ConditionGrammarTest : public BaseGameTest { +class ConditionGrammarTest : public CommonGameTestFixture { protected: typedef ConditionGrammar Grammar; @@ -43,7 +43,7 @@ protected: success_(false) {} inline void SetUp() { - BaseGameTest::SetUp(); + CommonGameTestFixture::SetUp(); // Write out an empty resource file. ASSERT_NO_THROW(boost::filesystem::create_directories(resourcePath.parent_path())); @@ -53,7 +53,7 @@ protected: } inline void TearDown() { - BaseGameTest::TearDown(); + CommonGameTestFixture::TearDown(); ASSERT_NO_THROW(boost::filesystem::remove(resourcePath)); } diff --git a/src/tests/backend/metadata/conditional_metadata_test.h b/src/tests/backend/metadata/conditional_metadata_test.h index 988deb44..8e3e6645 100644 --- a/src/tests/backend/metadata/conditional_metadata_test.h +++ b/src/tests/backend/metadata/conditional_metadata_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/metadata/conditional_metadata.h" #include "loot/error.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class ConditionalMetadataTest : public BaseGameTest { +class ConditionalMetadataTest : public CommonGameTestFixture { protected: ConditionalMetadata conditionalMetadata_; }; diff --git a/src/tests/backend/metadata/message_test.h b/src/tests/backend/metadata/message_test.h index 8b276cd2..47cb3245 100644 --- a/src/tests/backend/metadata/message_test.h +++ b/src/tests/backend/metadata/message_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/metadata/message.h" #include "backend/game/game.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class MessageTest : public BaseGameTest { +class MessageTest : public CommonGameTestFixture { protected: typedef std::vector MessageContents; diff --git a/src/tests/backend/metadata/plugin_cleaning_data_test.h b/src/tests/backend/metadata/plugin_cleaning_data_test.h index 6a490aa0..551699dc 100644 --- a/src/tests/backend/metadata/plugin_cleaning_data_test.h +++ b/src/tests/backend/metadata/plugin_cleaning_data_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/metadata/plugin_cleaning_data.h" #include "backend/game/game.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class PluginCleaningDataTest : public BaseGameTest { +class PluginCleaningDataTest : public CommonGameTestFixture { protected: PluginCleaningDataTest() : info_(std::vector({ MessageContent("info", LanguageCode::english), diff --git a/src/tests/backend/metadata/plugin_metadata_test.h b/src/tests/backend/metadata/plugin_metadata_test.h index 72b6f22f..b0b4ce57 100644 --- a/src/tests/backend/metadata/plugin_metadata_test.h +++ b/src/tests/backend/metadata/plugin_metadata_test.h @@ -27,11 +27,11 @@ along with LOOT. If not, see #include "backend/metadata/plugin_metadata.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class PluginMetadataTest : public BaseGameTest { +class PluginMetadataTest : public CommonGameTestFixture { protected: PluginMetadataTest() : info_(std::vector({ MessageContent("info", LanguageCode::english), diff --git a/src/tests/backend/metadata_list_test.h b/src/tests/backend/metadata_list_test.h index e93b191b..fe2132b9 100644 --- a/src/tests/backend/metadata_list_test.h +++ b/src/tests/backend/metadata_list_test.h @@ -27,11 +27,11 @@ along with LOOT. If not, see #include "backend/metadata_list.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class MetadataListTest : public BaseGameTest { +class MetadataListTest : public CommonGameTestFixture { protected: MetadataListTest() : metadataPath("./testing-metadata/masterlist.yaml"), @@ -40,7 +40,7 @@ protected: invalidMetadataPaths({"./testing-metadata/invalid/non_unique.yaml"}) {} inline virtual void SetUp() { - BaseGameTest::SetUp(); + CommonGameTestFixture::SetUp(); ASSERT_TRUE(boost::filesystem::exists(metadataPath)); ASSERT_FALSE(boost::filesystem::exists(savedMetadataPath)); @@ -51,7 +51,7 @@ protected: } inline virtual void TearDown() { - BaseGameTest::TearDown(); + CommonGameTestFixture::TearDown(); ASSERT_TRUE(boost::filesystem::exists(metadataPath)); ASSERT_NO_THROW(boost::filesystem::remove(savedMetadataPath)); diff --git a/src/tests/backend/plugin/plugin_sorter_test.h b/src/tests/backend/plugin/plugin_sorter_test.h index 4d7143d0..a0f54d45 100644 --- a/src/tests/backend/plugin/plugin_sorter_test.h +++ b/src/tests/backend/plugin/plugin_sorter_test.h @@ -27,14 +27,14 @@ along with LOOT. If not, see #include "backend/plugin/plugin_sorter.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class PluginSorterTest : public BaseGameTest { +class PluginSorterTest : public CommonGameTestFixture { protected: inline virtual void SetUp() { - BaseGameTest::SetUp(); + CommonGameTestFixture::SetUp(); game_ = Game(GetParam()); game_.SetGamePath(dataPath.parent_path()); diff --git a/src/tests/backend/plugin/plugin_test.h b/src/tests/backend/plugin/plugin_test.h index 08707984..91921e4b 100644 --- a/src/tests/backend/plugin/plugin_test.h +++ b/src/tests/backend/plugin/plugin_test.h @@ -28,11 +28,11 @@ along with LOOT. If not, see #include "backend/plugin/plugin.h" #include "backend/game/game.h" -#include "tests/backend/base_game_test.h" +#include "tests/common_game_test_fixture.h" namespace loot { namespace test { -class PluginTest : public BaseGameTest { +class PluginTest : public CommonGameTestFixture { protected: PluginTest() : emptyFile("EmptyFile.esm"), @@ -41,7 +41,7 @@ protected: blankSuffixArchive("Blank - Different - suffix" + Game(GetParam()).GetArchiveFileExtension()) {} void SetUp() { - BaseGameTest::SetUp(); + CommonGameTestFixture::SetUp(); game_ = Game(GetParam()); game_.SetGamePath(dataPath.parent_path()); @@ -66,7 +66,7 @@ protected: } void TearDown() { - BaseGameTest::TearDown(); + CommonGameTestFixture::TearDown(); boost::filesystem::remove(dataPath / emptyFile); boost::filesystem::remove(dataPath / nonPluginFile); diff --git a/src/tests/common_game_test_fixture.h b/src/tests/common_game_test_fixture.h index 9c3999fa..f381eabc 100644 --- a/src/tests/common_game_test_fixture.h +++ b/src/tests/common_game_test_fixture.h @@ -33,12 +33,13 @@ along with LOOT. If not, see #include #include +#include "loot/game_type.h" + namespace loot { namespace test { -class CommonGameTestFixture { +class CommonGameTestFixture : public ::testing::TestWithParam { protected: - CommonGameTestFixture(unsigned int gameType) : - gameType(gameType), + CommonGameTestFixture() : missingPath("./missing"), dataPath(getPluginsPath()), localPath(getLocalPath()), @@ -56,7 +57,7 @@ protected: blankDifferentPluginDependentEsp("Blank - Different Plugin Dependent.esp"), blankEsmCrc(getBlankEsmCrc()) {} - void setUp() { + void SetUp() { ASSERT_NO_THROW(boost::filesystem::create_directories(localPath)); ASSERT_TRUE(boost::filesystem::exists(localPath)); @@ -88,7 +89,7 @@ protected: ASSERT_TRUE(boost::filesystem::exists(dataPath / (blankMasterDependentEsm + ".ghost"))); } - void tearDown() { + void TearDown() { ASSERT_NO_THROW(boost::filesystem::remove_all(localPath)); ASSERT_NO_THROW(boost::filesystem::remove(dataPath / masterFile)); @@ -101,7 +102,7 @@ protected: std::vector getLoadOrder() { std::vector actual; - if (isLoadOrderTimestampBased(gameType)) { + if (isLoadOrderTimestampBased(GetParam())) { std::map loadOrder; for (boost::filesystem::directory_iterator it(dataPath); it != boost::filesystem::directory_iterator(); ++it) { if (boost::filesystem::is_regular_file(it->status())) { @@ -114,7 +115,7 @@ protected: } for (const auto& plugin : loadOrder) actual.push_back(plugin.second); - } else if (gameType == tes5) { + } else if (GetParam() == GameType::tes5) { boost::filesystem::ifstream in(localPath / "loadorder.txt"); while (in) { std::string line; @@ -157,10 +158,6 @@ protected: }); } -private: - // This needs to be here to ensure the correct initialisation order. - const unsigned int gameType; - protected: const boost::filesystem::path missingPath; const boost::filesystem::path dataPath; @@ -182,41 +179,35 @@ protected: const uint32_t blankEsmCrc; private: - static const unsigned int tes4 = 1; - static const unsigned int tes5 = 2; - static const unsigned int fo3 = 3; - static const unsigned int fonv = 4; - static const unsigned int fo4 = 5; - inline boost::filesystem::path getLocalPath() const { - if (gameType == tes4) + if (GetParam() == GameType::tes4) return "./local/Oblivion"; else return "./local/Skyrim"; } inline boost::filesystem::path getPluginsPath() const { - if (gameType == tes4) + if (GetParam() == GameType::tes4) return "./Oblivion/Data"; else return "./Skyrim/Data"; } inline std::string getMasterFile() const { - if (gameType == tes4) + if (GetParam() == GameType::tes4) return "Oblivion.esm"; - else if (gameType == tes5) + else if (GetParam() == GameType::tes5) return "Skyrim.esm"; - else if (gameType == fo3) + else if (GetParam() == GameType::fo3) return "Fallout3.esm"; - else if (gameType == fonv) + else if (GetParam() == GameType::fonv) return "FalloutNV.esm"; else return "Fallout4.esm"; } inline uint32_t getBlankEsmCrc() const { - if (gameType == tes4) + if (GetParam() == GameType::tes4) return 0x374E2A6F; else return 0x187BE342; @@ -225,15 +216,15 @@ private: void setLoadOrder(const std::vector>& loadOrder) const { boost::filesystem::ofstream out(localPath / "plugins.txt"); for (const auto &plugin : loadOrder) { - if (gameType == fo4 && plugin.second) + if (GetParam() == GameType::fo4 && plugin.second) out << '*'; - else if (gameType != fo4 && !plugin.second) + else if (GetParam() != GameType::fo4 && !plugin.second) continue; out << plugin.first << std::endl; } - if (isLoadOrderTimestampBased(gameType)) { + if (isLoadOrderTimestampBased(GetParam())) { time_t modificationTime = time(NULL); // Current time. for (const auto &plugin : loadOrder) { if (boost::filesystem::exists(dataPath / boost::filesystem::path(plugin.first + ".ghost"))) { @@ -243,15 +234,15 @@ private: } modificationTime += 60; } - } else if (gameType == tes5) { + } else if (GetParam() == GameType::tes5) { boost::filesystem::ofstream out(localPath / "loadorder.txt"); for (const auto &plugin : loadOrder) out << plugin.first << std::endl; } } - inline static bool isLoadOrderTimestampBased(unsigned int gameId) { - return gameId == tes4 || gameId == fo3 || gameId == fonv; + inline static bool isLoadOrderTimestampBased(GameType gameType) { + return gameType == GameType::tes4 || gameType == GameType::fo3 || gameType == GameType::fonv; } }; }