build: add unit-test harness (Catch2) and coverage tooling#10
Merged
Conversation
- New CMake options MADS_BUILD_TESTS and MADS_COVERAGE (both OFF by default) - tests/: Catch2 v3.7.1 via FetchContent, one executable per test_*.cpp, shared helpers (loopback endpoints, RunningGuard, wait_for) and README - Coverage: per-target instrumentation of MadsCore, gcovr.cfg with curated denominator, coverage CMake target, codecov.yml (informational for now) - CI: .github/workflows/coverage.yml builds with clang, runs ctest and uploads to codecov.io (tolerates missing CODECOV_TOKEN) - Fix: exclude mongo_fetch.cpp from the build when MADS_ENABLE_MONGOCXX=OFF - Remove orphaned legacy test/ directory (referenced dead targets)
…c_path, watcher Coverage (line %): mads.hpp 100% (46/46), execution_time_window_stats.hpp 100% (64/64), goback.hpp 100% (22/22), exec_path.hpp 84.6% (11/13), watcher.hpp 91.3% (21/23).
…ol, and LazyPayload Covers src/agent.hpp's LazyPayload text/doc lazy caching (test_lazy_payload.cpp); Agent pub/sub messaging, blob publishing, topic filtering, accessors, error paths, disconnect/shutdown safety, and Delivery::LastKnownValue over an in-process ZeroMQ loopback (test_agent_pubsub.cpp); the WireFormat x Compression codec matrix, the Compression::Auto threshold, a large Compression::Snappy round trip, and dropped_messages() on malformed frames injected via a raw zmqpp socket (test_agent_wire.cpp); and Agent::loop() pacing/exception handling plus remote_control()'s restart/shutdown/info/no-op commands (test_agent_loop.cpp). All ports are in the assigned 42100-42199 range. Coverage (ctest --test-dir build-cov, full 82-test suite, gcovr): src/agent.cpp 58.5% lines (531/908) src/agent.hpp 96.4% lines (27/28)
With the default infinite linger, a settings request queued toward an unreachable broker keeps the ZMQ context alive and Agent::shutdown() blocks forever in zmq_ctx_term. Found by the new broker-timeout unit test.
Local TOML settings loading (fixtures for valid/malformed/missing cases, sub_topic string vs array, save_settings guard) and an in-process fake broker covering the REQ/REP settings protocol: happy path, attachments, timecode offset, version mismatch, refusals, timeouts, start_agent. Coverage: src/agent.cpp 71% (from 58.5%).
… helper tests/test_agent_c.cpp (23 cases, 137 assertions): free functions (mads_version/mads_default_settings_uri/mads_free), full create/init/destroy lifecycle and accessor sweep (id, pub/sub topics incl. malloc-on-demand convention, receive timeout, high watermark, wire format, compression, settings getters against tests/fixtures/settings/valid.toml), agent_init error paths (missing file, malformed TOML, missing section, double-init), agent_connect/agent_disconnect guarded states, agent_register_event before/after connect, discover_broker_settings parameter guards (no network touched), CURVE setup via the C ABI (string-based and file-based key paths), a full C-ABI pub/sub round trip over tcp://127.0.0.1:42303 (endpoint/cross wiring done via the underlying Mads::Agent*, since agent_c.h has no equivalent), a mixed C++-publisher/C-API-subscriber blob-receive check, and message_type_t/event_type_t <-> Mads::message_type/event_type enum parity. tests/test_curve.cpp (13 cases, 35 assertions): CurveAuth::fetch_public_keys, set_key_dir, key getters/setters, setup_curve_server and both setup_curve_client overloads (missing-file and happy paths, all keys generated at runtime via zmqpp::curve::generate_keypair() into a temp dir), setup_auth, and a full CURVE-encrypted REQ/REP round trip with ZAP authentication over tcp://127.0.0.1:42350. Full suite: 138/138 ctest passing (102 pre-existing + 36 new). Coverage: src/agent_c.cpp 78% lines (0% before), src/curve.hpp 96% lines. Source bug found (not fixed, per work-package scope): in src/agent_c.cpp, agent_set_client_public_key/secret_key/server_public_key guard against "CurveAuth not initialized" with `if (!ag->curve_auth())`, but Agent::curve_auth() returns `&_curve_auth` (address of a member), which is never null. Calling any of these three setters before agent_setup_crypto() dereferences a null CurveAuth* and segfaults instead of returning -1; the "before setup" precondition is therefore untestable without triggering a crash and is not exercised. Remaining uncovered lines in agent_c.cpp are: discover_broker_settings's real-discovery body (network access via UDP broadcast, out of scope per the no-network-in-tests rule), the six lines of that dead branch above, several catch(...) "Unexpected" fallbacks that require throwing a non-std::exception type, and a malloc-failure branch -- all judged not worth a hostile setup per the work-package guidance.
agent_set_client_public_key/secret_key/server_public_key guarded with !ag->curve_auth(), but curve_auth() returns the address of the member unique_ptr, which is never null. Calling any of these setters before agent_setup_crypto() dereferenced a null CurveAuth and crashed instead of returning -1 as documented. Check the pointed-to unique_ptr instead; re-enable the unit-test assertions for the precondition.
The repo-wide *.toml gitignore rule silently kept tests/fixtures out of the WP-C commit (tests passed locally against untracked files, failed on CI with 'File could not be opened for reading'); negate the rule for tests/fixtures/**. Also derive the same-minor version string in the check_version test from Mads::version() instead of hardcoding v0.0.999, which only matched on tag-less clones.
47 Catch2 test cases / 189 assertions across helpers (line_of_offset, is_ident_char, find_matching_paren, read_file/write_file, run_command), detect_protocol/bump_git_tag, apply_literal/apply_regex, load_migrations against the real share/plugin_migrations chain, SpanRewriter/MethodTransform on tricky snippets (multi-line signatures, parens in comments/strings, call vs. declaration disambiguation), and run() end to end against copies of a new fixture plugin project (tests/fixtures/plugin_p6/): dry-run, full P6->P8 migration with backups, already-at-target, missing CMakeLists.txt, from/to overrides, missing chain step, undeterminable target, no-op run, and undetectable protocol. Full suite: 185/185 ctest passed (138 pre-existing + 47 new). src/main/plugin_migrate.hpp coverage: 87% lines (445/507), 88.8% functions. Uncovered: the Tier B compile-check block (invokes cmake/network, out of scope for a no-network unit suite), the Windows _popen branch and a popen() failure branch, and a few lexer edge branches (escaped chars, char literals) already exercised indirectly via find_matching_paren's own test cases.
…ning flag Agent run state is now owned by a Mads::Runtime object. Each agent gets its own Runtime by default, so shutting down or destroying one agent no longer stops the loops, LKV drain threads, or watchdogs of other agents in the same process, and a disconnected agent can reconnect and loop again. Agents that should stop together can share a Runtime via Agent::set_runtime(); signal handlers and the remote shutdown/restart commands request a process-wide stop (Runtime::stop_process()) that overrides every Runtime. Mads::running remains as a [[deprecated]] alias of the process-wide flag, so existing code keeps compiling and behaving as before, with a compile-time warning pointing at the replacement API. New tests: Runtime semantics, agent/runtime wiring, regression tests for the historical cross-agent stop and reconnect hazards, and the deprecated shim. Full suite: 193/193; coverage 82.4% lines.
New RUNTIME.md topical page (wired into doc/Doxyfile INPUT, alongside CRYPTO.md) documenting the Mads::Runtime model: the three stop levels (agent, Runtime group, process), usage examples, and a migration table from the deprecated Mads::running flag. The Runtime class doc and the deprecated flag doc now link to it via \ref.
v2.4.0 hasn't shipped yet, so the test suite/coverage infrastructure and the Mads::Runtime refactor belong in its CHANGES.md entry rather than a hypothetical v2.5. Also fixes a stray 'v2.5' reference in RUNTIME.md.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
shared helpers (loopback endpoints, RunningGuard, wait_for) and README
denominator, coverage CMake target, codecov.yml (informational for now)
uploads to codecov.io (tolerates missing CODECOV_TOKEN)