From 0440a9f0ce524aa48c2e57da912054258cafbde7 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Tue, 30 Apr 2024 16:05:01 +1000 Subject: [PATCH] Change the CI test units to be configurable using defines (#106) The osx runners in GHA are so slow they can't keep up with a 1/25 event timing. Change the event timing to be in a single file and configurable so that the macos CI runners can have a slower tolerance --- .github/workflows/macos.yaml | 2 + src/extension/network/NUClearNetwork.hpp | 11 ++-- tests/dsl/emit/Delay.cpp | 46 ++++++++--------- tests/individual/TimeTravel.cpp | 40 +++++++-------- tests/test_util/TimeUnit.hpp | 65 ++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 51 deletions(-) create mode 100644 tests/test_util/TimeUnit.hpp diff --git a/.github/workflows/macos.yaml b/.github/workflows/macos.yaml index 89f765623..576920269 100644 --- a/.github/workflows/macos.yaml +++ b/.github/workflows/macos.yaml @@ -38,6 +38,8 @@ jobs: ninjaVersion: 1.11.1 - name: Configure CMake + env: + CXXFLAGS: -DNUCLEAR_TEST_TIME_UNIT_DEN=10 run: | cmake -E make_directory build cmake -S . -B build \ diff --git a/src/extension/network/NUClearNetwork.hpp b/src/extension/network/NUClearNetwork.hpp index fc3d93656..e7ee03407 100644 --- a/src/extension/network/NUClearNetwork.hpp +++ b/src/extension/network/NUClearNetwork.hpp @@ -93,8 +93,8 @@ namespace extension { inline void measure_round_trip(std::chrono::steady_clock::duration time) { // Make our measurement into a float seconds type - const std::chrono::duration> m = - std::chrono::duration_cast>>(time); + const std::chrono::duration m = + std::chrono::duration_cast>(time); // Alias variables const auto& Q = round_trip_kf.process_noise; @@ -111,7 +111,7 @@ namespace extension { // Put result into our variable round_trip_time = std::chrono::duration_cast( - std::chrono::duration>(X)); + std::chrono::duration(X)); } }; @@ -130,7 +130,10 @@ namespace extension { * @param target who we are sending to (blank means everyone) * @param reliable if the delivery of the data should be ensured */ - void send(const uint64_t& hash, const std::vector& payload, const std::string& target, bool reliable); + void send(const uint64_t& hash, + const std::vector& payload, + const std::string& target, + bool reliable); /** * @brief Set the callback to use when a data packet is completed diff --git a/tests/dsl/emit/Delay.cpp b/tests/dsl/emit/Delay.cpp index 8e5fb5c1a..60fadc96b 100644 --- a/tests/dsl/emit/Delay.cpp +++ b/tests/dsl/emit/Delay.cpp @@ -23,16 +23,17 @@ #include #include -#include "../../test_util/TestBase.hpp" +#include "test_util/TestBase.hpp" +#include "test_util/TimeUnit.hpp" // Anonymous namespace to keep everything file local namespace { +using TimeUnit = test_util::TimeUnit; + /// @brief Events that occur during the test std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -/// @brief Test units are the time units the test is performed in -using TestUnits = std::chrono::duration>; /// @brief Perform this many different time points for the test constexpr int test_loops = 5; @@ -50,14 +51,14 @@ 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) { // Measure when messages were sent and received and print those values on>().then([](const DelayedMessage& m) { - auto true_delta = std::chrono::duration_cast(NUClear::clock::now() - m.time); - auto delta = std::chrono::duration_cast(m.delay); + auto true_delta = test_util::round_to_test_units(NUClear::clock::now() - m.time); + auto delta = test_util::round_to_test_units(m.delay); // Print the debug message events.push_back("delayed " + std::to_string(true_delta.count()) + " received " @@ -65,8 +66,8 @@ class TestReactor : public test_util::TestBase { }); on>().then([](const TargetTimeMessage& m) { - auto true_delta = std::chrono::duration_cast(NUClear::clock::now() - m.time); - auto delta = std::chrono::duration_cast(m.target - m.time); + auto true_delta = test_util::round_to_test_units(NUClear::clock::now() - m.time); + auto delta = test_util::round_to_test_units(m.target - m.time); // Print the debug message events.push_back("at_time " + std::to_string(true_delta.count()) + " received " @@ -78,25 +79,20 @@ class TestReactor : public test_util::TestBase { powerplant.shutdown(); }); - - on().then([this] { - // Get our jump size in milliseconds - const int jump_unit = (TestUnits::period::num * 1000) / TestUnits::period::den; - // Delay with consistent jumps + on>>().then([this] { + // Interleave absolute and relative events for (int i = 0; i < test_loops; ++i) { - auto delay = std::chrono::milliseconds(jump_unit * i); + auto delay = TimeUnit(i * 2); emit(std::make_unique(delay), delay); - } - - // Target time with consistent jumps that interleave the first set - for (int i = 0; i < test_loops; ++i) { - auto target = NUClear::clock::now() + std::chrono::milliseconds(jump_unit / 2 + jump_unit * i); + auto target = NUClear::clock::now() + TimeUnit((i * 2) + 1); emit(std::make_unique(target), target); } // Emit a shutdown one time unit after - emit(std::make_unique(), std::chrono::milliseconds(jump_unit * (test_loops + 1))); + emit(std::make_unique(), TimeUnit((test_loops + 1) * 2)); }); + + on().then([this] { emit(std::make_unique>()); }); } }; } // namespace @@ -110,15 +106,15 @@ TEST_CASE("Testing the delay emit", "[api][emit][delay]") { const std::vector expected = { "delayed 0 received 0", - "at_time 0 received 0", - "delayed 1 received 1", "at_time 1 received 1", "delayed 2 received 2", - "at_time 2 received 2", - "delayed 3 received 3", "at_time 3 received 3", "delayed 4 received 4", - "at_time 4 received 4", + "at_time 5 received 5", + "delayed 6 received 6", + "at_time 7 received 7", + "delayed 8 received 8", + "at_time 9 received 9", "Finished", }; diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 11a70640d..5921fa34c 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -3,10 +3,12 @@ #include #include "test_util/TestBase.hpp" +#include "test_util/TimeUnit.hpp" namespace { -using TestUnits = std::chrono::duration>; +using TimeUnit = test_util::TimeUnit; + constexpr int64_t EVENT_1_TIME = 4; constexpr int64_t EVENT_2_TIME = 8; @@ -36,7 +38,7 @@ class TestReactor : public test_util::TestBase { results.events[0] = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; return false; }, - NUClear::clock::time_point(TestUnits(EVENT_1_TIME)), + NUClear::clock::time_point(TimeUnit(EVENT_1_TIME)), 1)); // Emit a chrono task to run at time EVENT_2_TIME, and shutdown @@ -46,7 +48,7 @@ class TestReactor : public test_util::TestBase { powerplant.shutdown(); return false; }, - NUClear::clock::time_point(TestUnits(EVENT_2_TIME)), + NUClear::clock::time_point(TimeUnit(EVENT_2_TIME)), 2)); // Time travel! @@ -86,7 +88,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time const double rtf = GENERATE(0.5, 1.0, 2.0); CAPTURE(action, adjustment, rtf); reactor.action = action; - reactor.adjustment = TestUnits(adjustment); + reactor.adjustment = TimeUnit(adjustment); reactor.rtf = rtf; // Start the powerplant @@ -110,32 +112,26 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time default: throw std::runtime_error("Unknown action"); } - std::array expected_nuclear = {TestUnits(expected[0]), TestUnits(expected[1])}; - std::array expected_steady = {TestUnits(std::lround(double(expected[0]) / rtf)), - TestUnits(std::lround(double(expected[1]) / rtf))}; + std::array expected_nuclear = {TimeUnit(expected[0]), TimeUnit(expected[1])}; + std::array expected_steady = {TimeUnit(std::lround(double(expected[0]) / rtf)), + TimeUnit(std::lround(double(expected[1]) / rtf))}; const auto& r = reactor.results; const auto& n_start = reactor.results.start.nuclear; const auto& s_start = reactor.results.start.steady; - auto round_to_test_units = [](const auto& duration) { - const double d = std::chrono::duration_cast>(duration).count(); - const double t = (TestUnits::period::den * d) / TestUnits::period::num; - return TestUnits(std::lround(t)); - }; - - std::array actual_nuclear = { - round_to_test_units(r.events[0].nuclear - n_start), - round_to_test_units(r.events[1].nuclear - n_start), + std::array actual_nuclear = { + test_util::round_to_test_units(r.events[0].nuclear - n_start), + test_util::round_to_test_units(r.events[1].nuclear - n_start), }; - std::array actual_steady = { - round_to_test_units(r.events[0].steady - s_start), - round_to_test_units(r.events[1].steady - s_start), + std::array actual_steady = { + test_util::round_to_test_units(r.events[0].steady - s_start), + test_util::round_to_test_units(r.events[1].steady - s_start), }; - const TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); - const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); - CHECK(round_to_test_units(r.zero.nuclear.time_since_epoch()) == TestUnits(0)); + const TimeUnit actual_adjustment(test_util::round_to_test_units(r.start.nuclear - r.zero.nuclear)); + const TimeUnit expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); + CHECK(test_util::round_to_test_units(r.zero.nuclear.time_since_epoch()) == TimeUnit(0)); CHECK(expected_nuclear[0] == actual_nuclear[0]); CHECK(expected_nuclear[1] == actual_nuclear[1]); CHECK(expected_steady[0] == actual_steady[0]); diff --git a/tests/test_util/TimeUnit.hpp b/tests/test_util/TimeUnit.hpp new file mode 100644 index 000000000..6acca6531 --- /dev/null +++ b/tests/test_util/TimeUnit.hpp @@ -0,0 +1,65 @@ +/* + * MIT License + * + * Copyright (c) 2023 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#ifndef TEST_UTIL_TIME_UNIT_HPP +#define TEST_UTIL_TIME_UNIT_HPP + +#ifndef NUCLEAR_TEST_TIME_UNIT_NUM + // NOLINTNEXTLINE(cppcoreguidelines-macro-usage) + #define NUCLEAR_TEST_TIME_UNIT_NUM 1 +#endif +#ifndef NUCLEAR_TEST_TIME_UNIT_DEN + // NOLINTNEXTLINE(cppcoreguidelines-macro-usage) + #define NUCLEAR_TEST_TIME_UNIT_DEN 20 +#endif + +#include + +namespace test_util { + +/** + * Units that time based tests should use to measure time. + * This can be changed so that slower systems (such as CI) can run the tests with a larger time unit since they can be + * slow and fail. + * To change from the default define NUCLEAR_TEST_TIME_UNIT_NUM and NUCLEAR_TEST_TIME_UNIT_DEN to the desired time unit. + */ +using TimeUnit = std::chrono::duration>; + +/** + * Rounds the given duration to the nearest TimeUnit. + * + * @tparam T The type of the duration. + * + * @param duration The duration to be rounded. + * + * @return The rounded duration in TimeUnit. + */ +template +TimeUnit round_to_test_units(const T& duration) { + const double d = std::chrono::duration_cast>(duration).count(); + const double t = (TimeUnit::period::den * d) / TimeUnit::period::num; + return TimeUnit(std::lround(t)); +} + +} // namespace test_util + +#endif // TEST_UTIL_TIME_UNIT_HPP