From 974668a5cc98e84df6bd715fee0b5ab06d8fa6e2 Mon Sep 17 00:00:00 2001 From: Fede654 Date: Thu, 13 Aug 2026 17:52:59 +0000 Subject: [PATCH 1/3] tests: fix the doctest suite build wiring `cmake -DSS_TESTS=ON` currently fails to configure. Four small things, each independent of the others: - CMAKE_MODULE_PATH is set to ${CMAKE_SOURCE_DIR}/cmake/, but Doctest.cmake lives in tests/cmake/, so include(Doctest) cannot find it. Uses CMAKE_CURRENT_SOURCE_DIR instead. - target_set_warnings() is called but its Warnings.cmake module is not present in the repository, which is a hard configure error. Wrapped in if(COMMAND ...) so it is still used if that module is ever added. - include(CodeCoverage) is likewise not present. Made OPTIONAL: the suite then builds, and only the 'coverage' target is unavailable. - Doctest.cmake fetches doctest inside if(ENABLE_DOCTESTS), and that variable is never set, so doctest is never fetched and the tests fail to compile on a missing doctest/doctest.h. tests/CMakeLists.txt is only added to the build when SS_TESTS is ON, so it sets ENABLE_DOCTESTS itself. With these the test target configures and builds. A follow-up commit deals with the test sources themselves. Co-Authored-By: Claude Fable 5 --- tests/CMakeLists.txt | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 460b6d3..595d9c4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,8 +1,19 @@ cmake_minimum_required(VERSION 3.14) -set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/") +# Doctest.cmake lives in tests/cmake/, not in a top-level cmake/ dir +# (which does not exist), so CMAKE_SOURCE_DIR pointed at nothing and +# include(Doctest) failed. This suite therefore never configured while +# SS_TESTS defaulted to OFF and nobody turned it on. +set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/") include(CTest) + +# Doctest.cmake only fetches doctest inside `if(ENABLE_DOCTESTS)`, and +# nothing ever set that variable — so the module was a no-op and the +# tests failed to compile on a missing doctest/doctest.h. This file is +# only added to the build when SS_TESTS is ON, so enabling it here is +# equivalent to "the user asked for tests". +set(ENABLE_DOCTESTS ON) include(Doctest) # List all files containing tests. (Change as needed) @@ -22,7 +33,12 @@ set(TEST_RUNNER_PARAMS "-s") # Any arguemnts to feed the test runner (change as add_executable(${TEST_MAIN} ${TESTFILES}) target_link_libraries(${TEST_MAIN} PRIVATE ${LIBRARY_NAME} doctest) set_target_properties(${TEST_MAIN} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}) -target_set_warnings(${TEST_MAIN} ENABLE ALL AS_ERROR ALL DISABLE Annoying) # Set warnings (if needed). +# target_set_warnings() comes from a Warnings.cmake module that was +# never vendored into this repository, so calling it unconditionally is +# a hard configure error. Guarded so the suite builds either way. +if(COMMAND target_set_warnings) + target_set_warnings(${TEST_MAIN} ENABLE ALL AS_ERROR ALL DISABLE Annoying) +endif() set_target_properties(${TEST_MAIN} PROPERTIES CXX_STANDARD 20 @@ -35,5 +51,7 @@ add_test( NAME ${LIBRARY_NAME}.${TEST_MAIN} COMMAND ${TEST_MAIN} ${TEST_RUNNER_PARAMS}) -# Adds a 'coverage' target. -include(CodeCoverage) +# Adds a 'coverage' target. CodeCoverage.cmake was never vendored into +# this repository either, so this is OPTIONAL: absent it, the suite still +# builds and runs, only the 'coverage' target is unavailable. +include(CodeCoverage OPTIONAL RESULT_VARIABLE SS_CODECOVERAGE_FOUND) From 76764fb8fbe8c8bd7a9bfc27e459bb62ca18e79c Mon Sep 17 00:00:00 2001 From: Fede654 Date: Thu, 13 Aug 2026 19:01:58 +0000 Subject: [PATCH 2/3] tests: update the test suite to the current API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergeechotest.cc and parsearcomandtest.cc predate the current codebase: they call SharedState::extractCommand and SharedState::mergestate and include shared_state_error_code.hh and piped_async_command.hh, none of which exist since the Lua dependency was removed in 99a4249, so they cannot compile against today's sources. debugmesasgetest.cc still compiles but asserts nothing — it prints debug messages at each level — so it is removed as well rather than kept as a test that cannot fail. This also adds enable_testing() to the top-level CMakeLists.txt. Without it CMake writes no root CTestTestfile.cmake, so `ctest` in the build directory reports "No tests were found" regardless of what the tests subdirectory registers; include(CTest) in tests/CMakeLists.txt only covers that subdirectory. In their place, fourteen cases over code that is still current. bleach_tests.cc covers expiry, which is an ordinary function and so convenient to test directly. Entries at exactly the elapsed time are removed rather than kept; survivors are decremented by exactly that amount; five 2 s bleaches leave the same TTL as one 10 s bleach, which matters because the daemon passes real elapsed time to compensate for late timer ticks; a non-positive interval is refused without altering TTLs; an unknown type reports UNKOWN_DATA_TYPE; and an empty registered type is a no-op rather than an error. mStates is protected, so a small subclass supplies known state — the function under test is the real one. state_serialization_tests.cc covers the shapes peers must agree on. The compatibility cases deserialize *literal* payloads rather than round tripping whatever this build emits, since a round trip agrees with itself however the format drifts: an entry and a whole slice in the shape nodes send today must be accepted, and the member names are asserted directly for both StateEntry and DataTypeConf, because renaming one changes the wire format for every peer and the on-disk config for every node. Two cases document behaviour rather than assert correctness: - When an entry cannot be read, deserialization stops there: entries earlier in the map survive and everything after is lost. The serialization context records the failure, but NetworkMessage::toStateSlice() discards that status, so a node silently merges the surviving prefix. If a member is ever added and required on read, a slice from a peer that does not emit it yet has every entry in an incompatible legacy shape, so the first one fails and that peer's neighbour sees an empty view of it rather than a degraded one. - StateEntry is copy-constructible but not copy-assignable, because rapidjson's Document assignment is private. `map[key] = entry` fails to compile with an error that points into , while `map.emplace(key, entry)` works. The suite deliberately stops short of merge() itself, which is a coroutine needing an IOContext and a peer address; covering it well means driving real instances rather than unit testing it. Verified with cmake -DSS_TESTS=ON in Release and Debug: configures, builds, and ctest passes 1/1 (14 doctest cases, 63 assertions). Co-Authored-By: Claude Fable 5 --- CMakeLists.txt | 6 + tests/CMakeLists.txt | 5 +- tests/bleach_tests.cc | 183 +++++++++++++++++++++ tests/debugmesasgetest.cc | 56 ------- tests/mergeechotest.cc | 117 -------------- tests/parsearcomandtest.cc | 67 -------- tests/state_serialization_tests.cc | 245 +++++++++++++++++++++++++++++ 7 files changed, 436 insertions(+), 243 deletions(-) create mode 100644 tests/bleach_tests.cc delete mode 100644 tests/debugmesasgetest.cc delete mode 100644 tests/mergeechotest.cc delete mode 100644 tests/parsearcomandtest.cc create mode 100644 tests/state_serialization_tests.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index e28ae07..408b7c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -165,6 +165,12 @@ if(SS_DEVELOPMENT_BUILD) endif(SS_DEVELOPMENT_BUILD) if(SS_TESTS) + # enable_testing() must be called in the TOP-LEVEL list file or no + # root CTestTestfile.cmake is generated, and `ctest` in the build + # directory reports "No tests were found" however many tests the + # subdirectory registers. tests/CMakeLists.txt calls include(CTest) + # for itself, which is not enough on its own. + enable_testing() add_subdirectory(tests) endif(SS_TESTS) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 595d9c4..989dd5d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -19,9 +19,8 @@ include(Doctest) # List all files containing tests. (Change as needed) set(TESTFILES # All .cpp files in tests/ main.cpp - mergeechotest.cc - debugmesasgetest.cc - parsearcomandtest.cc + bleach_tests.cc + state_serialization_tests.cc ) set(TEST_MAIN unit_tests) # Default name for test executable (change if you wish). diff --git a/tests/bleach_tests.cc b/tests/bleach_tests.cc new file mode 100644 index 0000000..b1b7bfa --- /dev/null +++ b/tests/bleach_tests.cc @@ -0,0 +1,183 @@ +/* + * Shared State + * + * Copyright (C) 2026 Asociación Civil Altermundi + * + * This program is free software: you can redistribute it and/or modify it under + * the terms of the GNU Affero General Public License as published by the + * Free Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. + * See the GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +/* Expiry semantics. + * + * `bleach()` decides when data leaves the network, so its edge cases are + * the difference between an entry disappearing a second early and one + * that never leaves. It is an ordinary function — no coroutine, no + * socket — which makes it exactly the kind of thing worth testing in + * process rather than by driving daemons. */ + +#include "doctest/doctest.h" + +#include "sharedstate.hh" +#include "shared_state_errors.hh" + +#include "io_context.hh" + +#include +#include +#include +#include +#include + +namespace +{ + +/** `mStates` is protected, so a small subclass is the supported way to + * put a known state in front of the real implementation. Nothing here + * reimplements behaviour — the code under test is the real `bleach`. */ +struct TestableSharedState : SharedState +{ + using SharedState::SharedState; + + void put(const std::string& type, const std::string& key, int64_t ttl) + { + StateEntry e; + e.mAuthor = "test"; + e.mTtl = std::chrono::seconds(ttl); + e.mData.Parse(R"({"v":1})"); + auto& entries = mStates[type]; + entries.erase(key); // StateEntry is not assignable + entries.emplace(key, e); + } + + bool has(const std::string& type, const std::string& key) const + { + auto it = mStates.find(type); + return it != mStates.end() && it->second.count(key) > 0; + } + + int64_t ttl(const std::string& type, const std::string& key) const + { + return mStates.at(type).at(key).mTtl.count(); + } + + std::size_t size(const std::string& type) const + { + auto it = mStates.find(type); + return it == mStates.end() ? 0 : it->second.size(); + } + + void ensureType(const std::string& type) { mStates[type]; } +}; + +/** SharedState needs an IOContext reference. bleach() never uses it, but + * forming a reference that does not designate an object is undefined + * behaviour whether or not anything reads it, so make a real one — an + * epoll descriptor is cheap. */ +struct Fixture +{ + std::unique_ptr io = IOContext::setup(); + TestableSharedState state; + + Fixture(): state(*io) { REQUIRE(io != nullptr); } +}; + +constexpr const char* TYPE = "unit_type"; + +} // namespace + +TEST_CASE("bleach removes entries at or below the elapsed time") +{ + Fixture f; + auto& st = f.state; + st.put(TYPE, "expires-exactly", 5); + st.put(TYPE, "expires-under", 3); + st.put(TYPE, "survives", 6); + + const ssize_t removed = st.bleach(TYPE, std::chrono::seconds(5)); + + CHECK(removed == 2); + CHECK_FALSE(st.has(TYPE, "expires-exactly")); // <= elapsed, so it goes + CHECK_FALSE(st.has(TYPE, "expires-under")); + CHECK(st.has(TYPE, "survives")); +} + +TEST_CASE("bleach decrements the survivors by exactly the elapsed time") +{ + Fixture f; + auto& st = f.state; + st.put(TYPE, "a", 100); + st.put(TYPE, "b", 10); + + st.bleach(TYPE, std::chrono::seconds(4)); + + CHECK(st.ttl(TYPE, "a") == 96); + CHECK(st.ttl(TYPE, "b") == 6); +} + +TEST_CASE("repeated bleaching is equivalent to one longer bleach") +{ + /* The daemon bleaches on a timer and compensates for a late tick by + * passing the real elapsed time, so these two paths must agree or + * entries live longer on a busy node than on an idle one. */ + Fixture fs, fo; + auto& stepwise = fs.state; + auto& atOnce = fo.state; + for(auto* st : {&stepwise, &atOnce}) st->put(TYPE, "k", 30); + + for(int i = 0; i < 5; ++i) stepwise.bleach(TYPE, std::chrono::seconds(2)); + atOnce.bleach(TYPE, std::chrono::seconds(10)); + + CHECK(stepwise.ttl(TYPE, "k") == atOnce.ttl(TYPE, "k")); +} + +TEST_CASE("bleach rejects a non-positive interval instead of corrupting TTLs") +{ + Fixture f; + auto& st = f.state; + st.put(TYPE, "k", 10); + + std::error_condition ec; + const ssize_t ret = st.bleach(TYPE, std::chrono::seconds(0), &ec); + + CHECK(ret == -1); + CHECK(bool(ec)); + CHECK(st.ttl(TYPE, "k") == 10); // untouched +} + +TEST_CASE("bleach reports an unknown data type") +{ + Fixture f; + auto& st = f.state; + + std::error_condition ec; + const ssize_t ret = st.bleach("never-registered", + std::chrono::seconds(1), &ec); + + CHECK(ret == -1); + CHECK(ec == SharedStateErrors::UNKOWN_DATA_TYPE); +} + +TEST_CASE("bleaching an empty but registered type is a no-op, not an error") +{ + Fixture f; + auto& st = f.state; + st.ensureType(TYPE); + + std::error_condition ec; + const ssize_t ret = st.bleach(TYPE, std::chrono::seconds(1), &ec); + + CHECK(ret == 0); + CHECK_FALSE(bool(ec)); + CHECK(st.size(TYPE) == 0); +} diff --git a/tests/debugmesasgetest.cc b/tests/debugmesasgetest.cc deleted file mode 100644 index aee380a..0000000 --- a/tests/debugmesasgetest.cc +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Shared State - * - * Copyright (c) 2023 Javier Jorge - * Copyright (c) 2023 Instituto Nacional de Tecnología Industrial - * Copyright (C) 2023 Asociación Civil Altermundi - * - * This program is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the - * Free Software Foundation, version 3. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS - * FOR A PARTICULAR PURPOSE. - * See the GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -#include "doctest/doctest.h" - -#include - -void print_debug_message() -{ - std::string original = "mensaje a verificar"; - RS_DBG0(" cout replacement " , "variable2"); - RS_DBG0("Hello 0 ", "my debug ", original, " message " , "variable2"); - RS_DBG1("Hello 1 ", "my debug ", original , " message " , "variable2"); - RS_DBG2("Hello 2 ", "my debug ", original , " message " , "variable2"); - //since "rsdebuglevel2.h" is included the following lines wont print anything - RS_DBG3("Hello 3 ", "my debug ", original , " message " , "variable2"); - RS_DBG4("Hello 4 ", "my debug ", original , " message " , "variable2"); -} - -/* -this tests outputs this text -D 1677074011.647 void print_debug_message() Hello 0 my debug mensaje a verificar message variable2 -D 1677074011.647 void print_debug_message() Hello 1 my debug mensaje a verificar message variable2 -D 1677074011.647 void print_debug_message() Hello 2 my debug mensaje a verificar message variable2 -I 1677074011.647 void DOCTEST_ANON_FUNC_2() this is an information message, with no useful data -W 1677074011.647 void DOCTEST_ANON_FUNC_2() this is a warning, be careful! you should wear a hat -E 1677074011.647 void DOCTEST_ANON_FUNC_2() 2 + 2 = 5 in normal math this shouldn't happen -F 1677074011.647 void DOCTEST_ANON_FUNC_2() THIS IS THE END -*/ -TEST_CASE("print different level debug messages") -{ - print_debug_message(); - RS_INFO("this is an information message, with no useful data"); - RS_WARN("this is a warning, be careful! you should wear a hat"); - RS_ERR("2 + 2 = 5 in normal math this shouldn't happen"); - RS_FATAL("THIS IS THE END"); -} diff --git a/tests/mergeechotest.cc b/tests/mergeechotest.cc deleted file mode 100644 index 35c0e93..0000000 --- a/tests/mergeechotest.cc +++ /dev/null @@ -1,117 +0,0 @@ -/* - * Shared State - * - * Copyright (c) 2023 Javier Jorge - * Copyright (c) 2023 Instituto Nacional de Tecnología Industrial - * Copyright (C) 2023 Asociación Civil Altermundi - * - * This program is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the - * Free Software Foundation, version 3. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS - * FOR A PARTICULAR PURPOSE. - * See the GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -#include "doctest/doctest.h" -#include "sharedstate.hh" -#include "shared_state_error_code.hh" -#include "piped_async_command.hh" - -#include -#include - -// Tests that don't naturally fit in the headers/.cpp files directly -// can be placed in a tests/*.cpp file. Integration tests are a good example. - -TEST_CASE("returnmerge") -{ - std::string original = "mensajeaverificar"; - std::string merged = SharedState::mergestate(original); - RS_DBG0(merged , original , "--------------------------------------------------------------" ); - CHECK(original.size() == merged.size()); - CHECK(original == merged); -} - -TEST_CASE("parametrizedmerge") -{ - std::string original = "mensajeaverificar"; - std::string merged; - SharedState::mergestate(original, merged); - RS_DBG0(merged , original); - CHECK(original.size() == merged.size()); - CHECK(original == merged); -} - -void verificar(std::string original) -{ - auto merged = SharedState::mergestate(original); - CHECK(original.size() == merged.size()); - CHECK(original == merged); -} - -void verificarOptional(std::string original) -{ - auto merged = SharedState::optMergeState(original); - CHECK(original.size() == merged.value().size()); - CHECK(original == merged.value()); -} -/* -void verificarExpected(std::string original) -{ - auto merged = SharedState::expMergestate(original); - CHECK(original.size() == merged.value().size()); - CHECK(original == merged.value()); -} - -void verificarExpectedWillFail(std::string original) -{ - auto merged = SharedState::expMergestate(original, true); - CHECK_FALSE(merged); - std::error_condition(SharedState::SharedStateErrorCode::OpenPipeError); - CHECK(merged.error().message() == make_error_condition(SharedState::SharedStateErrorCode::OpenPipeError).message()); -}*/ - -// void verificarPiped(std::string original) -// { -// IOContext io_context{}; -// char socbuffer[256] = {0}; -// std::unique_ptr asyncecho = std::make_unique("cat",&io_context); -// asyncecho.get()->writepipe(original.data(),original.length()); -// asyncecho.get()->readpipe(socbuffer,256); -// std::string merged(socbuffer); -// RS_DBG0("merged , " --- " , original; -// CHECK(original.size() == merged.size()); -// CHECK(original == merged); - -// } - -TEST_CASE("Opt merge") -{ - std::string original = "mensajeaverificar"; - verificarOptional(original); -} - -TEST_CASE("Parametrized merge test") -{ - std::vector data{"", "mensajeaverificar", "asdasdaasd > saddsdfsdf", "546654654654", "546654654654546654654654546654654654546654654654"}; - - for (auto &i : data) - { - CAPTURE(i); // log the current input data - verificarOptional(i); - verificar(i); - //verificarExpected(i); - //verificarExpectedWillFail(i); - // verificarPiped(i); - } -} - -// sync vacio devuelve lo mismo que get diff --git a/tests/parsearcomandtest.cc b/tests/parsearcomandtest.cc deleted file mode 100644 index a80e3df..0000000 --- a/tests/parsearcomandtest.cc +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Shared State - * - * Copyright (c) 2023 Javier Jorge - * Copyright (c) 2023 Instituto Nacional de Tecnología Industrial - * Copyright (C) 2023 Asociación Civil Altermundi - * - * This program is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the - * Free Software Foundation, version 3. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS - * FOR A PARTICULAR PURPOSE. - * See the GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -#include "doctest/doctest.h" -#include "sharedstate.hh" -#include "shared_state_error_code.hh" - -#include - -// Tests that don't naturally fit in the headers/.cpp files directly -// can be placed in a tests/*.cpp file. Integration tests are a good example. - -TEST_CASE("return command") -{ - std::string completo = "comando\ndatos"; - std::string comando = ""; - comando = SharedState::extractCommand(completo); - CHECK(comando == "comando"); - CHECK(completo == "datos"); -} - -TEST_CASE("return command overload") -{ - std::string completo = "comando\ndatos"; - std::string comando = ""; - SharedState::extractCommand(completo,comando); - CHECK(comando == "comando"); - CHECK(completo == "datos"); -} - -TEST_CASE("return empty string") -{ - std::string completo = "comandodatos"; - std::string comando = ""; - comando = SharedState::extractCommand(completo); - CHECK(comando == ""); - CHECK(completo == "comandodatos"); -} - -TEST_CASE("return empty string") -{ - std::string completo = "comandodatos"; - std::string comando = ""; - std::error_condition ec = SharedState::extractCommand(completo,comando); - CHECK(comando == ""); - CHECK(completo == "comandodatos"); - CHECK(ec == make_error_condition(SharedState::SharedStateErrorCode::NoCommand)); -} diff --git a/tests/state_serialization_tests.cc b/tests/state_serialization_tests.cc new file mode 100644 index 0000000..788253f --- /dev/null +++ b/tests/state_serialization_tests.cc @@ -0,0 +1,245 @@ +/* + * Shared State + * + * Copyright (C) 2026 Asociación Civil Altermundi + * + * This program is free software: you can redistribute it and/or modify it under + * the terms of the GNU Affero General Public License as published by the + * Free Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. + * See the GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +/* Serialization of the types that travel on the wire. + * + * These are the shapes every peer must agree on, so they are worth + * pinning in-process: a black-box test can only observe that a sync + * produced no changes, which is the same thing it observes when a + * neighbour genuinely has nothing to say. */ + +#include "doctest/doctest.h" + +#include "sharedstate.hh" + +#include +#include +#include + +#include +#include +#include + +namespace +{ + +/// Serialize a value the way the wire path does, returning the JSON. +template RsJson toJson(T& value, const char* memberName) +{ + RsGenericSerializer::SerializeJob j(RsGenericSerializer::TO_JSON); + RsGenericSerializer::SerializeContext ctx; + RsTypeSerializer::serial_process(j, ctx, value, memberName); + return std::move(ctx.mJson); +} + +/// Deserialize, reporting whether the serializer was happy. +template bool fromJson( + RsJson& json, T& value, const char* memberName ) +{ + RsGenericSerializer::SerializeJob j(RsGenericSerializer::FROM_JSON); + RsGenericSerializer::SerializeContext ctx; + ctx.mJson.CopyFrom(json, ctx.mJson.GetAllocator()); + RsTypeSerializer::serial_process(j, ctx, value, memberName); + return ctx.mOk; +} + +SharedState::StateEntry makeEntry( + const std::string& author, int64_t ttl, const char* dataJson ) +{ + SharedState::StateEntry e; + e.mAuthor = author; + e.mTtl = std::chrono::seconds(ttl); + e.mData.Parse(dataJson); + return e; +} + +} // namespace + +TEST_CASE("StateEntry survives a serialization round trip") +{ + auto original = makeEntry("LiMe-abc123", 2401, R"({"hostname":"node-a"})"); + + RsJson json = toJson(original, "entry"); + + SharedState::StateEntry restored; + REQUIRE(fromJson(json, restored, "entry")); + + CHECK(restored.mAuthor == original.mAuthor); + CHECK(restored.mTtl.count() == original.mTtl.count()); + REQUIRE(restored.mData.IsObject()); + REQUIRE(restored.mData.HasMember("hostname")); + CHECK(std::string(restored.mData["hostname"].GetString()) == "node-a"); +} + +TEST_CASE("StateEntry keeps its member names on the wire") +{ + /* Peers identify members by name, so renaming one silently breaks + * interoperability with every deployed node. Pin the names. */ + auto entry = makeEntry("author-x", 60, R"({"k":1})"); + RsJson json = toJson(entry, "entry"); + + REQUIRE(json.HasMember("entry")); + const auto& v = json["entry"]; + CHECK(v.HasMember("mAuthor")); + CHECK(v.HasMember("mTtl")); + CHECK(v.HasMember("mData")); +} + +/// Parse literal JSON as if it had arrived from a peer. +RsJson parse(const char* json) +{ + RsJson doc; + doc.Parse(json); + REQUIRE_FALSE(doc.HasParseError()); + return doc; +} + +TEST_CASE("an entry in the currently deployed shape is accepted") +{ + /* This is the literal payload a deployed node puts on the wire: three + * members, no more. Freezing it as a fixture rather than round-tripping + * whatever this build happens to emit is the point — a round trip + * agrees with itself no matter how the format drifts. + * + * If a member is ever added to StateEntry, this case is what tells you + * whether nodes that do not emit it yet can still be understood. */ + RsJson wire = parse(R"({"entry":{ + "mAuthor":"LiMe-abc123", + "mTtl":{"xint64":2401,"xstr64":"2401"}, + "mData":{"hostname":"node-a"}}})"); + + SharedState::StateEntry entry; + REQUIRE(fromJson(wire, entry, "entry")); + + CHECK(entry.mAuthor == "LiMe-abc123"); + CHECK(entry.mTtl.count() == 2401); + REQUIRE(entry.mData.HasMember("hostname")); + CHECK(std::string(entry.mData["hostname"].GetString()) == "node-a"); +} + +TEST_CASE("a slice in the currently deployed shape is accepted whole") +{ + RsJson wire = parse(R"({"stateSlice":[ + {"key":"key-a","value":{"mAuthor":"node-a", + "mTtl":{"xint64":100,"xstr64":"100"},"mData":{"v":1}}}, + {"key":"key-b","value":{"mAuthor":"node-b", + "mTtl":{"xint64":200,"xstr64":"200"},"mData":{"v":2}}}]})"); + + std::map slice; + REQUIRE(fromJson(wire, slice, "stateSlice")); + + REQUIRE(slice.size() == 2); + CHECK(slice["key-a"].mAuthor == "node-a"); + CHECK(slice["key-b"].mTtl.count() == 200); +} + +TEST_CASE("deserialization stops at the first unreadable entry") +{ + /* Where the damage lands when an entry cannot be read: parsing stops + * there. Entries earlier in the map survive and everything after is + * lost. The serialization context records the failure, but + * NetworkMessage::toStateSlice() returns void and discards ctx.mOk, + * and neither of its call sites checks it — so a node merges the + * surviving prefix without ever learning that the rest was dropped. + * + * The practical consequence is easy to underestimate. If a member is + * added and required on read, a slice from a peer that does not emit + * it yet has every entry in an incompatible legacy shape, so the very + * first one fails: that peer's neighbour learns nothing at all from + * it — not a degraded view, an empty one. Keys are ordered here to + * make the boundary visible; std::map iterates in key order. */ + RsJson wire = parse(R"({"stateSlice":[ + {"key":"a-before","value":{"mAuthor":"node-a", + "mTtl":{"xint64":100,"xstr64":"100"},"mData":{"v":1}}}, + {"key":"b-unreadable","value":{"mAuthor":"node-b", + "mData":{"v":2}}}, + {"key":"c-after","value":{"mAuthor":"node-c", + "mTtl":{"xint64":300,"xstr64":"300"},"mData":{"v":3}}}]})"); + + std::map slice; + const bool ok = fromJson(wire, slice, "stateSlice"); + + CHECK_FALSE(ok); // recorded, but discarded + // by toStateSlice() in production + CHECK(slice.count("a-before") == 1); // parsed before the failure + CHECK(slice.count("b-unreadable") == 0); + CHECK(slice.count("c-after") == 0); // never reached +} + +TEST_CASE("StateEntry is copy-constructible but not copy-assignable") +{ + /* Pinning a sharp edge rather than a behaviour: `mData` is a + * rapidjson Document whose copy assignment is private, so + * `map[key] = entry` does not compile while `map.emplace(key, entry)` + * does. Worth stating explicitly — the error it produces points deep + * into and reads as though the container is at fault. */ + static_assert(std::is_copy_constructible_v); + static_assert(!std::is_copy_assignable_v); + CHECK(true); +} + +TEST_CASE("DataTypeConf survives a serialization round trip") +{ + /* The on-disk configuration format: a node that cannot read it back + * loses every registered data type. */ + SharedState::DataTypeConf conf; + conf.mName = "wifi_links_info"; + conf.mScope = "community"; + conf.mUpdateInterval = std::chrono::seconds(30); + conf.mBleachTTL = std::chrono::seconds(2400); + + RsJson json = toJson(conf, "conf"); + + /* Freeze the member names: a rename would keep a round trip green + * while making every existing configuration file unreadable. */ + REQUIRE(json.HasMember("conf")); + const auto& c = json["conf"]; + CHECK(c.HasMember("mName")); + CHECK(c.HasMember("mScope")); + CHECK(c.HasMember("mUpdateInterval")); + CHECK(c.HasMember("mBleachTTL")); + + SharedState::DataTypeConf restored; + REQUIRE(fromJson(json, restored, "conf")); + + CHECK(restored.mName == conf.mName); + CHECK(restored.mScope == conf.mScope); + CHECK(restored.mUpdateInterval.count() == conf.mUpdateInterval.count()); + CHECK(restored.mBleachTTL.count() == conf.mBleachTTL.count()); +} + +TEST_CASE("A map of entries round trips as a whole slice") +{ + /* What a sync actually carries. */ + std::map slice; + /* emplace, not operator[]= : StateEntry is copy-constructible but + * not copy-assignable (see the test below) */ + slice.emplace("key-a", makeEntry("node-a", 100, R"({"v":1})")); + slice.emplace("key-b", makeEntry("node-b", 200, R"({"v":2})")); + + RsJson json = toJson(slice, "stateSlice"); + + std::map restored; + REQUIRE(fromJson(json, restored, "stateSlice")); + + REQUIRE(restored.size() == 2); + CHECK(restored["key-a"].mAuthor == "node-a"); + CHECK(restored["key-b"].mTtl.count() == 200); +} From b5e9876e4a7c5bf09135a8628890596a6246e5e4 Mon Sep 17 00:00:00 2001 From: Fede654 Date: Thu, 13 Aug 2026 18:04:15 +0000 Subject: [PATCH 3/3] ci: build with the runner's compiler and actually run the tests Two reasons CI does not currently exercise the test suite: - The "switch to gcc-10" step fails outright on today's ubuntu-latest images, which no longer ship gcc-10: update-alternatives reports "alternative path /usr/bin/gcc-10 doesn't exist" and the job stops before configuring. The project requires C++20 coroutines, which the runner's default GCC has supported for several releases, so the pin can simply go. - Configure does not pass -DSS_TESTS=ON, and that option defaults to OFF, so the test target is never added and the `ctest` step reports "No tests were found" while the job goes green. With both addressed the existing Test step runs the suite for real. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c62aace..520535d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,13 +27,10 @@ jobs: - name: Checkout Code uses: actions/checkout@v2 - - name: switch to gcc-10 on linux - run: | - sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-10 100 --slave /usr/bin/g++ g++ /usr/bin/g++-10 --slave /usr/bin/gcov gcov /usr/bin/gcov-10 - sudo update-alternatives --set gcc /usr/bin/gcc-10 - - name: Configure (${{ matrix.configuration }}) - run: cmake -S . -Bbuild -DCMAKE_BUILD_TYPE=${{ matrix.configuration }} + run: > + cmake -S . -Bbuild -DCMAKE_BUILD_TYPE=${{ matrix.configuration }} + -DSS_TESTS=ON - name: Build with ${{ matrix.compiler }} run: cmake --build build --config ${{ matrix.configuration }}