From 62949025b11a549f59ed513cfdaa1e52eae1f37e Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Fri, 18 Dec 2020 22:21:40 +0000 Subject: [PATCH] Merge param-tests for the transport adapter (#1217) fixes: #1216 fixes: #1087 --- sdk/core/azure-core/test/ut/CMakeLists.txt | 10 +-- .../test/ut/transport_adapter_base.hpp | 19 ++++- .../test/ut/transport_adapter_curl.cpp | 33 ++------- .../ut/transport_adapter_implementation.cpp | 73 +++++++++++++++++++ .../test/ut/transport_adapter_winhttp.cpp | 41 ----------- 5 files changed, 98 insertions(+), 78 deletions(-) create mode 100644 sdk/core/azure-core/test/ut/transport_adapter_implementation.cpp delete mode 100644 sdk/core/azure-core/test/ut/transport_adapter_winhttp.cpp diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index f7c414bc4..e02e65147 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -25,10 +25,6 @@ if(BUILD_TRANSPORT_CURL) SET(CURL_TRANSPORT_TESTS transport_adapter_curl.cpp) endif() -if(BUILD_TRANSPORT_WINHTTP) - SET(WINHTTP_TRANSPORT_TESTS transport_adapter_winhttp.cpp) -endif() - include(GoogleTest) add_executable ( @@ -51,7 +47,7 @@ add_executable ( telemetry_policy.cpp transport_adapter_base.cpp ${CURL_TRANSPORT_TESTS} - ${WINHTTP_TRANSPORT_TESTS} + transport_adapter_implementation.cpp url.cpp uuid.cpp ) @@ -64,4 +60,6 @@ target_link_libraries(azure-core-test PRIVATE azure-core gtest gmock) # gtest_discover_tests will scan the test from azure-core-test and call add_test # for each test to ctest. This enables `ctest -r` to run specific tests directly. gtest_discover_tests(azure-core-test - TEST_PREFIX azure-core.) + TEST_PREFIX azure-core. + NO_PRETTY_TYPES + NO_PRETTY_VALUES) diff --git a/sdk/core/azure-core/test/ut/transport_adapter_base.hpp b/sdk/core/azure-core/test/ut/transport_adapter_base.hpp index 803011996..4abcb197e 100644 --- a/sdk/core/azure-core/test/ut/transport_adapter_base.hpp +++ b/sdk/core/azure-core/test/ut/transport_adapter_base.hpp @@ -19,8 +19,20 @@ namespace Azure { namespace Core { namespace Test { - class TransportAdapter - : public testing::TestWithParam { + struct TransportAdaptersTestParameter + { + std::string Suffix; + Azure::Core::Http::TransportPolicyOptions TransportAdapter; + + TransportAdaptersTestParameter( + std::string suffix, + Azure::Core::Http::TransportPolicyOptions options) + : Suffix(std::move(suffix)), TransportAdapter(std::move(options)) + { + } + }; + + class TransportAdapter : public testing::TestWithParam { protected: std::unique_ptr m_pipeline; @@ -35,7 +47,8 @@ namespace Azure { namespace Core { namespace Test { policies.push_back(std::make_unique(opt)); // Will get transport policy options from test param // auto param = GetParam(); - policies.push_back(std::make_unique(GetParam())); + policies.push_back( + std::make_unique(GetParam().TransportAdapter)); m_pipeline = std::make_unique(policies); } diff --git a/sdk/core/azure-core/test/ut/transport_adapter_curl.cpp b/sdk/core/azure-core/test/ut/transport_adapter_curl.cpp index e0bce4eb0..5106e3cbd 100644 --- a/sdk/core/azure-core/test/ut/transport_adapter_curl.cpp +++ b/sdk/core/azure-core/test/ut/transport_adapter_curl.cpp @@ -3,11 +3,16 @@ #include "transport_adapter_base.hpp" #include +#include #include + #include #include #include +// The next includes are from Azure Core private headers. +// That's why the path starts from `sdk/core/azure-core/src/` +// They are included to test the connection pool from the curl transport adapter implementation. #include #include @@ -15,27 +20,6 @@ using testing::ValuesIn; namespace Azure { namespace Core { namespace Test { - /********************** Define the parameters for the base test and a suffix ***************/ - namespace { - static Azure::Core::Http::TransportPolicyOptions GetTransportOptions() - { - Azure::Core::Http::TransportPolicyOptions options; - options.Transport = std::make_shared(); - return options; - } - - // When adding more than one parameter, this function should return a unique string. - // But since we are only using one parameter (the libcurl transport adapter) this is fine. - static std::string GetSuffix(const testing::TestParamInfo& info) - { - // Can't use empty spaces or underscores (_) as per google test documentation - // https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#specifying-names-for-value-parameterized-test-parameters - (void)(info); - std::string suffix("curlImplementation"); - return suffix; - } - } // namespace - /*********************** Unique Tests for Libcurl ********************************/ // Disabling test with INCLUDE_DISABLED_TESTS. The test name cannot be changed because it depends // on a friend class definition. Hence, it can't use the gtest DISABLE_. @@ -114,11 +98,4 @@ namespace Azure { namespace Core { namespace Test { } #endif - /*********************** Base Transporter Adapter Tests ******************************/ - INSTANTIATE_TEST_SUITE_P( - TransportAdapterCurlImpl, - TransportAdapter, - testing::Values(GetTransportOptions()), - GetSuffix); - }}} // namespace Azure::Core::Test diff --git a/sdk/core/azure-core/test/ut/transport_adapter_implementation.cpp b/sdk/core/azure-core/test/ut/transport_adapter_implementation.cpp new file mode 100644 index 000000000..7ac039e20 --- /dev/null +++ b/sdk/core/azure-core/test/ut/transport_adapter_implementation.cpp @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "transport_adapter_base.hpp" + +#include +#include + +#include + +using testing::ValuesIn; + +namespace Azure { namespace Core { namespace Test { + + /********************** Define the parameters for the base test and a suffix ***************/ + namespace { + // Produces a parameter for the transport adapters tests based in a suffix and an specific + // adapter implementation + static TransportAdaptersTestParameter GetTransportOptions( + std::string suffix, + std::shared_ptr adapter) + { + Azure::Core::Http::TransportPolicyOptions options; + options.Transport = adapter; + return TransportAdaptersTestParameter(std::move(suffix), options); + } + + // When adding more than one parameter, this function should return a unique string. + static std::string GetSuffix(const testing::TestParamInfo& info) + { + // Can't use empty spaces or underscores (_) as per google test documentation + // https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#specifying-names-for-value-parameterized-test-parameters + return info.param.Suffix; + } + } // namespace + + /*********************** Transporter Adapter Tests ******************************/ + /* + * MSVC does not support adding the pre-processor `#if` condition inside the MACRO parameters, + * this is why we need to duplicate each case based on the transport adapters built. + */ +#if defined(BUILD_TRANSPORT_WINHTTP_ADAPTER) && defined(BUILD_CURL_HTTP_TRANSPORT_ADAPTER) + /* WinHttp + LibCurl */ + INSTANTIATE_TEST_SUITE_P( + Test, + TransportAdapter, + testing::Values( + GetTransportOptions("winHttp", std::make_shared()), + GetTransportOptions("libCurl", std::make_shared())), + GetSuffix); + +#elif defined(BUILD_TRANSPORT_WINHTTP_ADAPTER) + /* WinHttp */ + INSTANTIATE_TEST_SUITE_P( + Test, + TransportAdapter, + testing::Values( + GetTransportOptions("winHttp", std::make_shared())), + GetSuffix); + +#elif defined(BUILD_CURL_HTTP_TRANSPORT_ADAPTER) + /* LibCurl */ + INSTANTIATE_TEST_SUITE_P( + Test, + TransportAdapter, + testing::Values( + GetTransportOptions("libCurl", std::make_shared())), + GetSuffix); +#else + /* Custom adapter. Not adding tests */ +#endif + +}}} // namespace Azure::Core::Test diff --git a/sdk/core/azure-core/test/ut/transport_adapter_winhttp.cpp b/sdk/core/azure-core/test/ut/transport_adapter_winhttp.cpp deleted file mode 100644 index 4b55d971f..000000000 --- a/sdk/core/azure-core/test/ut/transport_adapter_winhttp.cpp +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// SPDX-License-Identifier: MIT - -#include "transport_adapter_base.hpp" -#include - -#include - -using testing::ValuesIn; - -namespace Azure { namespace Core { namespace Test { - - /********************** Define the parameters for the base test and a suffix ***************/ - namespace { - static Azure::Core::Http::TransportPolicyOptions GetTransportOptions() - { - Azure::Core::Http::TransportPolicyOptions options; - options.Transport = std::make_shared(); - return options; - } - - // When adding more than one parameter, this function should return a unique string. - // But since we are only using one parameter (the winhttp transport adapter) this is fine. - static std::string GetSuffix(const testing::TestParamInfo& info) - { - // Can't use empty spaces or underscores (_) as per google test documentation - // https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#specifying-names-for-value-parameterized-test-parameters - (void)(info); - std::string suffix("winHttpImplementation"); - return suffix; - } - } // namespace - - /*********************** Base Transporter Adapter Tests ******************************/ - INSTANTIATE_TEST_SUITE_P( - TransportAdapterWinHttpImpl, - TransportAdapter, - testing::Values(GetTransportOptions()), - GetSuffix); - -}}} // namespace Azure::Core::Test