Repository navigation
Fix benchmark methodology and enforce 95% production coverage (v0.5.2) - #7
Merged
Merged
Conversation
…nct sequence Receive benches refill outside the timer and check every message; the view bench gets its own consumer; recv_batch(32) counts 32 messages. Benches use unique self-removing ring paths and validate replies. Replication examples end on the last sequence or an idle timeout instead of hanging, report loss, duplicates and reordering, pace without drift, propagate mirror errors, and replication_stages stamps pushes lock-free and checks clock drift. Adds docs/benchmarking.md, scripts/bench-smoke.sh and harness tests, and withdraws the README try_recv/recv_batch figures until re-measured.
- pingpong: split lost/late/unexpected counters into measured vs warm-up, and check the deadline every wait-loop iteration so a continuous stream of late/unexpected replies cannot evade it. - replication_stages: bound clock_offset's connect/read/write with a connect timeout and socket read/write timeouts, breaking the probe loop instead of hanging or panicking on a stalled peer. - examples/support::receive_until: only a new highest sequence resets the idle timer, and the idle check runs every 256 polls regardless of Some/None, so a flood of duplicates or a poll that never returns None cannot stall it forever. - benches/support: add a Watchdog (heartbeat-based, no per-iteration clock read) so blocking waits with no built-in timeout (recv_blocking, blocking socket/pipe I/O) abort the process instead of hanging when a peer stays alive but stops progressing; wire it into ipc_compare and add full 64-byte payload validation there (previously only the sequence was checked). - timed_chunks: add timed_chunks_with_clock so its setup-exclusion property can be tested with an injectable fake clock instead of a wall-clock sleep raced against an upper bound.
…ringfire CLI sig_handler called libc::exit() from a signal handler (async-signal-unsafe) purely to fake a successful exit on SIGINT/SIGTERM so coverage counters would flush; this made every "signal termination" test assert a fabricated success status instead of real Unix signal semantics. Removed it along with RINGFIRE_TOP_ITERATIONS, a hidden env var that bypassed the documented --iterations flag. top --iterations now rejects zero/invalid values instead of silently ignoring them, and both flags are documented in --help. Rewrote cli_coverage_tests.rs to match: subprocess helpers (ChildGuard, LineWatcher) give RAII kill/reap on panic and bounded waits instead of sleep-then-kill; serve's bind address is read from its own stdout/stderr announcement rather than a pre-allocated port; signal-terminated processes are asserted via ExitStatusExt instead of a fabricated success; and the replication and network-flag tests do a real mirror handshake with exact 1..=50 / 1..=10 content checks rather than launch/sleep/kill. All paths/ports are now unique per test run. Honest coverage for src/bin/ringfire.rs alone (cargo llvm-cov, this test scope): 87.87% lines (was a fabricated 97%). The remaining gap is structural: any serve/ mirror/top invocation that legitimately runs until an external signal has its LLVM coverage counters lost, because they flush only on normal process exit and a raw SIGTERM/SIGINT kill never reaches one. Recovering that number would require either the removed async-signal-unsafe hack or adding real graceful-shutdown signal handling, which is a production behavior change outside this bounded fix.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
Receive benchmarks previously timed empty polls/refills, some network examples could hang or hide losses, and the multiprocess stress children did not actually execute their test. This release corrects the harnesses and adds behavioral tests with a 95% production Rust line-coverage gate.
Validation: Linux AArch64 production coverage 4,961/5,132 lines (96.67%); all-features, release and no-default-features tests; Clippy in both feature modes; full benchmark smoke; verified crate packaging. CI independently runs Linux x86-64/AArch64 tests, coverage and smoke. GitHub x86-64 coverage on the final PR revision is 4,951/5,132 lines (96.47%). Coverage is line coverage, not exhaustive branch/concurrency coverage. Shared-host measurements include all repeat ranges; no new cross-host LAN/WAN claims.
Prepare v0.5.2. Library wire formats remain compatible with v0.5.x.
Closes #6.