4a5041811d79f95f11000fbc3fc310f9a4ae417a braney Sun Sep 13 15:36:26 2026 -0700 docent: a test run clears up after itself, refs #38252 A run left a log per script and a stills/ directory per script, and nothing ever removed them, so `git status` in the test directories reported 89 files that were not work. The obvious answer is a .gitignore, and it is the wrong one: the files stay on disk, and git is taught to look away from the directory new tests are written in. `make test` now removes a passing script's log, stills and sessions. Nothing reads any of it once the run is over -- nightly.sh reads this target's OUTPUT, and the failure branch prints a failing log into that output while the file is still there. A failing script keeps its log, and so does an xfail that passed, which is the flip `make proof` is about and the one morning someone will want to read the whole run. `parity` does the same with its three logs and the mp4 its slow run records. WARNING lines are echoed before the log goes. make test sends each script's output to its log and prints only "ok", so a docent warning on a PASSING script reached a file nobody opens -- and "tooltip never showed its own text" means the step measured nothing. None of the 67 scripts warns today; the point is that one that starts to will say so. Five orphan logs are removed by hand in passing: rm36212.xfail.log, rm36540.log, rm37388ui.xfail.log, rm37389.xfail.log and rm38272.xfail.log, left behind when those scripts were renamed. make test deliberately does not sweep logs it did not just write, since a rename in progress should not lose its evidence; make clean is still there for a full wipe. Measured: 67 of 67 green in 10m39s and the directory came back with nothing untracked in it, the fifteen language tests next door pass and clear their sessions/ too, and a script made to fail on purpose kept its log and its stills. diff --git src/hg/utils/docent/tests/docentTest.mk src/hg/utils/docent/tests/docentTest.mk index b74d6158b18..b7e79ccc1c9 100644 --- src/hg/utils/docent/tests/docentTest.mk +++ src/hg/utils/docent/tests/docentTest.mk @@ -1,140 +1,170 @@ # Shared rules for a directory of Docent tests. Included by tests/makefile and by # tests/regress/makefile, so both directories run the same code rather than a copy of it. # # An including makefile sets, before the include: # DOCENT path to docent.js from THIS directory (required) # PREFLIGHT path to preflight.js from THIS directory (required) # PROOF path to proof.js from THIS directory (default: beside PREFLIGHT) # PARITY which script `make parity` runs (default: the first one found) # # Everything else -- which scripts are tests, which have derive baselines -- comes from # what is on disk here, so a new *.docent.yaml is picked up with no edit. ifndef DOCENT $(error include docentTest.mk only after setting DOCENT, e.g. DOCENT = ../docent.js) endif ifndef PREFLIGHT $(error include docentTest.mk only after setting PREFLIGHT, e.g. PREFLIGHT = ./preflight.js) endif PW_DIR ?= /hive/groups/browser/uiTest/pw PW_ENV ?= PLAYWRIGHT_BROWSERS_PATH=$(PW_DIR)/browsers NODE_PATH=$(PW_DIR)/node_modules T ?= # `make parity` needs one script that is expected to PASS, so an .xfail one is no use as # the default. An including makefile can name a better one. PASSING := $(filter-out %.xfail,$(patsubst %.docent.yaml,%,$(wildcard *.docent.yaml))) PARITY ?= $(firstword $(PASSING)) TESTS := $(if $(T),$(addsuffix .docent.yaml,$(T)),$(wildcard *.docent.yaml)) PROOF ?= $(dir $(PREFLIGHT))proof.js .PHONY: test parity clean preflight proof # The fixtures the scripts here name but do not contain: saved sessions, hub URLs, the # server itself. No browser, so this is seconds, and it is what separates "the fixtures # went away" from "a bug came back" -- which are the same red without it. Run it before # the suite, and on its own as often as you like. preflight: @$(PW_ENV) node $(PREFLIGHT) . # What evidence each script has that it would catch its bug, and the tally. No browser # and no network, so it costs nothing to run and the number can go straight into a commit # message or a ticket. It is a separate target and not part of `make test` on purpose: a # script with no proof is not a failure, it is a script whose evidence has not been # collected yet, and the two must not arrive as the same red. # # It DOES fail on a malformed or unknown proof line, because a vocabulary nobody enforces # turns into free text and free text cannot be counted. proof: @$(PW_ENV) node $(PROOF) . $(T) +# A run leaves a log per script, plus a stills/ and a sessions/ directory per script that +# takes a shot: or writes a session:. Nothing reads any of it once the run is over -- the +# nightly reads this target's OUTPUT, and the failure branches below print a failing log +# into that output while the file is still there -- so a passing script's log is cleared +# up rather than left for `git status` to report. Ignoring them instead would leave the +# same files on disk and teach git to look away from the directory new tests are written +# in, which is the wrong half of the problem to solve. +# +# A failing script keeps its log. It is already echoed here, but a file is easier to page +# through than a terminal, and an xfail that PASSED keeps its log too: that is the flip +# `make proof` is about, and the morning it happens is the one morning someone will want +# to read the whole run. +# +# The stills and the session file a passing script wrote go with the log, for the same +# reason: both are rewritten from scratch by the next run, and a script that reads its own +# session back (`loadSession: {file: ...}`) does so during the run, not after it. +# +# WARNING lines are printed before the log goes. docent warns without failing -- a +# mouseover whose tooltip never showed its own text is the one that matters, since it +# means the step measured nothing -- and today those lines reach a file that nobody opens. test: @if [ -z "$(strip $(TESTS))" ]; then \ echo "no *.docent.yaml here -- nothing was tested"; exit 1; fi @fail=0; \ for f in $(TESTS); do \ b=$${f%.docent.yaml}; want=0; \ case $$b in *.xfail) want=1;; esac; \ if [ $$want = 1 ]; then printf '=== %s (expected to fail)\n' "$$b"; \ else printf '=== %s\n' "$$b"; fi; \ $(PW_ENV) node $(DOCENT) $$f > $$b.log 2>&1; got=$$?; \ if [ $$got -ne 0 ] && [ $$want -eq 0 ]; then \ echo " FAILED -- run said:"; sed 's/^/ /' $$b.log; fail=1; \ elif [ $$got -eq 0 ] && [ $$want -eq 1 ]; then \ echo " FAILED -- this was supposed to fail, and it passed"; fail=1; \ - else echo " ok"; fi; \ + else \ + echo " ok"; \ + grep -h 'WARNING' $$b.log 2>/dev/null | sed 's/^/ /' || true; \ + rm -f $$b.log; \ + [ -n "$$b" ] && rm -rf stills/$$b sessions/$$b; \ + fi; \ done; \ + rmdir stills sessions 2>/dev/null || true; \ if [ $$fail -eq 0 ]; then echo "docent tests passed"; else echo "docent tests FAILED"; exit 1; fi # Two invariants that need the same script run more than once, so they cannot be # written as a script of their own: # FAST parity -- FAST drops the dwells, the cursor animation and the recording. # It must not change what the page ends up showing. # rerun stability -- a second run in the same directory must reach the same state. # Cart bleed between runs would show up here and nowhere else. parity: @echo "=== $(PARITY) fast"; \ DOCENT_FAST=1 $(PW_ENV) node $(DOCENT) $(PARITY).docent.yaml > parity.fast.log 2>&1 \ || { sed 's/^/ /' parity.fast.log; exit 1; } @echo "=== $(PARITY) slow (records an mp4, so this one is not quick)"; \ $(PW_ENV) node $(DOCENT) $(PARITY).docent.yaml > parity.slow.log 2>&1 \ || { sed 's/^/ /' parity.slow.log; exit 1; } @echo "=== $(PARITY) again, to catch state left behind by the last run"; \ DOCENT_FAST=1 $(PW_ENV) node $(DOCENT) $(PARITY).docent.yaml > parity.rerun.log 2>&1 \ || { sed 's/^/ /' parity.rerun.log; exit 1; } - @echo "parity passed" +# Same rule as `test`: the three logs and the mp4 the slow run records are kept only when +# a step failed, and a failing step exits above before this line is reached. + @rm -f parity.fast.log parity.slow.log parity.rerun.log $(PARITY).mp4; \ + rmdir stills/$(PARITY) sessions/$(PARITY) stills sessions 2>/dev/null || true; \ + echo "parity passed" # The derivation on its own: DOCENT_DERIVE=1 resolves each `track:` step against the # server's trackDb and prints the cart variables, with no browser and no navigation. That # is where Docent's own decisions are, and it runs in about a second, so it is worth # checking against a baseline. # # Only the scripts with a file in expected/ are checked. The output depends on LIVE # trackDb, so a baseline can go stale for an honest reason -- a new member of a superTrack, # a retired subtrack. When that happens, read the diff before believing it: # # make derive # diff every baseline # make derive-accept # rewrite the baselines, then `git diff` them # # Scripts whose derivation is large and churny (views, 188 variables from one view-level # hideKids) deliberately have NO baseline: it would fail every time ENCODE gained a cell # line, and the browser test already covers the behaviour. # # One line has to be stripped before the diff. docent.js caches the trackDb listing in # $TMPDIR for a day, and prints `trackDb: N tracks for DB from .../hubApi` only when it # actually fetches. So the first run of the day carries a line that every run after it # does not, and a baseline captured warm would fail against a cold run for a reason that # is not about trackDb at all. Both targets strip exactly that line, so it cannot get # into a baseline either. The other two trackDb lines -- a hub genome, an unreachable # hubApi -- are real news about the derivation and are left in. DERIVE_ENV = DOCENT_DERIVE=1 $(PW_ENV) DERIVE_FILTER = sed '/^trackDb: [0-9][0-9]* tracks for /d' BASELINES := $(patsubst expected/%.derive,%,$(wildcard expected/*.derive)) .PHONY: derive derive-accept derive: @if [ -z "$(strip $(BASELINES))" ]; then \ echo "no baselines in expected/ -- nothing was checked"; exit 1; fi @fail=0; \ for b in $(BASELINES); do \ $(DERIVE_ENV) node $(DOCENT) $$b.docent.yaml 2>&1 | $(DERIVE_FILTER) > $$b.derive.out; \ if diff -u expected/$$b.derive $$b.derive.out > $$b.derive.diff; then \ echo "=== $$b"; echo " ok"; rm -f $$b.derive.diff; \ else \ echo "=== $$b"; echo " CHANGED -- read this before accepting it:"; \ sed 's/^/ /' $$b.derive.diff; fail=1; \ fi; \ rm -f $$b.derive.out; \ done; \ if [ $$fail -eq 0 ]; then echo "derivation baselines match"; else echo "derivation CHANGED"; exit 1; fi derive-accept: @mkdir -p expected @for b in $(BASELINES); do \ $(DERIVE_ENV) node $(DOCENT) $$b.docent.yaml 2>&1 | $(DERIVE_FILTER) > expected/$$b.derive; \ echo "rewrote expected/$$b.derive"; \ done @echo "now read: git diff expected/" clean: rm -rf stills sessions *.log *.derive.out *.derive.diff *.mp4