Menu

#229 cli: pin scan --json over --quiet with test and docs

closed
nobody
None
2026-08-03
2026-08-02
Anonymous
No

Originally created by: AshSgDe29071999

Summary

doberman scan --json --quiet already prints JSON because the JSON branch returns before the quiet check. That precedence is sensible but was untested and undocumented.

  • Add a regression test that --json --quiet still emits valid JSON with the same exit code as --json alone
  • Document in docs/CLI.md that --json wins over --quiet

Fixes [#225]

Related

Tickets: #225
Tickets: #230
Tickets: #297

Discussion

  • Anonymous

    Anonymous - 2026-08-03

    Originally posted by: fu351

    Reviewed. I pushed one commit to your branch (d63cf98) instead of sending you a round trip for it. Two fixes, and the second is the one worth knowing about.

    The ruff format failure was a missing blank line before the new test function. Mechanical.

    The docs change had a real rendering bug, and the root cause is a GFM rule rather than anything about your sentence: a blank line terminates a Markdown table. Your note went between two rows, so the table ended after the --quiet row, and the --path row below it stopped being a row at all — it would have rendered as stray pipe-text underneath. The fix is placement only: --path moved back into the table, your sentence moved below it, wording untouched. Worth filing away, because it bites anywhere an explanatory line feels natural mid-table.

    On the substance, the test is why this PR is worth having. I didn't take the description's word for it — I reverted the precedence in a scratch worktree two different ways and reran the suite. Against the realistic bug (quiet winning, returning ahead of the JSON branch), your test is the only one in the suite that goes red. All three pre-existing scan tests sail straight through it.

    That comes down to a specific choice you made: asserting json.loads(both.stdout) == json.loads(json_only.stdout) rather than the easier "stdout is non-empty". The weaker assertion would have passed against the bug. Pinning behaviour that currently holds only by accident of statement order is precisely what a regression test is for, and the assertion has to be strong enough to notice when the accident changes.

    CI should go green on the new commit.

     
  • Anonymous

    Anonymous - 2026-08-03

    Ticket changed by: fu351

    • status: open --> closed
     

Log in to post a comment.