Skip to content

The assertion checker prints all 638 offenders and marks none of them new, so a +1 is unreadable #892

Description

@fujibee

Four people independently mis-read this checker's output tonight, on the same red. That is
not four careless readings; it is what the output invites.

What it prints

When the tree is over baseline, it reports the count and then lists every offender:

check-enforced-assertions: 639 unenforceable assertions, baseline is 638.

  test_actas_integration.bats:52: [[[ ]]] [[ "$output" =~ "status=ok" ]]
  ... 638 more lines ...

Measured with the baseline forced low: 638 offenders reported, 638 lines printed. The list
is complete and it is not truncated.

The signal is +1. The output is 639 lines, and nothing marks which one is new.

What that produced

  • one reader took the first file in the list as the cause;
  • another compared the lists between two heads and saw a hundred-line difference — line
    numbers shift for every line below an insertion, so the diff is mostly noise;
  • a third concluded the listing must be truncated, because a hundred-line difference for a
    one-line change cannot be right;
  • a fourth had the answer in a file list they had produced for another purpose and did not
    recognise it.

Everyone eventually got there by the same route: run the checker at each commit and watch
where the count changes. That is the method that works, and the tool does not suggest it.

Directions

  • Print what changed, not what exists. The baseline is a number, so the tool cannot know
    which entries are new — but it can be made to: store the offender list alongside the count
    and diff against it. Then a red says "these two are new" and the 638 that were already
    tolerated stay quiet.
  • Failing that, say what the list is. One line above it — "the list below is every
    offender in the tree, not the ones that pushed it over the baseline; to find those,
    re-run at the previous commit and compare counts" — costs nothing and would have saved
    four people an hour each.
  • The count-per-commit walk is the reliable method today. If it stays the method, it belongs
    in the tool's own output rather than in four people's heads.

Not a complaint about the ratchet

It caught a real defect: a test premise that could not fail (#887). The check is earning its
place. This is about the part after the count.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions