Skip to content

Conversation

lplewa
Copy link
Owner

@lplewa lplewa commented Jun 10, 2025

Summary

  • expand log tests to capture output strings
  • verify timestamp output matches YYYY-MM-DDTHH:MM:SS format
  • verify PID/TID header exists when enabled

Testing

  • cmake --build build -j$(nproc)
  • ctest -R test_logger --output-on-failure

https://chatgpt.com/codex/tasks/task_e_684823a83050832194419c71f85a46ed


This change is Reviewable

@lplewa lplewa force-pushed the main branch 2 times, most recently from b11534f to cde4cb8 Compare June 25, 2025 13:52
Copy link

@KFilipek KFilipek left a comment

Choose a reason for hiding this comment

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

LGTM

@KFilipek reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lplewa)


test/utils/utils_log.cpp line 7 at r1 (raw file):

#include "base.hpp"
#include "test_helpers.h"
#include <regex>

Separate include block

@KFilipek
Copy link

There are failing build jobs, please fix it before merge.

Comment on lines +360 to +363

std::regex r("^\\[\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2} DEBUG UMF\\] "
+ MOCK_FN_NAME + ": example log\\n$");
EXPECT_TRUE(std::regex_match(last_message, r));

Choose a reason for hiding this comment

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

Can you describe this regexp as a comment?

@@ -4,6 +4,7 @@

#include "base.hpp"
#include "test_helpers.h"
#include <regex>

Choose a reason for hiding this comment

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

Move it to the separate block

@@ -257,6 +260,7 @@ TEST_F(test, parseEnv) {
template <typename... Args> void helper_test_log(Args... args) {
fput_count = 0;
fflush_count = 0;
last_message.clear();

Choose a reason for hiding this comment

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

Add empty line after this call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants