Improved discoverability of tests; handle exceptions thrown during te…

…st execution (#4021)

* Improved discoverability of tests on WIndows; handle exceptions thrown during test execution better

CMakeSettings.json

@@ -290,7 +290,7 @@
       "configurationType": "Debug",
       "buildRoot": "${projectDir}\\out\\build\\${name}",
       "installRoot": "${projectDir}\\out\\install\\${name}",
-      "cmakeCommandArgs": "-DINSTALL_GTEST=OFF -DBUILD_TESTING=ON -DBUILD_TRANSPORT_CURL=ON -DBUILD_SAMPLES=ON -DBUILD_PERFORMANCE_TESTS=ON",
+      "cmakeCommandArgs": "",
       "buildCommandArgs": "-v",
       "inheritEnvironments": [ "msvc_x64_x64" ],
       "variables": [
@@ -299,6 +299,88 @@
           "value": "x64-windows-static",
           "type": "STRING"
         },
+        {
+          "name": "INSTALL_GTEST",
+          "value": "False",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_TESTING",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_SAMPLES",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_PERFORMANCE_TESTS",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_TRANSPORT_WINHTTP",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_TRANSPORT_CURL",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "MSVC_USE_STATIC_CRT",
+          "value": "True",
+          "type": "BOOL"
+        }
+      ]
+    },
+    {
+      "name": "x64-ReleaseWithPerfTest",
+      "generator": "Ninja",
+      "configurationType": "RelWithDebInfo",
+      "buildRoot": "${projectDir}\\out\\build\\${name}",
+      "installRoot": "${projectDir}\\out\\install\\${name}",
+      "cmakeCommandArgs": "",
+      "buildCommandArgs": "-v",
+      "inheritEnvironments": [ "msvc_x64_x64" ],
+      "variables": [
+        {
+          "name": "VCPKG_TARGET_TRIPLET",
+          "value": "x64-windows-static",
+          "type": "STRING"
+        },
+        {
+          "name": "INSTALL_GTEST",
+          "value": "False",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_TESTING",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_SAMPLES",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_PERFORMANCE_TESTS",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_TRANSPORT_WINHTTP",
+          "value": "True",
+          "type": "BOOL"
+        },
+        {
+          "name": "BUILD_TRANSPORT_CURL",
+          "value": "True",
+          "type": "BOOL"
+        },
         {
           "name": "MSVC_USE_STATIC_CRT",
           "value": "True",

sdk/core/perf/README.md

@@ -46,6 +46,7 @@ usage: azure-perf-test testName [options]
 >Note: You can use the option `-h` to print out the available options for a test name.
 
 The next options can be used for any test:
+
 | Option     | Activators | Description | Default | Example |
 | ---------- | ---        | ---| ---| --- |
 | Duration   | -d, --duration   | Duration of the test in seconds                  | 10    | -d 5

sdk/core/perf/inc/azure/perf/argagg.hpp

@@ -1436,7 +1436,7 @@ inline fmt_ostream::~fmt_ostream() { output << fmt_string(this->str()); }
 
 inline std::string lstrip(const std::string& text)
 {
-  auto result = text;
+  std::string result = text;
 
   result.erase(result.begin(), std::find_if(result.begin(), result.end(), [](int ch) {
                  return !std::isspace(ch);
@@ -1447,7 +1447,7 @@ inline std::string lstrip(const std::string& text)
 
 inline std::string rstrip(const std::string& text)
 {
-  auto result = text;
+  std::string result = text;
 
   result.erase(
       std::find_if(result.rbegin(), result.rend(), [](int ch) { return !std::isspace(ch); }).base(),

sdk/core/perf/src/arg_parser.cpp

@@ -18,12 +18,12 @@ argagg::parser_results Azure::Perf::Program::ArgParser::Parse(
   // Option Name, Activate options, display message and number of expected args.
   argagg::parser argParser;
   auto optionsMetadata = Azure::Perf::GlobalTestOptions::GetOptionMetadata();
-  for (auto option : testOptions)
+  for (auto const& option : testOptions)
   {
     argParser.definitions.push_back(
         {option.Name, option.Activators, option.DisplayMessage, option.ExpectedArgs});
   }
-  for (auto option : optionsMetadata)
+  for (auto const& option : optionsMetadata)
   {
     argParser.definitions.push_back(
         {option.Name, option.Activators, option.DisplayMessage, option.ExpectedArgs});

sdk/core/perf/src/base_test.cpp

@@ -32,7 +32,7 @@ class ProxyPolicy final : public HttpPolicy {
   ProxyPolicy(ProxyPolicy const& other) : ProxyPolicy{other.m_testContext} {}
 
   // move
-  ProxyPolicy(ProxyPolicy&& other) : m_testContext{other.m_testContext} {}
+  ProxyPolicy(ProxyPolicy&& other) noexcept : m_testContext{other.m_testContext} {}
 
   std::unique_ptr<RawResponse> Send(
       Request& request,

sdk/core/perf/src/program.cpp

@@ -6,23 +6,23 @@
 
 #include <azure/core/internal/json/json.hpp>
 #include <azure/core/internal/strings.hpp>
+#include <azure/core/platform.hpp>
 
 #include <chrono>
+#include <csignal>
 #include <iostream>
 #include <thread>
 
 namespace {
 
-inline std::unique_ptr<Azure::Perf::PerfTest> PrintAvailableTests(
-    std::vector<Azure::Perf::TestMetadata> const& tests)
+inline void PrintAvailableTests(std::vector<Azure::Perf::TestMetadata> const& tests)
 {
   std::cout << "No test name found in the input. Available tests to run:" << std::endl;
   std::cout << std::endl << "Name\t\tDescription" << std::endl << "---\t\t---" << std::endl;
-  for (auto test : tests)
+  for (auto const& test : tests)
   {
     std::cout << test.Name << "\t\t" << test.Description << std::endl;
   }
-  return nullptr;
 }
 
 inline Azure::Perf::TestMetadata const* GetTestMetadata(
@@ -37,14 +37,17 @@ inline Azure::Perf::TestMetadata const* GetTestMetadata(
   argagg::parser argParser;
   auto args = argParser.parse(argc, argv, true);
 
-  auto testName = std::string(args.pos[0]);
-
-  for (auto& test : tests)
+  if (!args.pos.empty())
   {
-    if (Azure::Core::_internal::StringExtensions::LocaleInvariantCaseInsensitiveEqual(
-            test.Name, testName))
+    auto testName = std::string(args.pos[0]);
+
+    for (auto& test : tests)
     {
-      return &test;
+      if (Azure::Core::_internal::StringExtensions::LocaleInvariantCaseInsensitiveEqual(
+              test.Name, testName))
+      {
+        return &test;
+      }
     }
   }
   return nullptr;
@@ -79,12 +82,13 @@ inline void PrintOptions(
   {
     std::cout << std::endl << "=== Test Options ===" << std::endl;
     Azure::Core::Json::_internal::json optionsAsJson;
-    for (auto option : testOptions)
+    for (auto const& option : testOptions)
     {
       try
       {
-        optionsAsJson[option.Name]
-            = option.SensitiveData ? "***" : parsedArgs[option.Name].as<std::string>();
+        auto optionName{option.Name};
+        optionsAsJson[optionName]
+            = option.SensitiveData ? "***" : parsedArgs[optionName].as<std::string>();
       }
       catch (std::out_of_range const&)
       {
@@ -199,14 +203,14 @@ inline void RunTests(
   std::vector<std::chrono::nanoseconds> lastCompletionTimes(parallelTestsCount);
 
   /********************* Progress Reporter ******************************/
-  Azure::Core::Context progresToken;
+  Azure::Core::Context progressToken;
   uint64_t lastCompleted = 0;
   auto progressThread = std::thread(
-      [&title, &completedOperations, &lastCompletionTimes, &lastCompleted, &progresToken]() {
+      [&title, &completedOperations, &lastCompletionTimes, &lastCompleted, &progressToken]() {
         std::cout << std::endl
                   << "=== " << title << " ===" << std::endl
                   << "Current\t\tTotal\t\tAverage" << std::endl;
-        while (!progresToken.IsCancelled())
+        while (!progressToken.IsCancelled())
         {
           using namespace std::chrono_literals;
           std::this_thread::sleep_for(1000ms);
@@ -228,7 +232,6 @@ inline void RunTests(
           bool isCancelled = false;
           // Azure::Context is not good performer for checking cancellation inside the test loop
           auto manualCancellation = std::thread([&deadLineSeconds, &isCancelled] {
-            using namespace std::chrono_literals;
             std::this_thread::sleep_for(deadLineSeconds);
             isCancelled = true;
           });
@@ -251,7 +254,7 @@ inline void RunTests(
   }
 
   // Stop progress
-  progresToken.Cancel();
+  progressToken.Cancel();
   progressThread.join();
 
   std::cout << std::endl << "=== Results ===";
@@ -278,15 +281,37 @@ void Azure::Perf::Program::Run(
     int argc,
     char** argv)
 {
+  // Ensure that all calls to abort() no longer pop up a modal dialog on Windows.
+#if defined(_DEBUG) && defined(_MSC_VER)
+  _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_DEBUG);
+#endif
+
+// Declare a signal handler to report unhandled exceptions on Windows - this is not needed for other
+// OS's as they will print the exception to stderr in their terminate() function.
+#if defined(AZ_PLATFORM_WINDOWS)
+  signal(SIGABRT, [](int) {
+    try
+    {
+      throw;
+    }
+    catch (std::exception const& ex)
+    {
+      std::cout << "Exception thrown: " << ex.what() << std::endl;
+    }
+  });
+#endif // AZ_PLATFORM_WINDOWS
+
   // Parse args only to get the test name first
   auto testMetadata = GetTestMetadata(tests, argc, argv);
   auto const& testGenerator = testMetadata->Factory;
   if (testMetadata == nullptr)
   {
     // Wrong input. Print what are the options.
     PrintAvailableTests(tests);
+
     return;
   }
+
   // Initial test to get it's options, we can use a dummy parser results
   argagg::parser_results argResults;
   auto test = testGenerator(Azure::Perf::TestOptions(argResults));
@@ -351,7 +376,7 @@ void Azure::Perf::Program::Run(
   {
     if (!options.TestProxies.empty())
     {
-      std::cout << " - Creating test recordgins for each test using test-proxies..." << std::endl;
+      std::cout << " - Creating test recordings for each test using test-proxies..." << std::endl;
       std::cout << " - Enabling test-proxy playback" << std::endl;
     }
 
@@ -375,20 +400,13 @@ void Azure::Perf::Program::Run(
 
   /******************** Tests ******************************/
   std::string iterationInfo;
-  try
+  for (int iteration = 0; iteration < options.Iterations; iteration++)
   {
-    for (int iteration = 0; iteration < options.Iterations; iteration++)
+    if (iteration > 0)
     {
-      if (iteration > 0)
-      {
-        iterationInfo.append(FormatNumber(iteration));
-      }
-      RunTests(context, parallelTest, options, "Test" + iterationInfo);
+      iterationInfo.append(FormatNumber(iteration));
     }
-  }
-  catch (std::exception const& error)
-  {
-    std::cout << "Error: " << error.what();
+    RunTests(context, parallelTest, options, "Test" + iterationInfo);
   }
 
   std::cout << std::endl << "=== Pre-Cleanup ===" << std::endl;

sdk/core/perf/test/inc/azure/perf/test/curl_http_client_get_test.hpp

@@ -36,7 +36,7 @@ namespace Azure { namespace Perf { namespace Test {
      */
     void GlobalSetup() override
     {
-      _detail::HttpClient = std::make_unique<Azure::Core::Http::CurlTransport>();
+      m_httpClient = std::make_unique<Azure::Core::Http::CurlTransport>();
     }
 
     /**

sdk/core/perf/test/inc/azure/perf/test/http_client_get_test.hpp

@@ -20,17 +20,14 @@
 
 namespace Azure { namespace Perf { namespace Test {
 
-  namespace _detail {
-    static std::unique_ptr<Azure::Core::Http::HttpTransport> HttpClient;
-  } // namespace _detail
-
   /**
    * @brief A performance test that defines a test option.
    *
    */
   class HttpClientGetTest : public Azure::Perf::PerfTest {
   protected:
     Azure::Core::Url m_url;
+    static std::unique_ptr<Azure::Core::Http::HttpTransport> m_httpClient;
 
   public:
     /**
@@ -57,7 +54,7 @@ namespace Azure { namespace Perf { namespace Test {
     void Run(Azure::Core::Context const& ctx) override
     {
       Azure::Core::Http::Request request(Azure::Core::Http::HttpMethod::Get, m_url);
-      auto response = _detail::HttpClient->Send(request, ctx);
+      auto response = m_httpClient->Send(request, ctx);
       // Read the body from network
       auto bodyStream = response->ExtractBodyStream();
       response->SetBody(bodyStream->ReadToEnd(ctx));
@@ -74,4 +71,6 @@ namespace Azure { namespace Perf { namespace Test {
     }
   };
 
+  std::unique_ptr<Azure::Core::Http::HttpTransport> HttpClientGetTest::m_httpClient;
+
 }}} // namespace Azure::Perf::Test