Repository: doodspav/patomic (language: python)

Issue:
Setup binary and unit tests with GTest framework
## Description

- Create `test` folder with binary and unit tests (and any other tests) using GTest framework

## Acceptance Criteria

- [x] Running `ctest .` runs all tests that were built
- [x] Tests can only be built as part of the library if the library as built as a top level project, unless overriden with option
- [x] The user of the test shall provide GTest, the test shall only look for it using `find_package`
- [x] The tests must be able to be built separately to the library, and link with the library using `find_package`
- [x] It shall be possible to only build certain test types (binary or unit test)
- [x] It shall be possible to only build certain suites from a test type
- [x] It shall be possible to install tests, but they will be excluded from all


Discussion / hints:

Fix patch:
diff --git a/CMakeLists.txt b/CMakeLists.txt
index bad77f7fc..cb7e357bb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -79,3 +79,12 @@ target_compile_features(patomic_patomic PUBLIC c_std_90)
 if(NOT CMAKE_SKIP_INSTALL_RULES)
     include(cmake/util/InstallRules.cmake)
 endif()
+
+
+# ---- Setup Tests ----
+
+if(patomic_BUILD_TESTING)
+    # need to enable testing in case BUILD_TESTING is disabled
+    enable_testing()
+    add_subdirectory(test)
+endif()
diff --git a/cmake/util/OptionVariables.cmake b/cmake/util/OptionVariables.cmake
index c3feb21a4..1226e214d 100644
--- a/cmake/util/OptionVariables.cmake
+++ b/cmake/util/OptionVariables.cmake
@@ -35,3 +35,20 @@ if(NOT PROJECT_IS_TOP_LEVEL)
         set(patomic_WARNING_GUARD SYSTEM)
     endif()
 endif()
+
+
+# ---- Enable Testing ----
+
+# By default tests aren't enabled even with BUILD_TESTING unless the library is
+# built as a top level project.
+# This is in order to cut down on unnecessary compile times, since it's unlikely
+# for users to want to run the tests of their dependencies.
+if(PROJECT_IS_TOP_LEVEL)
+    option(BUILD_TESTING "Build tests" OFF)
+endif()
+option(
+    patomic_BUILD_TESTING
+    "Override BUILD_TESTING for patomic library"
+    ${BUILD_TESTING}
+)
+mark_as_advanced(patomic_BUILD_TESTING)
diff --git a/cmake/util/PreventInSourceBuilds.cmake b/cmake/util/PreventInSourceBuilds.cmake
index c78f18825..ef5b59ebb 100644
--- a/cmake/util/PreventInSourceBuilds.cmake
+++ b/cmake/util/PreventInSourceBuilds.cmake
@@ -3,7 +3,6 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
     message(
         FATAL_ERROR
         "In-source builds are not supported. "
-        "Please read the BUILDING document before trying to build this project. "
-        "You may need to delete 'CMakeCache.txt' and 'CMakeFiles/' first."
+        "You may need to delete 'CMakeCache.txt' and 'CMakeFiles/' before rebuilding this project."
     )
 endif()


