Skip to content

test(operator_commands): audit + replace low-value smoke tests with output assertions - #4145

Merged
rysweet merged 1 commit into
mainfrom
engineer/audit-simard-s-test-coverage-and-raise-it-to-70-4d27c91a-1784169728-800f95
Jul 22, 2026
Merged

test(operator_commands): audit + replace low-value smoke tests with output assertions#4145
rysweet merged 1 commit into
mainfrom
engineer/audit-simard-s-test-coverage-and-raise-it-to-70-4d27c91a-1784169728-800f95

Conversation

@rysweet

@rysweet rysweet commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Summary

Test-quality audit of the Simard suite. The CI coverage metric
(cargo llvm-cov --workspace --lib --bins --ignore-filename-regex 'tests?/')
already reports 82.84% line coverage — well above the 70% target — so the
useful work here is the quality half of the mandate: finding tests that
execute code without verifying behavior and turning them into real
behavior assertions, rather than padding the metric.

The audit surfaced a coherent cluster of low-value tests in the
operator-command output layer: the renderers print directly via println!, so
their tests could only assert "does not panic" while discarding the actual
rendered text. That left the real behavior unverified — status filtering,
priority/title/slug sorting, count lines, <none> placeholders,
terminal-control sanitization, and concise-label formatting.

Changes

Behavior-preserving refactor: extract pure helpers that return the rendered
lines, and have the existing print_* wrappers forward them to stdout
(rendered output is byte-identical). Then rewrite the smoke tests to assert
exact output.

  • operator_commands::format — added format_labeled + *_section_lines
    helpers; converted 9 *_does_not_panic tests into output assertions
    (sanitization strips ANSI escapes, ItemsItem singularization, goal
    filtering + priority sort, empty <none> placeholders, terminal-handoff
    Some/None branches incl. the missing-last_output_line fallback).
  • operator_commands::goals — added render_lines on the goal register
    view/section; converted 2 print_does_not_panic tests into full-output
    assertions (per-section counts, <none>, concise-label rendering).
  • operator_commands_terminal::commandsterminal_recipe_list_does_not_panic
    discarded its Result (a probe can return Err without panicking); now
    asserts the probe succeeds and the descriptor list is non-empty and every
    descriptor carries a name.

Net: 12 assertion-free / discarded-result smoke tests replaced with
behavior-verifying assertions.
The thin print_* wrappers are still
exercised once each so the println! paths stay covered.

Audit methodology

Scanned all ~9,600 #[test]/#[tokio::test] functions for anti-patterns
(assert!(true), assert_eq!(x, x), assertion-free bodies, discarded
Results). Most heuristic hits were false positives (helper-based assertions,
e.g. overseer::signal's has() helper). The genuine low-value cluster was
the does_not_panic stdout smoke tests fixed here. No tautological
(assert!(true) / assert_eq!(x, x)) tests exist in the tree.

Evidence

  • Coverage (criterion: >70%)cargo +nightly-2026-07-01 llvm-cov --workspace --lib --bins --ignore-filename-regex 'tests?/' --json --summary-only: overall 82.84% lines (143676/173432); functions 83.43%,
    regions 83.90%. operator_commands module 80.9% before this change.
  • Tests — all 32 tests in the three touched modules pass; the strengthened
    assertions verify exact rendered output (see files).
  • fmtcargo fmt --all -- --check clean.
  • clippycargo clippy --lib --tests --all-features --locked -- -D warnings
    clean; pre-commit cargo clippy --release --no-deps -- -D warnings and
    pre-push cargo clippy --all-targets --all-features --locked -- -D warnings
    both passed.
  • Diff focused — 3 files, all in the operator-command rendering layer; no
    unrelated edits.

Surfaces changed

Internal only. The print_* functions render byte-identical stdout (the
*_lines helpers are the single source of truth the wrappers now use), so
there is no user-facing behavior change and no docs update is required.

…sertions

Test-quality audit follow-up. The operator-command output renderers printed
directly via `println!`, so their tests could only assert "does not panic"
while discarding the actual rendered text. That left the real behavior —
status filtering, priority/title/slug sorting, count lines, `<none>`
placeholders, terminal-control sanitization, and concise-label formatting —
unverified.

Extract behavior-preserving pure helpers that return the rendered lines and
have the `print_*` wrappers forward them to stdout (output is byte-identical),
then rewrite the smoke tests to assert exact output:

- operator_commands::format: add `format_labeled`, `*_section_lines`
  helpers; convert 9 `*_does_not_panic` tests into output assertions
  (sanitization, singularization, sorting/filtering, empty placeholders,
  terminal-handoff Some/None branches).
- operator_commands::goals: add `render_lines` on the goal register view /
  section; convert 2 `print_does_not_panic` tests into full-output assertions.
- operator_commands_terminal::commands: `terminal_recipe_list_does_not_panic`
  discarded its `Result`; assert the probe succeeds and descriptors are
  well-formed.

No user-facing surface change: rendered stdout is identical; the print
wrappers are still exercised for coverage. fmt clean; clippy
(`--lib --tests --all-features --locked -D warnings`) clean; all 32 tests in
the touched modules pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

📊 Coverage Summary

Generated by cargo llvm-cov --workspace --summary-only (nightly, excluding test files)

Module Lines Covered Coverage
Total 175449 145072 82.7%

Coverage data from CI run. Test files matching tests?/ are excluded from line counts.

@rysweet

rysweet commented Jul 16, 2026

Copy link
Copy Markdown
Owner Author

CI update: all 16 checks green (pre-commit/verify, coverage, install-real, e2e-dashboard, cargo-deny/audit/vet, npm-audit, GitGuardian). The CI coverage job independently confirms the metric: Total 145072/175449 lines = 82.7%, verifiably above the 70% target. No coverage regression from the audit — the 12 reworked tests assert byte-identical rendered output, and the thin print_* wrappers stay exercised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant