Skip to content

Commit bbc578b

Browse files
author
3code
committed
test tightening
1 parent 8097324 commit bbc578b

5 files changed

Lines changed: 224 additions & 18 deletions

File tree

‎bug-hall-of-fame.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,44 @@ client's second request raises instead of looping.
7474
progress count will spin on a half-dead connection. A cached connection is
7575
a liability as well as a latency win: the cache must be dropped the moment
7676
a write fails, and writes need a real bound, not an unbounded retry.
77+
78+
---
79+
80+
## The invisible space that failed every run (test-side, exit code 1)
81+
82+
**Symptom:** The tty functional suite reported 25/25 `[OK]` yet the test
83+
binary exited 1. A non-fatal `check` inside the *passing* `queued prompt
84+
typed during a turn` test printed `needle was hello` / `row was ❯ hello`
85+
and tripped Nim's failed-check counter. The test never *failed* — it only
86+
leaked a failed `check` — so the `[OK]` line and the exit code disagreed.
87+
88+
**Root cause:** A test-side assumption, not a program bug. The test types
89+
`" world"` into a queued prompt editor one char at a time, accumulating
90+
`acc` (`"hello"` → `"hello "` → `"hello w"` …) and asserting each step's
91+
`acc` is on the cursor row. But the editor's word-wrap — `lineSpans` in
92+
minline.nim — intentionally excludes trailing break-spaces from rendered
93+
rows: `contentEnd` tracks "just past the last non-space," so editor text
94+
`"hello "` renders as the row `"hello"` (no trailing space). The caret is
95+
correctly placed *after* the space; the space is simply never drawn.
96+
97+
The misleading part: the check failure prints `row was ❯ hello`, which
98+
*visually* looks like it contains `"hello"` — and it does. But the needle
99+
was `"hello "` (len 6, trailing space), and `"hello " in "❯ hello"` is
100+
false. The printed `needle was hello` drops the trailing space because
101+
terminal output trims it, hiding the real mismatch. Hex-dumping the cursor
102+
row at every loop step proved it: after the space, bytes are
103+
`E2 9D AF 20 68 65 6C 6C 6F` (`❯ hello`); only once the next non-space char
104+
`w` lands does the space appear (`❯ hello w`).
105+
106+
**Fix:** `requireVisibleEditorCaret` compares against the needle with
107+
trailing whitespace trimmed (`needle.strip(leading = false)`), with a
108+
comment citing the lineSpans invariant. No-op for the other callers (their
109+
needles have no trailing whitespace). Binary now exits 0.
110+
111+
**Lesson:** A trailing space in an assertion string is invisible in both
112+
the source and the failure message — the one place a human looks. When a
113+
`check ... in row` fails but the printed row *looks* like it contains the
114+
needle, suspect unprintable or trailing-whitespace bytes and hex-dump
115+
before reasoning further. And: a green `[OK]` with a non-zero exit code is
116+
itself a bug — a non-fatal `check` in a passing test is a silent failure
117+
that will break CI gating.

‎tests/test_empty_enter_freeze.nim‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,7 @@ suite "idle enter freeze regression":
8686
tty.send($ch)
8787
tty.drain(10)
8888
tty.send "\n"
89+
tty.drain(200)
90+
tty.expectAlive() # empty Enter + command must not exit the process
8991

9092
echo " PASS: empty Enter did not freeze the prompt"

‎tests/test_interrupt_prestream_freeze.nim‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ suite "interrupt during pre-stream freeze regression":
7878

7979
# The prompt must come back and accept a real prompt.
8080
tty.expect "\u276f"
81+
tty.expectAlive() # ESC during in-flight call must not exit
8182
tty.send "hello model"
8283
tty.expect "hello model"
8384
tty.send "\n"
@@ -117,6 +118,7 @@ suite "interrupt during pre-stream freeze regression":
117118
tty.expectInHistory "interrupted by user"
118119

