Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a logging function #643

Merged
merged 3 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ add_subdirectory("src/realizations/catchment")
add_subdirectory("src/forcing")
add_subdirectory("src/utilities/mdarray")
add_subdirectory("src/utilities/mdframe")
add_subdirectory("src/utilities/logging")

target_link_libraries(ngen
PUBLIC
Expand All @@ -207,6 +208,7 @@ target_link_libraries(ngen
NGen::realizations_catchment
NGen::forcing
NGen::core_mediator
NGen::logging
)

if(NGEN_WITH_SQLITE)
Expand Down
16 changes: 8 additions & 8 deletions include/realizations/catchment/Bmi_Module_Formulation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
#include <DataProvider.hpp>
#include <UnitsHelper.hpp>
#include "bmi_utilities.hpp"
#include "utilities/logging_utils.h"

using data_access::MEAN;
using data_access::SUM;


// Forward declaration to provide access to protected items in testing
class Bmi_Formulation_Test;
class Bmi_Multi_Formulation_Test;
Expand Down Expand Up @@ -254,7 +254,7 @@ namespace realization {
}
catch (const std::runtime_error& e){
#ifndef UDUNITS_QUIET
std::cerr<<"WARN: Unit conversion unsuccessful - Returning unconverted value! (\""<<e.what()<<"\")"<<std::endl;
logging::warning((std::string("WARN: Unit conversion unsuccessful - Returning unconverted value! (\"")+e.what()+"\")\n").c_str());
#endif
return values;
}
Expand Down Expand Up @@ -665,13 +665,13 @@ namespace realization {
//TODO consider some additional introspection/optimization for this?
param.second.as_vector(double_vec);
if(double_vec.size() == 0){
std::cout<<"Cannot pass non-numeric lists as a BMI parameter, skipping "<<param.first<<std::endl;
logging::warning(("Cannot pass non-numeric lists as a BMI parameter, skipping "+param.first+"\n").c_str());
continue;
}
value_ptr = get_values_as_type(type, double_vec.begin(), double_vec.end());
break;
default:
std::cout<<"Cannot pass parameter of type "<<geojson::get_propertytype_name(param.second.get_type())<<" as a BMI parameter, skipping "<<param.first<<std::endl;
logging::warning(("Cannot pass parameter of type "+geojson::get_propertytype_name(param.second.get_type())+" as a BMI parameter, skipping "+param.first+"\n").c_str());
continue;
}
try{
Expand All @@ -681,13 +681,13 @@ namespace realization {
}
catch (const std::exception &e)
{
std::cout<<"Exception setting parameter value: "<< e.what()<<std::endl;
std::cout<<"Skipping parameter: "<<param.first<<std::endl;
logging::warning((std::string("Exception setting parameter value: ")+e.what()).c_str());
logging::warning(("Skipping parameter: "+param.first+"\n").c_str());
}
catch (...)
{
std::cout<<"Unknown Exception setting parameter value: "<<std::endl;
std::cout<<"Skipping parameter: "<<param.first<<std::endl;
logging::warning((std::string("Unknown Exception setting parameter value: \n")).c_str());
logging::warning(("Skipping parameter: "+param.first+"\n").c_str());
}
long_vec.clear();
double_vec.clear();
Expand Down
46 changes: 46 additions & 0 deletions include/utilities/logging_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#ifndef NGEN_LOGGING_UTILS_H
#define NGEN_LOGGING_UTILS_H

#define MAX_STRING_SIZE 2048

#ifdef __cplusplus
extern "C" {
#endif

namespace logging {
/**
* Send debug output to std::cerr
* @param msg The variable carries the humanly readable debug text info.
*/
void debug(const char* msg);

/**
* Send info content to std::cerr
* @param msg The variable carries the humanly readable info text.
*/
void info(const char* msg);

/**
* Send warning to std::cerr
* @param msg The variable carries the humanly readable warning message.
*/
void warning(const char* msg);

/**
* Send error message to std::cerr
* @param msg The variable carries the humanly readable error message.
*/
void error(const char* msg);

/**
* Send critical message to std::cerr
* @param msg The variable carries the humanly readable critical message.
*/
void critical(const char* msg);
}

#ifdef __cplusplus
}
#endif

#endif //NGEN_LOGGING_UTILS_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, really any new functionality added should have some unit tests. This is pretty simple so the unit test doesn't have to be huge, but something that puts a message through each function and makes sure it comes out with the expected text would be good. You can look at this approach from one of @program-- 's old commits for an example of how to do that:
https://github.com/program--/ngen/blob/4a1d137bff0ff3324cfb2239ac1eaec72b66705d/test/forcing/CsvPerFeatureForcingProvider_Test.cpp#L202-L216

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. this should be done.

1 change: 1 addition & 0 deletions src/realizations/catchment/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ target_link_libraries(realizations_catchment PUBLIC
${CMAKE_DL_LIBS}
NGen::core_catchment
NGen::geojson
NGen::logging
)

if(NGEN_WITH_PYTHON)
Expand Down
3 changes: 3 additions & 0 deletions src/utilities/logging/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
add_library(logging logging_utils.cpp)
add_library(NGen::logging ALIAS logging)
target_include_directories(logging PUBLIC ${PROJECT_SOURCE_DIR}/include/utilities)
46 changes: 46 additions & 0 deletions src/utilities/logging/logging_utils.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#include <iostream>
#include <cstring>
#include <string>
#include "logging_utils.h"

namespace logging {

#ifdef __cplusplus
extern "C" {
#endif

void debug(const char* msg)
{
#ifndef NGEN_QUIET
std::cerr<<"DEBUG: " << std::string(msg);
#endif
}

void info(const char* msg)
{
#ifndef NGEN_QUIET
std::cerr<<"INFO: " << std::string(msg);
#endif
}

void warning(const char* msg)
{
#ifndef NGEN_QUIET
std::cerr<<"WARNING: " <<std::string(msg);
#endif
}

void error(const char* msg)
{
std::cerr<<"ERROR: " <<std::string(msg);
}

void critical(const char* msg)
{
std::cerr<<"CRITICAL: " <<std::string(msg);
}

#ifdef __cplusplus
}
#endif
}
14 changes: 14 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ ngen_add_test(

)

########################## Logging Unit Tests
ngen_add_test(
test_logging
OBJECTS
utils/logging_Test.cpp
LIBRARIES
NGen::logging

)

########################## Nexus Tests
ngen_add_test(
test_nexus
Expand Down Expand Up @@ -334,6 +344,7 @@ ngen_add_test(
utils/mdframe_Test.cpp
utils/mdframe_netcdf_Test.cpp
utils/mdframe_csv_Test.cpp
utils/logging_Test.cpp
LIBRARIES
gmock
NGen::core
Expand All @@ -344,6 +355,7 @@ ngen_add_test(
NGen::realizations_catchment
NGen::mdarray
NGen::mdframe
NGen::logging
testbmicppmodel

)
Expand All @@ -368,6 +380,7 @@ ngen_add_test(
utils/mdframe_Test.cpp
utils/mdframe_netcdf_Test.cpp
utils/mdframe_csv_Test.cpp
utils/logging_Test.cpp
LIBRARIES
NGen::core
gmock
Expand All @@ -378,5 +391,6 @@ ngen_add_test(
NGen::realizations_catchment
NGen::mdarray
NGen::mdframe
NGen::logging
testbmicppmodel
)
109 changes: 109 additions & 0 deletions test/utils/logging_Test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
#include "gtest/gtest.h"
#include <iostream>
#include <cstring>
#include <string>
#include "logging_utils.h"

using namespace logging;

class loggingTest : public ::testing::Test {

protected:

loggingTest() {}

~loggingTest() override {}

void SetUp() override;

void TearDown() override;

};

void loggingTest::SetUp() {
}

void loggingTest::TearDown() {
}

//Test logging debug function
TEST_F(loggingTest, TestDebug)
{
const std::string test_str = "Debugging output messages.\n";

testing::internal::CaptureStderr();
debug(test_str.c_str());

const std::string cerr_output = testing::internal::GetCapturedStderr();
const std::string str = "DEBUG: Debugging output messages.\n";

#ifndef NGEN_QUIET
EXPECT_EQ(cerr_output, str);
#else
EXPECT_EQ(cerr_output, "");
#endif
}

//Test logging info function
TEST_F(loggingTest, TestInfo)
{
const std::string test_str = "Runtime information messages.\n";

testing::internal::CaptureStderr();
info(test_str.c_str());

const std::string cerr_output = testing::internal::GetCapturedStderr();
const std::string str = "INFO: Runtime information messages.\n";

#ifndef NGEN_QUIET
EXPECT_EQ(cerr_output, str);
#else
EXPECT_EQ(cerr_output, "");
#endif
}

//Test logging warning function
TEST_F(loggingTest, TestWarning)
{
const std::string test_str = "Runtime warning messages.\n";

testing::internal::CaptureStderr();
warning(test_str.c_str());

const std::string cerr_output = testing::internal::GetCapturedStderr();
const std::string str = "WARNING: Runtime warning messages.\n";

#ifndef NGEN_QUIET
EXPECT_EQ(cerr_output, str);
#else
EXPECT_EQ(cerr_output, "");
#endif
}

//Test logging critical function
TEST_F(loggingTest, TestCritical)
{
const std::string test_str = "Critical runtime messages.\n";

testing::internal::CaptureStderr();
critical(test_str.c_str());

const std::string cerr_output = testing::internal::GetCapturedStderr();
const std::string str = "CRITICAL: Critical runtime messages.\n";

EXPECT_EQ(cerr_output, str);
}

//Test logging error function
TEST_F(loggingTest, TestError)
{
const std::string test_str = "An error has occurred during runtime.\n";

testing::internal::CaptureStderr();
error(test_str.c_str());

const std::string cerr_output = testing::internal::GetCapturedStderr();
const std::string str = "ERROR: An error has occurred during runtime.\n";

EXPECT_EQ(cerr_output, str);
}
Loading