From 3f323e419aea9d679ec14faa039aa930d31d84f8 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 12:41:21 -0700 Subject: [PATCH 1/8] ci: basic add_subdirectory cmake integration test --- .github/workflows/cmake.yml | 24 ++++ CMakeLists.txt | 27 ++++- cmake-tests/CMakeLists.txt | 2 + cmake-tests/README.md | 105 ++++++++++++++++++ cmake-tests/declareProjectTest.cmake | 100 +++++++++++++++++ .../test_add_subdirectory/CMakeLists.txt | 2 + .../project/CMakeLists.txt | 26 +++++ .../project/main_client.cpp | 25 +++++ .../project/main_server.cpp | 23 ++++ scripts/build-cmake-integration-tests.sh | 20 ++++ 10 files changed, 348 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/cmake.yml create mode 100644 cmake-tests/CMakeLists.txt create mode 100644 cmake-tests/README.md create mode 100644 cmake-tests/declareProjectTest.cmake create mode 100644 cmake-tests/test_add_subdirectory/CMakeLists.txt create mode 100644 cmake-tests/test_add_subdirectory/project/CMakeLists.txt create mode 100644 cmake-tests/test_add_subdirectory/project/main_client.cpp create mode 100644 cmake-tests/test_add_subdirectory/project/main_server.cpp create mode 100755 scripts/build-cmake-integration-tests.sh diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml new file mode 100644 index 000000000..0a14f4ce4 --- /dev/null +++ b/.github/workflows/cmake.yml @@ -0,0 +1,24 @@ +name: cmake-integration + +on: + push: + branches: [ main ] + paths-ignore: + - '**.md' # Do not need to run CI for markdown changes. + pull_request: + branches: [ "main", "feat/**" ] + paths-ignore: + - '**.md' + schedule: + # Run daily at midnight PST + - cron: '0 8 * * *' + +jobs: + test: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + - run: ./scripts/build-cmake-integration-tests.sh + - run: + export CTEST_OUTPUT_ON_FAILURE=1 + cd build/cmake-tests && ctest diff --git a/CMakeLists.txt b/CMakeLists.txt index 90c2287db..1fd6ee858 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -65,6 +65,14 @@ cmake_dependent_option(LD_BUILD_CONTRACT_TESTS OFF # otherwise, off ) +# Add an option for enabling the "CMake Integration Tests" (see cmake-tests README). +# These tests require testing to be enabled (BUILD_TESTING), but aren't unit tests, so are disabled by default. +cmake_dependent_option(LD_CMAKE_INTEGRATION_TESTS + "Test integration of SDK into other CMake projects" OFF # Default to disabling the cmake integration tests. + "BUILD_TESTING" # Only expose if testing is enabled. + OFF # otherwise, off. +) + # The general strategy is to produce a fat artifact containing all of our dependencies so users # only have a single thing to link. We should support this either being a static or shared library. # Because OpenSSL is a large, and security relevant dependency, we should have a separate option @@ -108,6 +116,13 @@ if (LD_BUILD_UNIT_TESTS) enable_testing() endif () +if (LD_CMAKE_INTEGRATION_TESTS) + message(STATUS "LaunchDarkly: building CMake integration tests") + add_subdirectory(cmake-tests) + enable_testing() +endif () + + if (LD_DYNAMIC_LINK_OPENSSL) message(STATUS "LaunchDarkly: searching for shared OpenSSL library") set(OPENSSL_USE_STATIC_LIBS OFF) @@ -130,13 +145,13 @@ endif () if (LD_BUILD_SHARED_LIBS) if (LD_BUILD_EXPORT_ALL_SYMBOLS) - message(STATUS "LaunchDarkly: exposing all symbols in shared libraries") + message(STATUS "LaunchDarkly: exposing all symbols in shared libraries") else () - message(STATUS "LaunchDarkly: hiding all symbols in shared libraries except for C API") - set(CMAKE_CXX_VISIBILITY_PRESET hidden) - set(CMAKE_VISIBILITY_INLINES_HIDDEN 1) - endif() -endif() + message(STATUS "LaunchDarkly: hiding all symbols in shared libraries except for C API") + set(CMAKE_CXX_VISIBILITY_PRESET hidden) + set(CMAKE_VISIBILITY_INLINES_HIDDEN 1) + endif () +endif () set(Boost_USE_MULTITHREADED ON) set(Boost_USE_STATIC_RUNTIME OFF) diff --git a/cmake-tests/CMakeLists.txt b/cmake-tests/CMakeLists.txt new file mode 100644 index 000000000..9c0541576 --- /dev/null +++ b/cmake-tests/CMakeLists.txt @@ -0,0 +1,2 @@ +include(declareProjectTest.cmake) +add_subdirectory(test_add_subdirectory) diff --git a/cmake-tests/README.md b/cmake-tests/README.md new file mode 100644 index 000000000..ac7acb5b2 --- /dev/null +++ b/cmake-tests/README.md @@ -0,0 +1,105 @@ +## CMake project tests overview + +This directory contains tests for various integration techniques that users of the +LaunchDarkly C++ SDKs may employ. + +Each test takes the form of a minimal CMake project with a `CMakeLists.txt` and `main.c/main.cpp`. +An additional `CMakeLists.txt` sets up the test properties. + +Structure: + +``` +some_test_directory + project # Contains the CMake project under test. + - main.c[pp] # Minimal code that invokes LaunchDarkly SDK. + - CMakeLists.txt # CMake configuration that builds the project executable. + - CMakeLists.txt # CMake configuration that sets up the CTest machinery for this test. +``` + +*Important note about `main.c`/`main.cpp`*: + +The optimizer employed by whatever toolchain is building the project might omit function definitions in the SDK +during static linking, if those functions are proven to be unused. + +The code in the main file should not have any branches that allow this to happen +(such as a check for an SDK key, like in the hello demo projects.) + +This could obscure linker errors if the release artifacts are generated incorrectly. + +## CMake test setup + +The toplevel `CMakeLists.txt` in each subdirectory is responsible for setting up +the actual CTest tests that configure and build the projects. + +Note, the logic described below is encapsulated in two macros defined in `declareProjectTest.cmake`, so that +that new tests don't need to copy boilerplate. + +Test creation is generally done in two phases: + +1) Make a test that configures the project (simulating `cmake .. [options]`) +2) Make a test that builds the project (simulating `cmake --build .`) + +The tests are ordered via `set_tests_properties` to ensure the configure test +runs before the build test, as would be expected. + +The test creation logic harbors additional complexity because these tests are executed +in CI on multiple types of executors (Windows/Mac/Linux) in various configurations. + +In particular, some environment variables must be forwarded to each test project CMake configuration. +These include `C` and `CXX` variables, which are explicitly set/overridden in the `clang11` CI build. +Without setting these, the test would fail to build with the same compilers as the SDK. + +Additionally, certain variables must be forwarded to each test project CMake configuration. + +| Variable | Explanation | +|-----------------------------------------|--------------------------------------------------------------------------------------------------------------------------| +| `BOOST_LIBRARY`/`BOOST_INCLUDE_DIR` | Windows build uses Boost downloaded at runtime, there's no system Boost. | +| `OPENSSL_LIBRARY`/`OPENSSL_INCLUDE_DIR` | Windows build uses OpenSSL downloaded at runtime. | +| `CMAKE_GENERATOR_PLATFORM` | Windows build explicitly specifies x64 build, whereas the default project build would be x86. Linker errors would ensue. | + +The creation logic uses a series of CMake generator expressions (`$<...>`) to forward the variables +in the table above from the main SDK project (which `add_subdirectory`'d each test) to the test projects. + +Simply specifying the variables directly using `-DVARIABLE=${VARIABLE}` as is normally done on the command line +wouldn't work correctly. If the variable is empty in the SDK project (like when a system package can be used), +then passing that empty string to the test project would cause the eventual `find_package(Boost/OpenSSL)` to fail, as +passing those variables implies the user wants to override the find scripts. + +The generator expressions omit the `-D` entirely if the original variable is empty, otherwise they add it. + +## Tests + +### cmake_projects/test_add_subdirectory + +Checks that a project can include the SDK as a sub-project, via `add_subdirectory`. +This would be a likely use-case when the repo is a submodule of another project. + +### cmake_projects/test_find_package + +Checks that a project can include the SDK via `find_package(ldserverapi)`. +This would be a likely use-case if the SDK was installed on the system by the user. + +**NOTE:** Requires SDK to be installed. + +### cmake_projects/test_find_package_cpp + +Checks that a C++ project can include the SDK via `find_package(ldserverapi)`. +Also checks that C++ bindings can be included without compilation issues. + +**NOTE:** Requires SDK to be installed. + +### cmake_projects/test_find_package_compatible_version + +Checks that a project can include the SDK via `find_package(ldserverapi [version])`. +This would be a likely use-case if the user depends on a particular version of the SDK, +rather than accepting any version. + +**NOTE:** Requires SDK to be installed. + +### cmake_projects/test_find_package_incompatible_version + +Checks that a project will *fail* to configure if `find_package(ldserverapi [version])` +is invoked with a version that isn't present on the system. The test uses a fictional +version `10.0.0`. + +**NOTE:** Requires SDK to be installed. diff --git a/cmake-tests/declareProjectTest.cmake b/cmake-tests/declareProjectTest.cmake new file mode 100644 index 000000000..3e6fbc299 --- /dev/null +++ b/cmake-tests/declareProjectTest.cmake @@ -0,0 +1,100 @@ +# This file defines macros which can be used to setup +# new cmake project tests without introducing excessive boilerplate. + +# declare_add_subdirectory_test(): +# Use when the test depends on launchdarkly via add_subdirectory. + +# declare_find_package_test(): +# Use when the test depends on launchdarkly via find_package. + +# add_build_step(): +# By default, the declare_* macros result in a test where "cmake -DSOMEVARIABLE=WHATEVER .." +# (the cmake configure step) is invoked. This may be sufficient for a particular test, +# for example testing that the configure step fails. +# If the test should also invoke "cmake --build .", use this macro. + +# require_configure_failure(): +# Asserts that the cmake configure step should fail. For example, this would +# happen if a required version of a dependency couldn't be satisfied with find_package. + +# require_build_failure(): +# Asserts that the cmake build step should fail. + +macro(declare_add_subdirectory_test name) + set(test_prefix ${name}) + + add_test( + NAME ${test_prefix}_configure + COMMAND + ${CMAKE_COMMAND} + # Since project/CMakeLists.txt is going to call add_subdirectory(), it needs to know where + # the SDK's project is (which is actually a couple directories above this particular file; not normally the case.) + # The variable name is arbitrary. + -DLAUNCHDARKLY_SOURCE_DIR=${PROJECT_SOURCE_DIR} + # Do not setup all of the SDK's testing machinery, which would normally happen when calling add_subdirectory. + -DBUILD_TESTING=OFF + # Forward variables from the SDK project to the test project, if set. + $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> + $<$:-DBOOST_LIBRARY=${BOOST_LIBRARY}> + $<$:-DBOOST_INCLUDE_DIR=${BOOST_INCLUDE_DIR}> + $<$:-DOPENSSL_LIBRARY=${OPENSSL_LIBRARY}> + $<$:-DOPENSSL_INCLUDE_DIR=${OPENSSL_INCLUDE_DIR}> + ${CMAKE_CURRENT_SOURCE_DIR}/project + ) + + set_tests_properties(${test_prefix}_configure + PROPERTIES + FIXTURES_SETUP ${test_prefix} + # Forward along the CC and CXX environment variables, because clang11 CI build uses them. + ENVIRONMENT "CC=${CMAKE_C_COMPILER};CXX=${CMAKE_CXX_COMPILER}" + ) +endmacro() + +macro(require_configure_failure name) + set_tests_properties(${name}_configure PROPERTIES WILL_FAIL TRUE) +endmacro() + +macro(require_build_failure name) + set_tests_properties(${name}_build PROPERTIES WILL_FAIL TRUE) +endmacro() + +macro(add_build_step name) + # Setup a 'test' to perform the cmake build step. + add_test( + NAME ${name}_build + COMMAND ${CMAKE_COMMAND} --build . + ) + + set_tests_properties(${name}_build + PROPERTIES + FIXTURES_REQUIRED ${name} + ) +endmacro() + +macro(declare_find_package_test name) + # This test assumes that the SDK has been installed at CMAKE_INSTALL_PREFIX. + set(test_prefix ${name}) + + add_test( + NAME ${test_prefix}_configure + COMMAND + ${CMAKE_COMMAND} + # Since project/CMakeLists.txt uses find_package(), it needs to know where to find + # ldserverapiConfig.cmake. That can be found where the SDK is installed, which is CMAKE_INSTALL_PREFIX. + -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX} + # Forward variables from the SDK project to the test project, if set. + $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> + $<$:-DBOOST_LIBRARY=${BOOST_LIBRARY}> + $<$:-DBOOST_INCLUDE_DIR=${BOOST_INCLUDE_DIR}> + $<$:-DOPENSSL_LIBRARY=${OPENSSL_LIBRARY}> + $<$:-DOPENSSL_INCLUDE_DIR=${OPENSSL_INCLUDE_DIR}> + ${CMAKE_CURRENT_SOURCE_DIR}/project + ) + + set_tests_properties(${test_prefix}_configure + PROPERTIES + FIXTURES_SETUP ${test_prefix} + # Forward along the CC and CXX environment variables, because clang11 CI build uses them. + ENVIRONMENT "CC=${CMAKE_C_COMPILER};CXX=${CMAKE_CXX_COMPILER}" + ) +endmacro() diff --git a/cmake-tests/test_add_subdirectory/CMakeLists.txt b/cmake-tests/test_add_subdirectory/CMakeLists.txt new file mode 100644 index 000000000..950c3dd8d --- /dev/null +++ b/cmake-tests/test_add_subdirectory/CMakeLists.txt @@ -0,0 +1,2 @@ +declare_add_subdirectory_test(test_add_subdirectory) +add_build_step(test_add_subdirectory) diff --git a/cmake-tests/test_add_subdirectory/project/CMakeLists.txt b/cmake-tests/test_add_subdirectory/project/CMakeLists.txt new file mode 100644 index 000000000..7b18814c2 --- /dev/null +++ b/cmake-tests/test_add_subdirectory/project/CMakeLists.txt @@ -0,0 +1,26 @@ +cmake_minimum_required(VERSION 3.19) + +project(AddSubdirectoryTest) + +add_subdirectory( + # Source directory where the SDK's CMakeLists.txt is located. + ${LAUNCHDARKLY_SOURCE_DIR} + # Binary directory must be specified when using an out-of-tree source. + ${CMAKE_CURRENT_BINARY_DIR}/launchdarkly +) + +set(TARGET_PREFIX add_subdirectory) + +# Server-side +add_executable(${TARGET_PREFIX}_server_cpp main_server.cpp) +target_link_libraries(${TARGET_PREFIX}_server_cpp launchdarkly-cpp-server) + +add_executable(${TARGET_PREFIX}_server_cpp_alias main_server.cpp) +target_link_libraries(${TARGET_PREFIX}_server_cpp_alias launchdarkly::server) + +# Client-side +add_executable(${TARGET_PREFIX}_client_cpp main_client.cpp) +target_link_libraries(${TARGET_PREFIX}_client_cpp launchdarkly-cpp-client) + +add_executable(${TARGET_PREFIX}_client_cpp_alias main_client.cpp) +target_link_libraries(${TARGET_PREFIX}_client_cpp_alias launchdarkly::client) diff --git a/cmake-tests/test_add_subdirectory/project/main_client.cpp b/cmake-tests/test_add_subdirectory/project/main_client.cpp new file mode 100644 index 000000000..6c548ab8f --- /dev/null +++ b/cmake-tests/test_add_subdirectory/project/main_client.cpp @@ -0,0 +1,25 @@ +#include +#include + +#include +#include + +using namespace launchdarkly; +using namespace launchdarkly::client_side; + +int main() { + auto config = ConfigBuilder("sdk-key").Build(); + if (!config) { + std::cout << "error: config is invalid: " << config.error() << '\n'; + return 1; + } + + auto context = + ContextBuilder().Kind("user", "example-user-key").Name("Sandy").Build(); + + auto client = Client(std::move(*config), std::move(context)); + + client.StartAsync(); + + std::cout << client.Initialized() << '\n'; +} diff --git a/cmake-tests/test_add_subdirectory/project/main_server.cpp b/cmake-tests/test_add_subdirectory/project/main_server.cpp new file mode 100644 index 000000000..aa55e3575 --- /dev/null +++ b/cmake-tests/test_add_subdirectory/project/main_server.cpp @@ -0,0 +1,23 @@ +#include +#include + +#include +#include + +using namespace launchdarkly; +using namespace launchdarkly::server_side; + +int main() { + auto config = ConfigBuilder("sdk-key").Build(); + if (!config) { + std::cout << "error: config is invalid: " << config.error() << '\n'; + return 1; + } + + auto client = Client(std::move(*config)); + + client.StartAsync(); + + std::cout << client.Initialized() << '\n'; + +} diff --git a/scripts/build-cmake-integration-tests.sh b/scripts/build-cmake-integration-tests.sh new file mode 100755 index 000000000..3849e92ff --- /dev/null +++ b/scripts/build-cmake-integration-tests.sh @@ -0,0 +1,20 @@ +#!/bin/bash -e + +function cleanup { + cd .. +} + +mkdir -p build +cd build +# After we enter the directory we want to make sure we always exit it when the +# script ends. +trap cleanup EXIT + + + +cmake -G Ninja -D CMAKE_COMPILE_WARNING_AS_ERROR=TRUE \ + -D BUILD_TESTING=ON \ + -D LD_CMAKE_INTEGRATION_TESTS=ON \ + -D LD_BUILD_EXAMPLES=OFF .. + +cmake --build . From cd925a15cb20b84a8773bcef39b705b51827cafe Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 15:26:46 -0700 Subject: [PATCH 2/8] get build deps --- .github/workflows/cmake.yml | 19 ++++++++- cmake-tests/README.md | 40 +++---------------- cmake-tests/declareProjectTest.cmake | 12 ++---- ...h => configure-cmake-integration-tests.sh} | 2 - 4 files changed, 26 insertions(+), 47 deletions(-) rename scripts/{build-cmake-integration-tests.sh => configure-cmake-integration-tests.sh} (95%) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 0a14f4ce4..f316ca1e6 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -18,7 +18,22 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - - run: ./scripts/build-cmake-integration-tests.sh - - run: + - name: Install Ninja + uses: ./.github/actions/install-ninja + - name: Install boost + uses: ./.github/actions/install-boost + id: install-boost + with: + platform_version: "22.04" + - name: Install OpenSSL + uses: ./.github/actions/install-openssl + id: install-openssl + - run: ./scripts/configure-cmake-integration-tests.sh + env: + # Note: these are consumed by the SDK's CMake project, and then will be passed into the cmake integration tests + # so that the test projects will know where to find Boost and OpenSSL. More info in the README. + BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} + OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} + - run: | export CTEST_OUTPUT_ON_FAILURE=1 cd build/cmake-tests && ctest diff --git a/cmake-tests/README.md b/cmake-tests/README.md index ac7acb5b2..1729fa2d2 100644 --- a/cmake-tests/README.md +++ b/cmake-tests/README.md @@ -51,11 +51,11 @@ Without setting these, the test would fail to build with the same compilers as t Additionally, certain variables must be forwarded to each test project CMake configuration. -| Variable | Explanation | -|-----------------------------------------|--------------------------------------------------------------------------------------------------------------------------| -| `BOOST_LIBRARY`/`BOOST_INCLUDE_DIR` | Windows build uses Boost downloaded at runtime, there's no system Boost. | -| `OPENSSL_LIBRARY`/`OPENSSL_INCLUDE_DIR` | Windows build uses OpenSSL downloaded at runtime. | -| `CMAKE_GENERATOR_PLATFORM` | Windows build explicitly specifies x64 build, whereas the default project build would be x86. Linker errors would ensue. | +| Variable | Explanation | +|----------------------------|--------------------------------------------------------------------------------------------------------------------------| +| `BOOST_ROOT` | Path to Boost. | +| `OPENSSL_ROOT_DIR` | Path to OpenSSL. | +| `CMAKE_GENERATOR_PLATFORM` | Windows build explicitly specifies x64 build, whereas the default project build would be x86. Linker errors would ensue. | The creation logic uses a series of CMake generator expressions (`$<...>`) to forward the variables in the table above from the main SDK project (which `add_subdirectory`'d each test) to the test projects. @@ -73,33 +73,3 @@ The generator expressions omit the `-D` entirely if the original variable is emp Checks that a project can include the SDK as a sub-project, via `add_subdirectory`. This would be a likely use-case when the repo is a submodule of another project. - -### cmake_projects/test_find_package - -Checks that a project can include the SDK via `find_package(ldserverapi)`. -This would be a likely use-case if the SDK was installed on the system by the user. - -**NOTE:** Requires SDK to be installed. - -### cmake_projects/test_find_package_cpp - -Checks that a C++ project can include the SDK via `find_package(ldserverapi)`. -Also checks that C++ bindings can be included without compilation issues. - -**NOTE:** Requires SDK to be installed. - -### cmake_projects/test_find_package_compatible_version - -Checks that a project can include the SDK via `find_package(ldserverapi [version])`. -This would be a likely use-case if the user depends on a particular version of the SDK, -rather than accepting any version. - -**NOTE:** Requires SDK to be installed. - -### cmake_projects/test_find_package_incompatible_version - -Checks that a project will *fail* to configure if `find_package(ldserverapi [version])` -is invoked with a version that isn't present on the system. The test uses a fictional -version `10.0.0`. - -**NOTE:** Requires SDK to be installed. diff --git a/cmake-tests/declareProjectTest.cmake b/cmake-tests/declareProjectTest.cmake index 3e6fbc299..f6f75b8fc 100644 --- a/cmake-tests/declareProjectTest.cmake +++ b/cmake-tests/declareProjectTest.cmake @@ -35,10 +35,8 @@ macro(declare_add_subdirectory_test name) -DBUILD_TESTING=OFF # Forward variables from the SDK project to the test project, if set. $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> - $<$:-DBOOST_LIBRARY=${BOOST_LIBRARY}> - $<$:-DBOOST_INCLUDE_DIR=${BOOST_INCLUDE_DIR}> - $<$:-DOPENSSL_LIBRARY=${OPENSSL_LIBRARY}> - $<$:-DOPENSSL_INCLUDE_DIR=${OPENSSL_INCLUDE_DIR}> + $<$:-DBOOST_ROOT=${BOOST_ROOT}> + $<$:-DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR}> ${CMAKE_CURRENT_SOURCE_DIR}/project ) @@ -84,10 +82,8 @@ macro(declare_find_package_test name) -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX} # Forward variables from the SDK project to the test project, if set. $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> - $<$:-DBOOST_LIBRARY=${BOOST_LIBRARY}> - $<$:-DBOOST_INCLUDE_DIR=${BOOST_INCLUDE_DIR}> - $<$:-DOPENSSL_LIBRARY=${OPENSSL_LIBRARY}> - $<$:-DOPENSSL_INCLUDE_DIR=${OPENSSL_INCLUDE_DIR}> + $<$:-DBOOST_ROOT=${BOOST_ROOT}> + $<$:-DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR}> ${CMAKE_CURRENT_SOURCE_DIR}/project ) diff --git a/scripts/build-cmake-integration-tests.sh b/scripts/configure-cmake-integration-tests.sh similarity index 95% rename from scripts/build-cmake-integration-tests.sh rename to scripts/configure-cmake-integration-tests.sh index 3849e92ff..c4ad1562f 100755 --- a/scripts/build-cmake-integration-tests.sh +++ b/scripts/configure-cmake-integration-tests.sh @@ -16,5 +16,3 @@ cmake -G Ninja -D CMAKE_COMPILE_WARNING_AS_ERROR=TRUE \ -D BUILD_TESTING=ON \ -D LD_CMAKE_INTEGRATION_TESTS=ON \ -D LD_BUILD_EXAMPLES=OFF .. - -cmake --build . From 2d25c68b328b733aa55f2413320de8fb3bd60e24 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 15:49:48 -0700 Subject: [PATCH 3/8] try not passing boost_root and openssl_root_dir --- cmake-tests/declareProjectTest.cmake | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmake-tests/declareProjectTest.cmake b/cmake-tests/declareProjectTest.cmake index f6f75b8fc..0cca96356 100644 --- a/cmake-tests/declareProjectTest.cmake +++ b/cmake-tests/declareProjectTest.cmake @@ -35,8 +35,6 @@ macro(declare_add_subdirectory_test name) -DBUILD_TESTING=OFF # Forward variables from the SDK project to the test project, if set. $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> - $<$:-DBOOST_ROOT=${BOOST_ROOT}> - $<$:-DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR}> ${CMAKE_CURRENT_SOURCE_DIR}/project ) @@ -82,8 +80,6 @@ macro(declare_find_package_test name) -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX} # Forward variables from the SDK project to the test project, if set. $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> - $<$:-DBOOST_ROOT=${BOOST_ROOT}> - $<$:-DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR}> ${CMAKE_CURRENT_SOURCE_DIR}/project ) From 818133f67134a9f909063085cd54cce0750eb2db Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 15:53:26 -0700 Subject: [PATCH 4/8] try using env variables' --- .github/workflows/cmake.yml | 11 +++++++++-- cmake-tests/declareProjectTest.cmake | 4 ---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index f316ca1e6..15f15259b 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -28,12 +28,19 @@ jobs: - name: Install OpenSSL uses: ./.github/actions/install-openssl id: install-openssl - - run: ./scripts/configure-cmake-integration-tests.sh + - name: Configure CMake Integration Tests + run: ./scripts/configure-cmake-integration-tests.sh env: # Note: these are consumed by the SDK's CMake project, and then will be passed into the cmake integration tests # so that the test projects will know where to find Boost and OpenSSL. More info in the README. BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} - - run: | + - name: Run CMake Integration Tests + run: | export CTEST_OUTPUT_ON_FAILURE=1 cd build/cmake-tests && ctest + env: + # Note: these are consumed by the SDK's CMake project, and then will be passed into the cmake integration tests + # so that the test projects will know where to find Boost and OpenSSL. More info in the README. + BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} + OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} diff --git a/cmake-tests/declareProjectTest.cmake b/cmake-tests/declareProjectTest.cmake index 0cca96356..2d6cd1dc7 100644 --- a/cmake-tests/declareProjectTest.cmake +++ b/cmake-tests/declareProjectTest.cmake @@ -33,8 +33,6 @@ macro(declare_add_subdirectory_test name) -DLAUNCHDARKLY_SOURCE_DIR=${PROJECT_SOURCE_DIR} # Do not setup all of the SDK's testing machinery, which would normally happen when calling add_subdirectory. -DBUILD_TESTING=OFF - # Forward variables from the SDK project to the test project, if set. - $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> ${CMAKE_CURRENT_SOURCE_DIR}/project ) @@ -78,8 +76,6 @@ macro(declare_find_package_test name) # Since project/CMakeLists.txt uses find_package(), it needs to know where to find # ldserverapiConfig.cmake. That can be found where the SDK is installed, which is CMAKE_INSTALL_PREFIX. -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX} - # Forward variables from the SDK project to the test project, if set. - $<$:-DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}> ${CMAKE_CURRENT_SOURCE_DIR}/project ) From 3c03de5fcab03b28d6107ac22d2fe96ccfec8b6d Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 16:05:05 -0700 Subject: [PATCH 5/8] try passing on command line --- .github/workflows/cmake.yml | 10 +++------- cmake-tests/declareProjectTest.cmake | 2 ++ scripts/configure-cmake-integration-tests.sh | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 15f15259b..737809158 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -31,16 +31,12 @@ jobs: - name: Configure CMake Integration Tests run: ./scripts/configure-cmake-integration-tests.sh env: - # Note: these are consumed by the SDK's CMake project, and then will be passed into the cmake integration tests - # so that the test projects will know where to find Boost and OpenSSL. More info in the README. + # These will be injected into the SDK CMake project on the command line, via "-D BOOST_ROOT=..." + # and "-D OPENSSL_ROOT_DIR=...". When the integration tests are configured, they will then be passed + # along in the same manner to those test projects via the command line. BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} - name: Run CMake Integration Tests run: | export CTEST_OUTPUT_ON_FAILURE=1 cd build/cmake-tests && ctest - env: - # Note: these are consumed by the SDK's CMake project, and then will be passed into the cmake integration tests - # so that the test projects will know where to find Boost and OpenSSL. More info in the README. - BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} - OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} diff --git a/cmake-tests/declareProjectTest.cmake b/cmake-tests/declareProjectTest.cmake index 2d6cd1dc7..0adbcf547 100644 --- a/cmake-tests/declareProjectTest.cmake +++ b/cmake-tests/declareProjectTest.cmake @@ -33,6 +33,8 @@ macro(declare_add_subdirectory_test name) -DLAUNCHDARKLY_SOURCE_DIR=${PROJECT_SOURCE_DIR} # Do not setup all of the SDK's testing machinery, which would normally happen when calling add_subdirectory. -DBUILD_TESTING=OFF + -DBOOST_ROOT=${BOOST_ROOT} + -DOPENSSL_ROOT_DIR=${OPENSSL_ROOT_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/project ) diff --git a/scripts/configure-cmake-integration-tests.sh b/scripts/configure-cmake-integration-tests.sh index c4ad1562f..acc052f71 100755 --- a/scripts/configure-cmake-integration-tests.sh +++ b/scripts/configure-cmake-integration-tests.sh @@ -1,5 +1,18 @@ #!/bin/bash -e + +if [ -z "$BOOST_ROOT" ]; then + echo "BOOST_ROOT is not set. Please set it to the root directory boost so it can be forwarded to CMake integration + tests." + exit 1 +fi + +if [ -z "$OPENSSL_ROOT_DIR" ]; then + echo "OPENSSL_ROOT_DIR is not set. Please set it to the root directory of OpenSSL so it can be forwarded to CMake + integration tests." + exit 1 +fi + function cleanup { cd .. } @@ -15,4 +28,6 @@ trap cleanup EXIT cmake -G Ninja -D CMAKE_COMPILE_WARNING_AS_ERROR=TRUE \ -D BUILD_TESTING=ON \ -D LD_CMAKE_INTEGRATION_TESTS=ON \ + -D BOOST_ROOT="$BOOST_ROOT" \ + -D OPENSSL_ROOT_DIR="$OPENSSL_ROOT_DIR" \ -D LD_BUILD_EXAMPLES=OFF .. From ef513bf8f421a1da65fb31a563db6208da981d0e Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 16:11:55 -0700 Subject: [PATCH 6/8] try passing in empty variable --- scripts/configure-cmake-integration-tests.sh | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/scripts/configure-cmake-integration-tests.sh b/scripts/configure-cmake-integration-tests.sh index acc052f71..531d25712 100755 --- a/scripts/configure-cmake-integration-tests.sh +++ b/scripts/configure-cmake-integration-tests.sh @@ -1,18 +1,5 @@ #!/bin/bash -e - -if [ -z "$BOOST_ROOT" ]; then - echo "BOOST_ROOT is not set. Please set it to the root directory boost so it can be forwarded to CMake integration - tests." - exit 1 -fi - -if [ -z "$OPENSSL_ROOT_DIR" ]; then - echo "OPENSSL_ROOT_DIR is not set. Please set it to the root directory of OpenSSL so it can be forwarded to CMake - integration tests." - exit 1 -fi - function cleanup { cd .. } From 981a1f7c5de3bacdddc35e935fb75987da7904b9 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 16:50:20 -0700 Subject: [PATCH 7/8] more platforms --- .github/actions/cmake-test/action.yml | 43 +++++++++++++++++++++++++++ .github/workflows/cmake.yml | 43 ++++++++++++++------------- 2 files changed, 65 insertions(+), 21 deletions(-) create mode 100644 .github/actions/cmake-test/action.yml diff --git a/.github/actions/cmake-test/action.yml b/.github/actions/cmake-test/action.yml new file mode 100644 index 000000000..f036b9134 --- /dev/null +++ b/.github/actions/cmake-test/action.yml @@ -0,0 +1,43 @@ +# This is a composite to allow sharing these steps into other workflows. +# It isn't a shared workflow, because then it isn't convenient to add +# additional package-specific steps. +name: CMake Integration Test +description: 'CMake integration test suitable for running on multiple platforms.' +inputs: + platform_version: + description: 'Boost platform version' + required: false + default: "22.04" + toolset: + description: 'Boost toolset' + required: false + + +runs: + using: composite + steps: + - name: Install Ninja + uses: ./.github/actions/install-ninja + - name: Install boost + uses: ./.github/actions/install-boost + id: install-boost + with: + platform_version: ${{ inputs.platform_version }} + toolset: ${{ inputs.toolset }} + - name: Install OpenSSL + uses: ./.github/actions/install-openssl + id: install-openssl + - name: Configure CMake Integration Tests + shell: bash + run: ./scripts/configure-cmake-integration-tests.sh + env: + # These will be injected into the SDK CMake project on the command line, via "-D BOOST_ROOT=..." + # and "-D OPENSSL_ROOT_DIR=...". When the integration tests are configured, they will then be passed + # along in the same manner to those test projects via the command line. + BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} + OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} + - name: Run CMake Integration Tests + shell: bash + run: | + export CTEST_OUTPUT_ON_FAILURE=1 + cd build/cmake-tests && ctest diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 737809158..7f1be3f88 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -14,29 +14,30 @@ on: - cron: '0 8 * * *' jobs: - test: + test-ubuntu: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - - name: Install Ninja - uses: ./.github/actions/install-ninja - - name: Install boost - uses: ./.github/actions/install-boost - id: install-boost + - uses: ./.github/actions/cmake-test with: - platform_version: "22.04" - - name: Install OpenSSL - uses: ./.github/actions/install-openssl - id: install-openssl - - name: Configure CMake Integration Tests - run: ./scripts/configure-cmake-integration-tests.sh + platform_version: '22.04' + + test-macos: + runs-on: macos-12 + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/cmake-test + with: + platform_version: '12' + + test-windows: + runs-on: windows-2022 + steps: + - uses: actions/checkout@v4 + - uses: ilammy/msvc-dev-cmd@v1 + - uses: ./.github/actions/cmake-test env: - # These will be injected into the SDK CMake project on the command line, via "-D BOOST_ROOT=..." - # and "-D OPENSSL_ROOT_DIR=...". When the integration tests are configured, they will then be passed - # along in the same manner to those test projects via the command line. - BOOST_ROOT: ${{ steps.install-boost.outputs.BOOST_ROOT }} - OPENSSL_ROOT_DIR: ${{ steps.install-openssl.outputs.OPENSSL_ROOT_DIR }} - - name: Run CMake Integration Tests - run: | - export CTEST_OUTPUT_ON_FAILURE=1 - cd build/cmake-tests && ctest + BOOST_ROOT: 'C:\local\boost_1_81_0\lib64-msvc-14.3' + with: + platform_version: 2022 + toolset: msvc From b577037523267c4c490b8fcbedd64f887362a1cd Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Wed, 2 Oct 2024 17:16:05 -0700 Subject: [PATCH 8/8] docs --- cmake-tests/README.md | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/cmake-tests/README.md b/cmake-tests/README.md index 1729fa2d2..a899bdc18 100644 --- a/cmake-tests/README.md +++ b/cmake-tests/README.md @@ -3,7 +3,7 @@ This directory contains tests for various integration techniques that users of the LaunchDarkly C++ SDKs may employ. -Each test takes the form of a minimal CMake project with a `CMakeLists.txt` and `main.c/main.cpp`. +Each test takes the form of a minimal CMake project with a `CMakeLists.txt` and `main.cpp`. An additional `CMakeLists.txt` sets up the test properties. Structure: @@ -11,12 +11,12 @@ Structure: ``` some_test_directory project # Contains the CMake project under test. - - main.c[pp] # Minimal code that invokes LaunchDarkly SDK. + - main.cpp # Minimal code that invokes LaunchDarkly SDK. - CMakeLists.txt # CMake configuration that builds the project executable. - CMakeLists.txt # CMake configuration that sets up the CTest machinery for this test. ``` -*Important note about `main.c`/`main.cpp`*: +*Important note about `main.cpp`*: The optimizer employed by whatever toolchain is building the project might omit function definitions in the SDK during static linking, if those functions are proven to be unused. @@ -24,7 +24,7 @@ during static linking, if those functions are proven to be unused. The code in the main file should not have any branches that allow this to happen (such as a check for an SDK key, like in the hello demo projects.) -This could obscure linker errors if the release artifacts are generated incorrectly. +This could obscure linker errors that would have otherwise been caught. ## CMake test setup @@ -51,21 +51,10 @@ Without setting these, the test would fail to build with the same compilers as t Additionally, certain variables must be forwarded to each test project CMake configuration. -| Variable | Explanation | -|----------------------------|--------------------------------------------------------------------------------------------------------------------------| -| `BOOST_ROOT` | Path to Boost. | -| `OPENSSL_ROOT_DIR` | Path to OpenSSL. | -| `CMAKE_GENERATOR_PLATFORM` | Windows build explicitly specifies x64 build, whereas the default project build would be x86. Linker errors would ensue. | - -The creation logic uses a series of CMake generator expressions (`$<...>`) to forward the variables -in the table above from the main SDK project (which `add_subdirectory`'d each test) to the test projects. - -Simply specifying the variables directly using `-DVARIABLE=${VARIABLE}` as is normally done on the command line -wouldn't work correctly. If the variable is empty in the SDK project (like when a system package can be used), -then passing that empty string to the test project would cause the eventual `find_package(Boost/OpenSSL)` to fail, as -passing those variables implies the user wants to override the find scripts. - -The generator expressions omit the `-D` entirely if the original variable is empty, otherwise they add it. +| Variable | Explanation | +|--------------------|------------------| +| `BOOST_ROOT` | Path to Boost. | +| `OPENSSL_ROOT_DIR` | Path to OpenSSL. | ## Tests