Skip to content

701 Update LogService class in CppMicroServices (#1009)#1163

Merged
jeffdiclemente merged 3 commits intoCppMicroServices:c++14-compliantfrom
insi-eb:701_logservice
Jul 25, 2025
Merged

701 Update LogService class in CppMicroServices (#1009)#1163
jeffdiclemente merged 3 commits intoCppMicroServices:c++14-compliantfrom
insi-eb:701_logservice

Conversation

@RobertLauferElektrobit
Copy link
Copy Markdown

cherry-pick of commit 070fd93 (PR #1009)

plus additional commit for C++14 compliance

see discussion #701

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

* @remarks This class is thread safe.
*/
class LogService
class LogService : public LoggerFactory
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: class 'LogService' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

        class LogService : public LoggerFactory
              ^

#include <functional>
#include <string>

namespace cppmicroservices::logservice
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: nested namespace definition is a C++17 extension; define each namespace separately [clang-diagnostic-c++17-extensions]

Suggested change
namespace cppmicroservices::logservice
namespace cppmicroservices { namespace logservice

compendium/LogService/include/cppmicroservices/logservice/Logger.hpp:317:

- } // namespace cppmicroservices::logservice
+ } } // namespace cppmicroservices::logservice

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Suggested change
namespace cppmicroservices::logservice
namespace cppmicroservices
{
namespace logservice

* @remarks This class is thread-safe.
*/

class Logger
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: class 'Logger' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

    class Logger
          ^

* @remarks This class is thread safe.
*/

class LoggerFactory
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: class 'LoggerFactory' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

	class LoggerFactory
       ^

return stream.str();
}

namespace cppmicroservices::logservice
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: nested namespace definition is a C++17 extension; define each namespace separately [clang-diagnostic-c++17-extensions]

Suggested change
namespace cppmicroservices::logservice
namespace cppmicroservices { namespace logservice

compendium/LogServiceImpl/src/LogServiceImpl.cpp:96:

- } // namespace cppmicroservices::logservice
+ } } // namespace cppmicroservices::logservice

EXPECT_NO_THROW(logger->info("Test invalid exception_ptr", exp));
EXPECT_TRUE(ContainsRegex(log_preamble + "Test invalid exception_ptr(\\n)" + exception_preamble + "none"));

EXPECT_NO_THROW(logger->info("Test invalid ServiceReferenceBase object", cppmicroservices::ServiceReferenceU {},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: avoid using 'goto' for flow control [cppcoreguidelines-avoid-goto]

    EXPECT_NO_THROW(logger->info("Test invalid ServiceReferenceBase object", cppmicroservices::ServiceReferenceU {},
    ^

expanded from here

+ "Invalid service reference(\\n)" + exception_preamble + "none"));
}

TEST_F(LoggerImplTests, ThreadSafety)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: initializing non-owner argument of type 'TestFactoryBase *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

TEST_F(LoggerImplTests, ThreadSafety)
^
Additional context

third_party/googletest/googletest/include/gtest/gtest.h:2207: expanded from macro 'TEST_F'

#define TEST_F(test_fixture, test_name) GTEST_TEST_F(test_fixture, test_name)
                                        ^

third_party/googletest/googletest/include/gtest/gtest.h:2204: expanded from macro 'GTEST_TEST_F'

  GTEST_TEST_(test_fixture, test_name, test_fixture, \
  ^

third_party/googletest/googletest/include/gtest/internal/gtest-internal.h:1555: expanded from macro 'GTEST_TEST_'

          new ::testing::internal::TestFactoryImpl<GTEST_TEST_CLASS_NAME_(     \
          ^

#include <memory>
#include <thread>

#include <gmock/gmock.h>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: 'gmock/gmock.h' file not found [clang-diagnostic-error]

#include <gmock/gmock.h>
         ^

namespace cfrimpl
{
// The fixture for testing class CFRLogger.
class CFRLoggerTest : public ::testing::Test
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: class 'CFRLoggerTest' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

        class CFRLoggerTest : public ::testing::Test
              ^

namespace cfrimpl
{
// The fixture for testing class CFRLogger.
class CFRLoggerTest : public ::testing::Test
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: destructor of 'CFRLoggerTest' is protected and virtual [cppcoreguidelines-virtual-class-destructor]

        class CFRLoggerTest : public ::testing::Test
              ^
Additional context

framework/test/gtest/CFRLoggerTest.cpp:55: make it protected and non-virtual

        class CFRLoggerTest : public ::testing::Test
              ^

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.41%. Comparing base (e5ed312) to head (e8fd8f0).
⚠️ Report is 3 commits behind head on c++14-compliant.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##           c++14-compliant    #1163      +/-   ##
===================================================
- Coverage            86.04%   83.41%   -2.63%     
===================================================
  Files                  133      123      -10     
  Lines                 7508     5126    -2382     
===================================================
- Hits                  6460     4276    -2184     
+ Misses                1048      850     -198     
Files with missing lines Coverage Δ
framework/src/bundle/CoreBundleContext.cpp 95.40% <100.00%> (+0.25%) ⬆️
framework/src/util/CFRLogger.cpp 96.00% <100.00%> (+24.07%) ⬆️

... and 142 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jeffdiclemente jeffdiclemente merged commit 4687f17 into CppMicroServices:c++14-compliant Jul 25, 2025
9 of 54 checks passed
@RobertLauferElektrobit RobertLauferElektrobit deleted the 701_logservice branch July 25, 2025 18:06
RobertLauferElektrobit added a commit to insi-eb/CppMicroServices-cpp14 that referenced this pull request Jul 28, 2025
jeffdiclemente pushed a commit that referenced this pull request Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants