Skip to content

The harness compares empty measurements: check "" "" passes, and four suites can produce one #418

Description

@ChronicallyJD

A suite that produces an empty measurement and then fails a comparison is one step from
one that produces an empty measurement and passes. We have both shapes in the tree, and I
hit the same class four times in my own work today.

The instance that surfaced it

test/column_projection.sh pipes its buffer count through bc:

bufs() {
	... | grep -oE '(hit|read)=[0-9]+' | cut -d= -f2 | paste -sd+ | bc
}

bc is not installed on the bench. The function returned empty, and the checks read:

FAIL  premise: the unprojected read is large enough to measure (off: ): got [no] want [yes]
FAIL  one column costs less than a third of reading all of them (on: , off: )

It failed, which is the lucky direction. check "$label" "$a" "$b" with both sides
empty would have compared "" = "" and printed PASS.

That is not hypothetical. It happened to me twice today:

  • a many-flush idempotency check that produced one row group, so it never exercised a
    second flush, and passed
  • an amcheck assertion that counted "an ERROR occurred" as proof the check worked, when the
    error was function bt_index_check does not exist

Both were green and both measured nothing.

The general shape

Every one of these is checking a proxy instead of the thing:

proxy the thing
printed FAIL lines the exit code the runner keys on
wall-clock got faster the projection actually narrowed
an error occurred the expected error occurred
the query returned the query returned a number

The last one is the cheapest to close and it covers the most cases.

Suggested: make the shared harness refuse an empty measurement

In test/lib.sh, beside check:

# Compare two measurements, refusing to compare nothing with nothing. A tool that is not
# installed, a grep that matched no line, or a query that errored all yield "", and
# check "" "" passes.
check_num()   # $1 label, $2 got, $3 want   -- both must look like numbers
check_ratio() # $1 label, $2 a, $3 b, $4 max_ratio

Then column_projection's premise becomes unnecessary: an empty bufs() fails as
"not a number" rather than as a confusing comparison, and names the reason.

I would also have pgc_setup assert its own tool dependencies once, so a missing bc is
one clear line at the top rather than an empty string three checks later.

Scope

I am not proposing a sweep of every suite. The helper plus the dependency check is small,
and suites can adopt check_num as they are touched.

Happy to take this. It came out of my own mistakes more than anyone else's, and #416 has
the audit trail if you want the detail.

Also worth fixing separately

test/audit.sh ends with exit $fail rather than pgc_summary, so it exits non-zero
while printing no FAIL line. That is what made my first control report "0 failures" for a
suite the matrix was failing. Whatever it does, it should say.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions