Merge topic '394-logging-cleanup'

e3ab59fb2 limit the count to the actual number of function calls
b71421c8a have Run() take int references (so that multiple calls with argc will allow loguru to correctly modify the value)
cc86740c9 bad whitespace fix
8917bbe72 Supply the default log level only through cmake args
a0b164b42 remove static var, directly init logging to INFO level for testing inside Run Function
374989fc2 remove duplicate include
325c75fd7 store What as a member var
ede1f7832 something is still fishy
...

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !1774

CMake/VTKmCheckCopyright.cmake

@@ -67,7 +67,7 @@ find_path(BUILD_DIR CMakeCache.txt .)
 get_filename_component(abs_build_dir ${BUILD_DIR} ABSOLUTE)
 get_filename_component(build_dir_name ${abs_build_dir} NAME)
 set(EXCEPTIONS ${EXCEPTIONS} ${build_dir_name}/*)
-message("${EXCEPTIONS}")
+message("Copyright Check Exceptions: ${EXCEPTIONS}")
 
 # Gets the current year (if possible).
 function (get_year var)

CMake/testing/VTKmTestWrappers.cmake

@@ -97,6 +97,9 @@ function(vtkm_unit_tests)
     set(test_prog "UnitTests_${kit}")
   endif()
 
+  # For Testing Purposes, we will set the default logging level to INFO
+  list(APPEND vtkm_default_test_log_level "-v" "INFO")
+
   if(VTKm_UT_MPI)
     # for MPI tests, suffix test name and add MPI_Init/MPI_Finalize calls.
     set(test_prog "${test_prog}_mpi")
@@ -156,12 +159,13 @@ function(vtkm_unit_tests)
       if(VTKm_UT_MPI AND VTKm_ENABLE_MPI)
         add_test(NAME ${tname}${upper_backend}
           COMMAND ${MPIEXEC} ${MPIEXEC_NUMPROC_FLAG} 3 ${MPIEXEC_PREFLAGS}
-                  $<TARGET_FILE:${test_prog}> ${tname} ${device_command_line_argument} ${VTKm_UT_TEST_ARGS}
-                  ${MPIEXEC_POSTFLAGS}
+                  $<TARGET_FILE:${test_prog}> ${tname} ${device_command_line_argument}
+                  ${vtkm_default_test_log_level} ${VTKm_UT_TEST_ARGS} ${MPIEXEC_POSTFLAGS}
           )
       else()
         add_test(NAME ${tname}${upper_backend}
-          COMMAND ${test_prog} ${tname} ${device_command_line_argument} ${VTKm_UT_TEST_ARGS}
+          COMMAND ${test_prog} ${tname} ${device_command_line_argument}
+                  ${vtkm_default_test_log_level} ${VTKm_UT_TEST_ARGS}
           )
       endif()
 

CMakeLists.txt

@@ -83,7 +83,6 @@ vtkm_option(VTKm_ENABLE_TBB "Enable TBB support" OFF)
 vtkm_option(VTKm_ENABLE_OPENMP "Enable OpenMP support" OFF)
 vtkm_option(VTKm_ENABLE_RENDERING "Enable rendering library" ON)
 vtkm_option(VTKm_ENABLE_BENCHMARKS "Enable VTKm Benchmarking" OFF)
-vtkm_option(VTKm_ENABLE_LOGGING "Enable VTKm Logging" OFF)
 vtkm_option(VTKm_ENABLE_MPI "Enable MPI support" OFF)
 vtkm_option(VTKm_ENABLE_DOCUMENTATION "Build Doxygen documentation" OFF)
 vtkm_option(VTKm_ENABLE_EXAMPLES "Build examples" OFF)
@@ -98,6 +97,12 @@ endif()
 vtkm_option(VTKm_USE_DOUBLE_PRECISION "Use double precision for floating point calculations" OFF)
 vtkm_option(VTKm_USE_64BIT_IDS "Use 64-bit indices." ON)
 
+# VTK-m will turn on logging by default, but will set the default
+# logging level to WARN.  This option should not be visible by default
+# in the GUI, as ERROR and WARN level logging should not interfere
+# with the performance of vtk-m
+vtkm_option(VTKm_ENABLE_LOGGING "Enable VTKm Logging" ON)
+
 # When VTK-m is embedded into larger projects they may desire to turn off
 # VTK-m internal assert checks when in debug mode to improve debug runtime
 # performance.
@@ -124,6 +129,7 @@ set(VTKm_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
 vtkm_option(VTKm_ENABLE_DEVELOPER_FLAGS "Enable compiler flags that are useful while developing VTK-m" ON)
 
 mark_as_advanced(
+  VTKm_ENABLE_LOGGING
   VTKm_NO_ASSERT
   VTKm_INSTALL_ONLY_LIBRARIES
   VTKm_USE_DEFAULT_SYMBOL_VISIBILITY

vtkm/cont/Error.h

@@ -37,6 +37,7 @@ public:
 #ifndef GetMessage
   const std::string& GetMessage() const { return this->Message; }
 #endif
+  const std::string& GetStackTrace() const { return this->StackTrace; }
 
 //GetMessage is a macro defined by <windows.h> to redirrect to
 //GetMessageA or W depending on if you are using ansi or unicode.
@@ -47,7 +48,7 @@ public:
 #endif
 
   // For std::exception compatibility:
-  const char* what() const noexcept override { return this->Message.c_str(); }
+  const char* what() const noexcept override { return this->What.c_str(); }
 
   /// Returns true if this exception is device independent. For exceptions that
   /// are not device independent, `vtkm::TryExecute`, for example, may try
@@ -58,17 +59,18 @@ protected:
   Error() {}
   Error(const std::string& message, bool is_device_independent = false)
     : Message(message)
+    , StackTrace(vtkm::cont::GetStackTrace(1))
+    , What(Message + "\n" + StackTrace)
     , IsDeviceIndependent(is_device_independent)
   {
-    VTKM_LOG_S(vtkm::cont::LogLevel::Warn,
-               "Exception raised: " << message << "\n"
-                                    << vtkm::cont::GetStackTrace(1)); // 1 = skip this ctor frame.
   }
 
   void SetMessage(const std::string& message) { this->Message = message; }
 
 private:
   std::string Message;
+  std::string StackTrace;
+  std::string What;
   bool IsDeviceIndependent;
 };
 

vtkm/cont/Logging.cxx

@@ -123,6 +123,8 @@ void InitLogging(int& argc, char* argv[])
   loguru::set_verbosity_to_name_callback(&verbosityToNameCallback);
   loguru::set_name_to_verbosity_callback(&nameToVerbosityCallback);
 
+  // Set the default log level to warning
+  SetStderrLogLevel(vtkm::cont::LogLevel::Warn);
   loguru::init(argc, argv);
 
   LOG_F(INFO, "Logging initialized.");

vtkm/cont/testing/CMakeLists.txt

@@ -68,6 +68,7 @@ set(unit_tests
   UnitTestDeviceAdapterAlgorithmDependency.cxx
   UnitTestDeviceAdapterAlgorithmGeneral.cxx
   UnitTestDynamicCellSet.cxx
+  UnitTestError.cxx
   UnitTestFieldRangeCompute.cxx
   UnitTestInitialize.cxx
   UnitTestLogging.cxx

vtkm/cont/testing/Testing.h

@@ -36,7 +36,7 @@ struct Testing
 {
 public:
   template <class Func>
-  static VTKM_CONT int Run(Func function, int argc, char* argv[])
+  static VTKM_CONT int Run(Func function, int& argc, char* argv[])
   {
     vtkm::cont::Initialize(argc, argv);
 

vtkm/cont/testing/UnitTestError.cxx

@@ -0,0 +1,76 @@
+//============================================================================
+//  Copyright (c) Kitware, Inc.
+//  All rights reserved.
+//  See LICENSE.txt for details.
+//
+//  This software is distributed WITHOUT ANY WARRANTY; without even
+//  the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+//  PURPOSE.  See the above copyright notice for more information.
+//============================================================================
+
+#include <vtkm/cont/Error.h>
+#include <vtkm/cont/ErrorBadValue.h>
+#include <vtkm/cont/Logging.h>
+#include <vtkm/cont/testing/Testing.h>
+
+namespace
+{
+
+void RecursiveFunction(int recurse)
+{
+  if (recurse < 5)
+  {
+    RecursiveFunction(++recurse);
+  }
+  else
+  {
+    throw vtkm::cont::ErrorBadValue("Too much recursion");
+  }
+}
+
+void ValidateError(const vtkm::cont::Error& error)
+{
+  std::string message = "Too much recursion";
+  std::string stackTrace = error.GetStackTrace();
+  std::stringstream stackTraceStream(stackTrace);
+  std::string tmp;
+  size_t count = 0;
+  while (std::getline(stackTraceStream, tmp))
+  {
+    count++;
+  }
+
+  // StackTrace may be unavailable on certain Devices
+  if (stackTrace == "(Stack trace unavailable)")
+  {
+    VTKM_TEST_ASSERT(count == 1, "Logging disabled, stack trace shouldn't be available");
+  }
+  else
+  {
+    std::string assert_msg = "StackTrace did not recurse enough: " + std::to_string(count);
+    VTKM_TEST_ASSERT(count > 6, assert_msg);
+  }
+  VTKM_TEST_ASSERT(test_equal(message, error.GetMessage()), "Message was incorrect");
+  VTKM_TEST_ASSERT(test_equal(message + "\n" + stackTrace, std::string(error.what())),
+                   "what() was incorrect");
+}
+
+void DoErrorTest()
+{
+  VTKM_LOG_S(vtkm::cont::LogLevel::Info, "Check base error messages");
+  try
+  {
+    RecursiveFunction(0);
+  }
+  catch (const vtkm::cont::Error& e)
+  {
+    ValidateError(e);
+  }
+}
+
+} // anonymous namespace
+
+int UnitTestError(int argc, char* argv[])
+{
+  return vtkm::cont::testing::Testing::Run(DoErrorTest, argc, argv);
+}

vtkm/testing/Testing.h

@@ -314,10 +314,16 @@ public:
   /// \endcode
   ///
   template <class Func>
-  static VTKM_CONT int Run(Func function, int argc, char* argv[])
+  static VTKM_CONT int Run(Func function, int& argc, char* argv[])
   {
-
-    vtkm::cont::InitLogging(argc, argv);
+    if (argc == 0 || argv == nullptr)
+    {
+      vtkm::cont::InitLogging();
+    }
+    else
+    {
+      vtkm::cont::InitLogging(argc, argv);
+    }
 
     try
     {