eccdddfb22cf35d4e5698f72a4b12707aa43b349 braney Sun Sep 20 14:27:27 2026 -0700 testRegistry: check the docent column's "-" as well as its names The docent column says which browser test watches the same ticket, and a named script was already checked: it has to exist and has to belong to that ticket. A "-" was checked by nothing, and that was the wrong half to leave out. `unwatched` -- the list of tickets where nothing anywhere would go red if the bug came back -- is built entirely out of those "-" values. They were entered by hand, by reading the docent directory once, so a script written the next day left the ticket sitting on that queue with nothing to notice. For the four tickets that are on it because their code lives inside a CGI, somebody writing the docent script is the expected outcome, which made it the likeliest kind of row to go stale. So a row claiming no docent script is now checked against the tree the same way a named one is: if rm<ticket>.*docent.yaml turns up, the check fails and asks for the row to be filled in. The failure is somebody doing the right thing and the table not having heard about it. Watched to fail and then pass: blanking the docent column of #36212, whose script is in the tree, turns the real table red with the name of the script it found. refs #38391 diff --git src/utils/testRegistry/testRegistry src/utils/testRegistry/testRegistry index 776ccea7be6..5cbe8ed139f 100755 --- src/utils/testRegistry/testRegistry +++ src/utils/testRegistry/testRegistry @@ -10,31 +10,37 @@ testRegistry ticket 38320 what defends this ticket testRegistry test faSpeedRead which tickets a test defends testRegistry release 504 everything that ships in one release testRegistry needed the tickets waiting for a test to be written testRegistry unwatched the tickets with no test of any kind testRegistry why perf one reason at a time: invisible, perf, library testRegistry tickets just the numbers that have one, for a join testRegistry check every row still names a live test A row whose test column is "-" is a ticket that needs a unit test and does not have one, with what the test would have to hold down in its note. Those rows are half the value of the table: a ticket nobody has looked at and a ticket somebody looked at and could not test are different states, and without them the two are indistinguishable. -The docent column names the browser test that watches the same ticket, or "-". +The docent column names the browser test that watches the same ticket, or "-", +and BOTH values are checked. A named script has to exist and has to belong to +that ticket; a "-" has to still be true, so a script appearing later for a +ticket whose row says "-" is a failure rather than a silence. Without that +second half the "-" rows would be the only thing here nothing verified, and +`unwatched` -- the list of tickets nothing would catch -- is built entirely out +of them. It is not a second copy of the docent suite: it is here so that "nothing is watching this ticket at all" is a question the tool answers, which is the question worth acting on. A docent script does not make a unit test pointless where the why is invisible -- rm38309 cannot see a read past the end of an array, it asserts something next to it -- but a ticket with neither is a ticket nobody would notice breaking again. The why column is the part that took the most arguing. "The fix is in a library file" is the wrong rule in both directions: a wording change in a library file needs no unit test, and a performance rewrite in hgTracks needs one badly, since nothing else can tell that the old slow path has come back. So the column names the reason a browser test cannot do the job, and a perf row says what has to be measured, because a test that times a loop is a test that goes red on a busy machine and gets deleted. @@ -42,30 +48,31 @@ that is renamed or deleted takes its row's meaning with it, and a registry that quietly names files nobody has, saying a bug is covered when it is not, is worse than no registry. So `check` fails the build the day a row rots, and tests/makefile runs it, which is how it reaches `make test` from src. It reads the table and the file system and nothing else: no network, no database, no Redmine. That is deliberate, because a check that needs a server is a check that gets skipped. Coverage questions that need the set of all tickets ("which v504 tickets have no unit test") are a join, not a mode here: `testRegistry tickets` prints the covered numbers and redmineCli prints the others. """ import argparse +import glob import os import sys # Weakest first, and the order is the point. This is proof.js's vocabulary from # the docent suite (hg/utils/docent/tests/proof.js), minus xfail and server-flip, # which need a browser and a server to mean anything. The two scales are kept # compatible on purpose, so one reader can compare a unit row with a docent one. # "unrecorded" is not in proof.js; it is this file's way of saying nobody has # written down what was seen. LEVELS = [ "unrecorded", "assertion-only", "sandbox-ab", "release-ab", "caught-regression", @@ -176,30 +183,44 @@ if not os.path.exists(row.path()): # The rot this file exists to catch. bad(row, "%s is not in the tree" % row.fileName()) for ticket, group in byTicketMap(rows).items(): if len(group) > 1 and any(r.test == "-" for r in group): problems.append("%s: ticket %s says both covered and waiting" % (fileName, ticket)) # One ticket, one answer about its browser test. Two rows disagreeing # is how a table starts lying about what is watching a ticket. if len({r.docent for r in group}) > 1: problems.append("%s: ticket %s names more than one docent script" % (fileName, ticket)) + # A "-" is a claim that no browser test watches this ticket, and + # `unwatched` is built out of those claims, so it is checked like any + # other. Somebody writing the docent script is the good outcome here; + # the failure exists so the row gets updated rather than going stale and + # leaving the ticket on a queue it has left. + if all(r.docent == "-" for r in group): + found = sorted(glob.glob(os.path.join(KENT_SRC, DOCENT_DIR, + "rm%s.*docent.yaml" % ticket))) + if found: + problems.append("%s: ticket %s says no docent script, but %s " + "is in the tree; put it in the docent column" % + (fileName, ticket, + os.path.basename(found[0]))) + ordered = sorted(rows, key=lambda r: (int(r.ticket) if r.ticket.isdigit() else 0, r.test)) if [r.lineNo for r in ordered] != [r.lineNo for r in rows]: problems.append("%s: rows are not sorted by ticket then test" % fileName) for line in problems: print(line) return len(problems) def byTicketMap(rows): """Rows grouped by ticket.""" out = {} for row in rows: out.setdefault(row.ticket, []).append(row)