From 31677d9d919bd52d91809412ce90c586aa8aa63a Mon Sep 17 00:00:00 2001 From: Raphael Maenle Date: Tue, 9 Jun 2026 10:42:46 +0200 Subject: [PATCH] tests: SECS-I T1 / T2-recv timer firings; T3/T4 no-op assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_secsi.cpp covered T2 on the send side (retry) and a tick-based back-to-back exchange. This commit fills in the rest of the timer matrix at FSM level: T1 in RecvBlock → abort, reason mentions "T1" T1 outside RecvBlock → ignored T2 in RecvEotSent → abort T2 in RecvBlock → abort (mid-block stall) T3 / T4 → FSM-level no-op (documented as upper-layer driven) T2 contrast → send-side retries, recv-side aborts (same timer, different recovery, both demonstrated in one test) If a future commit moves T3 or T4 enforcement into the FSM, the no-op test breaks loudly so protocol.hpp can be updated alongside. Closes #7 in the test-gap backlog. Co-Authored-By: Claude Opus 4.7 --- CMakeLists.txt | 1 + tests/test_secsi_timers.cpp | 146 ++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 tests/test_secsi_timers.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 23aeb7f..326be40 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -101,6 +101,7 @@ add_executable(secsgem_tests tests/test_hsms_timers.cpp tests/test_hsms_s9.cpp tests/test_secsi.cpp + tests/test_secsi_timers.cpp tests/test_secsi_tcp.cpp tests/test_host_handler.cpp tests/test_control_state.cpp diff --git a/tests/test_secsi_timers.cpp b/tests/test_secsi_timers.cpp new file mode 100644 index 0000000..7aecf65 --- /dev/null +++ b/tests/test_secsi_timers.cpp @@ -0,0 +1,146 @@ +// SECS-I FSM timer firing tests. +// +// The existing test_secsi.cpp covered T2 in the send direction. This +// file fills in: +// +// T1 inter-character — fires while RecvBlock is mid-buffer, aborts. +// T2 protocol — fires while in receive states (RecvEotSent or +// RecvBlock), aborts. +// T3 / T4 — documented as upper-layer-driven; the FSM is +// a no-op on those, asserted here so a future +// commit that adds in-FSM enforcement breaks +// this test loudly. + +#include + +#include +#include +#include + +#include "secsgem/secsi/block.hpp" +#include "secsgem/secsi/protocol.hpp" + +using namespace secsgem::secsi; + +namespace { + +bool has_abort(const std::vector& out) { + return std::any_of(out.begin(), out.end(), [](const Action& a) { + return std::holds_alternative(a); + }); +} + +// Drive the FSM into RecvBlock by simulating "peer sends ENQ, then a +// length byte (say 3), then nothing further". +void drive_into_recv_block(Protocol& p, std::vector& out) { + p.on_event(EventByte{kENQ}, out); + REQUIRE(p.state() == Protocol::State::RecvEotSent); + out.clear(); + // Now feed the length byte — FSM moves into RecvBlock. + p.on_event(EventByte{3}, out); + REQUIRE(p.state() == Protocol::State::RecvBlock); + out.clear(); + // One more byte: still in RecvBlock since recv_expected_ = 1 + 3 + 2 = 6. + p.on_event(EventByte{0xAA}, out); + REQUIRE(p.state() == Protocol::State::RecvBlock); + out.clear(); +} + +} // namespace + +TEST_CASE("SECS-I T1: inter-character timeout in RecvBlock aborts") { + Protocol p(Role::Master); + std::vector out; + + drive_into_recv_block(p, out); + + p.on_event(EventTimeout{Timer::T1}, out); + CHECK(p.state() == Protocol::State::Idle); + CHECK(has_abort(out)); + // The abort reason should mention T1 so operators can grep logs for it. + bool t1_mentioned = std::any_of(out.begin(), out.end(), [](const Action& a) { + auto* err = std::get_if(&a); + return err && err->reason.find("T1") != std::string::npos; + }); + CHECK(t1_mentioned); +} + +TEST_CASE("SECS-I T1: ignored outside RecvBlock") { + Protocol p(Role::Master); + std::vector out; + CHECK(p.state() == Protocol::State::Idle); + p.on_event(EventTimeout{Timer::T1}, out); + CHECK(p.state() == Protocol::State::Idle); + CHECK_FALSE(has_abort(out)); +} + +TEST_CASE("SECS-I T2: timeout in RecvEotSent aborts") { + Protocol p(Role::Master); + std::vector out; + + p.on_event(EventByte{kENQ}, out); + REQUIRE(p.state() == Protocol::State::RecvEotSent); + out.clear(); + + p.on_event(EventTimeout{Timer::T2}, out); + CHECK(p.state() == Protocol::State::Idle); + CHECK(has_abort(out)); +} + +TEST_CASE("SECS-I T2: timeout in RecvBlock aborts (mid-block stall)") { + Protocol p(Role::Master); + std::vector out; + drive_into_recv_block(p, out); + + p.on_event(EventTimeout{Timer::T2}, out); + CHECK(p.state() == Protocol::State::Idle); + CHECK(has_abort(out)); +} + +TEST_CASE("SECS-I T3 / T4: FSM-level no-ops (upper layer enforces)") { + // The protocol docs in protocol.hpp note T3 (reply) and T4 (inter-block) + // are driven from the higher SECS-II / message layer. If a future + // commit adds in-FSM handling for either, this test will break and + // the documentation should be updated alongside. + Protocol p(Role::Master); + std::vector out; + p.on_event(EventTimeout{Timer::T3}, out); + CHECK(p.state() == Protocol::State::Idle); + CHECK(out.empty()); + p.on_event(EventTimeout{Timer::T4}, out); + CHECK(p.state() == Protocol::State::Idle); + CHECK(out.empty()); +} + +TEST_CASE("SECS-I T2: in send state retries (existing) AND in recv state aborts (new)") { + // Round-trip the existing T2-send-retry to keep the contrast obvious: + // T2 in send → retry, T2 in recv → abort. Same timer, different + // recovery strategy depending on which half of the protocol is + // active. + Block blk; + blk.header.stream = 1; + blk.header.function = 1; + blk.header.block_number = 1; + blk.header.end_block = true; + blk.header.w_bit = true; + + Protocol sender(Role::Master); + std::vector out; + sender.on_event(EventSend{blk}, out); + REQUIRE(sender.state() == Protocol::State::SendEnqSent); + const auto rty_before = sender.rty_remaining(); + out.clear(); + sender.on_event(EventTimeout{Timer::T2}, out); + CHECK(sender.state() == Protocol::State::SendEnqSent); + CHECK(sender.rty_remaining() == rty_before - 1); + + // Now the receive side: T2 there *aborts* instead of retrying. + Protocol receiver(Role::Master); + out.clear(); + receiver.on_event(EventByte{kENQ}, out); + REQUIRE(receiver.state() == Protocol::State::RecvEotSent); + out.clear(); + receiver.on_event(EventTimeout{Timer::T2}, out); + CHECK(receiver.state() == Protocol::State::Idle); + CHECK(has_abort(out)); +}