119120
tty.expect "\u276f"
121+
tty.expectAlive() # Ctrl-C during in-flight call must not exit
120122
tty.send "hello model"
121123
tty.expect "hello model"
122124
tty.send "\n"
@@ -163,4 +165,5 @@ suite "interrupt during pre-stream freeze regression":
163165
# follow-up must produce the second response. A freeze shows up as a
164166
# timeout here.
165167
tty.expectInHistory "ok."
168+
tty.expectAlive() # the ESC+follow-up race must not exit the process
166169
echo " PASS: ESC + immediate follow-up did not freeze"

‎tests/test_tty_functional.nim‎

Lines changed: 160 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ suite "terminal visual contract":
291291
tty.expect "cannot run :provider add while a turn is active"
292292
tty.continueStubApi()
293293
tty.expectInHistory "Active command turn complete."
294+
tty.expectAlive() # an active turn that ran colon commands must not exit
294295
tty.drain(200)
295296

296297
let log = sessionLogText(root)
@@ -324,6 +325,7 @@ suite "terminal visual contract":
324325
tty.send "\x1b"
325326
tty.expect "cancelled"
326327
tty.expectNo "nvapi-visible-secret"
328+
tty.expectAlive() # cancelling :provider add must not exit the process
327329

328330
test "harness commands are transcript items":
329331
let root = newFixture("harness_commands")
@@ -379,6 +381,7 @@ suite "terminal visual contract":
379381
tty.expectInHistory "❯ " & command
380382
tty.expectInHistory marker
381383
tty.drain(120)
384+
tty.expectAlive() # no command in the suite exited the process
382385
tty.expectInHistory "unknown command: :toknes did you mean :tokens?"
383386
tty.send "\x1b[A"
384387
tty.expect "❯ :toknes"
@@ -408,17 +411,20 @@ suite "terminal visual contract":
408411
tty.send "\n"
409412
tty.drain(200)
410413
tty.expect "\u276f" # prompt must return
414+
tty.expectAlive() # must not exit after :provider
411415
# Send a message — if the program exited this times out
412416
tty.send "hi"
413417
tty.expect "hi"
414418
tty.send "\n"
415419
tty.expectInHistory "hello"
416420
tty.expect "\u276f"
421+
tty.expectAlive()
417422
# Change model
418423
tty.send ":model stub-large"
419424
tty.send "\n"
420425
tty.drain(200)
421426
tty.expect "\u276f" # prompt must return
427+
tty.expectAlive() # must not exit after :model
422428
tty.send "hey"
423429
tty.expect "hey"
424430
tty.send "\n"
@@ -456,6 +462,10 @@ suite "terminal visual contract":
456462
tty.send "\n"
457463
tty.expectInHistory "Thinking about the test prompt"
458464
tty.expectInHistory "This is a test response."
465+
# Prompt row committed exactly once (not duplicated, not dropped);
466+
# the reply streamed exactly once (not duplicated, not dropped).
467+
tty.expectRowAppearsOnce("❯ This is a test prompt")
468+
tty.expectCount("This is a test response.", 1, where = "raw")
459469
tty.expectTokenBar(["○", "↑120", "↓24"])
460470
tty.drain(200)
461471
tty.expectMeaningfulFrameArtifact(
@@ -617,10 +627,14 @@ suite "terminal visual contract":
617627
tty.expectActiveSpinner()
618628
tty.send "\x1b"
619629
tty.expectInHistory "interrupted by user"
620-
tty.expect "❯"
630+
tty.expectOnScreen "❯" # prompt painted on the grid after ESC, not just raw bytes
631+
tty.expectAlive()
621632
tty.send "second"
622633
tty.send "\n"
623634
tty.expectInHistory "second turn ok"
635+
# The cancelled turn's reply never reached scrollback (dropped, not
636+
# leaked into the next turn).
637+
tty.expectNeverInHistory("should not appear")
624638

625639
test "ctrl-c cancels during network-quiet":
626640
let root = newFixture("quiet_cancel_ctrlc")
@@ -653,10 +667,14 @@ suite "terminal visual contract":
653667
tty.expectActiveSpinner()
654668
tty.send "\x03"
655669
tty.expectInHistory "interrupted by user"
656-
tty.expect "❯"
670+
tty.expectOnScreen "❯" # prompt painted on the grid after Ctrl-C, not just raw bytes
671+
tty.expectAlive()
657672
tty.send "second"
658673
tty.send "\n"
659674
tty.expectInHistory "second turn ok"
675+
# The cancelled turn's reply never reached scrollback (dropped, not
676+
# leaked into the next turn).
677+
tty.expectNeverInHistory("should not appear")
660678

661679
test "ctrl-c during bash tool then prompt accepts input":
662680
let root = newFixture("ctrlc_during_bash")
@@ -695,7 +713,8 @@ suite "terminal visual contract":
695713
tty.drain(300)
696714
tty.send "\x03"
697715
tty.expectInHistory "interrupted by user"
698-
tty.expect "❯"
716+
tty.expectOnScreen "❯" # prompt painted on the grid after Ctrl-C, not just raw bytes
717+
tty.expectAlive()
699718
# The regression: after interrupt the prompt must accept typing.
700719
tty.send "hello"
701720
tty.expectTypedAtPrompt("hello")
@@ -734,7 +753,8 @@ suite "terminal visual contract":
734753
tty.drain(300)
735754
tty.send "\x03"
736755
tty.expectInHistory "interrupted by user"
737-
tty.expect "❯"
756+
tty.expectOnScreen "❯" # prompt painted on the grid after Ctrl-C, not just raw bytes
757+
tty.expectAlive()
738758
tty.send "hello"
739759
tty.expectTypedAtPrompt("hello")
740760
tty.send "\n"
@@ -802,6 +822,9 @@ suite "terminal visual contract":
802822
tty.expectInHistory "❯ queued line one"
803823
tty.expectInHistory " queued line two"
804824
tty.expectInHistory "Queued multiline response."
825+
# Each reply streamed exactly once (not duplicated, not dropped).
826+
tty.expectCount("First multiline response.", 1, where = "raw")
827+
tty.expectCount("Queued multiline response.", 1, where = "raw")
805828
tty.expectTokenBar(["○", "↑180", "↓32"])
806829
tty.drain(200)
807830
tty.expectMeaningfulFrameArtifact(
@@ -1029,8 +1052,12 @@ suite "terminal visual contract":
10291052
# as the next user turn, not dropped.
10301053
tty.send "\x03"
10311054
tty.expectInHistory "interrupted by user"
1055+
tty.expectAlive()
10321056
tty.expectInHistory "❯ queued prompt"
10331057
tty.expectInHistory "Reply to queued prompt."
1058+
# The queued turn was sent exactly once: its reply reached scrollback
1059+
# once (not dropped by the cancel, not double-sent).
1060+
tty.expectCount("Reply to queued prompt.", 1, where = "raw")
10341061

10351062
test "bare escape during a queued mid-turn prompt sends the queue next":
10361063
# Same contract as the Ctrl-C variant above, exercised with a bare Esc.
@@ -1082,8 +1109,12 @@ suite "terminal visual contract":
10821109
# be sent as the next user turn, not dropped.
10831110
tty.send "\x1b"
10841111
tty.expectInHistory "interrupted by user"
1112+
tty.expectAlive()
10851113
tty.expectInHistory "❯ queued prompt"
10861114
tty.expectInHistory "Reply to queued prompt."
1115+
# The queued turn was sent exactly once: its reply reached scrollback
1116+
# once (not dropped by the cancel, not double-sent).
1117+
tty.expectCount("Reply to queued prompt.", 1, where = "raw")
10871118

10881119
test "bash tool success and nonzero exit":
10891120
let root = newFixture("bash_tool_visual_test")
@@ -1254,12 +1285,22 @@ suite "terminal visual contract":
12541285
firstMarkerFrame = i
12551286
break
12561287
require firstMarkerFrame >= 0
1288+
# The flicker regression is an erase-then-redraw pair: a sync frame
1289+
# clears the viewport (`\x1b[J`) without the marker, and the next frame
1290+
# rewrites the marker as committed scrollback. Report the offending pair
1291+
# so a regression points straight at the erase.
1292+
var flickerAt = -1
12571293
for i in firstMarkerFrame ..< syncPayloads.len - 1:
12581294
if "\x1b[J" in syncPayloads[i] and
12591295
"flicker-marker" notin syncPayloads[i] and
12601296
"flicker-marker" in syncPayloads[i + 1]:
1261-
check false
1297+
flickerAt = i
12621298
break
1299+
doAssert flickerAt < 0, "bash tool viewport flickered blank: sync frame " &
1300+
$flickerAt & " erases the viewport (ESC[J) while flicker-marker is " &
1301+
"absent, then frame " & $(flickerAt + 1) &
1302+
" redraws it — an erase-then-redraw pair.\n" &
1303+
tty.dumpFramesAround("flicker-marker")
12631304

12641305
test "non-bash tool transcript shapes":
12651306
let root = newFixture("other_tools_visual_test")
@@ -1434,13 +1475,21 @@ suite "terminal visual contract":
14341475
# scrollback: that is the visual symptom of the extra-line bug.
14351476
let rows = if tty.frames.len > 0: tty.frames[^1].rows else: @[]
14361477
var maxRun = 0
1478+
var maxRunStart = -1
14371479
var curRun = 0
1438-
for r in rows:
1480+
var curRunStart = -1
1481+
for idx, r in rows:
14391482
if r.strip.len == 0:
1440-
inc curRun; maxRun = max(maxRun, curRun)
1483+
if curRun == 0: curRunStart = idx
1484+
inc curRun
1485+
if curRun > maxRun:
1486+
maxRun = curRun; maxRunStart = curRunStart
14411487
else:
14421488
curRun = 0
1443-
check maxRun <= 1
1489+
doAssert maxRun <= 1, "scrollback has " & $maxRun &
1490+
" consecutive blank rows at rows " & $maxRunStart & ".." &
1491+
$(maxRunStart + maxRun - 1) & " (the extra-line bug):\n" &
1492+
tty.dumpFramesAround("")
14441493

14451494
test "every prompt first line survives a reasoning-ticker to content transition":
14461495
# Regression: when a thinking ticker clears at the start of streamed
@@ -1542,9 +1591,98 @@ suite "terminal visual contract":
15421591
tty.expectInHistory "reply after bg-fg"
15431592
# After the turn, a new prompt appears. Type into it.
15441593
tty.expect "❯"
1594+
tty.expectAlive() # bg-fg resume must not have exited the process
15451595
tty.send "still working"
15461596
tty.expect "still working"
15471597

1598+
test "shakedown: one session exercises the full REPL contract":
1599+
# The canonical consolidated contract test (report.md Pattern C). If this
1600+
# one test is green, the three regression classes are closed in a single
1601+
# session: (1) premature exit, (2) prompt echoed twice / line swallowed,
1602+
# (3) committed row overwritten. It threads expectAlive, expectCount and
1603+
# expectNeverInHistory through every interaction rather than relying on a
1604+
# golden file at the end.
1605+
let root = newFixture("shakedown_contract")
1606+
writeHarnessProviders(root)
1607+
writeStubResponses(root, %*[
1608+
# Turn 1: plain prompt and reply.
1609+
{"role": "assistant", "preStreamDelayMs": 50,
1610+
"content": "turn one reply", "contentChunks": ["turn one reply"],
1611+
"usage": {"promptTokens": 10, "completionTokens": 3,
1612+
"totalTokens": 13, "cachedTokens": 0}},
1613+
# Turn 2: held open so we can interrupt it mid-stream.
1614+
{"role": "assistant", "waitForTestContinue": true,
1615+
"content": "streaming that should be interrupted",
1616+
"contentChunks": ["streaming that should be interrupted"],
1617+
"usage": {"promptTokens": 10, "completionTokens": 5,
1618+
"totalTokens": 15, "cachedTokens": 0}},
1619+
# Turn 3: reply after interrupt.
1620+
{"role": "assistant", "content": "turn three reply",
1621+
"contentChunks": ["turn three reply"],
1622+
"usage": {"promptTokens": 10, "completionTokens": 3,
1623+
"totalTokens": 13, "cachedTokens": 0}},
1624+
# Turn 4: reply to a multiline prompt.
1625+
{"role": "assistant", "content": "turn four reply",
1626+
"contentChunks": ["turn four reply"],
1627+
"usage": {"promptTokens": 10, "completionTokens": 3,
1628+
"totalTokens": 13, "cachedTokens": 0}}
1629+
])
1630+
1631+
let tty = startStub(root)
1632+
defer: tty.close()
1633+
1634+
# --- Turn 1: plain prompt and reply (not duplicated, not dropped). ---
1635+
tty.expect "❯"
1636+
tty.expectAlive()
1637+
tty.send "turn one"
1638+
tty.send "\n"
1639+
tty.expectInHistory "turn one"
1640+
tty.expectCount("turn one reply", 1, where = "raw")
1641+
tty.expect "❯"
1642+
tty.expectAlive()
1643+
1644+
# --- Turn 2: interrupt mid-stream; process stays alive, cancelled reply
1645+
# never reaches scrollback, prompt accepts a fresh prompt. ---
1646+
tty.send "turn two"
1647+
tty.send "\n"
1648+
tty.drain(300)
1649+
tty.send "\x03"
1650+
tty.expectInHistory "interrupted by user"
1651+
# The cancelled turn's reply must not leak (not-duplicated / not-merged).
1652+
tty.expectNeverInHistory("streaming that should be interrupted")
1653+
tty.expectOnScreen "❯" # prompt painted on the grid, not just raw bytes
1654+
tty.expectAlive()
1655+
1656+
# --- Turn 3: a second prompt after interrupt (process still interactive). ---
1657+
tty.send "turn three"
1658+
tty.send "\n"
1659+
tty.expectInHistory "turn three"
1660+
tty.expectCount("turn three reply", 1, where = "raw")
1661+
tty.expect "❯"
1662+
tty.expectAlive()
1663+
1664+
# --- Edit the model via :model; process must not exit, prompt returns. ---
1665+
tty.send ":model stub-large"
1666+
tty.send "\n"
1667+
tty.drain(200)
1668+
tty.expect "❯" # prompt must return
1669+
tty.expectAlive() # must not exit after :model
1670+
1671+
# --- Multiline prompt; every line must survive into scrollback. ---
1672+
tty.send "line alpha"
1673+
tty.send "\x1b[13;2u"
1674+
tty.send "line beta"
1675+
tty.expect "line beta"
1676+
tty.send "\n"
1677+
tty.expectInHistory "❯ line alpha"
1678+
tty.expectInHistory " line beta"
1679+
tty.expectCount("turn four reply", 1, where = "raw")
1680+
tty.expect "❯"
1681+
tty.expectAlive()
1682+
# Prompt is live and interactive at session end (the deferred close reaps
1683+
# the child; a clean :q exit is not asserted because the harness's
1684+
# shutdown frame-sync drain is known to hang, per the cwd-removed test).
1685+
15481686
when false:
15491687
suite "disabled terminal visual contract tests":
15501688
test "stub provider streams bash output without replaying it later":
@@ -1667,7 +1805,19 @@ when false:
16671805
editorText.add row.dropRunes(2)
16681806
elif inEditor and row.strip().len == 0:
16691807
break
1670-
check editorText == "this is a long enough line of idle text to wrap when narrowed"
1808+
const expected = "this is a long enough line of idle text to wrap when narrowed"
1809+
if editorText != expected:
1810+
var diverge = 0
1811+
while diverge < min(editorText.len, expected.len) and
1812+
editorText[diverge] == expected[diverge]:
1813+
inc diverge
1814+
doAssert false, "editor text did not survive the narrow rewrap: " &
1815+
"first divergence at byte " & $diverge &
1816+
" (got len " & $editorText.len & ", want " & $expected.len & ").\n" &
1817+
"got: " & editorText & "\n" &
1818+
"expect: " & expected & "\n" &
1819+
tty.dumpFramesAround("")
1820+
check true
16711821

16721822
test "main visual test":
16731823
if getEnv("THREECODE_TTY_ONLY").len > 0 and
@@ -1829,4 +1979,4 @@ when false:
18291979
tty.expectInHistory "Sure is. Let me know when you have a real task."
18301980
tty.expectMeaningfulFrameArtifact(
18311981
MainVisualTestFrames,
1832-
root / "main_visual_test_actual.txt")
1982+
root / "main_visual_test_actual.txt")

0 commit comments

Comments
 (0)