From 760fc8f14b1ab1a1e62688782c6808afd726c01a Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 11:37:58 +0100 Subject: [PATCH 1/8] Build the singleton test on windows. --- api/test/singleton/CMakeLists.txt | 106 +++++++++++++-------------- api/test/singleton/singleton_test.cc | 40 ++++++++-- 2 files changed, 83 insertions(+), 63 deletions(-) diff --git a/api/test/singleton/CMakeLists.txt b/api/test/singleton/CMakeLists.txt index 450308cd82..7c30d6e571 100644 --- a/api/test/singleton/CMakeLists.txt +++ b/api/test/singleton/CMakeLists.txt @@ -3,59 +3,53 @@ include(GoogleTest) -# Header only singletons are not available in windows yet. - -if(NOT WIN32) - - add_library(component_a STATIC component_a.cc) - target_link_libraries(component_a opentelemetry_api) - - add_library(component_b STATIC component_b.cc) - target_link_libraries(component_b opentelemetry_api) - - add_library(component_c SHARED component_c.cc) - set_target_properties(component_c PROPERTIES CXX_VISIBILITY_PRESET default) - target_link_libraries(component_c opentelemetry_api) - - add_library(component_d SHARED component_d.cc) - set_target_properties(component_d PROPERTIES CXX_VISIBILITY_PRESET hidden) - target_link_libraries(component_d opentelemetry_api) - - add_library(component_e SHARED component_e.cc) - set_target_properties(component_e PROPERTIES CXX_VISIBILITY_PRESET default) - target_link_libraries(component_e opentelemetry_api) - - add_library(component_f SHARED component_f.cc) - set_target_properties(component_f PROPERTIES CXX_VISIBILITY_PRESET hidden) - target_link_libraries(component_f opentelemetry_api) - - add_library(component_g SHARED component_g.cc) - set_target_properties(component_g PROPERTIES CXX_VISIBILITY_PRESET default) - target_link_libraries(component_g opentelemetry_api) - - add_library(component_h SHARED component_h.cc) - set_target_properties(component_h PROPERTIES CXX_VISIBILITY_PRESET hidden) - target_link_libraries(component_h opentelemetry_api) - - add_executable(singleton_test singleton_test.cc) - - # Not linking with component_g and component_h on purpose - target_link_libraries( - singleton_test - component_a - component_b - component_c - component_d - component_e - component_f - ${GTEST_BOTH_LIBRARIES} - ${CMAKE_THREAD_LIBS_INIT} - ${CMAKE_DL_LIBS} - opentelemetry_api) - - gtest_add_tests( - TARGET singleton_test - TEST_PREFIX singleton. - TEST_LIST singleton_test) - -endif() +add_library(component_a STATIC component_a.cc) +target_link_libraries(component_a opentelemetry_api) + +add_library(component_b STATIC component_b.cc) +target_link_libraries(component_b opentelemetry_api) + +add_library(component_c SHARED component_c.cc) +set_target_properties(component_c PROPERTIES CXX_VISIBILITY_PRESET default) +target_link_libraries(component_c opentelemetry_api) + +add_library(component_d SHARED component_d.cc) +set_target_properties(component_d PROPERTIES CXX_VISIBILITY_PRESET hidden) +target_link_libraries(component_d opentelemetry_api) + +add_library(component_e SHARED component_e.cc) +set_target_properties(component_e PROPERTIES CXX_VISIBILITY_PRESET default) +target_link_libraries(component_e opentelemetry_api) + +add_library(component_f SHARED component_f.cc) +set_target_properties(component_f PROPERTIES CXX_VISIBILITY_PRESET hidden) +target_link_libraries(component_f opentelemetry_api) + +add_library(component_g SHARED component_g.cc) +set_target_properties(component_g PROPERTIES CXX_VISIBILITY_PRESET default) +target_link_libraries(component_g opentelemetry_api) + +add_library(component_h SHARED component_h.cc) +set_target_properties(component_h PROPERTIES CXX_VISIBILITY_PRESET hidden) +target_link_libraries(component_h opentelemetry_api) + +add_executable(singleton_test singleton_test.cc) + +# Not linking with component_g and component_h on purpose +target_link_libraries( + singleton_test + component_a + component_b + component_c + component_d + component_e + component_f + ${GTEST_BOTH_LIBRARIES} + ${CMAKE_THREAD_LIBS_INIT} + ${CMAKE_DL_LIBS} + opentelemetry_api) + +gtest_add_tests( + TARGET singleton_test + TEST_PREFIX singleton. + TEST_LIST singleton_test) diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index d212221118..084b84a302 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -4,12 +4,9 @@ #include #include -/* - TODO: - Once singleton are supported for windows, - expand this test to use ::LoadLibrary, ::GetProcAddress, ::FreeLibrary -*/ -#ifndef _WIN32 +#ifdef _WIN32 +# include +#else # include #endif @@ -58,28 +55,57 @@ void do_something() #ifndef BAZEL_BUILD /* Call do_something_in_g() */ +# ifdef _WIN32 + HMODULE component_g = LoadLibraryA("component_g.dll"); +# else void *component_g = dlopen("libcomponent_g.so", RTLD_NOW); +# endif + EXPECT_NE(component_g, nullptr); +# ifdef _WIN32 + auto *func_g = reinterpret_cast(GetProcAddress(component_g, "do_something_in_g")); +# else auto *func_g = reinterpret_cast(dlsym(component_g, "do_something_in_g")); +# endif + EXPECT_NE(func_g, nullptr); (*func_g)(); +# ifdef _WIN32 + FreeLibrary(component_g); +# else dlclose(component_g); +# endif /* Call do_something_in_h() */ +# ifdef _WIN32 + HMODULE component_g = LoadLibraryA("component_h.dll"); +# else void *component_h = dlopen("libcomponent_h.so", RTLD_NOW); +# endif + EXPECT_NE(component_h, nullptr); +# ifdef _WIN32 + auto *func_h = reinterpret_cast(GetProcAddress(component_g, "do_something_in_h")); +# else auto *func_h = reinterpret_cast(dlsym(component_h, "do_something_in_h")); +# endif + EXPECT_NE(func_h, nullptr); (*func_h)(); +# ifdef _WIN32 + FreeLibrary(component_h); +# else dlclose(component_h); -#endif +# endif + +#endif /* BAZEL_BUILD */ } int span_a_lib_count = 0; From f664eb8570b809234e8a0efe2040a4fc5da4dde0 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 11:49:39 +0100 Subject: [PATCH 2/8] cleanup --- api/test/singleton/singleton_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index 084b84a302..3cd9eb9e04 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -82,7 +82,7 @@ void do_something() /* Call do_something_in_h() */ # ifdef _WIN32 - HMODULE component_g = LoadLibraryA("component_h.dll"); + HMODULE component_h = LoadLibraryA("component_h.dll"); # else void *component_h = dlopen("libcomponent_h.so", RTLD_NOW); # endif @@ -90,7 +90,7 @@ void do_something() EXPECT_NE(component_h, nullptr); # ifdef _WIN32 - auto *func_h = reinterpret_cast(GetProcAddress(component_g, "do_something_in_h")); + auto *func_h = reinterpret_cast(GetProcAddress(component_h, "do_something_in_h")); # else auto *func_h = reinterpret_cast(dlsym(component_h, "do_something_in_h")); # endif From 9faacaf3ae64f75312df2e91f4b9ab90f0e1dda8 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 12:17:55 +0100 Subject: [PATCH 3/8] Output test failure on windows --- ci/do_ci.ps1 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/do_ci.ps1 b/ci/do_ci.ps1 index e9c1f64646..98d5fba7f4 100644 --- a/ci/do_ci.ps1 +++ b/ci/do_ci.ps1 @@ -27,6 +27,8 @@ $PLUGIN_DIR = Join-Path "$SRC_DIR" "plugin" $VCPKG_DIR = Join-Path "$SRC_DIR" "tools" "vcpkg" +$CTEST_OUTPUT_ON_FAILURE = 1 + switch ($action) { "bazel.build" { bazel $BAZEL_STARTUP_OPTIONS build $BAZEL_OPTIONS --action_env=VCPKG_DIR=$VCPKG_DIR --deleted_packages=opentracing-shim -- //... From 4dfe92ccb9d2829aaa69c8d88d35b317f28bfab9 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 12:32:17 +0100 Subject: [PATCH 4/8] windows --- ci/do_ci.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/do_ci.ps1 b/ci/do_ci.ps1 index 98d5fba7f4..a04ec938f5 100644 --- a/ci/do_ci.ps1 +++ b/ci/do_ci.ps1 @@ -27,7 +27,7 @@ $PLUGIN_DIR = Join-Path "$SRC_DIR" "plugin" $VCPKG_DIR = Join-Path "$SRC_DIR" "tools" "vcpkg" -$CTEST_OUTPUT_ON_FAILURE = 1 +$Env:CTEST_OUTPUT_ON_FAILURE = "1" switch ($action) { "bazel.build" { From 9153193bfeb1b01061ad464f180f117fa1060bc8 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 14:53:06 +0100 Subject: [PATCH 5/8] Disable failing test on windows --- api/test/singleton/singleton_test.cc | 40 +++++++++++++++------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index 3cd9eb9e04..d8c0ab1067 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -342,7 +342,11 @@ void cleanup_otel() trace_api::Provider::SetTracerProvider(provider); } +#ifdef _WIN32 +TEST(SingletonTest, DISABLED_Uniqueness) +#else TEST(SingletonTest, Uniqueness) +#endif { do_something(); @@ -383,26 +387,26 @@ TEST(SingletonTest, Uniqueness) EXPECT_EQ(span_b_lib_count, 1); EXPECT_EQ(span_b_f1_count, 2); EXPECT_EQ(span_b_f2_count, 1); - EXPECT_EQ(span_c_lib_count, 1); - EXPECT_EQ(span_c_f1_count, 2); - EXPECT_EQ(span_c_f2_count, 1); - EXPECT_EQ(span_d_lib_count, 1); - EXPECT_EQ(span_d_f1_count, 2); - EXPECT_EQ(span_d_f2_count, 1); - EXPECT_EQ(span_e_lib_count, 1); - EXPECT_EQ(span_e_f1_count, 2); - EXPECT_EQ(span_e_f2_count, 1); - EXPECT_EQ(span_f_lib_count, 1); - EXPECT_EQ(span_f_f1_count, 2); - EXPECT_EQ(span_f_f2_count, 1); + EXPECT_EQ(span_c_lib_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_c_f1_count, 2); // Fails with shared libraries on Windows + EXPECT_EQ(span_c_f2_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_d_lib_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_d_f1_count, 2); // Fails with shared libraries on Windows + EXPECT_EQ(span_d_f2_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_e_lib_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_e_f1_count, 2); // Fails with shared libraries on Windows + EXPECT_EQ(span_e_f2_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_f_lib_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_f_f1_count, 2); // Fails with shared libraries on Windows + EXPECT_EQ(span_f_f2_count, 1); // Fails with shared libraries on Windows #ifndef BAZEL_BUILD - EXPECT_EQ(span_g_lib_count, 1); - EXPECT_EQ(span_g_f1_count, 2); - EXPECT_EQ(span_g_f2_count, 1); - EXPECT_EQ(span_h_lib_count, 1); - EXPECT_EQ(span_h_f1_count, 2); - EXPECT_EQ(span_h_f2_count, 1); + EXPECT_EQ(span_g_lib_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_g_f1_count, 2); // Fails with shared libraries on Windows + EXPECT_EQ(span_g_f2_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_h_lib_count, 1); // Fails with shared libraries on Windows + EXPECT_EQ(span_h_f1_count, 2); // Fails with shared libraries on Windows + EXPECT_EQ(span_h_f2_count, 1); // Fails with shared libraries on Windows #endif EXPECT_EQ(unknown_span_count, 0); From 3fd20f1565b8610a5e9449ed68666d0401fbfe4f Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 15:06:18 +0100 Subject: [PATCH 6/8] cleanup --- api/test/singleton/singleton_test.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index d8c0ab1067..4f8d6ffbdb 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -343,10 +343,12 @@ void cleanup_otel() } #ifdef _WIN32 -TEST(SingletonTest, DISABLED_Uniqueness) +#define RUN_FAILING_WINDOWS_TEST 0 #else -TEST(SingletonTest, Uniqueness) +#define RUN_FAILING_WINDOWS_TEST 1 #endif + +TEST(SingletonTest, Uniqueness) { do_something(); @@ -387,6 +389,8 @@ TEST(SingletonTest, Uniqueness) EXPECT_EQ(span_b_lib_count, 1); EXPECT_EQ(span_b_f1_count, 2); EXPECT_EQ(span_b_f2_count, 1); + +#if RUN_FAILING_WINDOWS_TEST EXPECT_EQ(span_c_lib_count, 1); // Fails with shared libraries on Windows EXPECT_EQ(span_c_f1_count, 2); // Fails with shared libraries on Windows EXPECT_EQ(span_c_f2_count, 1); // Fails with shared libraries on Windows @@ -399,14 +403,17 @@ TEST(SingletonTest, Uniqueness) EXPECT_EQ(span_f_lib_count, 1); // Fails with shared libraries on Windows EXPECT_EQ(span_f_f1_count, 2); // Fails with shared libraries on Windows EXPECT_EQ(span_f_f2_count, 1); // Fails with shared libraries on Windows +#endif #ifndef BAZEL_BUILD +# if RUN_FAILING_WINDOWS_TEST EXPECT_EQ(span_g_lib_count, 1); // Fails with shared libraries on Windows EXPECT_EQ(span_g_f1_count, 2); // Fails with shared libraries on Windows EXPECT_EQ(span_g_f2_count, 1); // Fails with shared libraries on Windows EXPECT_EQ(span_h_lib_count, 1); // Fails with shared libraries on Windows EXPECT_EQ(span_h_f1_count, 2); // Fails with shared libraries on Windows EXPECT_EQ(span_h_f2_count, 1); // Fails with shared libraries on Windows +# endif #endif EXPECT_EQ(unknown_span_count, 0); From 5a936a470f55695681738e3859251581378fed1d Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 3 Dec 2024 15:08:16 +0100 Subject: [PATCH 7/8] format --- api/test/singleton/singleton_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index 4f8d6ffbdb..d7afa73831 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -343,9 +343,9 @@ void cleanup_otel() } #ifdef _WIN32 -#define RUN_FAILING_WINDOWS_TEST 0 +# define RUN_FAILING_WINDOWS_TEST 0 #else -#define RUN_FAILING_WINDOWS_TEST 1 +# define RUN_FAILING_WINDOWS_TEST 1 #endif TEST(SingletonTest, Uniqueness) From cb4f90f8c9894e344513490f5087b43a4f2a4a2c Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 5 Dec 2024 14:32:17 +0100 Subject: [PATCH 8/8] Code review comments --- api/test/singleton/singleton_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index d7afa73831..8b8cb1587d 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -342,6 +342,8 @@ void cleanup_otel() trace_api::Provider::SetTracerProvider(provider); } +// TODO: Remove once windows api singletons are supported. +// See https://github.com/open-telemetry/opentelemetry-cpp/issues/2534 #ifdef _WIN32 # define RUN_FAILING_WINDOWS_TEST 0 #else