Skip to content

Commit f769193

Browse files
TheWayWithinclaude
andcommitted
fix(test): replace flaky shell-driven edit-server test with a deterministic Python harness
The shell test managed a background server, fixed ports, sleep-based readiness and curl — which raced on the macOS CI runner ("did not report ready") and reddened main, though the feature itself was fine. - edit-server.py now reports its actual bound port (supports --port 0, an OS-assigned free port) and drops an unused import. - tests/test-edit-server.py drives the server over an OS-assigned port via urllib with generous timeouts, reading the real port from the ready line: /load, /save (must exit 0 + overwrite + validate), /cancel (must exit 2 + leave the file untouched). No ports to guess, no curl, no sleeps. - run-tests.sh calls the harness as one check and dumps its log on failure. Green locally on both mechanisms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7ca3113 commit f769193

3 files changed

Lines changed: 142 additions & 27 deletions

File tree

tests/run-tests.sh

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -355,33 +355,14 @@ SHEET_DIR="$(sed -n 's|^ \(.*\)/share-1\.html$|\1|p' "$SHEET_OUT" | head -1)"
355355
grep -q "spool" "$SHEET_OUT" && ok "printer spool warning shown" || fail "printer spool warning missing"
356356

357357
echo "== review-in-browser: edit-server.py load/save/cancel =="
358-
if command -v python3 >/dev/null 2>&1 && command -v curl >/dev/null 2>&1; then
359-
ESRV="$ROOT/web/edit-server.py"
360-
cp "$ROOT/examples/estate.example.yaml" "$T/srv.yaml"
361-
python3 "$ESRV" "$T/srv.yaml" --port 8801 >/dev/null 2>"$T/srv.err" &
362-
SPID=$!
363-
ready=0; n=0; while [ "$n" -lt 20 ]; do grep -q "ready at" "$T/srv.err" 2>/dev/null && { ready=1; break; }; sleep 0.2; n=$((n+1)); done
364-
[ "$ready" = 1 ] && ok "edit-server starts and reports ready" || fail "edit-server did not report ready"
365-
PORT1="$(sed -n 's|.*127\.0\.0\.1:\([0-9][0-9]*\)/.*|\1|p' "$T/srv.err")"
366-
curl -s "http://127.0.0.1:${PORT1}/load" 2>/dev/null | grep -q "format_version" && ok "edit-server /load returns the file" || fail "edit-server /load did not return the file"
367-
printf "meta:\n format_version: 3\n owner: 'Rev Tester'\n updated: 2026-07-18\n jurisdictions: [UK]\n password_manager: 'Bitwarden'\nassets:\n - id: A001\n provider: 'Bank'\n type: cash\n identifier: 'a/c ...9'\n priority: high\n ownership: sole\n status: active\n last_confirmed: 2026-07-18\n preferred_action: liquidate\n action_notes: 'Edited in review.'\n" > "$T/save-body.yaml"
368-
curl -s -X POST --data-binary @"$T/save-body.yaml" "http://127.0.0.1:${PORT1}/save" >/dev/null 2>&1
369-
n=0; while kill -0 "$SPID" 2>/dev/null && [ "$n" -lt 30 ]; do sleep 0.2; n=$((n+1)); done
370-
if kill -0 "$SPID" 2>/dev/null; then kill "$SPID" 2>/dev/null; fail "edit-server did not exit after save"; else wait "$SPID"; check "edit-server exits 0 after save" 0 $?; fi
371-
grep -q "Rev Tester" "$T/srv.yaml" && ok "edit-server wrote the saved register" || fail "edit-server did not write the saved content"
372-
sh "$ROOT/scripts/validate.sh" "$T/srv.yaml" >/dev/null 2>&1 && ok "the saved register validates" || fail "saved register did not validate"
373-
374-
cp "$ROOT/examples/estate.example.yaml" "$T/srv2.yaml"
375-
python3 "$ESRV" "$T/srv2.yaml" --port 8811 >/dev/null 2>"$T/srv2.err" &
376-
SPID2=$!
377-
n=0; while [ "$n" -lt 20 ]; do grep -q "ready at" "$T/srv2.err" 2>/dev/null && break; sleep 0.2; n=$((n+1)); done
378-
PORT2="$(sed -n 's|.*127\.0\.0\.1:\([0-9][0-9]*\)/.*|\1|p' "$T/srv2.err")"
379-
curl -s -X POST "http://127.0.0.1:${PORT2}/cancel" >/dev/null 2>&1
380-
n=0; while kill -0 "$SPID2" 2>/dev/null && [ "$n" -lt 30 ]; do sleep 0.2; n=$((n+1)); done
381-
if kill -0 "$SPID2" 2>/dev/null; then kill "$SPID2" 2>/dev/null; fail "edit-server did not exit after cancel"; else wait "$SPID2"; check "edit-server exits 2 after cancel" 2 $?; fi
382-
cmp -s "$ROOT/examples/estate.example.yaml" "$T/srv2.yaml" && ok "cancel left the file unchanged" || fail "cancel changed the file"
358+
if command -v python3 >/dev/null 2>&1; then
359+
if python3 "$ROOT/tests/test-edit-server.py" > "$T/es.log" 2>&1; then
360+
ok "edit-server load/save(exit0)/cancel(exit2) round-trip"
361+
else
362+
fail "edit-server round-trip"; sed 's/^/ | /' "$T/es.log"
363+
fi
383364
else
384-
skip "edit-server test needs python3 + curl"
365+
skip "edit-server test needs python3"
385366
fi
386367

387368
echo "== rotate-shares.sh: full rotation, old shares dead =="

