From c865add8213ac2580d803d7bb0f9bb6814daf8fc Mon Sep 17 00:00:00 2001 From: patrick96 Date: Sat, 7 Apr 2018 22:16:55 +0200 Subject: [PATCH] refactor(tests): Migrate to googletest googletest (gtest) is more feature rich than the current implementation which only provides expect() which is basically an assertion. It is also quite intuitive to use, this can be seen in the rewrite of the command_line test where EXPECT_THROW replaces a whole try-catch block. I have also moved the source files the test depend on to be linked in CMakeLists.txt instead of including them directly because include .cpp files is bad. The two x11 tests were removed because they were written two years ago and a lot of the things they depend on, don't actually exist anymore in polybar (I think we switched to xpp after those tests were written) Tests are now compiled with the gcov lib which can be used to provide test coverage in a second step --- .travis.yml | 2 + CMakeLists.txt | 3 + tests/CMakeLists.txt | 68 +++-- tests/common/test.hpp | 1 + tests/unit_tests/components/command_line.cpp | 255 ++++++++----------- tests/unit_tests/utils/file.cpp | 11 +- tests/unit_tests/utils/math.cpp | 1 + tests/unit_tests/utils/memory.cpp | 33 ++- tests/unit_tests/utils/scope.cpp | 1 + tests/unit_tests/utils/string.cpp | 3 +- tests/unit_tests/x11/connection.cpp | 14 - tests/unit_tests/x11/winspec.cpp | 33 --- 12 files changed, 185 insertions(+), 240 deletions(-) delete mode 100644 tests/unit_tests/x11/connection.cpp delete mode 100644 tests/unit_tests/x11/winspec.cpp diff --git a/.travis.yml b/.travis.yml index 08783c04..d5e8c9c6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,6 +35,8 @@ addons: - python-xcbgen - xcb-proto - xutils-dev + - libgtest0 + - libgtest-dev env: global: diff --git a/CMakeLists.txt b/CMakeLists.txt index c23f6e2f..b6725b7e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,6 +26,9 @@ add_subdirectory(lib) add_subdirectory(man) add_subdirectory(src bin) +# We need to enable testing in the root folder so that 'ctest' and 'make test' +# can be run in the build directory +enable_testing() if(BUILD_TESTS) add_subdirectory(tests) else() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ce47a897..c8018f0f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,26 +1,62 @@ -# -# Based on https://github.com/modern-cpp-examples/match3/blob/master/test/CMakeLists.txt -# -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -include common/test.hpp") +# Compile and link with coverage +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fprofile-arcs -ftest-coverage") +set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-arcs -ftest-coverage") link_libraries(${libs}) include_directories(${dirs}) include_directories(${PROJECT_SOURCE_DIR}/src) include_directories(${CMAKE_CURRENT_LIST_DIR}) -function(unit_test file) +find_package(PkgConfig REQUIRED) +pkg_check_modules(GTEST REQUIRED gtest) + +function(unit_test file tests) + set(multi_value_args SOURCES) + + cmake_parse_arguments("BIN" "" "" "${multi_value_args}" ${ARGN}) + + # Prefix all sources needed by the tests with ../src/ so that the calls to the + # unit_test function become cleaner + SET(sources "") + FOREACH(f ${BIN_SOURCES}) + LIST(APPEND sources "../src/${f}") + ENDFOREACH(f) + string(REPLACE "/" "_" testname ${file}) - add_executable(unit_test.${testname} unit_tests/${file}.cpp) - add_test(unit_test.${testname} unit_test.${testname}) + set(name "unit_test.${testname}") + add_executable(${name} unit_tests/${file}.cpp ${sources}) + + # Link against googletest + target_link_libraries(${name} ${GTEST_LDFLAGS}) + target_compile_options(${name} PUBLIC ${GTEST_CFLAGS}) + + add_test(NAME ${name} COMMAND ${name}) + + # Add test to list of unit tests + list(APPEND ${tests} "${name}") + set(${tests} ${${tests}} PARENT_SCOPE) endfunction() -unit_test(utils/color) -unit_test(utils/math) -unit_test(utils/memory) -unit_test(utils/string) -unit_test(utils/file) -unit_test(components/command_line) +unit_test(utils/color unit_tests) +unit_test(utils/math unit_tests) +unit_test(utils/memory unit_tests) +unit_test(utils/string unit_tests + SOURCES + utils/string.cpp) +unit_test(utils/file unit_tests + SOURCES + utils/command.cpp + utils/file.cpp + utils/env.cpp + utils/process.cpp + utils/io.cpp + utils/string.cpp + utils/concurrency.cpp + components/logger.cpp) +unit_test(components/command_line unit_tests + SOURCES + components/command_line.cpp + utils/string.cpp) -# XXX: Requires mocked xcb connection -#unit_test("x11/connection") -#unit_test("x11/winspec") +# Compile all unit tests with 'make all_unit_tests' +add_custom_target("all_unit_tests" DEPENDS ${unit_tests}) diff --git a/tests/common/test.hpp b/tests/common/test.hpp index fa662b03..5ee1b5e5 100644 --- a/tests/common/test.hpp +++ b/tests/common/test.hpp @@ -9,6 +9,7 @@ #include #include +#include #define expect(...) \ (void)((__VA_ARGS__) || (expect_fail__(#__VA_ARGS__, __FILE__, __LINE__), 0)) diff --git a/tests/unit_tests/components/command_line.cpp b/tests/unit_tests/components/command_line.cpp index d93f470f..79873a70 100644 --- a/tests/unit_tests/components/command_line.cpp +++ b/tests/unit_tests/components/command_line.cpp @@ -1,160 +1,113 @@ -#include "components/command_line.cpp" -#include "utils/string.cpp" +#include "common/test.hpp" +#include "components/command_line.hpp" +#include "utils/string.hpp" -int main() { - using namespace polybar; +using namespace polybar; - const auto get_opts = []() -> const command_line::options { - // clang-format off - return command_line::options{ - command_line::option{"-f", "--flag", "Flag description"}, - command_line::option{"-o", "--option", "Option description", "OPTION", {"foo", "bar", "baz"}}, +class CommandLine : public ::testing::Test { + protected: + virtual void SetUp() { + set_cli(); + } + + virtual void set_cli() { + cli = command_line::parser::make("cmd", get_opts()); + } + + command_line::options get_opts() { + // clang-format off + return command_line::options { + command_line::option{"-f", "--flag", "Flag description"}, + command_line::option{"-o", "--option", "Option description", "OPTION", {"foo", "bar", "baz"}}, + }; + // clang-format on }; - // clang-format on - }; - "has_short"_test = [&] { - auto cli = command_line::parser::make("cmd", get_opts()); - cli->process_input(string_util::split("-f", ' ')); - expect(cli->has("flag")); - expect(!cli->has("option")); + command_line::parser::make_type cli; +}; - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("-f -o foo", ' ')); - expect(cli->has("flag")); - expect(cli->has("option")); - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("-o baz", ' ')); - expect(!cli->has("flag")); - expect(cli->has("option")); - }; +TEST_F(CommandLine, hasShort) { + cli->process_input(string_util::split("-f", ' ')); + EXPECT_TRUE(cli->has("flag")); + EXPECT_FALSE(cli->has("option")); - "has_long"_test = [&] { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--flag", ' ')); - expect(cli->has("flag")); - expect(!cli->has("option")); + set_cli(); + cli->process_input(string_util::split("-f -o foo", ' ')); + EXPECT_TRUE(cli->has("flag")); + EXPECT_TRUE(cli->has("option")); - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--flag --option=foo", ' ')); - expect(cli->has("flag")); - expect(cli->has("option")); - - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--option=foo --flag", ' ')); - expect(cli->has("flag")); - expect(cli->has("option")); - - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--option=baz", ' ')); - expect(!cli->has("flag")); - expect(cli->has("option")); - }; - - "compare"_test = [&] { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("-o baz", ' ')); - expect(cli->compare("option", "baz")); - - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--option=foo", ' ')); - expect(cli->compare("option", "foo")); - }; - - "get"_test = [&] { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--option=baz", ' ')); - expect("baz" == cli->get("option")); - - cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(string_util::split("--option=foo", ' ')); - expect("foo" == cli->get("option")); - }; - - "missing_value"_test = [&] { - auto input1 = string_util::split("--option", ' '); - auto input2 = string_util::split("-o", ' '); - auto input3 = string_util::split("--option baz", ' '); - - bool exception_thrown = false; - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input1); - } catch (const command_line::value_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - - exception_thrown = false; // reset - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input2); - } catch (const command_line::value_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - - exception_thrown = false; // reset - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input3); - } catch (const command_line::value_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - }; - - "invalid_value"_test = [&] { - auto input1 = string_util::split("--option=invalid", ' '); - auto input2 = string_util::split("-o invalid_value", ' '); - - bool exception_thrown = false; - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input1); - } catch (const command_line::value_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - - exception_thrown = false; // reset - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input2); - } catch (const command_line::value_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - }; - - "unrecognized"_test = [&] { - auto input1 = string_util::split("-x", ' '); - auto input2 = string_util::split("--unrecognized", ' '); - - bool exception_thrown = false; - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input1); - } catch (const command_line::argument_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - - exception_thrown = false; // reset - try { - auto cli = command_line::parser::make("cmd", get_opts());; - cli->process_input(input2); - } catch (const command_line::argument_error&) { - exception_thrown = true; - } catch (...) { - } - expect(exception_thrown); - }; + set_cli(); + cli->process_input(string_util::split("-o baz", ' ')); + EXPECT_FALSE(cli->has("flag")); + EXPECT_TRUE(cli->has("option")); +} + +TEST_F(CommandLine, hasLong) { + cli->process_input(string_util::split("--flag", ' ')); + EXPECT_TRUE(cli->has("flag")); + EXPECT_FALSE(cli->has("option")); + + set_cli(); + cli->process_input(string_util::split("--flag --option=foo", ' ')); + EXPECT_TRUE(cli->has("flag")); + EXPECT_TRUE(cli->has("option")); + + set_cli(); + cli->process_input(string_util::split("--option=foo --flag", ' ')); + EXPECT_TRUE(cli->has("flag")); + EXPECT_TRUE(cli->has("option")); + + set_cli(); + cli->process_input(string_util::split("--option=baz", ' ')); + EXPECT_FALSE(cli->has("flag")); + EXPECT_TRUE(cli->has("option")); +} + +TEST_F(CommandLine, compare) { + cli->process_input(string_util::split("-o baz", ' ')); + EXPECT_TRUE(cli->compare("option", "baz")); + + set_cli(); + cli->process_input(string_util::split("--option=foo", ' ')); + EXPECT_TRUE(cli->compare("option", "foo")); +} + +TEST_F(CommandLine, get) { + cli->process_input(string_util::split("--option=baz", ' ')); + EXPECT_EQ("baz", cli->get("option")); + + set_cli(); + cli->process_input(string_util::split("--option=foo", ' ')); + EXPECT_EQ("foo", cli->get("option")); +} + +TEST_F(CommandLine, missingValue) { + auto input1 = string_util::split("--option", ' '); + auto input2 = string_util::split("-o", ' '); + auto input3 = string_util::split("--option baz", ' '); + + EXPECT_THROW(cli->process_input(input1), command_line::value_error); + set_cli(); + EXPECT_THROW(cli->process_input(input2), command_line::value_error); + set_cli(); + EXPECT_THROW(cli->process_input(input3), command_line::value_error); +} + +TEST_F(CommandLine, invalidValue) { + auto input1 = string_util::split("--option=invalid", ' '); + auto input2 = string_util::split("-o invalid_value", ' '); + + EXPECT_THROW(cli->process_input(input1), command_line::value_error); + set_cli(); + EXPECT_THROW(cli->process_input(input2), command_line::value_error); +} + +TEST_F(CommandLine, unrecognized) { + auto input1 = string_util::split("-x", ' '); + auto input2 = string_util::split("--unrecognized", ' '); + + EXPECT_THROW(cli->process_input(input1), command_line::argument_error); + set_cli(); + EXPECT_THROW(cli->process_input(input2), command_line::argument_error); } diff --git a/tests/unit_tests/utils/file.cpp b/tests/unit_tests/utils/file.cpp index 4aacd80d..47e915ef 100644 --- a/tests/unit_tests/utils/file.cpp +++ b/tests/unit_tests/utils/file.cpp @@ -1,14 +1,9 @@ #include #include -#include "components/logger.cpp" -#include "utils/command.cpp" -#include "utils/concurrency.cpp" -#include "utils/env.cpp" -#include "utils/file.cpp" -#include "utils/io.cpp" -#include "utils/process.cpp" -#include "utils/string.cpp" +#include "common/test.hpp" +#include "utils/command.hpp" +#include "utils/file.hpp" int main() { using namespace polybar; diff --git a/tests/unit_tests/utils/math.cpp b/tests/unit_tests/utils/math.cpp index f338a98d..faceb18c 100644 --- a/tests/unit_tests/utils/math.cpp +++ b/tests/unit_tests/utils/math.cpp @@ -1,3 +1,4 @@ +#include "common/test.hpp" #include "utils/math.hpp" int main() { diff --git a/tests/unit_tests/utils/memory.cpp b/tests/unit_tests/utils/memory.cpp index 92dc43ba..5c50f8b2 100644 --- a/tests/unit_tests/utils/memory.cpp +++ b/tests/unit_tests/utils/memory.cpp @@ -1,24 +1,23 @@ +#include "common/test.hpp" #include "utils/memory.hpp" +using namespace polybar; + struct mytype { int x, y, z; }; -int main() { - using namespace polybar; - - "make_malloc_ptr"_test = [] { - auto ptr = memory_util::make_malloc_ptr(); - expect(sizeof(mytype*) == sizeof(ptr.get())); - ptr.reset(); - expect(ptr.get() == nullptr); - }; - - "countof"_test = [] { - mytype A[3]{{}, {}, {}}; - mytype B[8]{{}, {}, {}, {}, {}, {}, {}, {}}; - - expect(memory_util::countof(A) == size_t{3}); - expect(memory_util::countof(B) == size_t{8}); - }; +TEST(Memory, makeMallocPtr) { + auto ptr = memory_util::make_malloc_ptr(); + EXPECT_EQ(sizeof(mytype*), sizeof(ptr.get())); + ptr.reset(); + EXPECT_EQ(nullptr, ptr.get()); +} + +TEST(Memory, countof) { + mytype A[3]{{}, {}, {}}; + mytype B[8]{{}, {}, {}, {}, {}, {}, {}, {}}; + + EXPECT_EQ(memory_util::countof(A), size_t{3}); + EXPECT_EQ(memory_util::countof(B), size_t{8}); } diff --git a/tests/unit_tests/utils/scope.cpp b/tests/unit_tests/utils/scope.cpp index 839db83c..5324e0d8 100644 --- a/tests/unit_tests/utils/scope.cpp +++ b/tests/unit_tests/utils/scope.cpp @@ -1,3 +1,4 @@ +#include "common/test.hpp" #include "utils/scope.hpp" int main() { diff --git a/tests/unit_tests/utils/string.cpp b/tests/unit_tests/utils/string.cpp index 93e109f5..e52e52dd 100644 --- a/tests/unit_tests/utils/string.cpp +++ b/tests/unit_tests/utils/string.cpp @@ -1,6 +1,7 @@ #include -#include "utils/string.cpp" +#include "common/test.hpp" +#include "utils/string.hpp" int main() { using namespace polybar; diff --git a/tests/unit_tests/x11/connection.cpp b/tests/unit_tests/x11/connection.cpp deleted file mode 100644 index 4bce78a6..00000000 --- a/tests/unit_tests/x11/connection.cpp +++ /dev/null @@ -1,14 +0,0 @@ -#include "utils/string.cpp" -#include "x11/atoms.cpp" -#include "x11/connection.cpp" -#include "x11/xutils.cpp" -#include "x11/xlib.cpp" - -int main() { - using namespace polybar; - - "id"_test = [] { - connection& conn{configure_connection().create()}; - expect(conn.id(static_cast(0x12345678)) == "0x12345678"); - }; -} diff --git a/tests/unit_tests/x11/winspec.cpp b/tests/unit_tests/x11/winspec.cpp deleted file mode 100644 index d853e828..00000000 --- a/tests/unit_tests/x11/winspec.cpp +++ /dev/null @@ -1,33 +0,0 @@ -#include "utils/string.cpp" -#include "x11/atoms.cpp" -#include "x11/connection.cpp" -#include "x11/winspec.hpp" -#include "x11/xutils.cpp" -#include "x11/xlib.cpp" - -int main() { - using namespace polybar; - - "cw_create"_test = [] { - connection& conn{configure_connection().create()}; - auto id = conn.generate_id(); - - // clang-format off - auto win = winspec(conn, id) - << cw_size(100, 200) - << cw_pos(10, -20) - << cw_border(9) - << cw_class(XCB_WINDOW_CLASS_INPUT_ONLY) - << cw_parent(0x000110a) - ; - // clang-format on - - expect(static_cast(win) == id); - - xcb_rectangle_t rect{static_cast(win)}; - expect(rect.width == 100); - expect(rect.height == 200); - expect(rect.x == 10); - expect(rect.y == -20); - }; -}