mlevkov commented on PR #3798:
URL: https://github.com/apache/iggy/pull/3798#issuecomment-5160404774
Pushed a follow-up commit closing the coverage gaps worth closing. Kept it
as a
separate commit rather than amending, so the delta is reviewable on its own.
Patch coverage was 94.52% (2847 hits / 131 misses / 34 partials). Reading the
per-line report rather than the percentage turned up one gap that mattered:
**Both existing management auth tests reached only `GET /admin/endpoints`.**
Coverage regions are per call site, so `denied()` looked fully covered while
the
guard inside `register_endpoint`, `rotate_secret`, `revoke_endpoint`, and
`get_endpoint` had never taken its rejection branch. Deleting the check from
`revoke_endpoint` left the suite green — it now fails with 204 instead of
401.
Also newly pinned: `republish_or_close` answering 500 rather than reporting a
revoke that never reached the route table; the endpoint-id route conflict
keeping
the id to an 8-char prefix in its message; the `hmac_algorithm()` mapping; a
revoked *dynamic* endpoint's tombstone surviving restore with no static
counterpart; and the two `message_headers` drop branches.
Every new test was mutation-checked — each was confirmed to fail against a
deliberate break of the behaviour it claims to pin, rather than assumed to
work
because it passed.
One production line changed: `ServerState::new` is now `pub(crate)` so the
management tests can build a listener-less state and provoke the republish
failure. The other 328 added lines are tests.
Deliberately left uncovered, and why:
- `SharedServer::shutdown`'s abort branch — needs a connection wedged past
the
5s timeout, so pinning it costs a 5s+ unit test.
- The bind-failure and draining-listener-join races.
- `Metrics::encode`'s encoder-failure branch.
- ~30 of the remaining missed lines are **tracing-macro arguments**.
`tracing`
skips formatting when no subscriber enables the level, so those regions
never
execute under `cargo test` even where the surrounding branch is fully
exercised. Installing a subscriber would raise the number without testing
anything, so I left it alone — worth knowing when reading the residual
figure.
`Metrics::default` and `EndpointRegistry::is_empty` were uncovered and have
no
callers, but they can't be deleted: `clippy::new_without_default` and
`clippy::len_without_is_empty` require them. Covered with one assertion each.
Full local gate green (fmt, sort, workspace clippy `-D warnings`, build, 124
unit
tests, 5 http_source integration tests, taplo, hawkeye, typos, trailing
whitespace/newline).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
