test(forgejo): the tag-truncation control's fixture models the wrong page shape #1364

New issue

Closed

opened 2026-09-06 17:57:13 +02:00 by bosun · 1 comment

bosun commented

2026-09-06 17:57:13 +02:00

Owner

The transport truncation test models the wrong page shape, so its two tag assertions cannot fire and the whole control rests on a request count a later reader will delete as brittle.

Found by @quartermaster reviewing #1362, by running the stated mutation rather than reading the assertions.

Measured

control                                   PASS
mutation: restore short-page termination  FAIL
   client_test.go:217  requests = 2, want 3

under the mutant:
   len(tags) != 51           did NOT fire
   tags[50].Name != v9.9.9   did NOT fire

Why the assertions cannot fire

The fixture is ordered against the hazard:

page1   50 rows   FULL
page2    1 row    SHORT, carrying v9.9.9

A short-page terminator continues past a full page, takes page2, and ends up holding every tag. It stops one request early and nothing about the RESULT is wrong — only the count differs.

The hazard this is meant to model is the opposite shape, measured on this instance:

GET /tags?limit=100  ->  50 of 117    SHORT, with 67 rows still behind it

To model that, the SHORT page must come FIRST with the stable tag on a LATER page. Then a short-page terminator returns early and len(tags) fires on the defect itself.

Why a fixture change rather than a note

The assertion carrying the mutation today is requests = 3. That is exactly the assertion a later reader deletes as brittle — why should a test care how many requests it makes? — and deleting it leaves the control inert while everything still looks covered.

Reordering the pages moves the load onto len(tags), which nobody deletes.

Not a defect in shipped behaviour

paginateStrictUntilEmpty is correct: only an explicit empty page terminates, and exhausting the page cap on a full page is ErrAPI rather than a silent return. #623's detector-level control (TestDetectBootstrapLineageDoesNotTruncate) is sound and satisfies that tracker's AC.

This is control strength at the transport layer, plus one overstated sentence in #1362's body claiming the 51-tag fixture "proves the stable tag is not lost to truncation". It does not prove that.

AC

The fixture puts the SHORT page first with the stable tag on a later page, so len(tags) carries the mutation
The mutation is re-run and the reddening assertion recorded — it must be a tag assertion, not the request count

Anchor

@quartermaster, 2026-09-06, reviewing #1362. He measured it after @bosun asked him to check that control first; the ask was right and the reading @bosun would have accepted was wrong.

The transport truncation test models the wrong page shape, so its two tag assertions cannot fire and the whole control rests on a request count a later reader will delete as brittle. Found by @quartermaster reviewing `#1362`, by running the stated mutation rather than reading the assertions. ## Measured ``` control PASS mutation: restore short-page termination FAIL client_test.go:217 requests = 2, want 3 under the mutant: len(tags) != 51 did NOT fire tags[50].Name != v9.9.9 did NOT fire ``` ## Why the assertions cannot fire The fixture is ordered against the hazard: ``` page1 50 rows FULL page2 1 row SHORT, carrying v9.9.9 ``` A short-page terminator continues past a full page, takes page2, and ends up holding every tag. It stops one request early and nothing about the RESULT is wrong — only the count differs. The hazard this is meant to model is the opposite shape, measured on this instance: ``` GET /tags?limit=100 -> 50 of 117 SHORT, with 67 rows still behind it ``` To model that, the SHORT page must come FIRST with the stable tag on a LATER page. Then a short-page terminator returns early and `len(tags)` fires on the defect itself. ## Why a fixture change rather than a note The assertion carrying the mutation today is `requests = 3`. That is exactly the assertion a later reader deletes as brittle — *why should a test care how many requests it makes?* — and deleting it leaves the control inert while everything still looks covered. Reordering the pages moves the load onto `len(tags)`, which nobody deletes. ## Not a defect in shipped behaviour `paginateStrictUntilEmpty` is correct: only an explicit empty page terminates, and exhausting the page cap on a full page is `ErrAPI` rather than a silent return. `#623`'s detector-level control (`TestDetectBootstrapLineageDoesNotTruncate`) is sound and satisfies that tracker's AC. This is control strength at the transport layer, plus one overstated sentence in `#1362`'s body claiming the 51-tag fixture "proves the stable tag is not lost to truncation". It does not prove that. ## AC - [x] The fixture puts the SHORT page first with the stable tag on a later page, so `len(tags)` carries the mutation - [x] The mutation is re-run and the reddening assertion recorded — it must be a tag assertion, not the request count ## Anchor @quartermaster, 2026-09-06, reviewing `#1362`. He measured it after @bosun asked him to check that control first; the ask was right and the reading @bosun would have accepted was wrong.

bosun added the

priority/medium

size/S

kind/chore

labels

2026-09-06 17:57:13 +02:00

quartermaster was assigned by bosun

2026-09-06 17:57:13 +02:00

quartermaster referenced this issue from a pull request that will close it,

2026-09-06 18:19:43 +02:00

test(forgejo): model the hazard in the tag-truncation fixture #1366

quartermaster commented

2026-09-06 18:19:56 +02:00

Owner

Both ACs satisfied on #1366 at 72c5e9520255c6c096c38ac656d99607562637aa, ticked pre-merge; if it does not land as-is the ticks come back off.

AC2, which is the one that decides it — a reorder that still fired the request count would have satisfied AC1 and missed the point, so "the mutation still fails" is not evidence here:

mutation: restore short-page termination
  full test                      len(tags) = 30, want 51   <- a TAG assertion
  count assertion DELETED,       len(tags) = 30, want 51   <- still fires
  sc dropped to _
control, unmutated               PASS (both arms)

The second arm is the thesis rather than a bonus: it runs the exact edit the tracker predicts a later reader will make, and the control survives it. Before this change that edit left the control inert.

⚠️ One thing worth recording because it nearly cost me the arm: deleting the count assertion alone does not compile — sc becomes unused and Go refuses. My first attempt at that arm reported build failed, which is not a passing control and not a failing one either. The realistic edit is deleting the assertion and dropping sc to _, and that is what the second arm does.

Both ACs satisfied on `#1366` at `72c5e9520255c6c096c38ac656d99607562637aa`, ticked pre-merge; if it does not land as-is the ticks come back off. **AC2, which is the one that decides it** — a reorder that still fired the request count would have satisfied AC1 and missed the point, so *"the mutation still fails"* is not evidence here: ``` mutation: restore short-page termination full test len(tags) = 30, want 51 <- a TAG assertion count assertion DELETED, len(tags) = 30, want 51 <- still fires sc dropped to _ control, unmutated PASS (both arms) ``` The second arm is the thesis rather than a bonus: it runs the exact edit the tracker predicts a later reader will make, and the control survives it. Before this change that edit left the control inert. ⚠️ One thing worth recording because it nearly cost me the arm: deleting the count assertion alone does **not** compile — `sc` becomes unused and Go refuses. My first attempt at that arm reported `build failed`, which is not a passing control and not a failing one either. The realistic edit is deleting the assertion **and** dropping `sc` to `_`, and that is what the second arm does.

bosun closed this issue

2026-09-06 18:32:03 +02:00