Refactor tests to use an event style and fix some found bugs

src/PowerPlant.cpp

@@ -37,8 +37,9 @@ void PowerPlant::start() {
     // We are now running
     is_running.store(true);
 
-    // Direct emit startup event
+    // Direct emit startup event and command line arguments
     emit<dsl::word::emit::Direct>(std::make_unique<dsl::word::Startup>());
+    emit_shared<dsl::word::emit::Direct>(dsl::store::DataStore<message::CommandLineArguments>::get());
 
     // Start all of the threads
     scheduler.start();

src/PowerPlant.ipp

@@ -45,9 +45,8 @@ inline PowerPlant::PowerPlant(Configuration config, int argc, const char* argv[]
         args.emplace_back(argv[i]);
     }
 
-    // We emit this twice, so the data is available for extensions
+    // We emit this twice, so the data is available for extensions, it will also be emitted in start
     emit(std::make_unique<message::CommandLineArguments>(args));
-    emit<dsl::word::emit::Initialise>(std::make_unique<message::CommandLineArguments>(args));
 }
 
 template <typename T, enum LogLevel level>

tests/CMakeLists.txt

@@ -29,7 +29,9 @@ if(CATCH_FOUND)
       add_compile_options(-Wall -Wextra -pedantic -Werror)
     endif(MSVC)
 
-    file(GLOB test_src test.cpp "api/*.cpp" "dsl/*.cpp" "dsl/emit/*.cpp" "log/*.cpp")
+    add_compile_definitions(CATCH_CONFIG_CONSOLE_WIDTH=120)
+
+    file(GLOB test_src test.cpp "api/*.cpp" "dsl/*.cpp" "dsl/emit/*.cpp" "log/*.cpp" "test_util/*.cpp")
 
     # Some tests must be executed as individual binaries
     file(GLOB individual_tests "${CMAKE_CURRENT_SOURCE_DIR}/individual/*.cpp")