tests/test-edit-server.py

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
#!/usr/bin/env python3
2+
"""Deterministic test for web/edit-server.py — the review-in-browser bridge.
3+
4+
Starts the server as a subprocess on an OS-assigned port, reads the real
5+
port from its ready line, then drives /load, /save and /cancel over HTTP
6+
with generous timeouts (no shell timing races, no curl dependency). Save
7+
must overwrite the file and exit 0; cancel must leave it untouched and
8+
exit 2.
9+
10+
Exit 0 = all good; prints a diagnostic and exits 1 on any failure.
11+
"""
12+
13+
import os
14+
import subprocess
15+
import sys
16+
import tempfile
17+
import time
18+
import urllib.request
19+
20+
ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
21+
SERVER = os.path.join(ROOT, "web", "edit-server.py")
22+
EXAMPLE = os.path.join(ROOT, "examples", "estate.example.yaml")
23+
READY_TIMEOUT = 20.0
24+
EXIT_TIMEOUT = 20.0
25+
26+
SAVE_BODY = (
27+
"meta:\n"
28+
" format_version: 3\n"
29+
" owner: 'Rev Tester'\n"
30+
" updated: 2026-07-18\n"
31+
" jurisdictions: [UK]\n"
32+
" password_manager: 'Bitwarden'\n"
33+
"assets:\n"
34+
" - id: A001\n"
35+
" provider: 'Bank'\n"
36+
" type: cash\n"
37+
" identifier: 'a/c ...9'\n"
38+
" priority: high\n"
39+
" ownership: sole\n"
40+
" status: active\n"
41+
" last_confirmed: 2026-07-18\n"
42+
" preferred_action: liquidate\n"
43+
" action_notes: 'Edited in review.'\n"
44+
)
45+
46+
47+
def fail(msg, proc=None):
48+
print(f"FAIL: {msg}")
49+
if proc is not None:
50+
try:
51+
proc.kill()
52+
except OSError:
53+
pass
54+
sys.exit(1)
55+
56+
57+
def start(target):
58+
"""Launch the server on an OS-assigned port; return (proc, base_url)."""
59+
proc = subprocess.Popen(
60+
[sys.executable, SERVER, target, "--port", "0"],
61+
stderr=subprocess.PIPE, stdout=subprocess.DEVNULL, text=True,
62+
)
63+
deadline = time.time() + READY_TIMEOUT
64+
while time.time() < deadline:
65+
line = proc.stderr.readline()
66+
if not line:
67+
if proc.poll() is not None:
68+
fail(f"server exited early (code {proc.returncode})")
69+
continue
70+
if "ready at" in line:
71+
url = line.split("ready at", 1)[1].strip()
72+
return proc, url
73+
fail("server never reported ready", proc)
74+
75+
76+
def post(url, data=None):
77+
req = urllib.request.Request(url, data=(data.encode() if data else b""), method="POST")
78+
with urllib.request.urlopen(req, timeout=10) as r:
79+
return r.read().decode()
80+
81+
82+
def wait_exit(proc, expected):
83+
try:
84+
code = proc.wait(timeout=EXIT_TIMEOUT)
85+
except subprocess.TimeoutExpired:
86+
fail(f"server did not exit (expected {expected})", proc)
87+
if code != expected:
88+
fail(f"server exit code {code}, expected {expected}")
89+
90+
91+
def main():
92+
if not os.path.isfile(SERVER):
93+
fail(f"server not found at {SERVER}")
94+
95+
with tempfile.TemporaryDirectory() as d:
96+
# ---- save path ----
97+
target = os.path.join(d, "estate.yaml")
98+
with open(EXAMPLE) as f:
99+
open(target, "w").write(f.read())
100+
proc, url = start(target)
101+
with urllib.request.urlopen(url + "load", timeout=10) as r:
102+
loaded = r.read().decode()
103+
if "format_version" not in loaded:
104+
fail("/load did not return the current register", proc)
105+
post(url + "save", SAVE_BODY)
106+
wait_exit(proc, 0)
107+
saved = open(target).read()
108+
if "Rev Tester" not in saved:
109+
fail("save did not overwrite the file")
110+
# the saved register must validate (baseline tier)
111+
vr = subprocess.run(
112+
["sh", os.path.join(ROOT, "scripts", "validate.sh"), target],
113+
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
114+
)
115+
if vr.returncode != 0:
116+
fail("the saved register did not validate")
117+
118+
# ---- cancel path ----
119+
target2 = os.path.join(d, "estate2.yaml")
120+
with open(EXAMPLE) as f:
121+
original = f.read()
122+
open(target2, "w").write(original)
123+
proc2, url2 = start(target2)
124+
post(url2 + "cancel")
125+
wait_exit(proc2, 2)
126+
if open(target2).read() != original:
127+
fail("cancel changed the file")
128+
129+
print("ok: edit-server load/save(exit0)/cancel(exit2) all correct")
130+
return 0
131+
132+
133+
if __name__ == "__main__":
134+
sys.exit(main())

web/edit-server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
import http.server
2121
import os
22-
import socket
2322
import sys
2423
import threading
2524
import webbrowser
@@ -111,6 +110,7 @@ def log_message(self, *a):
111110
sys.stderr.write("error: could not bind a local port.\n")
112111
return 1
113112

113+
port = httpd.server_address[1] # the actual bound port (handles --port 0)
114114
url = f"http://127.0.0.1:{port}/"
115115
sys.stderr.write(f"Register editor ready at {url}\n")
116116
sys.stderr.flush()

0 commit comments

Comments
 (0)