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

Use UNITTEST_LOG_LEVEL in objectstore tests #7029

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

### Internals
* Update tests to use global logger. ([PR #6917](https://github.com/realm/realm-core/pull/6917))
* Objectstore tests now use the UNITTEST_LOG_LEVEL env variable to determine log level rather than a build-time setting. ([PR #7029](https://github.com/realm/realm-core/pull/7029))

----------------------------------------------

Expand Down
8 changes: 5 additions & 3 deletions evergreen/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ functions:
set_cmake_var realm_vars CMAKE_TOOLCHAIN_FILE PATH "${cmake_toolchain_file}"
fi
set_cmake_var realm_vars REALM_TEST_LOGGING BOOL On
set_cmake_var realm_vars REALM_TEST_LOGGING_LEVEL STRING "debug"
GENERATOR="${cmake_generator}"
if [ -z "${cmake_generator|}" ]; then
GENERATOR="Ninja Multi-Config"
Expand Down Expand Up @@ -257,6 +254,9 @@ functions:
if [[ -n "${run_with_encryption}" ]]; then
export UNITTEST_ENCRYPT_ALL=1
fi
if [[ -n "${test_log_level}" ]]; then
export UNITTEST_LOG_LEVEL="${test_log_level}"
fi
export TSAN_OPTIONS="suppressions=$(pwd)/test/tsan.suppress"
cd build
Expand Down Expand Up @@ -877,6 +877,7 @@ tasks:
vars:
test_label: objstore-local
test_executable_name: "realm-object-store-tests"
test_log_level: debug
verbose_test_output: true
- func: "check branch state"

Expand All @@ -896,6 +897,7 @@ tasks:
vars:
test_label: objstore-baas
test_executable_name: "realm-object-store-tests"
test_log_level: debug
verbose_test_output: true
- func: "check branch state"

Expand Down
34 changes: 33 additions & 1 deletion src/realm/util/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const char* Logger::get_level_prefix(Level level) noexcept
return "";
}

const std::string_view Logger::level_to_string(Level level) noexcept
std::string_view Logger::level_to_string(Level level) noexcept
{
switch (level) {
case Logger::Level::all:
Expand All @@ -110,6 +110,38 @@ const std::string_view Logger::level_to_string(Level level) noexcept
return "";
}

std::optional<Logger::Level> Logger::level_from_string(std::string_view str)
{
if (str == "all") {
return Logger::Level::all;
}
if (str == "trace") {
return Logger::Level::trace;
}
if (str == "debug") {
return Logger::Level::debug;
}
if (str == "detail") {
return Logger::Level::detail;
}
if (str == "info") {
return Logger::Level::info;
}
if (str == "warn") {
return Logger::Level::warn;
}
if (str == "error") {
return Logger::Level::error;
}
if (str == "fatal") {
return Logger::Level::fatal;
}
if (str == "off") {
return Logger::Level::off;
}
return std::nullopt;
}

void StderrLogger::do_log(Level level, const std::string& message)
{
// std::cerr is unbuffered, so no need to flush
Expand Down
43 changes: 5 additions & 38 deletions src/realm/util/logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,14 @@ class Logger {
return static_cast<int>(level) >= static_cast<int>(get_level_threshold());
}

virtual inline ~Logger() noexcept = default;
virtual ~Logger() noexcept = default;

static void set_default_logger(std::shared_ptr<util::Logger>) noexcept;
static std::shared_ptr<util::Logger>& get_default_logger() noexcept;
static void set_default_level_threshold(Level level) noexcept;
static Level get_default_level_threshold() noexcept;
static const std::string_view level_to_string(Level level) noexcept;
static std::string_view level_to_string(Level level) noexcept;
static std::optional<Level> level_from_string(std::string_view);

protected:
// Used by subclasses that link to a base logger
Expand Down Expand Up @@ -376,43 +377,9 @@ template <class C, class T>
std::basic_istream<C, T>& operator>>(std::basic_istream<C, T>& in, Logger::Level& level)
{
std::basic_string<C, T> str;
auto check = [&](const char* name) {
size_t n = strlen(name);
if (n != str.size())
return false;
for (size_t i = 0; i < n; ++i) {
if (in.widen(name[i]) != str[i])
return false;
}
return true;
};
if (in >> str) {
if (check("all")) {
level = Logger::Level::all;
}
else if (check("trace")) {
level = Logger::Level::trace;
}
else if (check("debug")) {
level = Logger::Level::debug;
}
else if (check("detail")) {
level = Logger::Level::detail;
}
else if (check("info")) {
level = Logger::Level::info;
}
else if (check("warn")) {
level = Logger::Level::warn;
}
else if (check("error")) {
level = Logger::Level::error;
}
else if (check("fatal")) {
level = Logger::Level::fatal;
}
else if (check("off")) {
level = Logger::Level::off;
if (auto parsed = Logger::level_from_string(str)) {
level = *parsed;
}
else {
in.setstate(std::ios_base::failbit);
Expand Down
15 changes: 0 additions & 15 deletions test/object-store/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -137,21 +137,6 @@ if(REALM_ENABLE_SYNC)
endif()
endif()

if(REALM_TEST_LOGGING)
target_compile_definitions(ObjectStoreTests PRIVATE
TEST_ENABLE_LOGGING=1
)

if(REALM_TEST_LOGGING_LEVEL)
message(STATUS "Test logging level: ${REALM_TEST_LOGGING_LEVEL}")
target_compile_definitions(ObjectStoreTests PRIVATE
TEST_LOGGING_LEVEL=${REALM_TEST_LOGGING_LEVEL}
)
else()
message(STATUS "Test logging enabled")
endif()
endif()

target_include_directories(ObjectStoreTests PRIVATE
${CATCH_INCLUDE_DIR}
${JSON_INCLUDE_DIR}
Expand Down
13 changes: 1 addition & 12 deletions test/object-store/util/test_file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ 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);
if (const char* crypt_key = test_util::crypt_key()) {
encryption_key = std::vector<char>(crypt_key, crypt_key + 64);
}
Expand Down Expand Up @@ -197,16 +196,8 @@ SyncServer::SyncServer(const SyncServer::Config& config)
, m_server(m_local_root_dir, util::none, ([&] {
using namespace std::literals::chrono_literals;

#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

sync::Server::Config c;
c.logger = m_logger;
c.logger = util::Logger::get_default_logger();
c.token_expiration_clock = this;
c.listen_address = "127.0.0.1";
c.disable_sync_to_disk = true;
Expand Down Expand Up @@ -339,7 +330,6 @@ TestAppSession::TestAppSession(AppSession session,
if (!m_transport)
m_transport = instance_of<SynchronousTestTransport>;
auto app_config = get_config(m_transport, *m_app_session);
util::Logger::set_default_level_threshold(realm::util::Logger::Level::TEST_LOGGING_LEVEL);
set_app_config_defaults(app_config, m_transport);

util::try_make_dir(m_base_file_path);
Expand Down Expand Up @@ -429,7 +419,6 @@ TestSyncManager::TestSyncManager(const Config& config, const SyncServer::Config&
{
app::App::Config app_config = config.app_config;
set_app_config_defaults(app_config, transport);
util::Logger::set_default_level_threshold(config.log_level);

SyncClientConfig sc_config;
m_base_file_path = config.base_path.empty() ? util::make_temp_dir() + random_string(10) : config.base_path;
Expand Down
15 changes: 0 additions & 15 deletions test/object-store/util/test_file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <realm/object-store/shared_realm.hpp>
#include <realm/util/tagged_bool.hpp>

#include <realm/util/logger.hpp>
#include <realm/util/optional.hpp>

#include <thread>
Expand Down Expand Up @@ -97,18 +96,6 @@ struct InMemoryTestFile : realm::Realm::Config {
void advance_and_notify(realm::Realm& realm);
void on_change_but_no_notify(realm::Realm& realm);

#ifndef TEST_ENABLE_LOGGING
#define TEST_ENABLE_LOGGING 0 // change to 1 to enable trace-level logging
#endif

#ifndef TEST_LOGGING_LEVEL
#if TEST_ENABLE_LOGGING
#define TEST_LOGGING_LEVEL all
#else
#define TEST_LOGGING_LEVEL off
#endif // TEST_ENABLE_LOGGING
#endif // TEST_LOGGING_LEVEL

#if REALM_ENABLE_SYNC

using StartImmediately = realm::util::TaggedBool<class StartImmediatelyTag>;
Expand Down Expand Up @@ -147,7 +134,6 @@ class SyncServer : private realm::sync::Clock {
friend class TestSyncManager;
SyncServer(const Config& config);
std::string m_local_root_dir;
std::shared_ptr<realm::util::Logger> m_logger;
realm::sync::Server m_server;
std::thread m_thread;
std::string m_url;
Expand Down Expand Up @@ -224,7 +210,6 @@ class TestSyncManager {
std::string base_path;
realm::SyncManager::MetadataMode metadata_mode = realm::SyncManager::MetadataMode::NoEncryption;
bool should_teardown_test_directory = true;
realm::util::Logger::Level log_level = realm::util::Logger::Level::TEST_LOGGING_LEVEL;
bool override_sync_route = true;
std::shared_ptr<realm::app::GenericNetworkTransport> transport;
bool start_sync_client = true;
Expand Down
15 changes: 7 additions & 8 deletions test/test_all.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,14 +470,13 @@ bool run_tests(const std::shared_ptr<realm::util::Logger>& logger = nullptr)
// Set intra test log level threshold
{
const char* str = getenv("UNITTEST_LOG_LEVEL");
if (str && strlen(str) != 0) {
std::istringstream in(str);
in.imbue(std::locale::classic());
in.flags(in.flags() & ~std::ios_base::skipws); // Do not accept white space
in >> config.intra_test_log_level;
bool bad = !in || in.get() != std::char_traits<char>::eof();
if (bad)
throw std::runtime_error("Bad intra test log level");
if (str && *str) {
if (auto level = realm::util::Logger::level_from_string(str)) {
config.intra_test_log_level = *level;
}
else {
throw std::runtime_error(util::format("Invalid log level '%1'", str));
}
}
}

Expand Down
23 changes: 13 additions & 10 deletions test/util/test_path.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,19 @@ bool initialize_test_path(int argc, const char* argv[])
g_path_prefix = argv[1];
}

Logger::Level log_level = Logger::Level::off;
if (const char* str = getenv("UNITTEST_LOG_LEVEL"); str && *str) {
if (auto custom_level = Logger::level_from_string(str)) {
log_level = *custom_level;
}
Comment on lines +164 to +166
Copy link
Contributor

Choose a reason for hiding this comment

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

What about writing the value of UNITTEST_LOG_LEVEL to stderr here, like we did in the makefile, or print an error (with the UNITTEST_LOG_LEVEL) if the value is invalid. Regardless, don't return exit the test on an invalid value.

}
if (log_level == Logger::Level::off) {
Logger::set_default_logger(std::make_shared<NullLogger>());
}
else {
Logger::set_default_logger(std::make_shared<util::StderrLogger>(log_level));
}

return true;
}

Expand Down Expand Up @@ -317,17 +330,7 @@ std::string TestDirNameGenerator::next()

std::shared_ptr<DB> get_test_db(const std::string& path, const char* crypt_key)
{
const char* str = getenv("UNITTEST_LOG_LEVEL");
realm::util::Logger::Level core_log_level = realm::util::Logger::Level::off;
if (str && strlen(str) != 0) {
std::istringstream in(str);
in.imbue(std::locale::classic());
in.flags(in.flags() & ~std::ios_base::skipws); // Do not accept white space
in >> core_log_level;
}

DBOptions options;
options.logger = std::make_shared<util::StderrLogger>(core_log_level);
options.encryption_key = crypt_key;
return DB::create(make_in_realm_history(), path, options);
}
Expand Down