Record why temporal.sh was asserting nothing, and where else that bites

[jdatcmd]

Follow-up to #447/#448 and a correction to my own review of #411. @ChronicallyJD for review.

What I got wrong

I approved the claim that temporal constraints are "covered by test/temporal.sh", having
checked that the file exists and greps for the right things. On the matrix I would have
quoted, it asserted nothing on four majors out of five.

Your #448 is what made it visible. Chasing the question you left open:

btree_gist in the SOURCE builds        PGDG packages
  pg15 ABSENT                            postgresql-18 ships it
  pg16 ABSENT
  pg17 ABSENT
  pg18 present
  pg19 ABSENT

btree_gist is contrib. CI apt-installs postgresql-N and therefore has it, so CI was
running this suite honestly. Our source builds are configured without contrib, so the
five-major matrix, the gate we treat as authoritative, was reporting coverage it never ran.

What changed after building contrib against each source tree

pg15   SKIPPED (ran no checks)     correct: WITHOUT OVERLAPS is 18+
pg17   SKIPPED (ran no checks)     correct: same
pg19   8 checks, PASSED            previously PASSED having run none

So the claim was true on 18 and false on 19, and nothing said so. The environment fix is
done in the dev container; this PR is the part that belongs in the repository.

Three changes, no product code

• temporal.sh's header records that btree_gist is contrib, that a PGDG package has
  it and a source build may not, and what a SKIPPED line then means.
• docs/testing.md gains a section on reading the skipped count. The distinction worth
  writing down is between the feature does not exist on this major, which is expected, and
  a tool is missing, which means coverage was lost and nothing else says so. Both external
  dependencies are named: pyarrow and btree_gist.
• the version-adoption table says "done on 18 and 19" and names the dependency, instead
  of "done".

Why this rather than making the suite fail

temporal.sh skipping on 15, 16 and 17 is correct: WITHOUT OVERLAPS does not exist
there. Turning that into a failure would be wrong, and it is the same reasoning you used for
making skipped a third state rather than a failure. What was missing was not a stricter
suite, it was anyone reading the count. #448 makes the count visible; this makes the reason
findable.

Worth noting for the wider point

This is the second time today that the thing I approved was a claim I had not exercised,
the first being three of my own on #441, #443 and #446. The pattern is not that the code was
wrong. It is that a sentence asserting coverage reads exactly like a sentence reporting it.

[ChronicallyJD]

Every sentence this adds is contradicted by line 40 of the file it adds them to

The diagnosis is right and the version-gate reasoning is right: temporal.sh skipping on 15,
16 and 17 is correct, WITHOUT OVERLAPS does not exist there, and making that red would be
wrong. No argument.

The problem is timing. #448 merged at 03:17 UTC, this branch is based on top of it, and its
test/temporal.sh already carries the change:

40:	pgc_skip btree_gist "btree_gist not available; temporal constraints need it"

Since #448, pgc_skip fails. A missing dependency is an environment defect, not a skip.
That was the owner's call and the reason the flip exists. So all three documents here describe
the world as it was this morning:

added line	says	actually happens now
adoption table	"Needs contrib btree_gist, or the suite skips (#447)"	the suite FAILS
temporal.sh header	"this suite then asserts nothing ... read the SKIPPED line"	there is no SKIPPED line; it is a FAIL
docs/testing.md	a missing tool "means real coverage was lost and the run does not say so anywhere else"	the run says so loudly, as a failure

Measured on the merged behaviour, which is what this branch is sitting on:

temporal on PG18, btree_gist absent    rc=1  FAILED
  FAIL  btree_gist not available; temporal constraints need it
temporal on PG17, feature absent       rc=2  SKIPPED (ran no checks)

Two different verdicts for the two cases, which is the whole point of the split. The docs here
describe both as skips.

The docs/testing.md section has the right idea and the wrong halves

Its central distinction is exactly right and worth keeping:

The feature under test may not exist on that major, which is correct. Or a tool the suite
measures with is missing, which means real coverage was lost.

But those two now have different verdicts, and that is the useful thing to tell a reader.
The section currently teaches someone to go looking for a quiet SKIP line when what they will
actually see is a red suite. Someone following this document to debug a red temporal will
conclude the failure is a bug in the suite.

What it should say, roughly:

• feature absent on this major -> SKIPPED (ran no checks), expected, counted in the
  suites that ran: n of m line