@@ -40,6 +42,7 @@ if(CATCH_FOUND)
       add_executable(${test_name} ${test_src})
       target_link_libraries(${test_name} NUClear::nuclear)
       set_target_properties(${test_name} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/individual")
+      target_include_directories(${test_name} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
       target_include_directories(
         ${test_name} SYSTEM
         PRIVATE ${CATCH_INCLUDE_DIRS} ${PROJECT_BINARY_DIR}/include "${PROJECT_SOURCE_DIR}/src"
@@ -51,6 +54,7 @@ if(CATCH_FOUND)
     endforeach(test_src)
 
     add_executable(test_nuclear ${test_src})
+    target_include_directories(test_nuclear PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
     target_link_libraries(test_nuclear NUClear::nuclear)
     target_include_directories(
       test_nuclear SYSTEM
@@ -60,6 +64,7 @@ if(CATCH_FOUND)
 
     add_executable(test_network networktest.cpp)
     target_link_libraries(test_network NUClear::nuclear)
+    target_include_directories(test_network PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
     target_include_directories(
       test_network SYSTEM
       PRIVATE ${CATCH_INCLUDE_DIRS} ${PROJECT_BINARY_DIR}/include "${PROJECT_SOURCE_DIR}/src"

tests/api/ReactionHandle.cpp

@@ -19,37 +19,67 @@
 #include <catch.hpp>
 #include <nuclear>
 
+#include "test_util/TestBase.hpp"
+
 // Anonymous namespace to keep everything file local
 namespace {
 
-template <int id>
-struct Message {};
+/// @brief Events that occur during the test
+std::vector<std::string> events;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+
+struct Message {
+    Message(int i) : i(i) {}
+    int i;
+};
 
-class TestReactor : public NUClear::Reactor {
+class TestReactor : public test_util::TestBase<TestReactor> {
 public:
-    TestReactor(std::unique_ptr<NUClear::Environment> environment) : Reactor(std::move(environment)) {
+    TestReactor(std::unique_ptr<NUClear::Environment> environment) : TestBase(std::move(environment)) {
 
         // Make an always disabled reaction
-        ReactionHandle a =
-            on<Trigger<Message<0>>, Priority::HIGH>().then([] { FAIL("This reaction is disabled always"); });
+        a = on<Trigger<Message>, Priority::HIGH>().then([](const Message& msg) {  //
+            events.push_back("Executed disabled reaction " + std::to_string(msg.i));
+        });
         a.disable();
 
-        const ReactionHandle b = on<Trigger<Message<0>>>().then([this] { powerplant.shutdown(); });
+        // Make a reaction that we toggle on and off
+        b = on<Trigger<Message>, Priority::HIGH>().then([this](const Message& msg) {  //
+            events.push_back("Executed toggled reaction " + std::to_string(msg.i));
+            b.disable();
+            emit(std::make_unique<Message>(1));
+        });
+
+        on<Trigger<Message>>().then([](const Message& msg) {  //
+            events.push_back("Executed enabled reaction " + std::to_string(msg.i));
+        });
 
         // Start our test
-        on<Startup>().then([this] { emit(std::make_unique<Message<0>>()); });
+        on<Startup>().then([this] {  //
+            emit(std::make_unique<Message>(0));
+        });
     }
+
+    ReactionHandle a{};
+    ReactionHandle b{};
 };
 }  // namespace
 
 TEST_CASE("Testing reaction handle functionality", "[api][reactionhandle]") {
-
     NUClear::PowerPlant::Configuration config;
     config.thread_count = 1;
     NUClear::PowerPlant plant(config);
-
-    // We are installing with an initial log level of debug
     plant.install<TestReactor>();
-
     plant.start();
+
+    std::vector<std::string> expected = {
+        "Executed toggled reaction 0",
+        "Executed enabled reaction 0",
+        "Executed enabled reaction 1",
+    };
+
+    // Make an info print the diff in an easy to read way if we fail
+    INFO(test_util::diff_string(expected, events));
+
+    // Check the events fired in order and only those events
+    REQUIRE(events == expected);
 }

tests/dsl/Always.cpp

@@ -19,52 +19,67 @@
 #include <catch.hpp>
 #include <nuclear>
 
+#include "test_util/TestBase.hpp"
+
 namespace {
-struct BlankMessage {};
 
-int i                = 0;      // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
-bool emitted_message = false;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+/// @brief Events that occur during the test
+std::vector<std::string> events;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
+
+struct SimpleMessage {};
 
-class TestReactor : public NUClear::Reactor {
+class TestReactor : public test_util::TestBase<TestReactor> {
 public:
-    TestReactor(std::unique_ptr<NUClear::Environment> environment) : Reactor(std::move(environment)) {
+    TestReactor(std::unique_ptr<NUClear::Environment> environment) : TestBase(std::move(environment), false) {
 
         on<Always>().then([this] {
-            // Run until it's 11 then emit the blank message
-            if (i > 10) {
-                if (!emitted_message) {
-                    emitted_message = true;
-                    emit(std::make_unique<BlankMessage>());
-                }
-            }
-            else {
+            if (i < 10) {
+                events.push_back("Always " + std::to_string(i));
                 ++i;
             }
+            else if (i == 10) {
+                emit(std::make_unique<SimpleMessage>());
+            }
         });
 
-        on<Always, With<BlankMessage>>().then([this] {
-            if (i == 11) {
-                ++i;
-                powerplant.shutdown();
-            }
+        on<Always, With<SimpleMessage>>().then([this] {
+            events.push_back("Always with SimpleMessage " + std::to_string(i));
+
+            // We need to shutdown manually as the default pool will always be idle
+            powerplant.shutdown();
         });
     }
+
+    /// Counter for the number of times we have run
+    int i = 0;
 };
 }  // namespace
 
-TEST_CASE("Testing on<Always> functionality (permanent run)", "[api][always]") {
+TEST_CASE("The Always DSL keyword runs continuously when it can", "[api][always]") {
 
     NUClear::PowerPlant::Configuration config;
     config.thread_count = 1;
     NUClear::PowerPlant plant(config);
+    plant.install<TestReactor>();
+    plant.start();
 
-    // We are installing with an initial log level of debug
-    plant.install<TestReactor, NUClear::DEBUG>();
-
-    plant.emit(std::make_unique<int>(5));
+    std::vector<std::string> expected = {
+        "Always 0",
+        "Always 1",
+        "Always 2",
+        "Always 3",
+        "Always 4",
+        "Always 5",
+        "Always 6",
+        "Always 7",
+        "Always 8",
+        "Always 9",
+        "Always with SimpleMessage 10",
+    };
 
-    plant.start();
+    // Make an info print the diff in an easy to read way if we fail
+    INFO(test_util::diff_string(expected, events));
 
-    REQUIRE(emitted_message);
-    REQUIRE(i == 12);
+    // Check the events fired in order and only those events
+    REQUIRE(events == expected);
 }