Make tests report an absent prerequisite as a skip - #563
Conversation
pdfinn
left a comment
There was a problem hiding this comment.
Right problem and the right mechanism — runner.b:170 already catches
"fail:skip" or "skip:*" and the comment there cites INFR-312, so this is
using an established convention rather than inventing one. A test that prints
FAIL and returns without raising is recorded as a pass, which is the worst of
the three outcomes.
I measured it rather than reading it. Because the branch is based on
pre-#559/#560 master, I applied the tests/ diff onto current master to
isolate your change from the base:
master 12 passed, 1 failed, 5 skipped
master + this PR 13 passed, 0 failed, 5 skipped
So SystemTxtTodoMandate is genuinely fixed, and nothing regressed.
wallet_policy_test now raises
skip:wallet9p not mounted at /n/wallet (run tests/host/wallet9p...), which
runner.b records as SKIP instead of five false failures. Good.
(First time round I compared the branch against master directly and saw four
extra failures. That was the stale base, not your change — flagging it in case
anyone else measures it the same way and draws the same wrong conclusion.)
Three of the six tests never run
msg_inject_test, msg_triage_test and wallet9p_test are not in
tests/mkfile's TARG, so they are not built and not executed:
msg_inject_test TARG=0 built=no
msg_triage_test TARG=0 built=no
wallet9p_test TARG=0 built=no
The fixes to them are correct but inert. That is the same class #559 dealt with
for 45 modules elsewhere — a source with no target is never compiled — and it
is one line each here. Worth adding in this PR, since otherwise the
wallet9p_test rework in particular (the factotum fixture, waiting on the
mount rather than sleeping 1.5s, the _marker export) is unverifiable.
If any of them is deliberately excluded, a comment in tests/mkfile saying so
would stop the next person restoring it and finding out the hard way.
Rebase needed
This branch predates #560 and still tracks 1011 .dis files. It applies
cleanly to current master, so it is only a rebase, not a rework. Note that
#559 added veltro_cc_alignment_test.dis to TARG — that is why the test runs
at all now, and why its failure was visible for you to fix.
Fix the three missing TARG entries and rebase, and this is good.
58d9a0f to
73f7c9d
Compare
|
Rebased onto current master — zero tracked
All three verified through the wired path rather than in isolation: The full-suite before/after you measured, I could not reproduce, and it is The underlying reason is worth your attention separately. In place of the full totals: a controlled 85-module before/after, whose only
|
All three had sources in tests/ but no TARG entry, so they were never compiled and never executed — the same never-built class the manifest guard exists for. Add the three targets and the matching tools/dis-manifest.txt lines so the build must produce them. msg_inject_test and msg_triage_test raise skip:* without their tests/inferno drivers; runner.b records that as SKIP. wallet9p_test starts its own factotum-backed fixture and tears it down, so it runs and exits inside the runner.
73f7c9d to
482fb78
Compare
|
Rebased onto current master (df34b02). Force-pushed 73f7c9d -> 482fb78.
Re-verified on macOS after the rebase.
#578 fixed the ClusterFuzzLite link break, so |
What this changes
Four tests reported something other than the truth when a prerequisite was
absent from the environment.
wallet_capability_testprinted a failure line and returned without raising,so the runner recorded it as a pass.
wallet_policy_testraisedfail:, sofive of its eleven cases failed only because
/n/walletwas not mounted.msg_inject_testandmsg_triage_testprinted a FAIL line and returned. Eachnow raises
skip:<reason>naming the driver script that supplies theprerequisite, which
runner.brecords as a clean SKIP.wallet9p_testis rebuilt around a factotum-backed fixture. It starts factotumonly when one is not already running, waits for the mount instead of sleeping a
fixed 1.5s, unmounts what it mounted, and drops the case for the raw signing
oracle that no longer exists. It also gains a
_markerexport: without one,joiniface()conflatesWallet9pTestwithWallet9pand the module does notload.
luciuisrv_test.bandtask_consistency_test.balready carry the sameworkaround.
veltro_cc_alignment_testasserts the planning mandate text the promptactually contains.
Tests
wallet_capability_testandwallet_policy_testare in TARG, so these two runtoday. Before:
After:
The other four are not in TARG, so they are inert until they are wired. Wiring
them is a separate change that depends on #559 and #560.