Allow to set logging level through env var for object store tests #7344
Conversation
|
I did something similar in #7029, and it turned out to not work. The problem is that UNITTEST_LOG_LEVEL is specifically the log level of the unit test framework itself, so setting UNITTEST_LOG_LEVEL=off results in test failures not printing anything. The log level of the unit test framework and the log level of the logger used in tests needs to be separate environment variables. The code checking the environment variable should also not be in the main library, as it's purely a testing thing. |
Pull Request Test Coverage Report for Build kirill.burtsev_130Details
💛 - Coveralls |
| @@ -152,20 +152,12 @@ if(REALM_ENABLE_SYNC) | |||
| endif() | |||
| endif() | |||
|
|
|||
| if(REALM_TEST_LOGGING) | |||
There was a problem hiding this comment.
I recommend keeping the TEST_ENABLE_LOGGING value so it works like before (based on the logic in test_file.hpp) and update the logic to something like this:
set(logging_level "off")
if(DEFINED REALM_TEST_LOGGING_LEVEL)
if (NOT "${REALM_TEST_LOGGING_LEVEL}" STREQUAL "")
set(logging_level ${REALM_TEST_LOGGING_LEVEL})
endif()
elseif(TEST_ENABLE_LOGGING)
set(logging_level "all")
endif()
set(REALM_TEST_LOGGING_LEVEL ${logging_level} CACHE STRING "Object-store tests logging level" FORCE)
message(STATUS "Test logging level: ${REALM_TEST_LOGGING_LEVEL}")
target_compile_definitions(ObjectStoreTestLib PRIVATE TEST_LOGGING_LEVEL=${REALM_TEST_LOGGING_LEVEL})
| bool Logger::get_env_log_level_if_set(Level& level) noexcept | ||
| { | ||
| const char* str = getenv("UNITTEST_LOG_LEVEL"); | ||
| if (!str || strlen(str) == 0) | ||
| return true; | ||
|
|
||
| std::istringstream in(str); | ||
| in.imbue(std::locale::classic()); | ||
| in.flags(in.flags() & ~std::ios_base::skipws); // Do not accept white space | ||
| in >> level; | ||
|
|
||
| return in && in.get() == std::char_traits<char>::eof(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Recommend moving this to the test directory (e.g. test/util/) instead of the Core base.
| @@ -70,7 +70,7 @@ TestFile::TestFile() | |||
| disable_sync_to_disk(); | |||
| m_temp_dir = util::make_temp_dir(); | |||
| path = (fs::path(m_temp_dir) / "realm.XXXXXX").string(); | |||
| util::Logger::set_default_level_threshold(realm::util::Logger::Level::TEST_LOGGING_LEVEL); | |||
| util::Logger::set_default_level_threshold(TestSyncManager::default_log_level()); | |||
| if (const char* crypt_key = test_util::crypt_key()) { | |||
| encryption_key = std::vector<char>(crypt_key, crypt_key + 64); | |||
| } | |||
There was a problem hiding this comment.
Don't forget to address line 200 of this file, since it also uses the TEST_ENABLE_LOGGING variable, which is no longer being set.
#if TEST_ENABLE_LOGGING
m_logger = util::Logger::get_default_logger();
#else
// Logging is disabled, use a NullLogger to prevent printing anything
m_logger.reset(new util::NullLogger());
#endif
What, How & Why?
☑️ ToDos
bindgen/spec.yml, if public C++ API changed