From a22d1310a3c580d139af90afe9fa42b682a05622 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 13:52:52 +0100 Subject: [PATCH 01/16] Started to implement Google Test unit tests. Added a project for the tests to the CMake config, and added Google Test as a dependency for it. Travis runs the tests. Some tests currently fail. --- .travis.yml | 14 ++- CMakeLists.txt | 68 +++++++++---- README.md | 1 + src/tests/api/api.h | 135 +++++++++++++++++++++++++ src/tests/fixtures.h | 228 +++++++++++++++++++++++++++++++++++++++++++ src/tests/main.cpp | 30 ++++++ 6 files changed, 454 insertions(+), 22 deletions(-) create mode 100644 src/tests/api/api.h create mode 100644 src/tests/fixtures.h create mode 100644 src/tests/main.cpp diff --git a/.travis.yml b/.travis.yml index 52942e4d..1c3ed166 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,13 +3,13 @@ compiler: - gcc before_install: + # Add a PPA for Boost 1.55. + - sudo add-apt-repository ppa:boost-latest/ppa -y - sudo apt-get update -qq install: # Currently inside the cloned repo path. - # Need Boost 1.54+, which isn't in the 12.04 repositories - install from a PPA. - - sudo add-apt-repository ppa:boost-latest/ppa -y - - sudo apt-get update -qq + # Install Boost. - sudo apt-get install libboost-log1.55-dev libboost-date-time1.55-dev libboost-thread1.55-dev libboost-filesystem1.55-dev libboost-locale1.55-dev libboost-iostreams1.55-dev # Install Alphanum - cd ../.. @@ -54,7 +54,11 @@ install: before_script: - mkdir build - cd build + # Fetch plugins to test with. + - wget https://github.com/WrinklyNinja/testing-plugins/archive/master.zip + - unzip master.zip + - mv testing-plugins-master/* ./ # Travis machines are 64 bit, and the deps use dynamic linking. - - cmake .. -DPROJECT_ARCH=64 -DPROJECT_STATIC_RUNTIME=OFF -DBUILD_SHARED_LIBS=OFF + - cmake .. -DPROJECT_ARCH=64 -DPROJECT_STATIC_RUNTIME=OFF -DBUILD_SHARED_LIBS=OFF -DGTEST_ROOT=../../gtest-1.7.0 -script: make loot64 +script: make tests && ./tests diff --git a/CMakeLists.txt b/CMakeLists.txt index 231ec74f..7ffe9664 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,7 +46,12 @@ set (Boost_USE_STATIC_LIBS ${PROJECT_STATIC_RUNTIME}) set (Boost_USE_MULTITHREADED ON) set (Boost_USE_STATIC_RUNTIME ${PROJECT_STATIC_RUNTIME}) +IF (NOT Boost_USE_STATIC_LIBS) + add_definitions(-DBOOST_LOG_DYN_LINK) +ENDIF () + find_package(Boost REQUIRED COMPONENTS log log_setup regex locale thread date_time chrono filesystem system iostreams) +find_package(GTest) set (LOOT_SRC "${CMAKE_SOURCE_DIR}/src/backend/metadata.cpp" "${CMAKE_SOURCE_DIR}/src/backend/game.cpp" @@ -88,7 +93,12 @@ set (LOOT_API_SRC ${LOOT_SRC} set (LOOT_API_HEADERS ${LOOT_HEADERS} "${CMAKE_SOURCE_DIR}/src/api/api.h") -source_group("Header Files" FILES ${LOOT_HEADERS} ${LOOT_GUI_HEADERS} ${LOOT_API_HEADERS}) +set (LOOT_TESTS_SRC "${CMAKE_SOURCE_DIR}/src/tests/main.cpp") + +set (LOOT_TESTS_HEADERS "${CMAKE_SOURCE_DIR}/src/tests/fixtures.h" + "${CMAKE_SOURCE_DIR}/src/tests/api/api.h") + +source_group("Header Files" FILES ${LOOT_HEADERS} ${LOOT_GUI_HEADERS} ${LOOT_API_HEADERS} ${LOOT_TESTS_HEADERS}) # Include source and library directories. include_directories ("${CMAKE_SOURCE_DIR}/src" @@ -98,7 +108,8 @@ include_directories ("${CMAKE_SOURCE_DIR}/src" "${LIBGIT2_ROOT}/include" ${CEF_ROOT} ${LIBESPM_ROOT} - ${Boost_INCLUDE_DIRS}) + ${Boost_INCLUDE_DIRS} + ${GTEST_INCLUDE_DIRS}) link_directories ("${LIBLOADORDER_ROOT}/build" "${LIBGIT2_ROOT}/build" @@ -113,12 +124,7 @@ link_directories ("${LIBLOADORDER_ROOT}/build" # Settings when compiling for Windows. Since it's a Windows-only app this is always true, but useful to check for copy/paste into other projects. IF (CMAKE_SYSTEM_NAME MATCHES "Windows") - add_definitions (-DUNICODE -D_UNICODE -DNDEBUG -DLIBLO_STATIC -DWIN32 -D_WINDOWS) - IF (BUILD_SHARED_LIBS) - add_definitions (-DLOOT_EXPORT) - ELSE () - add_definitions (-DLOOT_STATIC) - ENDIF () + add_definitions (-DUNICODE -D_UNICODE -DLIBLO_STATIC) ENDIF () # GCC and MinGW settings. @@ -144,8 +150,8 @@ IF (MINGW) set (LOOT_LIBS ${LOOT_LIBS} version - ws2_32 - shlwapi) + ws2_32 + shlwapi) set (LOOT_GUI_LIBS ${LOOT_LIBS} cef_sandbox libcef @@ -165,13 +171,12 @@ ELSEIF (MSVC) ENDFOREACH() ENDIF () - set (CMAKE_EXE_LINKER_FLAGS "/SUBSYSTEM:WINDOWS /LARGEADDRESSAWARE") set (LOOT_LIBS git2 - libyaml-cppmt - version - loadorder${PROJECT_ARCH} - ws2_32 - shlwapi) + libyaml-cppmt + version + loadorder${PROJECT_ARCH} + ws2_32 + shlwapi) set (LOOT_GUI_LIBS ${LOOT_LIBS} cef_sandbox libcef @@ -182,17 +187,46 @@ ENDIF () ############################## -# Actual Building +# Define Targets ############################## # Build API. add_library (loot${PROJECT_ARCH} ${LOOT_API_SRC} ${LOOT_API_HEADERS}) target_link_libraries (loot${PROJECT_ARCH} ${Boost_LIBRARIES} ${LOOT_LIBS}) +IF (${GTEST_FOUND}) + # Build tests. + add_executable(tests ${LOOT_TESTS_SRC} ${LOOT_TESTS_HEADERS}) + target_link_libraries(tests loot${PROJECT_ARCH} ${Boost_LIBRARIES} ${LOOT_LIBS} ${GTEST_BOTH_LIBRARIES}) +ENDIF () + # Build application. add_executable (LOOT ${LOOT_GUI_SRC} ${LOOT_GUI_HEADERS}) target_link_libraries (LOOT ${Boost_LIBRARIES} ${LOOT_GUI_LIBS}) + +############################## +# Set Target-Specific Flags +############################## + +IF (MSVC) + set_target_properties (LOOT PROPERTIES CMAKE_EXE_LINKER_FLAGS "/SUBSYSTEM:WINDOWS /LARGEADDRESSAWARE") +ENDIF () + + +IF (CMAKE_SYSTEM_NAME MATCHES "Windows") + IF (BUILD_SHARED_LIBS) + set_target_properties (loot${PROJECT_ARCH} PROPERTIES COMPILE_DEFINITIONS "${COMPILE_DEFINITIONS} LOOT_EXPORT") + ELSE () + set_target_properties (loot${PROJECT_ARCH} PROPERTIES COMPILE_DEFINITIONS "${COMPILE_DEFINITIONS} LOOT_STATIC") + ENDIF () +ENDIF () + + +############################## +# Post-Build Steps +############################## + add_custom_command( TARGET LOOT POST_BUILD diff --git a/README.md b/README.md index 66f21645..abafdf0f 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ LOOT uses [CMake](http://cmake.org) to generate build files, and requires the fo * [Alphanum](http://www.davekoelle.com/files/alphanum.hpp) * [Boost](http://www.boost.org) v1.56.0 * [Chromium Embedded Framework](https://code.google.com/p/chromiumembedded/) branch 2062 +* [Google Test](https://code.google.com/p/googletest/) v1.7: Required to build the LOOT API's tests, but not the API itself or the LOOT application. * [Libespm](http://github.com/WrinklyNinja/libespm) * [Libgit2](http://libgit2.github.com/) v0.21.1 * [Libloadorder](http://github.com/WrinklyNinja/libloadorder) diff --git a/src/tests/api/api.h b/src/tests/api/api.h new file mode 100644 index 00000000..53781f4e --- /dev/null +++ b/src/tests/api/api.h @@ -0,0 +1,135 @@ +/* LOOT + +A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and +Fallout: New Vegas. + +Copyright (C) 2014 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_API +#define LOOT_TEST_API + +#include "../../api/api.h" +#include "tests/fixtures.h" + +TEST(GetVersion, HandlesNullInput) { + unsigned int vMajor, vMinor, vPatch; + EXPECT_EQ(loot_error_invalid_args, loot_get_version(&vMajor, NULL, NULL)); + EXPECT_EQ(loot_error_invalid_args, loot_get_version(NULL, &vMinor, NULL)); + EXPECT_EQ(loot_error_invalid_args, loot_get_version(NULL, NULL, &vPatch)); + EXPECT_EQ(loot_error_invalid_args, loot_get_version(NULL, NULL, NULL)); +} + +TEST(GetVersion, HandlesValidInput) { + unsigned int vMajor, vMinor, vPatch; + EXPECT_EQ(loot_ok, loot_get_version(&vMajor, &vMinor, &vPatch)); +} + +TEST(IsCompatible, HandlesCompatibleVersion) { + unsigned int vMajor, vMinor, vPatch; + EXPECT_EQ(loot_ok, loot_get_version(&vMajor, &vMinor, &vPatch)); + + EXPECT_TRUE(loot_is_compatible(vMajor, vMinor, vPatch)); + // Test somewhat arbitrary variations. + EXPECT_TRUE(loot_is_compatible(vMajor, vMinor + 1, vPatch + 1)); + if (vMinor > 0 && vPatch > 0) + EXPECT_TRUE(loot_is_compatible(vMajor, vMinor - 1, vPatch - 1)); +} + +TEST(IsCompatible, HandlesIncompatibleVersion) { + unsigned int vMajor, vMinor, vPatch; + EXPECT_EQ(loot_ok, loot_get_version(&vMajor, &vMinor, &vPatch)); + + EXPECT_FALSE(loot_is_compatible(vMajor + 1, vMinor, vPatch)); + // Test somewhat arbitrary variations. + EXPECT_FALSE(loot_is_compatible(vMajor + 1, vMinor + 1, vPatch + 1)); + if (vMinor > 0 && vPatch > 0) + EXPECT_FALSE(loot_is_compatible(vMajor + 1, vMinor - 1, vPatch - 1)); +} + +TEST(GetErrorMessage, HandlesInputCorrectly) { + EXPECT_EQ(loot_error_invalid_args, loot_get_error_message(NULL)); + + const char * error; + EXPECT_EQ(loot_ok, loot_get_error_message(&error)); + ASSERT_STREQ("Null message pointer passed.", error); +} + +TEST(Cleanup, CleansUpAfterError) { + // First generate an error. + EXPECT_EQ(loot_error_invalid_args, loot_get_error_message(NULL)); + + // Check that the error message is non-null. + const char * error; + EXPECT_EQ(loot_ok, loot_get_error_message(&error)); + ASSERT_STREQ("Null message pointer passed.", error); + + ASSERT_NO_THROW(loot_cleanup()); + + // Now check that the error message pointer is null. + error = nullptr; + EXPECT_EQ(loot_ok, loot_get_error_message(&error)); + EXPECT_EQ(nullptr, error); +} + +TEST(Cleanup, HandlesNoError) { + ASSERT_NO_THROW(loot_cleanup()); + + const char * error = nullptr; + EXPECT_EQ(loot_ok, loot_get_error_message(&error)); + EXPECT_EQ(nullptr, error); +} + +TEST_F(OblivionTest, CreateDbHandlesValidInputs) { + EXPECT_EQ(loot_ok, loot_create_db(&db, loot_game_tes4, dataPath.parent_path().string().c_str(), localPath.string().c_str())); + ASSERT_NO_THROW(loot_destroy_db(db)); + db = nullptr; + + // Also test absolute paths. + boost::filesystem::path game = boost::filesystem::current_path() / dataPath.parent_path(); + boost::filesystem::path local = boost::filesystem::current_path() / localPath; + EXPECT_EQ(loot_ok, loot_create_db(&db, loot_game_tes4, game.string().c_str(), local.string().c_str())); +} + +TEST_F(OblivionTest, CreateDbHandlesInvalidHandleInput) { + EXPECT_EQ(loot_error_invalid_args, loot_create_db(NULL, loot_game_tes4, dataPath.parent_path().string().c_str(), localPath.string().c_str())); +} + +TEST_F(OblivionTest, CreateDbHandlesInvalidGameType) { + EXPECT_EQ(loot_error_invalid_args, loot_create_db(&db, UINT_MAX, dataPath.parent_path().string().c_str(), localPath.string().c_str())); +} + +TEST_F(OblivionTest, CreateDbHandlesInvalidGamePathInput) { + EXPECT_EQ(loot_error_invalid_args, loot_create_db(&db, loot_game_tes4, missingPath.string().c_str(), localPath.string().c_str())); +} + +TEST_F(OblivionTest, CreateDbHandlesInvalidLocalPathInput) { + EXPECT_EQ(loot_error_invalid_args, loot_create_db(&db, loot_game_tes4, dataPath.parent_path().string().c_str(), missingPath.string().c_str())); +} + +#ifdef _WIN32 +TEST_F(OblivionTest, CreateDbHandlesNullLocalPath) { + EXPECT_EQ(loot_ok, loot_create_db(&db, loot_game_tes4, dataPath.parent_path().string().c_str(), NULL)); +} +#endif + +TEST(GameHandleDestroyTest, HandledNullInput) { + ASSERT_NO_THROW(loot_destroy_db(NULL)); +} +#endif diff --git a/src/tests/fixtures.h b/src/tests/fixtures.h new file mode 100644 index 00000000..179a61f0 --- /dev/null +++ b/src/tests/fixtures.h @@ -0,0 +1,228 @@ +/* LOOT + +A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and +Fallout: New Vegas. + +Copyright (C) 2013-2014 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_FIXTURES +#define LOOT_TEST_FIXTURES + +#include "backend/streams.h" + +#include + +#ifdef __GNUC__ // Workaround for GCC linking error. +#pragma message("GCC detected: Defining BOOST_NO_CXX11_SCOPED_ENUMS and BOOST_NO_SCOPED_ENUMS to avoid linking errors for boost::filesystem::copy_file().") +#define BOOST_NO_CXX11_SCOPED_ENUMS +#define BOOST_NO_SCOPED_ENUMS // For older versions. +#endif +#include + +class GameTest : public ::testing::Test { +protected: + GameTest(const boost::filesystem::path& gameDataPath, const boost::filesystem::path& gameLocalPath) + : dataPath(gameDataPath), localPath(gameLocalPath), missingPath("./missing"), db(nullptr) {} + + inline virtual void SetUp() { + ASSERT_NO_THROW(boost::filesystem::create_directories(localPath)); + ASSERT_TRUE(boost::filesystem::exists(localPath)); + + ASSERT_FALSE(boost::filesystem::exists(missingPath)); + + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank.esm")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Different.esm")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Master Dependent.esm")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Different Master Dependent.esm")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank.esp")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Different.esp")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Master Dependent.esp")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Different Master Dependent.esp")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Plugin Dependent.esp")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Different Plugin Dependent.esp")); + + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Blank.esm.missing")); + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Blank.esp.missing")); + + // Ghost a plugin. + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Blank - Master Dependent.esm.ghost")); + ASSERT_NO_THROW(boost::filesystem::rename(dataPath / "Blank - Master Dependent.esm", dataPath / "Blank - Master Dependent.esm.ghost")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Master Dependent.esm.ghost")); + + // Write out an empty file. + loot::ofstream out(dataPath / "EmptyFile.esm"); + out.close(); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "EmptyFile.esm")); + + // Write out an non-empty, non-plugin file. + out.open(dataPath / "NotAPlugin.esm"); + out << "This isn't a valid plugin file."; + out.close(); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "NotAPlugin.esm")); + } + + inline virtual void TearDown() { + // Unghost the ghosted plugin. + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank - Master Dependent.esm.ghost")); + ASSERT_NO_THROW(boost::filesystem::rename(dataPath / "Blank - Master Dependent.esm.ghost", dataPath / "Blank - Master Dependent.esm")); + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Blank - Master Dependent.esm.ghost")); + + // Delete generated files. + ASSERT_NO_THROW(boost::filesystem::remove(dataPath / "EmptyFile.esm")); + ASSERT_NO_THROW(boost::filesystem::remove(dataPath / "NotAPlugin.esm")); + ASSERT_FALSE(boost::filesystem::exists(dataPath / "EmptyFile.esm")); + ASSERT_FALSE(boost::filesystem::exists(dataPath / "NotAPlugin.esm")); + + ASSERT_NO_THROW(loot_destroy_db(db)); + } + + const boost::filesystem::path dataPath; + const boost::filesystem::path localPath; + const boost::filesystem::path missingPath; + + loot_db db; +}; + +class OblivionTest : public GameTest { +protected: + OblivionTest() : GameTest("./Oblivion/Data", "./local/Oblivion") {} + + inline virtual void SetUp() { + GameTest::SetUp(); + + // LOOT expects the game master file to be present, so mock it. + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Oblivion.esm")); + ASSERT_NO_THROW(boost::filesystem::copy_file(dataPath / "Blank.esm", dataPath / "Oblivion.esm")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Oblivion.esm")); + + // Oblivion's load order is decided through timestamps, so reset them to a known order before each test. + std::list loadOrder = { + "Oblivion.esm", + "Blank.esm", + "Blank - Different.esm", + "Blank - Master Dependent.esm", // Ghosted + "Blank - Different Master Dependent.esm", + "Blank.esp", + "Blank - Different.esp", + "Blank - Master Dependent.esp", + "Blank - Different Master Dependent.esp", + "Blank - Plugin Dependent.esp", + "Blank - Different Plugin Dependent.esp" + }; + time_t modificationTime = time(NULL); // Current time. + for (const auto &plugin : loadOrder) { + if (boost::filesystem::exists(dataPath / boost::filesystem::path(plugin + ".ghost"))) { + boost::filesystem::last_write_time(dataPath / boost::filesystem::path(plugin + ".ghost"), modificationTime); + } + else { + boost::filesystem::last_write_time(dataPath / plugin, modificationTime); + } + modificationTime += 60; + } + + // Set Oblivion's active plugins to a known list before running the test. + loot::ofstream activePlugins(localPath / "plugins.txt"); + activePlugins + << "Oblivion.esm" << std::endl + << "Blank.esm" << std::endl; + activePlugins.close(); + } + + inline virtual void TearDown() { + GameTest::TearDown(); + + // Delete the mock Oblivion.esm. + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Oblivion.esm")); + ASSERT_NO_THROW(boost::filesystem::remove(dataPath / "Oblivion.esm")); + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Oblivion.esm")); + + // Delete existing plugins.txt. + ASSERT_NO_THROW(boost::filesystem::remove(localPath / "plugins.txt")); + }; +}; + +class OblivionAPIOperationsTest : public OblivionTest { +protected: + inline virtual void SetUp() { + OblivionTest::SetUp(); + + ASSERT_EQ(loot_ok, loot_create_db(&db, loot_game_tes4, dataPath.parent_path().string().c_str(), localPath.string().c_str())); + } +}; + +class SkyrimTest : public GameTest { +protected: + SkyrimTest() : GameTest("./Skyrim/Data", "./local/Skyrim") {} + + inline virtual void SetUp() { + GameTest::SetUp(); + + // Can't change Skyrim's main master file, so mock it. + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Skyrim.esm")); + ASSERT_NO_THROW(boost::filesystem::copy_file(dataPath / "Blank.esm", dataPath / "Skyrim.esm")); + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Skyrim.esm")); + + // Set Skyrim's load order to a known list before running the test. + loot::ofstream loadOrder(localPath / "loadorder.txt"); + loadOrder + << "Skyrim.esm" << std::endl + << "Blank.esm" << std::endl + << "Blank - Different.esm" << std::endl + << "Blank - Master Dependent.esm" << std::endl // Ghosted + << "Blank - Different Master Dependent.esm" << std::endl + << "Blank.esp" << std::endl + << "Blank - Different.esp" << std::endl + << "Blank - Master Dependent.esp" << std::endl + << "Blank - Different Master Dependent.esp" << std::endl + << "Blank - Plugin Dependent.esp" << std::endl + << "Blank - Different Plugin Dependent.esp" << std::endl; + loadOrder.close(); + + // Set Skyrim's active plugins to a known list before running the test. + loot::ofstream activePlugins(localPath / "plugins.txt"); + activePlugins + << "Blank.esm" << std::endl; + activePlugins.close(); + } + + inline virtual void TearDown() { + GameTest::TearDown(); + + // Delete the mock Skyrim.esm. + ASSERT_TRUE(boost::filesystem::exists(dataPath / "Skyrim.esm")); + ASSERT_NO_THROW(boost::filesystem::remove(dataPath / "Skyrim.esm")); + ASSERT_FALSE(boost::filesystem::exists(dataPath / "Skyrim.esm")); + + // Delete existing plugins.txt and loadorder.txt. + ASSERT_NO_THROW(boost::filesystem::remove(localPath / "plugins.txt")); + ASSERT_NO_THROW(boost::filesystem::remove(localPath / "loadorder.txt")); + }; +}; + +class SkyrimAPIOperationsTest : public SkyrimTest { +protected: + inline virtual void SetUp() { + SkyrimTest::SetUp(); + + ASSERT_EQ(loot_ok, loot_create_db(&db, loot_game_tes5, dataPath.parent_path().string().c_str(), localPath.string().c_str())); + } +}; + +#endif \ No newline at end of file diff --git a/src/tests/main.cpp b/src/tests/main.cpp new file mode 100644 index 00000000..a5824db2 --- /dev/null +++ b/src/tests/main.cpp @@ -0,0 +1,30 @@ +/* LOOT + + A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and + Fallout: New Vegas. + + Copyright (C) 2014 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 + . + */ + +#include "tests/api/api.h" + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 89a4389e58f58e7b1c527d41c476b694b4fd05bb Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 14:24:37 +0100 Subject: [PATCH 02/16] Removed debug message from GetLocalAppDataPath() It couldn't be turned off, so was basically console spam. --- src/backend/helpers.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backend/helpers.cpp b/src/backend/helpers.cpp index 3d71a536..2d37cbf6 100644 --- a/src/backend/helpers.cpp +++ b/src/backend/helpers.cpp @@ -200,7 +200,6 @@ namespace loot { HWND owner = 0; TCHAR path[MAX_PATH]; - BOOST_LOG_TRIVIAL(trace) << "Getting path to %LOCALAPPDATA%."; HRESULT res = SHGetFolderPath(owner, CSIDL_LOCAL_APPDATA, NULL, SHGFP_TYPE_CURRENT, path); if (res == S_OK) From b4b625a8ed10b20db6cbfa6f798eb58ecfc62170 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 14:27:24 +0100 Subject: [PATCH 03/16] Fixed minor version changes being incompatible. If they are, it's because I've screwed up. --- src/api/api.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index 070edfb9..228e5884 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -201,7 +201,7 @@ LOOT_API void loot_cleanup() { // Returns whether this version of LOOT supports the API from the given // LOOT version. Abstracts LOOT API stability policy away from clients. LOOT_API bool loot_is_compatible(const unsigned int versionMajor, const unsigned int versionMinor, const unsigned int versionPatch) { - return versionMajor == loot::g_version_major && versionMinor == loot::g_version_minor; + return versionMajor == loot::g_version_major; } // Returns the version string for this version of LOOT. From 28d1112dcb4ee39b9d4c6c503dfb871cc883c0d1 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 15:05:03 +0100 Subject: [PATCH 04/16] Fixed incorrect linker flags for LOOT.exe. --- CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ffe9664..da38d53a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -210,7 +210,12 @@ target_link_libraries (LOOT ${Boost_LIBRARIES} ${LOOT_GUI_LIBS}) ############################## IF (MSVC) - set_target_properties (LOOT PROPERTIES CMAKE_EXE_LINKER_FLAGS "/SUBSYSTEM:WINDOWS /LARGEADDRESSAWARE") + set (LOOT_LINK_FLAGS "/SUBSYSTEM:WINDOWS /LARGEADDRESSAWARE") + get_target_property (EXISTING_LINK_FLAGS LOOT LINK_FLAGS) + IF (EXISTING_LINK_FLAGS) + set (LOOT_LINK_FLAGS "${EXISTING_LINK_FLAGS} ${LOOT_LINK_FLAGS}") + ENDIF () + set_target_properties (LOOT PROPERTIES LINK_FLAGS ${LOOT_LINK_FLAGS}) ENDIF () From 3ed2c57a6d4258de410316fd946fa24b4d0f42e4 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 15:05:42 +0100 Subject: [PATCH 05/16] Switched back to using Boost.Regex. GCC 4.6 doesn't implement C++11 . --- src/api/api.cpp | 2 -- src/backend/generators.cpp | 2 -- src/backend/helpers.cpp | 5 ++++- src/backend/helpers.h | 4 ++-- src/backend/json.h | 10 +++------- src/backend/metadata.cpp | 7 +++++-- src/backend/parsers.h | 14 +++++++------- 7 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index 228e5884..14266ad6 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -36,12 +36,10 @@ #include #include #include -#include #include #include #include -#include #include const unsigned int loot_ok = loot::error::ok; diff --git a/src/backend/generators.cpp b/src/backend/generators.cpp index 4bdcf3bc..3dc73f8b 100644 --- a/src/backend/generators.cpp +++ b/src/backend/generators.cpp @@ -31,8 +31,6 @@ along with LOOT. If not, see #include #include -#include - using namespace std; namespace YAML { diff --git a/src/backend/helpers.cpp b/src/backend/helpers.cpp index 2d37cbf6..a3167a16 100644 --- a/src/backend/helpers.cpp +++ b/src/backend/helpers.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -41,7 +42,6 @@ #include #include #include -#include #ifdef _WIN32 # ifndef UNICODE @@ -59,6 +59,9 @@ namespace loot { using namespace std; using boost::algorithm::replace_all; using boost::algorithm::replace_first; + using boost::regex; + using boost::regex_match; + using boost::regex_search; namespace karma = boost::spirit::karma; namespace fs = boost::filesystem; namespace lc = boost::locale; diff --git a/src/backend/helpers.h b/src/backend/helpers.h index feae3c77..29f003f4 100644 --- a/src/backend/helpers.h +++ b/src/backend/helpers.h @@ -29,13 +29,13 @@ #include #include -#include +#include #include namespace loot { /// Array used to try each of the expressions defined using /// an iteration for each of them. - extern const std::regex version_checks[7]; + extern const boost::regex version_checks[7]; ////////////////////////////////////////////////////////////////////////// // Helper functions diff --git a/src/backend/json.h b/src/backend/json.h index ab9fec68..73fae02a 100644 --- a/src/backend/json.h +++ b/src/backend/json.h @@ -25,13 +25,11 @@ along with LOOT. If not, see #ifndef __LOOT_JSON__ #define __LOOT_JSON__ - #include -#include +#include namespace loot { - // Handy class for turning YAML objects into JSON and vice-versa. class JSON { public: @@ -41,7 +39,6 @@ namespace loot { } inline static std::string stringify(const YAML::Node& yaml) { - YAML::Emitter out; out.SetOutputCharset(YAML::EscapeNonAscii); out.SetStringFormat(YAML::DoubleQuoted); @@ -65,14 +62,13 @@ namespace loot { // Using the definition at . // Version numbers should be kept as strings though. - std::regex numbers("\"(?!version)([^\"]+)\": \"(-?(?:0|[1-9]\\d*)(?:\\.\\d+)?(?:[eE][+-]?\\d+)?)\"", std::regex::ECMAScript); + boost::regex numbers("\"(?!version)([^\"]+)\": \"(-?(?:0|[1-9]\\d*)(?:\\.\\d+)?(?:[eE][+-]?\\d+)?)\"", boost::regex::ECMAScript); - json = std::regex_replace(json, numbers, "\"$1\": $2"); + json = boost::regex_replace(json, numbers, "\"$1\": $2"); return json; } }; } - #endif \ No newline at end of file diff --git a/src/backend/metadata.cpp b/src/backend/metadata.cpp index c6c456f9..ba1774d9 100644 --- a/src/backend/metadata.cpp +++ b/src/backend/metadata.cpp @@ -27,16 +27,19 @@ #include "parsers.h" #include "streams.h" -#include - #include #include #include #include #include +#include using namespace std; +using boost::regex; +using boost::regex_match; +using boost::regex_search; +using boost::smatch; namespace loot { namespace lc = boost::locale; diff --git a/src/backend/parsers.h b/src/backend/parsers.h index 97d50972..4984bda2 100644 --- a/src/backend/parsers.h +++ b/src/backend/parsers.h @@ -39,7 +39,7 @@ #include "error.h" #include -#include +#include #include @@ -555,11 +555,11 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << "Checking to see if any files matching the regex \"" << regexStr << "\" exist."; - std::regex sepReg("/|(\\\\\\\\)", std::regex::ECMAScript | std::regex::icase); + boost::regex sepReg("/|(\\\\\\\\)", boost::regex::ECMAScript | boost::regex::icase); std::vector components; - std::sregex_token_iterator it(regexStr.begin(), regexStr.end(), sepReg, -1); - std::sregex_token_iterator itend; + boost::sregex_token_iterator it(regexStr.begin(), regexStr.end(), sepReg, -1); + boost::sregex_token_iterator itend; for (; it != itend; ++it) { components.push_back(*it); } @@ -589,9 +589,9 @@ namespace loot { return; } - std::regex reg; + boost::regex reg; try { - reg = std::regex(filename, std::regex::ECMAScript | std::regex::icase); + reg = boost::regex(filename, boost::regex::ECMAScript | boost::regex::icase); } catch (std::exception& /*e*/) { BOOST_LOG_TRIVIAL(error) << "Invalid regex string:" << filename; @@ -599,7 +599,7 @@ namespace loot { } for (boost::filesystem::directory_iterator itr(parent_path); itr != boost::filesystem::directory_iterator(); ++itr) { - if (regex_match(itr->path().filename().string(), reg)) { + if (boost::regex_match(itr->path().filename().string(), reg)) { result = true; BOOST_LOG_TRIVIAL(trace) << "Matching file found: " << itr->path(); return; From 3adf066ea3e0d302f3caf5f7b8d9cb96ba9f1cd7 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 17:59:49 +0100 Subject: [PATCH 06/16] Fixed false successes for db creation. * If not on Windows, a local data path must be supplied. * The supplied paths must be valid directories. * Added general exception catching. --- src/api/api.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index 14266ad6..f891d6e9 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -248,18 +248,30 @@ LOOT_API unsigned int loot_create_db(loot_db * const db, boost::filesystem::path game_local_path = ""; if (gameLocalPath != nullptr) game_local_path = gameLocalPath; +#ifndef _WIN32 + else + return c_error(loot_error_invalid_args, "A local data path must be supplied on non-Windows platforms."); +#endif - loot_db retVal = {0}; try { - retVal = new _loot_db_int(clientGame, game_path, game_local_path); - } + // Check for valid paths. + if (gamePath != nullptr && !boost::filesystem::is_directory(gamePath)) + return c_error(loot_error_invalid_args, "Given game path \"" + std::string(gamePath) + "\" is not a valid directory."); + + if (gameLocalPath != nullptr && !boost::filesystem::is_directory(gameLocalPath)) + return c_error(loot_error_invalid_args, "Given local data path \"" + std::string(gameLocalPath) + "\" is not a valid directory."); + + *db = new _loot_db_int(clientGame, game_path, game_local_path); +} catch (loot::error& e) { return c_error(e); } catch (std::bad_alloc& e) { return c_error(loot_error_no_mem, e.what()); } - *db = retVal; + catch (std::exception& e) { + return c_error(loot_error_invalid_args, e.what()); + } return loot_ok; } From 53ba49521c5011f28be95a1606b56751312b0531 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 11 Oct 2014 18:00:03 +0100 Subject: [PATCH 07/16] Fixed libloadorder init/error handling. --- src/backend/game.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/backend/game.cpp b/src/backend/game.cpp index 37871bfd..9f110b8e 100644 --- a/src/backend/game.cpp +++ b/src/backend/game.cpp @@ -472,8 +472,9 @@ namespace loot { void Game::InitLibloHandle() { const char * gameLocalDataPath = nullptr; - if (!_gameLocalDataPath.empty()) - gameLocalDataPath = _gameLocalDataPath.string().c_str(); + std::string localAppData = _gameLocalDataPath.string(); + if (!localAppData.empty()) + gameLocalDataPath = localAppData.c_str(); int ret; if (Id() == Game::tes4) @@ -509,6 +510,8 @@ namespace loot { string err; lo_get_error_message(&e); lo_destroy_handle(gh); + gh = nullptr; + if (e == nullptr) { BOOST_LOG_TRIVIAL(error) << "libloadorder failed to initialise game master file support. Details could not be fetched."; err = lc::translate("libloadorder failed to initialise game master file support. Details could not be fetched.").str(); @@ -532,7 +535,6 @@ namespace loot { const char * e = nullptr; string err; lo_get_error_message(&e); - lo_destroy_handle(gh); if (e == nullptr) { BOOST_LOG_TRIVIAL(error) << "libloadorder failed to get the active plugins list. Details could not be fetched."; err = lc::translate("libloadorder failed to get the active plugins list. Details could not be fetched.").str(); @@ -565,7 +567,6 @@ namespace loot { const char * e = nullptr; string err; lo_get_error_message(&e); - lo_destroy_handle(gh); if (e == nullptr) { BOOST_LOG_TRIVIAL(error) << "libloadorder failed to get the load order. Details could not be fetched."; err = lc::translate("libloadorder failed to get the load order. Details could not be fetched.").str(); @@ -591,7 +592,6 @@ namespace loot { const char * e = nullptr; string err; lo_get_error_message(&e); - lo_destroy_handle(gh); if (e == nullptr) { BOOST_LOG_TRIVIAL(error) << "libloadorder failed to set the load order. Details could not be fetched."; err = lc::translate("libloadorder failed to set the load order. Details could not be fetched.").str(); From 0c4441cb9c85bda676ae8be0fd8a5648a589ed63 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 19:19:28 +0100 Subject: [PATCH 08/16] Added more tests. New tests are for loot_load_lists, loot_eval_lists and loot_update_masterlist. --- src/tests/api/api.h | 59 ++++++++++++++++++++++++++++++++++++++++++++ src/tests/fixtures.h | 15 ++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/tests/api/api.h b/src/tests/api/api.h index 53781f4e..9b3894f3 100644 --- a/src/tests/api/api.h +++ b/src/tests/api/api.h @@ -132,4 +132,63 @@ TEST_F(OblivionTest, CreateDbHandlesNullLocalPath) { TEST(GameHandleDestroyTest, HandledNullInput) { ASSERT_NO_THROW(loot_destroy_db(NULL)); } + +TEST_F(OblivionAPIOperationsTest, UpdateMasterlist) { + bool updated; + EXPECT_EQ(loot_error_invalid_args, loot_update_masterlist(NULL, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", "master", &updated)); + EXPECT_EQ(loot_error_invalid_args, loot_update_masterlist(db, NULL, "https://github.com/loot/oblivion.git", "master", &updated)); + EXPECT_EQ(loot_error_invalid_args, loot_update_masterlist(db, masterlistPath.string().c_str(), NULL, "master", &updated)); + EXPECT_EQ(loot_error_invalid_args, loot_update_masterlist(db, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", NULL, &updated)); + EXPECT_EQ(loot_error_invalid_args, loot_update_masterlist(db, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", "master", NULL)); + + EXPECT_EQ(loot_ok, loot_update_masterlist(db, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", "master", &updated)); + EXPECT_TRUE(updated); +} + +TEST_F(OblivionAPIOperationsTest, LoadLists) { + EXPECT_EQ(loot_error_invalid_args, loot_load_lists(NULL, masterlistPath.string().c_str(), NULL)); + EXPECT_EQ(loot_error_invalid_args, loot_load_lists(db, NULL, NULL)); + + bool updated; + ASSERT_EQ(loot_ok, loot_update_masterlist(db, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", "master", &updated)); + EXPECT_EQ(loot_error_path_not_found, loot_load_lists(db, masterlistPath.string().c_str(), NULL)); + + ASSERT_NO_THROW(boost::filesystem::copy(masterlistPath, userlistPath)); + EXPECT_EQ(loot_error_path_not_found, loot_load_lists(db, masterlistPath.string().c_str(), userlistPath.string().c_str())); +} + +TEST_F(OblivionAPIOperationsTest, EvalLists) { + // No lists loaded. + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_any)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_english)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_spanish)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_russian)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_french)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_chinese)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_polish)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_brazilian_portuguese)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_finnish)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_german)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_danish)); + + // Invalid args. + EXPECT_EQ(loot_error_invalid_args, loot_eval_lists(NULL, loot_lang_any)); + EXPECT_EQ(loot_error_invalid_args, loot_eval_lists(db, (unsigned int)-1)); + + // Now test different languages with a list loaded. + bool updated; + ASSERT_EQ(loot_ok, loot_update_masterlist(db, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", "master", &updated)); + ASSERT_EQ(loot_ok, loot_load_lists(db, masterlistPath.string().c_str(), NULL)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_any)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_english)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_spanish)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_russian)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_french)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_chinese)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_polish)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_brazilian_portuguese)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_finnish)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_german)); + EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_danish)); +} #endif diff --git a/src/tests/fixtures.h b/src/tests/fixtures.h index 179a61f0..2fe04627 100644 --- a/src/tests/fixtures.h +++ b/src/tests/fixtures.h @@ -39,12 +39,15 @@ along with LOOT. If not, see class GameTest : public ::testing::Test { protected: GameTest(const boost::filesystem::path& gameDataPath, const boost::filesystem::path& gameLocalPath) - : dataPath(gameDataPath), localPath(gameLocalPath), missingPath("./missing"), db(nullptr) {} + : dataPath(gameDataPath), localPath(gameLocalPath), missingPath("./missing"), masterlistPath(localPath / "masterlist.yaml"), userlistPath(localPath / "userlist.yaml"), db(nullptr) {} inline virtual void SetUp() { ASSERT_NO_THROW(boost::filesystem::create_directories(localPath)); ASSERT_TRUE(boost::filesystem::exists(localPath)); + ASSERT_FALSE(boost::filesystem::exists(masterlistPath)); + ASSERT_FALSE(boost::filesystem::exists(userlistPath)); + ASSERT_FALSE(boost::filesystem::exists(localPath / ".git")); ASSERT_FALSE(boost::filesystem::exists(missingPath)); ASSERT_TRUE(boost::filesystem::exists(dataPath / "Blank.esm")); @@ -90,6 +93,13 @@ protected: ASSERT_FALSE(boost::filesystem::exists(dataPath / "EmptyFile.esm")); ASSERT_FALSE(boost::filesystem::exists(dataPath / "NotAPlugin.esm")); + // Masterlist & userlist may have been created during test, so delete them. + ASSERT_NO_THROW(boost::filesystem::remove(masterlistPath)); + ASSERT_NO_THROW(boost::filesystem::remove(userlistPath)); + + // Also remove the ".git" folder if it has been created. + ASSERT_NO_THROW(boost::filesystem::remove_all(localPath / ".git")); + ASSERT_NO_THROW(loot_destroy_db(db)); } @@ -97,6 +107,9 @@ protected: const boost::filesystem::path localPath; const boost::filesystem::path missingPath; + const boost::filesystem::path masterlistPath; + const boost::filesystem::path userlistPath; + loot_db db; }; From 573bd3875785aadde59a2c49b7903df76ae8c16e Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 19:19:45 +0100 Subject: [PATCH 09/16] Fixed git_error not having an API error code. --- src/api/api.cpp | 1 + src/api/api.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/api/api.cpp b/src/api/api.cpp index f891d6e9..55a56c52 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -53,6 +53,7 @@ const unsigned int loot_error_invalid_args = loot::error::invalid_args; const unsigned int loot_error_no_tag_map = loot::error::no_tag_map; const unsigned int loot_error_path_not_found = loot::error::path_not_found; const unsigned int loot_error_no_game_detected = loot::error::no_game_detected; +const unsigned int loot_error_git_error = loot::error::git_error; const unsigned int loot_error_windows_error = loot::error::windows_error; const unsigned int loot_error_sorting_error = loot::error::sorting_error; const unsigned int loot_return_max = loot_error_sorting_error; diff --git a/src/api/api.h b/src/api/api.h index 9c390e97..da880fcb 100644 --- a/src/api/api.h +++ b/src/api/api.h @@ -182,6 +182,7 @@ extern "C" LOOT_API extern const unsigned int loot_error_no_tag_map; /**< No Bash Tag map has been generated yet. */ LOOT_API extern const unsigned int loot_error_path_not_found; /**< A file or folder path could not be found. */ LOOT_API extern const unsigned int loot_error_no_game_detected; /**< The given game could not be found. */ + LOOT_API extern const unsigned int loot_error_git_error; /**< An error occurred while performing a git operation (updating or getting the masterlist version). */ LOOT_API extern const unsigned int loot_error_windows_error; /**< An error occurred during a call to the Windows API. */ LOOT_API extern const unsigned int loot_error_sorting_error; /**< An error occurred while sorting plugins. */ From 1dd8687e28f7f82f7dce01a9cc3adc911e233be2 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 19:33:57 +0100 Subject: [PATCH 10/16] Fixed bad permissions on LOOT .git folders. They were being fixed when LOOT needed to delete them, but left with too restrictive permissions otherwise. Now the permissions are write access for all on successful operations too. --- src/backend/git.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/backend/git.cpp b/src/backend/git.cpp index 7f92b4b1..d35960cf 100644 --- a/src/backend/git.cpp +++ b/src/backend/git.cpp @@ -40,6 +40,16 @@ namespace fs = boost::filesystem; namespace lc = boost::locale; namespace loot { + + // Removes the read-only flag from some files in git repositories created by libgit2. + void FixRepoPermissions(const fs::path& path) { + BOOST_LOG_TRIVIAL(trace) << "Recursively setting write permission on directory: " << path; + for (fs::recursive_directory_iterator it(path); it != fs::recursive_directory_iterator(); ++it) { + BOOST_LOG_TRIVIAL(trace) << "Setting write permission for: " << it->path(); + fs::permissions(it->path(), fs::add_perms | fs::owner_write | fs::group_write | fs::others_write); + } + } + struct git_handler { public: git_handler() : @@ -58,7 +68,10 @@ namespace loot { buf({0}) {} ~git_handler() { + string path(git_repository_path(repo)); free(); + + FixRepoPermissions(path); } void free() { @@ -131,15 +144,6 @@ namespace loot { return git_repository_open_ext(NULL, path.string().c_str(), GIT_REPOSITORY_OPEN_NO_SEARCH, NULL) == 0; } - // Removes the read-only flag from some files in git repositories created by libgit2. - void FixRepoPermissions(const fs::path& path) { - BOOST_LOG_TRIVIAL(trace) << "Recursively setting write permission on directory: " << path; - for (fs::recursive_directory_iterator it(path); it != fs::recursive_directory_iterator(); ++it) { - BOOST_LOG_TRIVIAL(trace) << "Setting write permission for: " << it->path(); - fs::permissions(it->path(), fs::add_perms | fs::owner_write | fs::group_write | fs::others_write); - } - } - int diffFileCallback(const git_diff_delta *delta, float progress, void * payload) { BOOST_LOG_TRIVIAL(trace) << "Checking diff for: " << delta->old_file.path; if (strcmp(delta->old_file.path, "masterlist.yaml") == 0) { From 6af3dbf6b11e5f95b76224e67b2b5ed89449f958 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 19:43:23 +0100 Subject: [PATCH 11/16] Fixed API loot_load_lists bugs. The userlist path was being ignored, and the masterlist loaded again as the userlist. The function now errors if either of the given paths does not exist. --- src/api/api.cpp | 8 +++++++- src/tests/api/api.h | 7 ++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index 55a56c52..6f5c789d 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -303,6 +303,9 @@ LOOT_API unsigned int loot_load_lists(loot_db db, const char * const masterlistP // We don't want to update the masterlist too. temp.MetadataList::Load(masterlistPath); } + else { + return c_error(loot_error_path_not_found, std::string("The given masterlist path does not exist: ") + masterlistPath); + } } catch (std::exception& e) { return c_error(loot_error_parse_fail, e.what()); @@ -311,7 +314,10 @@ LOOT_API unsigned int loot_load_lists(loot_db db, const char * const masterlistP try { if (userlistPath != nullptr) { if (boost::filesystem::exists(userlistPath)) { - userTemp.Load(masterlistPath); + userTemp.Load(userlistPath); + } + else { + return c_error(loot_error_path_not_found, std::string("The given userlist path does not exist: ") + userlistPath); } } } diff --git a/src/tests/api/api.h b/src/tests/api/api.h index 9b3894f3..2d306a58 100644 --- a/src/tests/api/api.h +++ b/src/tests/api/api.h @@ -149,12 +149,13 @@ TEST_F(OblivionAPIOperationsTest, LoadLists) { EXPECT_EQ(loot_error_invalid_args, loot_load_lists(NULL, masterlistPath.string().c_str(), NULL)); EXPECT_EQ(loot_error_invalid_args, loot_load_lists(db, NULL, NULL)); + EXPECT_EQ(loot_error_path_not_found, loot_load_lists(db, masterlistPath.string().c_str(), NULL)); bool updated; ASSERT_EQ(loot_ok, loot_update_masterlist(db, masterlistPath.string().c_str(), "https://github.com/loot/oblivion.git", "master", &updated)); - EXPECT_EQ(loot_error_path_not_found, loot_load_lists(db, masterlistPath.string().c_str(), NULL)); - - ASSERT_NO_THROW(boost::filesystem::copy(masterlistPath, userlistPath)); EXPECT_EQ(loot_error_path_not_found, loot_load_lists(db, masterlistPath.string().c_str(), userlistPath.string().c_str())); + + ASSERT_NO_THROW(boost::filesystem::copy(masterlistPath, userlistPath)); + EXPECT_EQ(loot_ok, loot_load_lists(db, masterlistPath.string().c_str(), userlistPath.string().c_str())); } TEST_F(OblivionAPIOperationsTest, EvalLists) { From 86e24c4feaed4d32a0b79bb8918e9f8a14926925 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 19:49:32 +0100 Subject: [PATCH 12/16] Fixed loot_eval_lists allowing invalid languages. --- src/api/api.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/api/api.cpp b/src/api/api.cpp index 6f5c789d..43ba6660 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -364,6 +364,18 @@ LOOT_API unsigned int loot_load_lists(loot_db db, const char * const masterlistP LOOT_API unsigned int loot_eval_lists(loot_db db, const unsigned int language) { if (db == nullptr) return c_error(loot_error_invalid_args, "Null pointer passed."); + if (language != loot_lang_any + && language != loot_lang_english + && language != loot_lang_spanish + && language != loot_lang_russian + && language != loot_lang_french + && language != loot_lang_chinese + && language != loot_lang_polish + && language != loot_lang_brazilian_portuguese + && language != loot_lang_finnish + && language != loot_lang_german + && language != loot_lang_danish) + return c_error(loot_error_invalid_args, "Invalid language code given."); // Clear caches before evaluating conditions. db->conditionCache.clear(); From 4c01a6f073cd3ed6c3def56ec8796cb76df936ff Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 21:38:15 +0100 Subject: [PATCH 13/16] Fixed crash at end of masterlist update. Was due to trying to get the repo path from a closed repo handle. --- src/backend/git.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/backend/git.cpp b/src/backend/git.cpp index d35960cf..60a7ae2c 100644 --- a/src/backend/git.cpp +++ b/src/backend/git.cpp @@ -68,10 +68,18 @@ namespace loot { buf({0}) {} ~git_handler() { - string path(git_repository_path(repo)); + string path; + if (repo != nullptr) + path = git_repository_path(repo); + free(); - FixRepoPermissions(path); + if (!path.empty()) { + try { + FixRepoPermissions(path); + } + catch (exception&) {} + } } void free() { From 9eb2362064bbc336482a5a15edc1b3fbc3453512 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 23 Oct 2014 21:38:29 +0100 Subject: [PATCH 14/16] Added test for loot_sort_plugins. --- src/tests/api/api.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/tests/api/api.h b/src/tests/api/api.h index 2d306a58..93128e7c 100644 --- a/src/tests/api/api.h +++ b/src/tests/api/api.h @@ -192,4 +192,37 @@ TEST_F(OblivionAPIOperationsTest, EvalLists) { EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_german)); EXPECT_EQ(loot_ok, loot_eval_lists(db, loot_lang_danish)); } + +TEST_F(OblivionAPIOperationsTest, SortPlugins) { + char ** sortedPlugins; + size_t numPlugins; + EXPECT_EQ(loot_error_invalid_args, loot_sort_plugins(NULL, &sortedPlugins, &numPlugins)); + EXPECT_EQ(loot_error_invalid_args, loot_sort_plugins(db, NULL, &numPlugins)); + EXPECT_EQ(loot_error_invalid_args, loot_sort_plugins(db, &sortedPlugins, NULL)); + + EXPECT_EQ(loot_ok, loot_sort_plugins(db, &sortedPlugins, &numPlugins)); + + // Expected order was obtained from running the API function once. + std::list expectedOrder = { + "Blank.esm", + "Blank - Master Dependent.esm", + "Oblivion.esm", + "Blank - Different.esm", + "Blank - Different Master Dependent.esm", + "NotAPlugin.esm", + "EmptyFile.esm", + "Blank - Master Dependent.esp", + "Blank.esp", + "Blank - Plugin Dependent.esp", + "Blank - Different Master Dependent.esp", + "Blank - Different.esp", + "Blank - Different Plugin Dependent.esp" + }; + std::list actualOrder; + for (size_t i = 0; i < numPlugins; ++i) { + actualOrder.push_back(sortedPlugins[i]); + } + EXPECT_EQ(13, numPlugins); + EXPECT_EQ(expectedOrder, actualOrder); +} #endif From 6ee60d39aaba4dd1c26bd040a5c9d95dc9c19672 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 1 Nov 2014 12:11:44 +0000 Subject: [PATCH 15/16] Fixed missing parameter in Doxygen comment. --- src/api/api.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/api/api.h b/src/api/api.h index da880fcb..f02949e4 100644 --- a/src/api/api.h +++ b/src/api/api.h @@ -469,6 +469,8 @@ extern "C" * @brief Get the given masterlist's revision. * @details Getting a masterlist's revision is only possible if it is * found inside a local Git repository. + * @param db + * The database the function acts on. * @param masterlistPath * A string containing the relative or absolute path to the masterlist * file that should be queried. From 791f517bb487c3b4b8cd3f627c09d424d93ac0ae Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 1 Nov 2014 12:18:49 +0000 Subject: [PATCH 16/16] [Doxygen] Truncate paths and create shorter PDFs. --- docs/Doxyfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Doxyfile b/docs/Doxyfile index da82cb3a..e4916bf7 100644 --- a/docs/Doxyfile +++ b/docs/Doxyfile @@ -119,7 +119,7 @@ INLINE_INHERITED_MEMB = NO # path before files name in the file list and in the header files. If set # to NO the shortest path that makes the file name unique will be used. -FULL_PATH_NAMES = YES +FULL_PATH_NAMES = NO # If the FULL_PATH_NAMES tag is set to YES then the STRIP_FROM_PATH tag # can be used to strip a user-defined part of the path. Stripping is @@ -1237,7 +1237,7 @@ MAKEINDEX_CMD_NAME = makeindex # LaTeX documents. This may be useful for small projects and may help to # save some trees in general. -COMPACT_LATEX = NO +COMPACT_LATEX = YES # The PAPER_TYPE tag can be used to set the paper type that is used # by the printer. Possible values are: a4, letter, legal and