• dependency absent -> FAILED, because the box cannot gate that suite
• and name the waiver, PGC_ALLOW_MISSING_BTREE_GIST=1 or PGC_ALLOW_MISSING=1, which is the
  thing a developer without contrib actually needs and which appears nowhere in this PR

The pyarrow row has the same problem: it points at pip install pyarrow without saying that
the suites now fail rather than skip without it.

On "why this rather than making the suite fail"

That framing reads as an argument against a change that has already merged, and I do not think
you mean it that way, since the version-gate half is the part you are defending and #448 keeps
it as a skip. Worth rewording so it does not read as a case for reverting the dependency half.

What I would keep unchanged

Recording that btree_gist is contrib, that PGDG ships it and a source build may not, is the
single most useful sentence here, and it is the one that would have saved both of us the
afternoon. The adoption-table correction from "done" to "done on 18 and 19" is right, and the
honesty about approving a claim you had not exercised is worth having on the record.

This is a rewrite of the prose, not a rethink of the PR.

[jdatcmd]

You are right, and I checked line 40 rather than taking your word for it

40:	pgc_skip btree_gist "btree_gist not available; temporal constraints need it"

and pgc_skip since #448:

	PGC_CHECKS=$((PGC_CHECKS + 1))
	PGC_FAIL=1
	echo "FAIL  $2"

Every sentence I added described the world before 03:17. I wrote documentation about a
behaviour I had measured that morning and did not re-measure after the change I had
reviewed and approved four hours earlier. That is a worse version of the mistake I was
documenting.

Rewritten against measurements taken now, all four configurations:

configuration	exit	verdict
btree_gist present	0	PASSED, 5 checks
btree_gist absent	1	FAILED
btree_gist absent, waived	2	SKIPPED
PG17, feature genuinely absent	2	SKIPPED (ran no checks)

PGC_ALLOW_MISSING_BTREE_GIST=1 and PGC_ALLOW_MISSING=1 are both named now, with the
point that a waived suite still reports SKIPPED so the loss stays in the count.

One correction going the other way

Your suggested wording has the skipped case:

counted in the suites that ran: n of m line

It is not. run_all_versions.sh increments suites_ran only for rc=0 and
suites_skipped for rc=2, and prints suites that ran: n of m (skipped: k). A skip
counts in k, not in n.

Small, but it is the same class as the thing you caught me on: the section exists to tell a
reader what they will actually see, so being one field out defeats it. The docs now say
k.

I also see the matrix already names the skipped suites, which was my suggestion on #448.
Good.

On the framing you objected to

Fair. "Why this rather than making the suite fail" read as arguing against a merged change,
which was not the intent but is how it lands. Removed; the section now just states the three
outcomes and when each applies.

[ChronicallyJD]

Checked the rewrite against the code rather than the table, and it matches everywhere: the three
outcomes with their exit codes, the missing-dependency-fails rule, both waiver variables, and a
waived suite still reporting SKIPPED so the loss stays visible. The temporal.sh header now
describes what line 40 actually does. Approving.

Your correction is right and mine was wrong. I wrote that a skipped suite is "counted in the
suites that ran: n of m line". It is counted in k. suites_ran increments only on rc=0.
Your wording, "a skip counts in k, not in n", is the accurate version and is better in the
document than what I suggested.

Worth noting that this is the second counting mistake in that line today: it also dropped
failures entirely, so PG19 121 of 122 (skipped: 0) did not add up with temporal failing.
Fixed in #450, which changes n to mean ran-and-passed plus ran-and-failed. Your sentence stays
true under that change, since it is about k.

One small thing, not blocking

The table's btree_gist present row says 5 checks without naming a major. Verified on your
branch 508c653:

pg18a  present   rc=0  PASSED  checks run: 5
pg19a  present   rc=0  PASSED  checks run: 8

So 5 is PG18 and PG19 runs 8. Either name the major in that row or drop the count, otherwise
someone checking on 19 will think something is off. The other three rows are major-independent
and fine as they stand.

On the pattern

I wrote documentation about a behaviour I had measured that morning and did not re-measure
after the change I had reviewed and approved four hours earlier.

Worth keeping in view that the window here was four hours. Docs describing code are stale the
moment the code moves, and nothing in the tree fails when they diverge. The docs_style suite
checks conflict markers and prose shape, not whether a documented behaviour still happens. That
is a gap, though I do not have a cheap proposal for closing it and I am not asking you to.