From 6ca6c069f478be8c7b24acb4c5c2b688215e79f2 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 23 Dec 2024 22:02:54 +1100 Subject: [PATCH 1/6] Make LogLevel a smart enum --- src/LogLevel.hpp | 197 ++++++++++++++++++++---------- src/PowerPlant.hpp | 6 +- src/{LogLevel.cpp => Reactor.cpp} | 36 ++---- src/Reactor.hpp | 15 ++- src/util/Logger.hpp | 2 +- tests/tests/log/Log.cpp | 122 +++++++++--------- 6 files changed, 223 insertions(+), 155 deletions(-) rename src/{LogLevel.cpp => Reactor.cpp} (57%) diff --git a/src/LogLevel.hpp b/src/LogLevel.hpp index 148a4e14..d821a3ec 100644 --- a/src/LogLevel.hpp +++ b/src/LogLevel.hpp @@ -39,93 +39,164 @@ namespace NUClear { * Log levels are used to provide different levels of detail on a per-reactor basis. * The logging level of a reactor can be changed by setting it in the install function. */ -enum LogLevel : uint8_t { - /** - * Don't use this log level when emitting logs, it is for setting reactor log level from non reactor sources. - * - * Specifically when a NUClear::log is called from code that is not running in a reaction (even transitively) then - * the reactor_level will be set to UNKNOWN. - */ - UNKNOWN, +class LogLevel { +public: + enum Value : uint8_t { + /** + * Don't use this log level when emitting logs, it is for setting reactor log level from non reactor sources. + * + * Specifically when a NUClear::log is called from code that is not running in a reaction (even transitively) + * then the reactor_level will be set to UNKNOWN. + */ + UNKNOWN, + + /** + * The Trace level contains messages that are used to trace the exact flow of execution. + * + * This level is extremely verbose and often has a message per line of code. + */ + TRACE, + + /** + * Debug contains messages that represent the inputs and outputs of different computation units. + * + * If you have a function that performs three steps to do something then it's likely that you will have a + * message for the input and output of those three steps. Additionally you would likely have messages that check + * if it hit different branches. + */ + DEBUG, + + /** + * The info level is used to provide high level goal messages such as function start or successful completion. + * + * This shows when key user-facing functionality is executed and tells us that everything is working without + * getting into the details. + */ + INFO, + + /** + * The warning level is used to notify us that everything might not be working perfectly. + * + * Warnings are errors or inconsistencies that aren't fatal and generally do not completely break the system. + * However a warning message should require action and should point to a section of the system that needs + * attention. + */ + WARN, + + /** + * The error level is used to report unexpected behavior. + + * This level doesn't need to prefix a program-crashing issue but should be used to report major unexpected + branches + * in logic or other constraint breaking problems such as failed assertions. + * All errors should require action from someone and should be addressed immediately. + */ + ERROR, + + /** + * Fatal is a program destroying error that needs to be addressed immediately. + * + * If a fatal message is sent it should point to something that should never ever happen and ideally provide as + * much information as possible as to why it crashed. Fatal messages require action immediately and should + * always be addressed. + */ + FATAL + }; /** - * The Trace level contains messages that are used to trace the exact flow of execution. + * Construct a LogLevel from a Value * - * This level is extremely verbose and often has a message per line of code. + * @param value The value to construct the LogLevel from */ - TRACE, + constexpr LogLevel(const Value& value = Value::UNKNOWN) : value(value) {}; /** - * Debug contains messages that represent the inputs and outputs of different computation units. + * Construct a LogLevel from a string * - * If you have a function that performs three steps to do something then it's likely that you will have a message - * for the input and output of those three steps. - * Additionally you would likely have messages that check if it hit different branches. + * @param level The string to construct the LogLevel from */ - DEBUG, + constexpr LogLevel(const std::string& level) + : value(level == "TRACE" ? LogLevel::TRACE + : level == "DEBUG" ? LogLevel::DEBUG + : level == "INFO" ? LogLevel::INFO + : level == "WARN" ? LogLevel::WARN + : level == "ERROR" ? LogLevel::ERROR + : level == "FATAL" ? LogLevel::FATAL + : LogLevel::UNKNOWN) {}; /** - * The info level is used to provide high level goal messages such as function start or successful completion. + * A call operator which will return the value of the LogLevel + * This can be useful in situations where the implicit conversion operators are ambiguous. * - * This shows when key user-facing functionality is executed and tells us that everything is working without getting - * into the details. + * @return The value of the LogLevel */ - INFO, + constexpr Value operator()() const { + return value; + } /** - * The warning level is used to notify us that everything might not be working perfectly. + * A conversion operator which will return the value of the LogLevel * - * Warnings are errors or inconsistencies that aren't fatal and generally do not completely break the system. - * However a warning message should require action and should point to a section of the system that needs attention. + * @return The value of the LogLevel */ - WARN, + constexpr operator Value() const { + return value; + } /** - * The error level is used to report unexpected behavior. - - * This level doesn't need to prefix a program-crashing issue but should be used to report major unexpected branches - * in logic or other constraint breaking problems such as failed assertions. - * All errors should require action from someone and should be addressed immediately. + * A conversion operator which will return the string representation of the LogLevel + * + * @return The string representation of the LogLevel */ - ERROR, + operator std::string() const { + return value == LogLevel::TRACE ? "TRACE" + : value == LogLevel::DEBUG ? "DEBUG" + : value == LogLevel::INFO ? "INFO" + : value == LogLevel::WARN ? "WARN" + : value == LogLevel::ERROR ? "ERROR" + : value == LogLevel::FATAL ? "FATAL" + : "UNKNOWN"; + } /** - * Fatal is a program destroying error that needs to be addressed immediately. + * Stream the LogLevel to an ostream, it will output the string representation of the LogLevel + * + * @param os The ostream to output to + * @param level The LogLevel to output * - * If a fatal message is sent it should point to something that should never ever happen and ideally provide as much - * information as possible as to why it crashed. - * Fatal messages require action immediately and should always be addressed. + * @return The ostream that was passed in */ - FATAL + friend std::ostream& operator<<(std::ostream& os, LogLevel level) { + return os << static_cast(level); + } + + // Operators to compare LogLevel values and LogLevel to Value + // clang-format off + friend constexpr bool operator<(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value < rhs.value; } + friend constexpr bool operator>(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value > rhs.value; } + friend constexpr bool operator<=(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value <= rhs.value; } + friend constexpr bool operator>=(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value >= rhs.value; } + friend constexpr bool operator==(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value == rhs.value; } + friend constexpr bool operator!=(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value != rhs.value; } + friend constexpr bool operator<(const LogLevel& lhs, const Value& rhs) { return lhs.value < rhs; } + friend constexpr bool operator>(const LogLevel& lhs, const Value& rhs) { return lhs.value > rhs; } + friend constexpr bool operator<=(const LogLevel& lhs, const Value& rhs) { return lhs.value <= rhs; } + friend constexpr bool operator>=(const LogLevel& lhs, const Value& rhs) { return lhs.value >= rhs; } + friend constexpr bool operator==(const LogLevel& lhs, const Value& rhs) { return lhs.value == rhs; } + friend constexpr bool operator!=(const LogLevel& lhs, const Value& rhs) { return lhs.value != rhs; } + friend constexpr bool operator<(const Value& lhs, const LogLevel& rhs) { return lhs < rhs.value; } + friend constexpr bool operator>(const Value& lhs, const LogLevel& rhs) { return lhs > rhs.value; } + friend constexpr bool operator<=(const Value& lhs, const LogLevel& rhs) { return lhs <= rhs.value; } + friend constexpr bool operator>=(const Value& lhs, const LogLevel& rhs) { return lhs >= rhs.value; } + friend constexpr bool operator==(const Value& lhs, const LogLevel& rhs) { return lhs == rhs.value; } + friend constexpr bool operator!=(const Value& lhs, const LogLevel& rhs) { return lhs != rhs.value; } + // clang-format on + + +private: + /// The stored enum value + Value value; }; - -/** - * This function is used to convert a LogLevel into a string - * - * @param level the LogLevel to convert - * - * @return the string representation of the LogLevel - */ -std::string to_string(const LogLevel& level); - -/** - * This function is used to convert a string into a LogLevel - * - * @param level the string to convert - * - * @return the LogLevel representation of the string - */ -LogLevel from_string(const std::string& level); - -/** - * This function is used to convert a LogLevel into a string for printing. - * - * @param os the output stream to write to - * @param level the LogLevel to convert - * @return the output stream - */ -std::ostream& operator<<(std::ostream& os, const LogLevel& level); - } // namespace NUClear #endif // NUCLEAR_LOGLEVEL_HPP diff --git a/src/PowerPlant.hpp b/src/PowerPlant.hpp index 2b783c01..98fae22f 100644 --- a/src/PowerPlant.hpp +++ b/src/PowerPlant.hpp @@ -194,7 +194,7 @@ class PowerPlant { * * @param args The arguments we are logging */ - template + template void log(Arguments&&... args) { logger.log(nullptr, level, std::forward(args)...); } @@ -216,7 +216,7 @@ class PowerPlant { * @param reactor The reactor that is logging * @param args The arguments we are logging */ - template + template void log(const Reactor* reactor, Arguments&&... args) { logger.log(reactor, level, std::forward(args)...); } @@ -349,7 +349,7 @@ class PowerPlant { * * @param args The arguments to log. */ -template +template void log(Arguments&&... args) { if (PowerPlant::powerplant != nullptr) { PowerPlant::powerplant->log(std::forward(args)...); diff --git a/src/LogLevel.cpp b/src/Reactor.cpp similarity index 57% rename from src/LogLevel.cpp rename to src/Reactor.cpp index e7dd3813..1b3c5214 100644 --- a/src/LogLevel.cpp +++ b/src/Reactor.cpp @@ -20,37 +20,15 @@ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#include "LogLevel.hpp" - -#include +#include "Reactor.hpp" namespace NUClear { -std::string to_string(const LogLevel& level) { - switch (level) { - case LogLevel::TRACE: return "TRACE"; - case LogLevel::DEBUG: return "DEBUG"; - case LogLevel::INFO: return "INFO"; - case LogLevel::WARN: return "WARN"; - case LogLevel::ERROR: return "ERROR"; - case LogLevel::FATAL: return "FATAL"; - default: - case LogLevel::UNKNOWN: return "UNKNOWN"; - } -} - -LogLevel from_string(const std::string& level) { - return level == "TRACE" ? LogLevel::TRACE - : level == "DEBUG" ? LogLevel::DEBUG - : level == "INFO" ? LogLevel::INFO - : level == "WARN" ? LogLevel::WARN - : level == "ERROR" ? LogLevel::ERROR - : level == "FATAL" ? LogLevel::FATAL - : LogLevel::UNKNOWN; -} - -std::ostream& operator<<(std::ostream& os, const LogLevel& level) { - return os << to_string(level); -} +constexpr LogLevel Reactor::TRACE; +constexpr LogLevel Reactor::DEBUG; +constexpr LogLevel Reactor::INFO; +constexpr LogLevel Reactor::WARN; +constexpr LogLevel Reactor::ERROR; +constexpr LogLevel Reactor::FATAL; } // namespace NUClear diff --git a/src/Reactor.hpp b/src/Reactor.hpp index ea9b5079..8e758167 100644 --- a/src/Reactor.hpp +++ b/src/Reactor.hpp @@ -320,6 +320,19 @@ class Reactor { using WATCHDOG = dsl::word::emit::Watchdog; }; + /// @copydoc NUClear::LogLevel::Value::TRACE + static constexpr LogLevel TRACE = LogLevel::TRACE; + // @copydoc NUClear::LogLevel::Value::DEBUG + static constexpr LogLevel DEBUG = LogLevel::DEBUG; + // @copydoc NUClear::LogLevel::Value::INFO + static constexpr LogLevel INFO = LogLevel::INFO; + // @copydoc NUClear::LogLevel::Value::WARN + static constexpr LogLevel WARN = LogLevel::WARN; + // @copydoc NUClear::LogLevel::Value::ERROR + static constexpr LogLevel ERROR = LogLevel::ERROR; + // @copydoc NUClear::LogLevel::Value::FATAL + static constexpr LogLevel FATAL = LogLevel::FATAL; + /// This provides functions to modify how an on statement runs after it has been created using ReactionHandle = threading::ReactionHandle; @@ -436,7 +449,7 @@ class Reactor { * * @param args The arguments we are logging */ - template + template void log(Arguments&&... args) const { // Short circuit here before going to the more expensive log function if (level >= min_log_level || level >= log_level) { diff --git a/src/util/Logger.hpp b/src/util/Logger.hpp index 7872b118..d9bb3271 100644 --- a/src/util/Logger.hpp +++ b/src/util/Logger.hpp @@ -67,7 +67,7 @@ namespace util { * Describes the log levels for a particular reactor. */ struct LogLevels { - LogLevels(LogLevel display_log_level, LogLevel min_log_level) + LogLevels(const LogLevel& display_log_level, const LogLevel& min_log_level) : display_log_level(display_log_level), min_log_level(min_log_level) {} /// The log level that should be displayed diff --git a/tests/tests/log/Log.cpp b/tests/tests/log/Log.cpp index 6761da34..a9dbb30c 100644 --- a/tests/tests/log/Log.cpp +++ b/tests/tests/log/Log.cpp @@ -28,7 +28,7 @@ #include "test_util/executable_path.hpp" // This is a free floating function that we can use to test the log function when not in a reactor -template +template void free_floating_log(const Args&... args) { NUClear::log(args...); } @@ -44,8 +44,12 @@ std::vector messages; // NOLINT(cppcoreguidelines-avoid-non-cons // All the log levels // NOLINTNEXTLINE(cert-err58-cpp,cppcoreguidelines-avoid-non-const-global-variables) -const std::vector levels = - {NUClear::TRACE, NUClear::DEBUG, NUClear::INFO, NUClear::WARN, NUClear::ERROR, NUClear::FATAL}; +const std::vector levels = {NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL}; struct TestLevel { TestLevel(NUClear::LogLevel level) : level(level) {} @@ -70,29 +74,29 @@ class TestReactor : public NUClear::Reactor { this->log_level = l.level; // Test logs from a reaction - log("Direct Reaction", NUClear::TRACE); - log("Direct Reaction", NUClear::DEBUG); - log("Direct Reaction", NUClear::INFO); - log("Direct Reaction", NUClear::WARN); - log("Direct Reaction", NUClear::ERROR); - log("Direct Reaction", NUClear::FATAL); + log("Direct Reaction", TRACE); + log("Direct Reaction", DEBUG); + log("Direct Reaction", INFO); + log("Direct Reaction", WARN); + log("Direct Reaction", ERROR); + log("Direct Reaction", FATAL); // Test logs from a free floating function (called from a reaction) - free_floating_log("Indirect Reaction", NUClear::TRACE); - free_floating_log("Indirect Reaction", NUClear::DEBUG); - free_floating_log("Indirect Reaction", NUClear::INFO); - free_floating_log("Indirect Reaction", NUClear::WARN); - free_floating_log("Indirect Reaction", NUClear::ERROR); - free_floating_log("Indirect Reaction", NUClear::FATAL); + free_floating_log("Indirect Reaction", TRACE); + free_floating_log("Indirect Reaction", DEBUG); + free_floating_log("Indirect Reaction", INFO); + free_floating_log("Indirect Reaction", WARN); + free_floating_log("Indirect Reaction", ERROR); + free_floating_log("Indirect Reaction", FATAL); // Test logs called from a free floating function in another thread std::thread([] { - free_floating_log("Non Reaction", NUClear::TRACE); - free_floating_log("Non Reaction", NUClear::DEBUG); - free_floating_log("Non Reaction", NUClear::INFO); - free_floating_log("Non Reaction", NUClear::WARN); - free_floating_log("Non Reaction", NUClear::ERROR); - free_floating_log("Non Reaction", NUClear::FATAL); + free_floating_log("Non Reaction", TRACE); + free_floating_log("Non Reaction", DEBUG); + free_floating_log("Non Reaction", INFO); + free_floating_log("Non Reaction", WARN); + free_floating_log("Non Reaction", ERROR); + free_floating_log("Non Reaction", FATAL); }).join(); }); @@ -100,27 +104,27 @@ class TestReactor : public NUClear::Reactor { on>().then([this] { powerplant.shutdown(); - free_floating_log("Post Powerplant Shutdown", NUClear::TRACE); - free_floating_log("Post Powerplant Shutdown", NUClear::DEBUG); - free_floating_log("Post Powerplant Shutdown", NUClear::INFO); - free_floating_log("Post Powerplant Shutdown", NUClear::WARN); - free_floating_log("Post Powerplant Shutdown", NUClear::ERROR); - free_floating_log("Post Powerplant Shutdown", NUClear::FATAL); + free_floating_log("Post Powerplant Shutdown", TRACE); + free_floating_log("Post Powerplant Shutdown", DEBUG); + free_floating_log("Post Powerplant Shutdown", INFO); + free_floating_log("Post Powerplant Shutdown", WARN); + free_floating_log("Post Powerplant Shutdown", ERROR); + free_floating_log("Post Powerplant Shutdown", FATAL); - log("Post Powerplant Shutdown", NUClear::TRACE); - log("Post Powerplant Shutdown", NUClear::DEBUG); - log("Post Powerplant Shutdown", NUClear::INFO); - log("Post Powerplant Shutdown", NUClear::WARN); - log("Post Powerplant Shutdown", NUClear::ERROR); - log("Post Powerplant Shutdown", NUClear::FATAL); + log("Post Powerplant Shutdown", TRACE); + log("Post Powerplant Shutdown", DEBUG); + log("Post Powerplant Shutdown", INFO); + log("Post Powerplant Shutdown", WARN); + log("Post Powerplant Shutdown", ERROR); + log("Post Powerplant Shutdown", FATAL); std::thread([] { - free_floating_log("Non Reaction", NUClear::TRACE); - free_floating_log("Non Reaction", NUClear::DEBUG); - free_floating_log("Non Reaction", NUClear::INFO); - free_floating_log("Non Reaction", NUClear::WARN); - free_floating_log("Non Reaction", NUClear::ERROR); - free_floating_log("Non Reaction", NUClear::FATAL); + free_floating_log("Non Reaction", TRACE); + free_floating_log("Non Reaction", DEBUG); + free_floating_log("Non Reaction", INFO); + free_floating_log("Non Reaction", WARN); + free_floating_log("Non Reaction", ERROR); + free_floating_log("Non Reaction", FATAL); }).join(); }); @@ -136,13 +140,15 @@ class TestReactor : public NUClear::Reactor { TEST_CASE("Testing the Log<>() function", "[api][log]") { + using LogLevel = NUClear::LogLevel; + // Try to call log before constructing a powerplant - free_floating_log("Pre Powerplant Construction", NUClear::TRACE); - free_floating_log("Pre Powerplant Construction", NUClear::DEBUG); - free_floating_log("Pre Powerplant Construction", NUClear::INFO); - free_floating_log("Pre Powerplant Construction", NUClear::WARN); - free_floating_log("Pre Powerplant Construction", NUClear::ERROR); - free_floating_log("Pre Powerplant Construction", NUClear::FATAL); + free_floating_log("Pre Powerplant Construction", LogLevel(LogLevel::TRACE)); + free_floating_log("Pre Powerplant Construction", LogLevel(LogLevel::DEBUG)); + free_floating_log("Pre Powerplant Construction", LogLevel(LogLevel::INFO)); + free_floating_log("Pre Powerplant Construction", LogLevel(LogLevel::WARN)); + free_floating_log("Pre Powerplant Construction", LogLevel(LogLevel::ERROR)); + free_floating_log("Pre Powerplant Construction", LogLevel(LogLevel::FATAL)); // Local scope to force powerplant destruction { @@ -158,12 +164,12 @@ TEST_CASE("Testing the Log<>() function", "[api][log]") { } // Try to call log after destructing the powerplant - free_floating_log("Post Powerplant Destruction", NUClear::TRACE); - free_floating_log("Post Powerplant Destruction", NUClear::DEBUG); - free_floating_log("Post Powerplant Destruction", NUClear::INFO); - free_floating_log("Post Powerplant Destruction", NUClear::WARN); - free_floating_log("Post Powerplant Destruction", NUClear::ERROR); - free_floating_log("Post Powerplant Destruction", NUClear::FATAL); + free_floating_log("Post Powerplant Destruction", LogLevel(LogLevel::TRACE)); + free_floating_log("Post Powerplant Destruction", LogLevel(LogLevel::DEBUG)); + free_floating_log("Post Powerplant Destruction", LogLevel(LogLevel::INFO)); + free_floating_log("Post Powerplant Destruction", LogLevel(LogLevel::WARN)); + free_floating_log("Post Powerplant Destruction", LogLevel(LogLevel::ERROR)); + free_floating_log("Post Powerplant Destruction", LogLevel(LogLevel::FATAL)); // Test that we have the correct number of messages size_t expected_count = 0; @@ -179,7 +185,7 @@ TEST_CASE("Testing the Log<>() function", "[api][log]") { // Test logs from reactions directly for (const auto& log_level : levels) { if (display_level <= log_level) { - const std::string expected = "Direct Reaction " + NUClear::to_string(log_level); + const std::string expected = "Direct Reaction " + static_cast(log_level); REQUIRE(messages[i].message == expected); REQUIRE(messages[i].level == log_level); REQUIRE(messages[i++].from_reaction); @@ -188,7 +194,7 @@ TEST_CASE("Testing the Log<>() function", "[api][log]") { // Test logs from reactions indirectly for (const auto& log_level : levels) { if (display_level <= log_level) { - const std::string expected = "Indirect Reaction " + NUClear::to_string(log_level); + const std::string expected = "Indirect Reaction " + static_cast(log_level); REQUIRE(messages[i].message == expected); REQUIRE(messages[i].level == log_level); REQUIRE(messages[i++].from_reaction); @@ -197,7 +203,7 @@ TEST_CASE("Testing the Log<>() function", "[api][log]") { // Test logs from free floating functions for (const auto& log_level : levels) { // No filter here, free floating prints everything - const std::string expected = "Non Reaction " + NUClear::to_string(log_level); + const std::string expected = "Non Reaction " + static_cast(log_level); REQUIRE(messages[i].message == expected); REQUIRE(messages[i].level == log_level); REQUIRE_FALSE(messages[i++].from_reaction); @@ -206,19 +212,19 @@ TEST_CASE("Testing the Log<>() function", "[api][log]") { // Test post-shutdown logs { - const std::string expected = "Post Powerplant Shutdown " + NUClear::to_string(NUClear::FATAL); + const std::string expected = "Post Powerplant Shutdown " + static_cast(LogLevel(LogLevel::FATAL)); REQUIRE(messages[i].message == expected); - REQUIRE(messages[i].level == NUClear::FATAL); + REQUIRE(messages[i].level == LogLevel::FATAL); REQUIRE(messages[i++].from_reaction); REQUIRE(messages[i].message == expected); - REQUIRE(messages[i].level == NUClear::FATAL); + REQUIRE(messages[i].level == LogLevel::FATAL); REQUIRE(messages[i++].from_reaction); } // Test logs from free floating functions for (const auto& log_level : levels) { // No filter here, free floating prints everything - const std::string expected = "Non Reaction " + NUClear::to_string(log_level); + const std::string expected = "Non Reaction " + static_cast(log_level); REQUIRE(messages[i].message == expected); REQUIRE(messages[i].level == log_level); REQUIRE_FALSE(messages[i++].from_reaction); From aedd2f0d84ed097f736382c4c6fc815c1b17e88f Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 23 Dec 2024 22:19:26 +1100 Subject: [PATCH 2/6] Add tests for the smart enum --- tests/tests/api/LogLevel.cpp | 198 +++++++++++++++++++++++++++++++++++ 1 file changed, 198 insertions(+) create mode 100644 tests/tests/api/LogLevel.cpp diff --git a/tests/tests/api/LogLevel.cpp b/tests/tests/api/LogLevel.cpp new file mode 100644 index 00000000..298282f1 --- /dev/null +++ b/tests/tests/api/LogLevel.cpp @@ -0,0 +1,198 @@ +#define CATCH_CONFIG_MAIN +#include "LogLevel.hpp" + +#include +#include +#include +#include + +SCENARIO("LogLevel can be constructed from Value") { + GIVEN("A LogLevel constructed from a Value") { + auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + NUClear::LogLevel log_level(value); + + WHEN("the value is retrieved") { + auto retrieved_value = log_level(); + + THEN("it should be equal to the original value") { + REQUIRE(retrieved_value == value); + } + } + } +} + +SCENARIO("LogLevel can be constructed from a string") { + GIVEN("A LogLevel constructed from a string") { + auto str = GENERATE("TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"); + NUClear::LogLevel log_level(str); + + WHEN("the value is retrieved") { + auto value = log_level(); + + THEN("it should be equal to the corresponding Value") { + REQUIRE(log_level == str); + } + } + } +} + +SCENARIO("LogLevel can be converted to a string") { + GIVEN("A LogLevel") { + auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + NUClear::LogLevel log_level(value); + + WHEN("it is converted to a string") { + std::string str = log_level; + + THEN("it should be equal to the corresponding string representation") { + REQUIRE(str == log_level); + } + } + } +} + +SCENARIO("LogLevel can be streamed to an ostream") { + GIVEN("A LogLevel") { + auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + NUClear::LogLevel log_level(value); + + WHEN("it is streamed to an ostream") { + std::ostringstream os; + os << log_level; + + THEN("the output should be the corresponding string representation") { + REQUIRE(os.str() == log_level); + } + } + } +} + +SCENARIO("LogLevel comparison operators work correctly") { + GIVEN("Two LogLevel enum values") { + NUClear::LogLevel::Value v1 = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + NUClear::LogLevel::Value v2 = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + + WHEN("two smart enum value is constructed") { + NUClear::LogLevel ll1(v1); + AND_WHEN("they are compared using ==") { + THEN("the result should be correct") { + REQUIRE((ll1 == v2) == (v1 == v2)); + } + } + AND_WHEN("they are compared using !=") { + THEN("the result should be correct") { + REQUIRE((ll1 != v2) == (v1 != v2)); + } + } + AND_WHEN("they are compared using <") { + THEN("the result should be correct") { + REQUIRE((ll1 < v2) == (v1 < v2)); + } + } + AND_WHEN("they are compared using >") { + THEN("the result should be correct") { + REQUIRE((ll1 > v2) == (v1 > v2)); + } + } + AND_WHEN("they are compared using <=") { + THEN("the result should be correct") { + REQUIRE((ll1 <= v2) == (v1 <= v2)); + } + } + AND_WHEN("they are compared using >=") { + THEN("the result should be correct") { + REQUIRE((ll1 >= v2) == (v1 >= v2)); + } + } + } + + WHEN("two smart enum values are constructed") { + NUClear::LogLevel ll1(v1); + NUClear::LogLevel ll2(v2); + AND_WHEN("they are compared using ==") { + THEN("the result should be correct") { + REQUIRE((ll1 == ll2) == (v1 == v2)); + } + } + AND_WHEN("they are compared using !=") { + THEN("the result should be correct") { + REQUIRE((ll1 != ll2) == (v1 != v2)); + } + } + AND_WHEN("they are compared using <") { + THEN("the result should be correct") { + REQUIRE((ll1 < ll2) == (v1 < v2)); + } + } + AND_WHEN("they are compared using >") { + THEN("the result should be correct") { + REQUIRE((ll1 > ll2) == (v1 > v2)); + } + } + AND_WHEN("they are compared using <=") { + THEN("the result should be correct") { + REQUIRE((ll1 <= ll2) == (v1 <= v2)); + } + } + AND_WHEN("they are compared using >=") { + THEN("the result should be correct") { + REQUIRE((ll1 >= ll2) == (v1 >= v2)); + } + } + } + } +} + +SCENARIO("LogLevel can be used in switch statements") { + GIVEN("A LogLevel") { + auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + NUClear::LogLevel log_level(value); + + WHEN("used in a switch statement") { + std::string result; + switch (log_level) { + case NUClear::LogLevel::TRACE: result = "TRACE"; break; + case NUClear::LogLevel::DEBUG: result = "DEBUG"; break; + case NUClear::LogLevel::INFO: result = "INFO"; break; + case NUClear::LogLevel::WARN: result = "WARN"; break; + case NUClear::LogLevel::ERROR: result = "ERROR"; break; + case NUClear::LogLevel::FATAL: result = "FATAL"; break; + default: result = "UNKNOWN"; break; + } + + THEN("the result should be the corresponding string representation") { + REQUIRE(result == log_level); + } + } + } +} From 8c79372947137c3fd28263804fc2709448410668 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 23 Dec 2024 22:22:29 +1100 Subject: [PATCH 3/6] If only c++17... --- src/LogLevel.hpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/LogLevel.hpp b/src/LogLevel.hpp index d821a3ec..6801d06f 100644 --- a/src/LogLevel.hpp +++ b/src/LogLevel.hpp @@ -115,7 +115,7 @@ class LogLevel { * * @param level The string to construct the LogLevel from */ - constexpr LogLevel(const std::string& level) + LogLevel(const std::string& level) : value(level == "TRACE" ? LogLevel::TRACE : level == "DEBUG" ? LogLevel::DEBUG : level == "INFO" ? LogLevel::INFO @@ -178,6 +178,7 @@ class LogLevel { friend constexpr bool operator>=(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value >= rhs.value; } friend constexpr bool operator==(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value == rhs.value; } friend constexpr bool operator!=(const LogLevel& lhs, const LogLevel& rhs) { return lhs.value != rhs.value; } + friend constexpr bool operator<(const LogLevel& lhs, const Value& rhs) { return lhs.value < rhs; } friend constexpr bool operator>(const LogLevel& lhs, const Value& rhs) { return lhs.value > rhs; } friend constexpr bool operator<=(const LogLevel& lhs, const Value& rhs) { return lhs.value <= rhs; } @@ -190,6 +191,19 @@ class LogLevel { friend constexpr bool operator>=(const Value& lhs, const LogLevel& rhs) { return lhs >= rhs.value; } friend constexpr bool operator==(const Value& lhs, const LogLevel& rhs) { return lhs == rhs.value; } friend constexpr bool operator!=(const Value& lhs, const LogLevel& rhs) { return lhs != rhs.value; } + + friend bool operator<(const LogLevel& lhs, const std::string& rhs) { return static_cast(lhs) < rhs; } + friend bool operator>(const LogLevel& lhs, const std::string& rhs) { return static_cast(lhs) > rhs; } + friend bool operator<=(const LogLevel& lhs, const std::string& rhs) { return static_cast(lhs) <= rhs; } + friend bool operator>=(const LogLevel& lhs, const std::string& rhs) { return static_cast(lhs) >= rhs; } + friend bool operator==(const LogLevel& lhs, const std::string& rhs) { return static_cast(lhs) == rhs; } + friend bool operator!=(const LogLevel& lhs, const std::string& rhs) { return static_cast(lhs) != rhs; } + friend bool operator<(const std::string& lhs, const LogLevel& rhs) { return lhs < static_cast(rhs); } + friend bool operator>(const std::string& lhs, const LogLevel& rhs) { return lhs > static_cast(rhs); } + friend bool operator<=(const std::string& lhs, const LogLevel& rhs) { return lhs <= static_cast(rhs); } + friend bool operator>=(const std::string& lhs, const LogLevel& rhs) { return lhs >= static_cast(rhs); } + friend bool operator==(const std::string& lhs, const LogLevel& rhs) { return lhs == static_cast(rhs); } + friend bool operator!=(const std::string& lhs, const LogLevel& rhs) { return lhs != static_cast(rhs); } // clang-format on From 82924a16f254220f6575f04a2b63573cf0cec419 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 23 Dec 2024 22:29:40 +1100 Subject: [PATCH 4/6] clang-tidy --- tests/tests/api/LogLevel.cpp | 78 ++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/tests/tests/api/LogLevel.cpp b/tests/tests/api/LogLevel.cpp index 298282f1..7ce95f88 100644 --- a/tests/tests/api/LogLevel.cpp +++ b/tests/tests/api/LogLevel.cpp @@ -14,7 +14,7 @@ SCENARIO("LogLevel can be constructed from Value") { NUClear::LogLevel::WARN, NUClear::LogLevel::ERROR, NUClear::LogLevel::FATAL); - NUClear::LogLevel log_level(value); + const NUClear::LogLevel log_level(value); WHEN("the value is retrieved") { auto retrieved_value = log_level(); @@ -28,8 +28,8 @@ SCENARIO("LogLevel can be constructed from Value") { SCENARIO("LogLevel can be constructed from a string") { GIVEN("A LogLevel constructed from a string") { - auto str = GENERATE("TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"); - NUClear::LogLevel log_level(str); + const auto str = GENERATE("TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"); + const NUClear::LogLevel log_level(str); WHEN("the value is retrieved") { auto value = log_level(); @@ -43,13 +43,13 @@ SCENARIO("LogLevel can be constructed from a string") { SCENARIO("LogLevel can be converted to a string") { GIVEN("A LogLevel") { - auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - NUClear::LogLevel log_level(value); + const auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + const NUClear::LogLevel log_level(value); WHEN("it is converted to a string") { std::string str = log_level; @@ -63,13 +63,13 @@ SCENARIO("LogLevel can be converted to a string") { SCENARIO("LogLevel can be streamed to an ostream") { GIVEN("A LogLevel") { - auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - NUClear::LogLevel log_level(value); + const auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + const NUClear::LogLevel log_level(value); WHEN("it is streamed to an ostream") { std::ostringstream os; @@ -84,21 +84,21 @@ SCENARIO("LogLevel can be streamed to an ostream") { SCENARIO("LogLevel comparison operators work correctly") { GIVEN("Two LogLevel enum values") { - NUClear::LogLevel::Value v1 = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - NUClear::LogLevel::Value v2 = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); + const NUClear::LogLevel::Value v1 = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + const NUClear::LogLevel::Value v2 = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); WHEN("two smart enum value is constructed") { - NUClear::LogLevel ll1(v1); + const NUClear::LogLevel ll1(v1); AND_WHEN("they are compared using ==") { THEN("the result should be correct") { REQUIRE((ll1 == v2) == (v1 == v2)); @@ -132,8 +132,8 @@ SCENARIO("LogLevel comparison operators work correctly") { } WHEN("two smart enum values are constructed") { - NUClear::LogLevel ll1(v1); - NUClear::LogLevel ll2(v2); + const NUClear::LogLevel ll1(v1); + const NUClear::LogLevel ll2(v2); AND_WHEN("they are compared using ==") { THEN("the result should be correct") { REQUIRE((ll1 == ll2) == (v1 == v2)); @@ -170,13 +170,13 @@ SCENARIO("LogLevel comparison operators work correctly") { SCENARIO("LogLevel can be used in switch statements") { GIVEN("A LogLevel") { - auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - NUClear::LogLevel log_level(value); + const auto value = GENERATE(NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL); + const NUClear::LogLevel log_level(value); WHEN("used in a switch statement") { std::string result; From 9d00f5368dd79da199cb1638d44114bbebcc1b68 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 23 Dec 2024 22:34:43 +1100 Subject: [PATCH 5/6] poor clang-tidy is getting confused about this being a pointer --- tests/tests/api/LogLevel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/api/LogLevel.cpp b/tests/tests/api/LogLevel.cpp index 7ce95f88..94fd7a03 100644 --- a/tests/tests/api/LogLevel.cpp +++ b/tests/tests/api/LogLevel.cpp @@ -28,7 +28,7 @@ SCENARIO("LogLevel can be constructed from Value") { SCENARIO("LogLevel can be constructed from a string") { GIVEN("A LogLevel constructed from a string") { - const auto str = GENERATE("TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"); + const std::string str = GENERATE("TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"); const NUClear::LogLevel log_level(str); WHEN("the value is retrieved") { From b293e72a0024500d86614f745228f6282b1a0a34 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 23 Dec 2024 22:52:48 +1100 Subject: [PATCH 6/6] Revamp the tests to get rid of some duplication --- tests/tests/api/LogLevel.cpp | 109 +++++++++++++++-------------------- 1 file changed, 48 insertions(+), 61 deletions(-) diff --git a/tests/tests/api/LogLevel.cpp b/tests/tests/api/LogLevel.cpp index 94fd7a03..1db06fb6 100644 --- a/tests/tests/api/LogLevel.cpp +++ b/tests/tests/api/LogLevel.cpp @@ -3,80 +3,65 @@ #include #include +#include #include #include +#include -SCENARIO("LogLevel can be constructed from Value") { - GIVEN("A LogLevel constructed from a Value") { - auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - const NUClear::LogLevel log_level(value); +SCENARIO("LogLevel smart enum values can be constructed and converted appropriately") { + GIVEN("A LogLevel and a corresponding string representation") { + const auto test = GENERATE( + table({std::make_tuple("TRACE", NUClear::LogLevel::TRACE), + std::make_tuple("DEBUG", NUClear::LogLevel::DEBUG), + std::make_tuple("INFO", NUClear::LogLevel::INFO), + std::make_tuple("WARN", NUClear::LogLevel::WARN), + std::make_tuple("ERROR", NUClear::LogLevel::ERROR), + std::make_tuple("FATAL", NUClear::LogLevel::FATAL)})); + + const auto& expected_str = std::get<0>(test); + const auto& expected_value = std::get<1>(test); - WHEN("the value is retrieved") { - auto retrieved_value = log_level(); + WHEN("constructing a LogLevel from the Value") { + const NUClear::LogLevel log_level(expected_value); - THEN("it should be equal to the original value") { - REQUIRE(retrieved_value == value); + THEN("it should be equal to the corresponding string representation") { + REQUIRE(static_cast(log_level) == expected_str); } } - } -} - -SCENARIO("LogLevel can be constructed from a string") { - GIVEN("A LogLevel constructed from a string") { - const std::string str = GENERATE("TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"); - const NUClear::LogLevel log_level(str); - WHEN("the value is retrieved") { - auto value = log_level(); + WHEN("constructing a LogLevel from the string") { + const NUClear::LogLevel log_level(expected_str); THEN("it should be equal to the corresponding Value") { - REQUIRE(log_level == str); + REQUIRE(log_level() == expected_value); + REQUIRE(log_level == expected_value); + REQUIRE(log_level == NUClear::LogLevel(expected_value)); } } - } -} - -SCENARIO("LogLevel can be converted to a string") { - GIVEN("A LogLevel") { - const auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - const NUClear::LogLevel log_level(value); - WHEN("it is converted to a string") { - std::string str = log_level; + WHEN("constructing a LogLevel from the Value") { + const NUClear::LogLevel log_level(expected_value); THEN("it should be equal to the corresponding string representation") { - REQUIRE(str == log_level); + REQUIRE(static_cast(log_level) == expected_str); + REQUIRE(log_level == expected_str); } } - } -} - -SCENARIO("LogLevel can be streamed to an ostream") { - GIVEN("A LogLevel") { - const auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - const NUClear::LogLevel log_level(value); - WHEN("it is streamed to an ostream") { + WHEN("streaming the LogLevel to an ostream") { std::ostringstream os; - os << log_level; + os << NUClear::LogLevel(expected_value); THEN("the output should be the corresponding string representation") { - REQUIRE(os.str() == log_level); + REQUIRE(os.str() == expected_str); + } + } + + WHEN("converting the LogLevel to a string") { + const std::string str = NUClear::LogLevel(expected_value); + + THEN("it should be equal to the corresponding string representation") { + REQUIRE(str == expected_str); } } } @@ -97,7 +82,7 @@ SCENARIO("LogLevel comparison operators work correctly") { NUClear::LogLevel::ERROR, NUClear::LogLevel::FATAL); - WHEN("two smart enum value is constructed") { + WHEN("one smart enum value is constructed") { const NUClear::LogLevel ll1(v1); AND_WHEN("they are compared using ==") { THEN("the result should be correct") { @@ -170,12 +155,14 @@ SCENARIO("LogLevel comparison operators work correctly") { SCENARIO("LogLevel can be used in switch statements") { GIVEN("A LogLevel") { - const auto value = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); + auto test = GENERATE(table({{"TRACE", NUClear::LogLevel::TRACE}, + {"DEBUG", NUClear::LogLevel::DEBUG}, + {"INFO", NUClear::LogLevel::INFO}, + {"WARN", NUClear::LogLevel::WARN}, + {"ERROR", NUClear::LogLevel::ERROR}, + {"FATAL", NUClear::LogLevel::FATAL}})); + const auto& str = std::get<0>(test); + const auto& value = std::get<1>(test); const NUClear::LogLevel log_level(value); WHEN("used in a switch statement") { @@ -191,7 +178,7 @@ SCENARIO("LogLevel can be used in switch statements") { } THEN("the result should be the corresponding string representation") { - REQUIRE(result == log_level); + REQUIRE(result == str); } } }