Skip to content

TESTS.md's totals line is written rather than derived, and collided three times in one day #908

Description

@OffgridwithJD

test/pytest/TESTS.md carries a hand-written totals line:

**97 tests in 8 files.** Eighty-two of them test the harness rather than the product.

test/selftest/350-the-pytest-corpus-must-be.sh reads that line back and compares it with the corpus on disk, which is what makes it a claim rather than decoration. That check is mine and it works — it has caught stale counts five times today. This issue is about the number being written rather than derived, which is a different problem and one the gate cannot solve.

The evidence: three collisions in one day, on one line

TESTS.md's totals line conflicted on every rebase between #903, #905, #906 and #902, and in every case both sides of the conflict were wrong for the composed tree — each branch's number was right for its own tree and neither was right for the merge. There is no correct side to pick.

merge one side other side correct
#905 onto post-#903 main 76 74 80
#906 onto rebased #905 87 76 97
#902 onto post-#903 main (this branch's 12) (#903's 4) 90

Both @jdatcmd and I independently resolved these by counting from disk with the gate's own function rather than choosing a side. Two people reaching the same workaround for the same line three times in a day is the signal.

And it has a second failure mode that is not about merges

On #902, --pgc-expect-tests 90 refused with collected 96 test(s) but expected 90. That is the guard working correctly and the two numbers answering different questions:

  • 90 counts test FUNCTIONS, which is what selftest/350 compares against the document
  • 96 counts collected ITEMS, because two functions are parametrized over four box sizes

So the file needs two numbers whose relationship is not obvious, and a reader who picks the wrong one gets a refusal that looks like a defect. Right now the header explains this in prose.

The proposal

Derive the totals rather than write them. Options, roughly in order of how much they change:

A. Make the gate generate the line. selftest/350 already computes the true numbers to compare against; it could write them instead. The document then never conflicts, because nobody edits that line. The cost: a generated line inside a hand-written document, and a gate that writes to the tree — which I do not love, and which would need care in CI where the tree should be read-only.

B. Move the totals out of TESTS.md into a generated fragment the document references, or into the gate's output only. The document stops making the claim; the gate keeps checking it. Cheapest to reason about, and the line stops being a merge target entirely.

C. Keep it written and accept the conflicts, with the resolution rule recorded: never pick a side, always recount from disk. This is where we are, it works, and it costs one careful resolution per merge that touches the corpus.

D. State only the counts that do not move — files, and the harness/product split — and drop the exact test count.

I lean B, and I would not do it without a decision here, because it changes what selftest/350 gates and that arm exists to stop TESTS.md claiming coverage it does not have. Removing the claim to remove the conflict is exactly the move that arm was written to prevent, so it needs to be deliberate rather than convenient.

C is a perfectly respectable answer and I will happily keep resolving them by hand — three collisions came from an unusually busy day, and the twin rule means files under test/pytest/ will be touched by more PRs from here, so the rate may not drop.

Raising it rather than acting on it because it is a change to a gate, and because the observation came out of @jdatcmd's merges as much as mine.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions