[codex] raise ctl core mesh responder coverage#362
Conversation
There was a problem hiding this comment.
Pull request overview
Raises innerwarden-ctl command-module coverage by adding focused tests and extracting a small piece of interactive responder logic into a testable helper.
Changes:
- Add coverage for
commands/core.rsdaily dispatch paths and capability listing. - Add coverage for
commands/mesh.rsenable/disable/add-peer/status flows using temporary config/state files. - Refactor
commands/responder.rsinteractive selection into aResponderModehelper and add tests for mode application and flag-driven configuration.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| crates/ctl/src/commands/core.rs | Adds tests covering daily command dispatch and list output wiring. |
| crates/ctl/src/commands/mesh.rs | Adds tests covering config edits and status rendering with temp files. |
| crates/ctl/src/commands/responder.rs | Introduces ResponderMode, refactors interactive flow, and adds responder config/mode tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let dir = tempfile::tempdir().unwrap(); | ||
| let cli = test_cli(&dir, false); | ||
|
|
||
| apply_responder_mode(&cli, ResponderMode::DryRun, false).unwrap(); | ||
|
|
There was a problem hiding this comment.
This test calls apply_responder_mode() with cli.dry_run = false, which triggers a systemctl restart innerwarden-agent via restart_agent(). Please gate or decouple restarts so unit tests don't invoke external service managers.
| let dir = tempfile::tempdir().unwrap(); | ||
| let cli = test_cli(&dir, false); | ||
|
|
||
| apply_responder_mode(&cli, ResponderMode::Live, true).unwrap(); | ||
|
|
There was a problem hiding this comment.
With cli.dry_run = false, apply_responder_mode(... Live, true) will call restart_agent(cli) and run systemctl restart innerwarden-agent. Unit tests should not perform real restarts; refactor to separate config mutation from restart side effects or add a mechanism to skip restarts in tests.
| let dir = tempfile::tempdir().unwrap(); | ||
| let cli = test_cli(&dir, false); | ||
|
|
||
| cmd_configure_responder(&cli, true, false, Some(true)).unwrap(); | ||
|
|
There was a problem hiding this comment.
These tests run with cli.dry_run = false, so cmd_configure_responder() will call restart_agent(cli), which spawns systemctl restart innerwarden-agent via helpers::restart_agent(). Unit tests should avoid invoking external service managers; consider refactoring responder configuration to separate “write config” from “restart agent”, or add a test-only/flagged way to skip restarts so the tests remain hermetic.
| let dir = tempfile::tempdir().unwrap(); | ||
| let cli = test_cli(&dir, false); | ||
|
|
||
| cmd_configure_responder(&cli, false, true, None).unwrap(); | ||
|
|
There was a problem hiding this comment.
This test invokes cmd_configure_responder() with cli.dry_run = false, which triggers restart_agent(cli) and therefore executes systemctl restart innerwarden-agent (errors ignored). That external side effect makes the test suite environment-dependent; please refactor or gate restarts so tests don't call systemctl.
| let dir = tempfile::tempdir().unwrap(); | ||
| let cli = test_cli(&dir, false); | ||
|
|
||
| cmd_configure_responder(&cli, false, false, Some(false)).unwrap(); | ||
|
|
There was a problem hiding this comment.
Calling cmd_configure_responder() here with cli.dry_run = false will run restart_agent(cli) and attempt systemctl restart innerwarden-agent. Please avoid system-level restarts in unit tests (e.g., split config writes into a pure helper and test that, or add a skip-restart flag used by tests).
| #[test] | ||
| fn apply_responder_mode_observe_only_writes_disabled_state() { | ||
| let dir = tempfile::tempdir().unwrap(); | ||
| let cli = test_cli(&dir, false); |
There was a problem hiding this comment.
apply_responder_mode() always calls restart_agent(cli) for ObserveOnly when cli.dry_run is false, which runs systemctl restart innerwarden-agent. This makes the unit test non-hermetic; consider injecting the restart behavior (or returning an action to restart) so tests can verify config writes without shelling out.
| let cli = test_cli(&dir, false); | |
| let cli = test_cli(&dir, true); |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
commands/core.rsdaily dispatch and capability listing paths.Coverage
cargo llvm-cov -p innerwarden-ctl --json --summary-only --output-path target/llvm-cov/ctl-lote1-summary.jsoncommands/core.rs: 91.53% line coveragecommands/mesh.rs: 98.31% line coveragecommands/responder.rs: 81.59% line coverageValidation
cargo test -p innerwarden-ctl corecargo test -p innerwarden-ctl meshcargo test -p innerwarden-ctl respondercargo test -p innerwarden-ctlCloses #236
Closes #244
Closes #245