From b517e6ef82fbf38943541e1fff179019220aa5f0 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Thu, 16 May 2024 23:59:33 +0300 Subject: [PATCH 1/3] feat: make system tests skipable via env --- cmake/Boost_DD.cmake | 2 +- tests/fixtures/fixtures.cpp | 99 +++++++++++++++++++++---------------- tests/fixtures/fixtures.h | 39 +++++++++------ 3 files changed, 81 insertions(+), 59 deletions(-) diff --git a/cmake/Boost_DD.cmake b/cmake/Boost_DD.cmake index 98ee145..ab73aba 100644 --- a/cmake/Boost_DD.cmake +++ b/cmake/Boost_DD.cmake @@ -6,7 +6,7 @@ include_guard(GLOBAL) find_package(Boost 1.85) if(NOT Boost_FOUND) - message(STATUS "Boost v1.85.x package not found in system. Falling back to Fetch.") + message(STATUS "Boost v1.85.x package not found in the system. Falling back to FetchContent.") include(FetchContent) # Avoid warning about DOWNLOAD_EXTRACT_TIMESTAMP in CMake 3.24: diff --git a/tests/fixtures/fixtures.cpp b/tests/fixtures/fixtures.cpp index 819cc8c..c3258cf 100644 --- a/tests/fixtures/fixtures.cpp +++ b/tests/fixtures/fixtures.cpp @@ -1,6 +1,9 @@ // header include #include "fixtures.h" +// system includes +#include + // local includes #include "displaydevice/logging.h" @@ -11,6 +14,11 @@ BaseTest::BaseTest(): void BaseTest::SetUp() { + if (const auto skip_reason { skipTest() }; !skip_reason.empty()) { + m_test_skipped_at_setup = true; + GTEST_SKIP() << skip_reason; + } + // todo: only run this one time, instead of every time a test is run // see: https://stackoverflow.com/questions/2435277/googletest-accessing-the-environment-from-a-test // get command line args from the test executable @@ -38,6 +46,13 @@ BaseTest::SetUp() { void BaseTest::TearDown() { + if (m_test_skipped_at_setup) { + // We are not using the IsSkipped() state here. Here we are skipping + // teardown, because we have skipped the setup entirely, but during normal + // skips we still want to do teardown. + return; + } + display_device::Logger::get().setCustomCallback(nullptr); // restore the default callback to avoid potential leaks std::cout.rdbuf(m_sbuf); // restore cout buffer @@ -67,6 +82,28 @@ BaseTest::TearDown() { } } +bool +BaseTest::isSystemTest() const { + return false; +} + +std::string +BaseTest::skipTest() const { + if (isSystemTest()) { + const static bool is_system_test_skipable { + []() { + const std::string value { std::getenv("SKIP_SYSTEM_TESTS") }; + return value == "1"; + }() + }; + + if (is_system_test_skipable) { + return "Skipping, this system test is disabled via ENV."; + } + } + return {}; +} + int BaseTest::exec(const char *cmd) { std::array buffer {}; @@ -81,62 +118,38 @@ BaseTest::exec(const char *cmd) { while (fgets(buffer.data(), buffer.size(), m_pipe_stderr) != nullptr) { m_stderr_buffer << buffer.data(); } - int returnCode = pclose(m_pipe_stdout); + int return_code = pclose(m_pipe_stdout); m_pipe_stdout = nullptr; - if (returnCode != 0) { + if (return_code != 0) { std::cout << "Error: " << m_stderr_buffer.str() << std::endl - << "Return code: " << returnCode << std::endl; + << "Return code: " << return_code << std::endl; } - return returnCode; -} - -void -LinuxTest::SetUp() { -#ifndef __linux__ - GTEST_SKIP_("Skipping, this test is for Linux only."); -#endif - BaseTest::SetUp(); + return return_code; } -void -LinuxTest::TearDown() { +std::string +LinuxTest::skipTest() const { #ifndef __linux__ - // This a noop case to skip the teardown - return; -#endif - BaseTest::TearDown(); -} - -void -MacOSTest::SetUp() { -#if !defined(__APPLE__) || !defined(__MACH__) - GTEST_SKIP_("Skipping, this test is for macOS only."); + return "Skipping, this test is for Linux only."; +#else + return BaseTest::skipTest(); #endif - BaseTest::SetUp(); } -void -MacOSTest::TearDown() { +std::string +MacOSTest::skipTest() const { #if !defined(__APPLE__) || !defined(__MACH__) - // This a noop case to skip the teardown - return; + return "Skipping, this test is for macOS only."; +#else + return BaseTest::skipTest(); #endif - BaseTest::TearDown(); } -void -WindowsTest::SetUp() { -#ifndef _WIN32 - GTEST_SKIP_("Skipping, this test is for Windows only."); -#endif - BaseTest::SetUp(); -} - -void -WindowsTest::TearDown() { +std::string +WindowsTest::skipTest() const { #ifndef _WIN32 - // This a noop case to skip the teardown - return; + return "Skipping, this test is for Windows only."; +#else + return BaseTest::skipTest(); #endif - BaseTest::TearDown(); } diff --git a/tests/fixtures/fixtures.h b/tests/fixtures/fixtures.h index 9f96bca..63d51f7 100644 --- a/tests/fixtures/fixtures.h +++ b/tests/fixtures/fixtures.h @@ -59,6 +59,21 @@ class BaseTest: public ::testing::Test { void TearDown() override; + /** + * @brief Check if the test interacts/modifies with the system settings. + * @returns True if it does, false otherwise. + * @note By setting SKIP_SYSTEM_TESTS=1 env, these tests will be skipped (useful during development). + */ + [[nodiscard]] virtual bool + isSystemTest() const; + + /** + * @brief Skip the test by specifying the reason. + * @returns A non-empty string (reason) if test needs to be skipped, empty string otherwise. + */ + [[nodiscard]] virtual std::string + skipTest() const; + int exec(const char *cmd); @@ -72,31 +87,25 @@ class BaseTest: public ::testing::Test { std::streambuf *m_sbuf; FILE *m_pipe_stdout; FILE *m_pipe_stderr; + +private: + bool m_test_skipped_at_setup { false }; }; class LinuxTest: public BaseTest { protected: - void - SetUp() override; - - void - TearDown() override; + [[nodiscard]] std::string + skipTest() const override; }; class MacOSTest: public BaseTest { protected: - void - SetUp() override; - - void - TearDown() override; + [[nodiscard]] std::string + skipTest() const override; }; class WindowsTest: public BaseTest { protected: - void - SetUp() override; - - void - TearDown() override; + [[nodiscard]] std::string + skipTest() const override; }; From d69a02a11b2ca7293d9d72691a1f28712f0b8496 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Fri, 17 May 2024 00:09:21 +0300 Subject: [PATCH 2/3] disable topology tests --- tests/fixtures/fixtures.cpp | 6 +++--- tests/unit/windows/test_windisplaydevicetopology.cpp | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/fixtures/fixtures.cpp b/tests/fixtures/fixtures.cpp index c3258cf..6a2fb90 100644 --- a/tests/fixtures/fixtures.cpp +++ b/tests/fixtures/fixtures.cpp @@ -90,15 +90,15 @@ BaseTest::isSystemTest() const { std::string BaseTest::skipTest() const { if (isSystemTest()) { - const static bool is_system_test_skipable { + const static bool is_system_test_skippable { []() { const std::string value { std::getenv("SKIP_SYSTEM_TESTS") }; return value == "1"; }() }; - if (is_system_test_skipable) { - return "Skipping, this system test is disabled via ENV."; + if (is_system_test_skippable) { + return "Skipping, this system test is disabled via SKIP_SYSTEM_TESTS=1 env."; } } return {}; diff --git a/tests/unit/windows/test_windisplaydevicetopology.cpp b/tests/unit/windows/test_windisplaydevicetopology.cpp index 8a5f816..8922ab7 100644 --- a/tests/unit/windows/test_windisplaydevicetopology.cpp +++ b/tests/unit/windows/test_windisplaydevicetopology.cpp @@ -17,6 +17,11 @@ namespace { // Test fixture(s) for this file class WinDisplayDeviceTopology: public BaseTest { public: + bool + isSystemTest() const override { + return true; + } + std::optional> getAvailableDevices() { const auto all_devices { m_layer->queryDisplayConfig(display_device::QueryType::All) }; From f6b5c66d7e2e4189fe742925d518257e1bd40f18 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Fri, 17 May 2024 00:21:23 +0300 Subject: [PATCH 3/3] meh --- tests/fixtures/fixtures.cpp | 5 +---- tests/fixtures/testutils.cpp | 11 +++++++++++ tests/fixtures/testutils.h | 17 +++++++++++++---- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/tests/fixtures/fixtures.cpp b/tests/fixtures/fixtures.cpp index 6a2fb90..82ba067 100644 --- a/tests/fixtures/fixtures.cpp +++ b/tests/fixtures/fixtures.cpp @@ -1,9 +1,6 @@ // header include #include "fixtures.h" -// system includes -#include - // local includes #include "displaydevice/logging.h" @@ -92,7 +89,7 @@ BaseTest::skipTest() const { if (isSystemTest()) { const static bool is_system_test_skippable { []() { - const std::string value { std::getenv("SKIP_SYSTEM_TESTS") }; + const auto value { getEnv("SKIP_SYSTEM_TESTS") }; return value == "1"; }() }; diff --git a/tests/fixtures/testutils.cpp b/tests/fixtures/testutils.cpp index 503dc45..1947562 100644 --- a/tests/fixtures/testutils.cpp +++ b/tests/fixtures/testutils.cpp @@ -1,6 +1,9 @@ // header include #include "testutils.h" +// system includes +#include + // system includes #include #include @@ -26,3 +29,11 @@ setEnv(const std::string &name, const std::string &value) { return setenv(name.c_str(), value.c_str(), 1); #endif } + +std::optional +getEnv(const std::string &name) { + if (const auto value { std::getenv(name.c_str()) }; value) { + return std::string { value }; + } + return std::nullopt; +} diff --git a/tests/fixtures/testutils.h b/tests/fixtures/testutils.h index f49271b..a73de6a 100644 --- a/tests/fixtures/testutils.h +++ b/tests/fixtures/testutils.h @@ -1,20 +1,29 @@ #pragma once // system includes +#include #include /** * @brief Test regular expression against string. - * @return True if string matches the regex, false otherwise + * @return True if string matches the regex, false otherwise. */ bool testRegex(const std::string &test_pattern, const std::string ®ex_pattern); /** * @brief Set an environment variable. - * @param name Name of the environment variable - * @param value Value of the environment variable - * @return 0 on success, non-zero error code on failure + * @param name Name of the environment variable. + * @param value Value of the environment variable. + * @return 0 on success, non-zero error code on failure. */ int setEnv(const std::string &name, const std::string &value); + +/** + * @brief Get an environment variable. + * @param name Name of the environment variable. + * @return String value of the variable or an empty optional otherwise. + */ +std::optional +getEnv(const std::string &name);