Test patch:
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
new file mode 100644
index 000000000..a13189af9
--- /dev/null
+++ b/test/CMakeLists.txt
@@ -0,0 +1,43 @@
+cmake_minimum_required(VERSION 3.14)
+
+include(cmake/PreventInSourceBuilds.cmake)
+
+project(
+    patomic_test
+    DESCRIPTION "Standalone tests for patomic library"
+    HOMEPAGE_URL "https://github.com/doodspav/patomic"
+    LANGUAGES CXX
+)
+
+include(cmake/OptionVariables.cmake)
+
+
+# ---- Dependencies ----
+
+include(cmake/ProjectIsTopLevel.cmake)
+
+if(PROJECT_IS_TOP_LEVEL)
+    find_package(patomic REQUIRED)
+    enable_testing()
+endif()
+
+# for Windows: prevent overriding the parent project's compiler/linker settings
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+find_package(GTest REQUIRED)
+include(GoogleTest)
+
+
+# ---- Declare Tests ----
+
+add_custom_target(patomic_test)
+
+# add all test subdirectories
+add_subdirectory(bt)
+add_subdirectory(ut)
+
+
+# ---- Support Packaging Library ----
+
+if(PROJECT_IS_TOP_LEVEL AND NOT CMAKE_SKIP_INSTALL_RULES)
+    include(CPack)
+endif()
diff --git a/test/bt/CMakeLists.txt b/test/bt/CMakeLists.txt
new file mode 100644
index 000000000..7469ab097
--- /dev/null
+++ b/test/bt/CMakeLists.txt
@@ -0,0 +1,17 @@
+# create_test requires windows_deps_path
+include(../cmake/CreateTest.cmake)
+include(../cmake/WindowsDependenciesPath.cmake)
+
+
+# ---- Setup Tests ----
+
+add_custom_target(patomic_bt)
+add_dependencies(patomic_test patomic_bt)
+
+create_test(BT example_add SOURCE example_add.cpp)
+create_test(BT example_sub SOURCE example_sub.cpp)
+
+
+# ---- Windows Path Issues ----
+
+create_test_win_deps_path_file(BT)
diff --git a/test/bt/example_add.cpp b/test/bt/example_add.cpp
new file mode 100644
index 000000000..238875ab1
--- /dev/null
+++ b/test/bt/example_add.cpp
@@ -0,0 +1,8 @@
+#include <gtest/gtest.h>
+#include <patomic/patomic.h>
+
+
+TEST(BtSuite, Add)
+{
+    ASSERT_EQ(5, patomic_example_add(2, 3));
+}
diff --git a/test/bt/example_sub.cpp b/test/bt/example_sub.cpp
new file mode 100644
index 000000000..8de1bddc9
--- /dev/null
+++ b/test/bt/example_sub.cpp
@@ -0,0 +1,8 @@
+#include <gtest/gtest.h>
+#include <patomic/patomic.h>
+
+
+TEST(BtSuite, Sub)
+{
+    ASSERT_EQ(5, patomic_example_add(2, 3));
+}
diff --git a/test/cmake/CreateTest.cmake b/test/cmake/CreateTest.cmake
new file mode 100644
index 000000000..cd6e584af
--- /dev/null
+++ b/test/cmake/CreateTest.cmake
@@ -0,0 +1,260 @@
+# ---- Create Test ----
+
+# Creates a target to build a test executable and registers it with CTest.
+# Expects a target named patomic_${kind} to exist.
+# E.g. if you call it as create_test(BT ...) then patomic_bt must exist.
+# The target is named patomic_${kind}_${name} but the name of the executable
+# produced is just ${name}.
+# When the executable is installed, it's installed as patomic/${kind}/${name}.
+#
+# create_test(
+#     BT|UT <name>
+#     [INCLUDE <item>...]
+#     [SOURCE <item>...]
+#     [LINK <item>...]
+# )
+function(create_test)
+
+    # setup what arguments we expect
+
+    set(all_kinds "BT;UT")  # list we can iterate over
+    set(all_kinds_option "BT|UT")  # string to use in debug message
+
+    cmake_parse_arguments(
+        "ARG"
+        ""
+        "${all_kinds}"
+        "INCLUDE;SOURCE;LINK"
+        ${ARGN}
+    )
+
+
+    # check what test kinds are passed
+
+    set(kind "")  # -> "bt"
+    set(name "")  # -> "${ARG_BT}"
+    # we turn kind into a list so that we can check its length (should be 1)
+    # and name is just the last name we process
+    foreach(ak IN LISTS all_kinds)
+        # if(ARG_BT)
+        if (ARG_${ak})
+            string(TOLOWER ${ak} ak_lower)
+            # kind.append(bt)
+            # name = ${ARG_BT}
+            list(APPEND kind "${ak_lower}")
+            set(name "${ARG_${ak}}")
+        endif()
+    endforeach()
+
+
+    # validate arguments
+
+    set(args_valid TRUE)
+    set(func_name "create_test")
+    set(LENGTH kind kinds_count)
+
+    if(kinds_count EQUAL 0)
+        message(WARNING "${all_kinds_option} option needs to be specified when invoking '${func_name}'")
+        set(args_valid FALSE)
+    elseif(kinds_count GREATER 1)
+        message(WARNING "Only a single ${all_kinds_option} option may be specified when invoking '${func_name}'")
+        set(args_valid FALSE)
+    elseif(TARGET ${name})
+        message(WARNING "Test name must not be an existing target when invoking '${func_name}', was passed: ${name}")
+        set(args_valid FALSE)
+    elseif("${name}" STREQUAL "")
+        message(WARNING "Test name must not be empty when invoking '${func_name}'")
+    endif()
+
+    if(DEFINED ARG_UNPARSED_ARGUMENTS)
+        message(WARNING "The following arguments were not recognised when invoking '${func_name}': ${ARG_UNPARSED_ARGUMENTS}")
+        set(args_valid FALSE)
+    endif()
+
+    if (NOT args_valid)
+        message(FATAL_ERROR "Aborting '${func_name}' due to invalid arguments")
+    endif()
+
+
+    # setup target
+
+    set(parent_target patomic_${kind})
+    set(target ${parent_target}_${name})
+    set(target_deps patomic::patomic GTest::gtest_main ${ARG_LINK})
+    set(output_name ${name})
+
+    if (NOT "${target}" MATCHES ${patomic_test_CREATE_TEST_TARGETS_MATCHING})
+        message(DEBUG "Skipping creation of test target ${target} (matches ${patomic_test_CREATE_TEST_TARGETS_MATCHING})")
+        return()
+    endif()
+
+    add_executable(
+        ${target}
+        ${ARG_INCLUDE}
+        ${ARG_SOURCE}
+    )
+
+    target_link_libraries(
+        ${target}
+        PRIVATE
+        ${target_deps}
+    )
+
+    target_compile_features(${target} PRIVATE cxx_std_14)
+
+    set_target_properties(${target} PROPERTIES OUTPUT_NAME ${output_name})
+
+
+    # register tests with CTest
+
+    # must be run in same directory scope as target
+    gtest_add_tests(
+        TARGET ${target}
+        TEST_LIST added_tests
+    )
+
+
+    # deal with Windows runtime linker issues for tests (with and without CTest)
+
+    if (CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows")
+
+        # check we actually care about Windows PATH stuff
+        if (NOT patomic_test_SET_CTEST_PATH_ENV_WINDOWS AND NOT patomic_test_CREATE_WINDOWS_PATH_FILE)
+            return()
+        endif()
+
+        # get paths to all shared library dependencies (DLLs)
+        windows_deps_path(
+            deps_path
+            ${target_deps}
+        )
+
+        # set environment variable for the each test so that CTest works automatically
+        if (deps_path AND patomic_test_SET_CTEST_PATH_ENV_WINDOWS)
+            foreach(test IN LISTS added_tests)
+                set_property(
+                    TEST "${test}"
+                    PROPERTY ENVIRONMENT "PATH=${deps_path}"
+                )
+            endforeach()
+        endif()
+
+        # make dependencies accessible from parent target
+        if (patomic_test_CREATE_WINDOWS_PATH_FILE)
+            set_property(
+                TARGET ${parent_target}
+                APPEND PROPERTY WIN_DEP_TARGETS ${target_deps}
+            )
+        endif()
+    endif()
+
+
+    # setup install of target
+
+    if(NOT CMAKE_SKIP_INSTALL_RULES)
+        # install as part of patomic_${kind} component
+        install(
+            TARGETS ${target}
+            RUNTIME #
+            COMPONENT "${parent_target}"
+            DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}"
+            EXCLUDE_FROM_ALL
+        )
+
+        # install as part of patomic_test component
+        install(
+            TARGETS ${target}
+            RUNTIME #
+            COMPONENT patomic_test
+            DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}"
+            EXCLUDE_FROM_ALL
+        )
+    endif()
+
+
+    # attach to parent target
+
+    add_dependencies(${parent_target} ${target})
+
+endfunction()
+
+
+# ---- Create Test Dependency File ----
+
+# Creates a file containing the output of windows_deps_path for all tests of
+# the given kind registered so far.
+# Expects a target named patomic_${kind} to exist
+# E.g. if you call it as create_test_win_deps_path_file(BT) then patomic_bt must
+# exist.
+#
+# create_test_win_deps_path_file(
+#     BT|UT
+# )
+function(create_test_win_deps_path_file ARG_KIND)
+
+    # check we actually want to generate file
+
+    if(NOT patomic_test_CREATE_WINDOWS_PATH_FILE)
+        return()
+    endif()
+
+
+    # check KIND is valid
+
+    set(all_kinds_option "BT|UT")
+    set(func_name "create_test_win_deps_path_file")
+
+    if (NOT ARG_KIND MATCHES "^(${all_kinds_option})$")
+        message(WARNING "${all_kinds_option} option needs to be specified when invoking '${func_name}'")
+        message(FATAL_ERROR "Aborting '${func_name}' due to invalid arguments")
+    endif()
+
+    string(TOLOWER ${ARG_KIND} kind)
+
+
+    # create and install file with dependencies path
+
+    if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows")
+
+        # get dependencies set by create_test from target
+        get_target_property(dep_targets patomic_${kind} WIN_DEP_TARGETS)
+        if("${dep_targets}" STREQUAL "dep_targets-NOTFOUND")
+            message(DEBUG "Skipping creation of Windows dependencies PATH file for ${ARG_KIND}; no relevant test targets created")
+            return()
+        endif()
+        list(REMOVE_DUPLICATES dep_targets)
+
+        # get paths to all shared library dependencies (DLLs)
+        windows_deps_path(
+            deps_path
+            ${dep_targets}
+        )
+
+        # create file
+        set(file_path "${CMAKE_CURRENT_BINARY_DIR}/windows_dependencies_path.txt")
+        file(GENERATE
+            OUTPUT ${file_path}
+            CONTENT "${deps_path}"
+        )
+
+        # copy file to install location
+        if(NOT CMAKE_SKIP_INSTALL_RULES)
+            # install as part of patomic_${kind} component
+            install(
+                FILES ${file_path}
+                COMPONENT patomic_${kind}
+                DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}"
+                EXCLUDE_FROM_ALL
+            )
+
+            # install as part of patomic_test component
+            install(
+                FILES ${file_path}
+                COMPONENT patomic_test
+                DESTINATION "${CMAKE_INSTALL_TESTDIR}/patomic/${kind}"
+                EXCLUDE_FROM_ALL
+            )
+        endif()
+    endif()
+
+endfunction()
diff --git a/test/cmake/OptionVariables.cmake b/test/cmake/OptionVariables.cmake
new file mode 100644
index 000000000..8703697ad
--- /dev/null
+++ b/test/cmake/OptionVariables.cmake
@@ -0,0 +1,57 @@
+# ---- Test Install Directory ----
+
+# Normally tests would be install in CMAKE_INSTALL_BINDIR by default since
+# they're executables.
+# This is undesirable, so this variable exists to override the install location
+# of test binaries separately.
+# It's not prefixed with patomic_test because it's ok for it to be shared and
+# overridden by parent projects.
+# Note: this is not an official CMake variable
+set(
+    CMAKE_INSTALL_TESTDIR "share/test"
+    CACHE PATH "(unofficial) Default test install location"
+)
+
+
+# ---- Test Build Selection ----
+
+# This option provides a way to selectively disable/enable tests based on target
+# name (not suite/case name, or name passed to create_test).
+# Regex must be written according to CMake Regex Specification.
+# By default all test targets are enabled.
+set(
+    patomic_test_CREATE_TEST_TARGETS_MATCHING "^(.*)$"
+    CACHE STRING "Only test targets matching regex are created and registered with CTest"
+)
+
+
+# ---- Windows Tests Path ----
+
+# By default we set PATH for tests run with CTest on Windows in order to prevent
+# linker errors.
+# Due to limitations in CMake, we can only completely override the PATH, rather
+# than prepend to it.
+# This gives users the option to disable this behaviour.
+option(
+    patomic_test_SET_CTEST_PATH_ENV_WINDOWS
+    "Set PATH environment variable for tests when run CTest on Windows"
+    ON
+)
+mark_as_advanced(patomic_test_SET_CTEST_PATH_ENV_WINDOWS)
+
+
+# ---- Windows Path File ----
+
+# By default on Windows we generate a file per test kind that contains a string
+# that can be prepended to PATH before running tests, in order to ensure that
+# runtime dependencies can be found.
+# Most of the time we don't need this file (since CTest will take care of that
+# for us), so we don't need to generate it.
+# Additionally disabled by default because it contains potentially private
+# information about the target platform.
+option(
+    patomic_test_CREATE_WINDOWS_PATH_FILE
+    "Create file with PATH environment variables for tests on Windows"
+    OFF
+)
+mark_as_advanced(patomic_test_CREATE_WINDOWS_PATH_FILE)
diff --git a/test/cmake/PreventInSourceBuilds.cmake b/test/cmake/PreventInSourceBuilds.cmake
new file mode 100644
index 000000000..ef5b59ebb
--- /dev/null
+++ b/test/cmake/PreventInSourceBuilds.cmake
@@ -0,0 +1,8 @@
+# in-source build guard
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
+    message(
+        FATAL_ERROR
+        "In-source builds are not supported. "
+        "You may need to delete 'CMakeCache.txt' and 'CMakeFiles/' before rebuilding this project."
+    )
+endif()
diff --git a/test/cmake/ProjectIsTopLevel.cmake b/test/cmake/ProjectIsTopLevel.cmake
new file mode 100644
index 000000000..fc2ef6c9f
--- /dev/null
+++ b/test/cmake/ProjectIsTopLevel.cmake
@@ -0,0 +1,6 @@
+# this variable is set by project() in CMake 3.21+
+string(
+    COMPARE EQUAL
+    "${CMAKE_SOURCE_DIR}" "${PROJECT_SOURCE_DIR}"
+    PROJECT_IS_TOP_LEVEL
+)
diff --git a/test/cmake/WindowsDependenciesPath.cmake b/test/cmake/WindowsDependenciesPath.cmake
new file mode 100644
index 000000000..a572db3e3
--- /dev/null
+++ b/test/cmake/WindowsDependenciesPath.cmake
@@ -0,0 +1,39 @@
+# ---- Windows Path Prefix ----
+
+# Windows doesn't support rpath, so when linking dynamically the libraries need
+# to either be in the same directory or on PATH.
+# This function sets a variable to a GENERATOR string that may be prepended to
+# path in order to find linked dependencies
+# See: https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order
+# Usage: windows_deps_path(<variable> <link-target>...)
+function(windows_deps_path VAR)
+    if(NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows")
+        set(${VAR} "" PARENT_SCOPE)
+        return()
+    endif()
+
+    set(paths "")
+    foreach(target IN LISTS ARGN)
+        # This will fail if passed a link option that isn't a target
+        # This is intentional; don't do that
+        # Instead, create an IMPORTED library, and set its target properties
+        # such as IMPORTED_LOCATION for the library (.a .so etc.) path and
+        # set INTERFACE_INCLUDE_DIRECTORIES to the directory containing any
+        # necessary header files
+        get_target_property(type "${target}" TYPE)
+        if(type STREQUAL "SHARED_LIBRARY")
+            list(APPEND paths "$<TARGET_FILE_DIR:${target}>")
+        endif()
+    endforeach()
+
+    list(REMOVE_DUPLICATES paths)
+
+    set(path "")
+    set(glue "")
+    foreach(p IN LISTS paths)
+        set(path "${path}${glue}${p}")
+        set(glue "\;")  # backslash is important
+    endforeach()
+
+    set(${VAR} "${path}" PARENT_SCOPE)
+endfunction()
diff --git a/test/ut/CMakeLists.txt b/test/ut/CMakeLists.txt
new file mode 100644
index 000000000..20b7cd490
--- /dev/null
+++ b/test/ut/CMakeLists.txt
@@ -0,0 +1,17 @@
+# create_test requires windows_deps_path
+include(../cmake/CreateTest.cmake)
+include(../cmake/WindowsDependenciesPath.cmake)
+
+
+# ---- Setup Tests ----
+
+add_custom_target(patomic_ut)
+add_dependencies(patomic_test patomic_ut)
+
+create_test(UT example_add SOURCE example_add.cpp)
+create_test(UT example_sub SOURCE example_sub.cpp)
+
+
+# ---- Windows Path Issues ----
+
+create_test_win_deps_path_file(UT)
diff --git a/test/ut/example_add.cpp b/test/ut/example_add.cpp
new file mode 100644
index 000000000..99d8ee625
--- /dev/null
+++ b/test/ut/example_add.cpp
@@ -0,0 +1,8 @@
+#include <gtest/gtest.h>
+#include <patomic/patomic.h>
+
+
+TEST(UtSuite, Add)
+{
+    ASSERT_EQ(5, patomic_example_add(2, 3));
+}
diff --git a/test/ut/example_sub.cpp b/test/ut/example_sub.cpp
new file mode 100644
index 000000000..4799d147d
--- /dev/null
+++ b/test/ut/example_sub.cpp
@@ -0,0 +1,8 @@
+#include <gtest/gtest.h>
+#include <patomic/patomic.h>
+
+
+TEST(UtSuite, Sub)
+{
+    ASSERT_EQ(5, patomic_example_add(2, 3));
+}