From 1f6ec75f6e0e65c5a2fcee5cde9c3dcc0f40568d Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 18 Aug 2024 13:49:03 +1000 Subject: [PATCH] Make tests pass in their timeout, and don't use watchdog as for tests like TimeTravel where we manipulate the clock it won't work. --- tests/test_util/TestBase.hpp | 42 ++++++++++++++++++++++++---- tests/tests/api/TimeTravelFrozen.cpp | 2 +- tests/tests/dsl/Every.cpp | 5 ++-- tests/tests/dsl/Idle.cpp | 2 +- tests/tests/dsl/IdleSync.cpp | 2 +- tests/tests/dsl/TCP.cpp | 5 ++-- tests/tests/dsl/Watchdog.cpp | 4 +-- tests/tests/dsl/emit/Delay.cpp | 5 ++-- 8 files changed, 51 insertions(+), 16 deletions(-) diff --git a/tests/test_util/TestBase.hpp b/tests/test_util/TestBase.hpp index 58382579..bcb4a09a 100644 --- a/tests/test_util/TestBase.hpp +++ b/tests/test_util/TestBase.hpp @@ -32,7 +32,7 @@ namespace test_util { -template +template class TestBase : public NUClear::Reactor { public: /** @@ -46,7 +46,17 @@ class TestBase : public NUClear::Reactor { template struct Step {}; - explicit TestBase(std::unique_ptr environment, const bool& shutdown_on_idle = true) + /** + * Emit this struct to fail the test + */ + struct Fail { + explicit Fail(std::string message) : message(std::move(message)) {} + std::string message; + }; + + explicit TestBase(std::unique_ptr environment, + const bool& shutdown_on_idle = true, + const std::chrono::milliseconds& timeout = std::chrono::milliseconds(1000)) : Reactor(std::move(environment)) { // Shutdown if the system is idle @@ -54,13 +64,35 @@ class TestBase : public NUClear::Reactor { on>().then([this] { powerplant.shutdown(); }); } + on().then([this] { + std::lock_guard lock(timeout_mutex); + clean_shutdown = true; + timeout_cv.notify_all(); + }); + // Timeout if the test doesn't complete in time - on, MainThread>().then([this] { - powerplant.shutdown(); - INFO("Test timed out"); + // Typically we would use a watchdog, however it is subject to Time Travel + // So instead spawn a thread that will wait for the timeout and then fail the test and shut down + on().then([this, timeout] { + std::unique_lock lock(timeout_mutex); + timeout_cv.wait_for(lock, timeout); + if (!clean_shutdown) { + powerplant.shutdown(); + emit(std::make_unique("Test timed out")); + } + }); + + on, MainThread>().then([this](const Fail& f) { + INFO(f.message); CHECK(false); + powerplant.shutdown(); }); } + +private: + std::mutex timeout_mutex; + std::condition_variable timeout_cv; + bool clean_shutdown = false; }; } // namespace test_util diff --git a/tests/tests/api/TimeTravelFrozen.cpp b/tests/tests/api/TimeTravelFrozen.cpp index 9ab5ae09..162b6023 100644 --- a/tests/tests/api/TimeTravelFrozen.cpp +++ b/tests/tests/api/TimeTravelFrozen.cpp @@ -13,7 +13,7 @@ constexpr std::chrono::milliseconds SHUTDOWN_TIME = std::chrono::milliseconds(12 struct WaitForShutdown {}; -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { diff --git a/tests/tests/dsl/Every.cpp b/tests/tests/dsl/Every.cpp index 63b0dbce..445b9650 100644 --- a/tests/tests/dsl/Every.cpp +++ b/tests/tests/dsl/Every.cpp @@ -33,10 +33,11 @@ std::vector every_times; // NOLINT(cppcoreguideli std::vector per_times; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) std::vector dynamic_times; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: - TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { + TestReactor(std::unique_ptr environment) + : TestBase(std::move(environment), false, std::chrono::milliseconds(10000)) { // Trigger on 3 different types of every on>>().then([]() { every_times.push_back(NUClear::clock::now()); }); diff --git a/tests/tests/dsl/Idle.cpp b/tests/tests/dsl/Idle.cpp index e2a86a2b..b2a043ff 100644 --- a/tests/tests/dsl/Idle.cpp +++ b/tests/tests/dsl/Idle.cpp @@ -37,7 +37,7 @@ struct SimpleMessage { constexpr int time_step = 50; -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: template struct CustomPool { diff --git a/tests/tests/dsl/IdleSync.cpp b/tests/tests/dsl/IdleSync.cpp index 4a5c860a..eccd9624 100644 --- a/tests/tests/dsl/IdleSync.cpp +++ b/tests/tests/dsl/IdleSync.cpp @@ -30,7 +30,7 @@ namespace { /// A vector of events that have happened std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { diff --git a/tests/tests/dsl/TCP.cpp b/tests/tests/dsl/TCP.cpp index 70c1175f..56a346de 100644 --- a/tests/tests/dsl/TCP.cpp +++ b/tests/tests/dsl/TCP.cpp @@ -57,7 +57,7 @@ struct TestConnection { struct Finished {}; -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: void handle_data(const std::string& name, const IO::Event& event) { // We have data to read @@ -78,7 +78,8 @@ class TestReactor : public test_util::TestBase { } } - TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { + TestReactor(std::unique_ptr environment) + : TestBase(std::move(environment), false, std::chrono::milliseconds(2000)) { for (const auto& t : active_tests) { switch (t) { diff --git a/tests/tests/dsl/Watchdog.cpp b/tests/tests/dsl/Watchdog.cpp index 6669bf8a..407c014b 100644 --- a/tests/tests/dsl/Watchdog.cpp +++ b/tests/tests/dsl/Watchdog.cpp @@ -35,10 +35,10 @@ std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-gl template struct Flag {}; -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: TestReactor(std::unique_ptr environment) - : TestBase(std::move(environment), false), start(NUClear::clock::now()) { + : TestBase(std::move(environment), false, std::chrono::milliseconds(10000)), start(NUClear::clock::now()) { on, 50, std::chrono::milliseconds>>().then([this] { events.push_back("Watchdog 1 triggered @ " + floored_time()); diff --git a/tests/tests/dsl/emit/Delay.cpp b/tests/tests/dsl/emit/Delay.cpp index a20e1aad..573f243a 100644 --- a/tests/tests/dsl/emit/Delay.cpp +++ b/tests/tests/dsl/emit/Delay.cpp @@ -51,9 +51,10 @@ struct TargetTimeMessage { struct FinishTest {}; -class TestReactor : public test_util::TestBase { +class TestReactor : public test_util::TestBase { public: - TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { + TestReactor(std::unique_ptr environment) + : TestBase(std::move(environment), false, std::chrono::milliseconds(2000)) { // Measure when messages were sent and received and print those values on>().then([](const DelayedMessage& m) {