From 8fbbeb6ec9ac67a7e54d6c3c5d42b64e012c62a6 Mon Sep 17 00:00:00 2001 From: Anachronaut Date: Fri, 28 Aug 2026 18:07:35 -0400 Subject: [PATCH] Make xfail compare the diagnostic, not just the failure The Test Manual said an xfail test records the assembler's refusal message and so catches both an error that stops being detected and a message that changes without anybody meaning it to. It did not. run.sh checked only that the assembler exited non zero, printed the first line for a person to read, and compared nothing; --bless recorded nothing for these sixteen tests at all. So an xfail passed four different ways that look identical from outside: the intended error fired, an unrelated error fired, the message changed, or the assembler fell over on its way to the point. That is the documentation describing behaviour the code does not have, which is the exact failure Tests/docs.sh exists to prevent, in the manual that argues for knowing what your evidence is worth. The diagnostic is now stripped of colour, given the same [exit N] line every other recorded result carries, and compared through check() like anything else. Sixteen results recorded; every existing one is byte for byte unchanged. Verified the way the manual asks: one diagnostic was broken on purpose, its test failed with the changed line in the diff, and its neighbour passed. Found by ChatGPT reviewing the manual. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01E2JrLzFvuFX9fgi1LDRjrW --- Tests/expected/diagAlignOutside.out | 4 ++++ Tests/expected/diagBareAlign.out | 3 +++ Tests/expected/diagBareInclude.out | 3 +++ Tests/expected/diagBareSWI.out | 4 ++++ Tests/expected/diagDuplicateLabel.out | 3 +++ Tests/expected/diagDuplicateVector.out | 3 +++ Tests/expected/diagPinnedRange.out | 6 +++++ Tests/expected/diagPinnedTaken.out | 4 ++++ Tests/expected/diagStringInProgram.out | 8 +++++++ Tests/expected/diagUnbasedSegment.out | 6 +++++ Tests/expected/diagUnknownVector.out | 4 ++++ Tests/expected/lib-print.out | 3 +++ Tests/expected/printDecimalTest.out | 3 +++ Tests/expected/printDigitTest.out | 3 +++ Tests/expected/printHexTest.out | 3 +++ Tests/expected/shiftTest.out | 3 +++ Tests/manifest | 6 +++-- Tests/run.sh | 32 ++++++++++++++++++++++---- 18 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 Tests/expected/diagAlignOutside.out create mode 100644 Tests/expected/diagBareAlign.out create mode 100644 Tests/expected/diagBareInclude.out create mode 100644 Tests/expected/diagBareSWI.out create mode 100644 Tests/expected/diagDuplicateLabel.out create mode 100644 Tests/expected/diagDuplicateVector.out create mode 100644 Tests/expected/diagPinnedRange.out create mode 100644 Tests/expected/diagPinnedTaken.out create mode 100644 Tests/expected/diagStringInProgram.out create mode 100644 Tests/expected/diagUnbasedSegment.out create mode 100644 Tests/expected/diagUnknownVector.out create mode 100644 Tests/expected/lib-print.out create mode 100644 Tests/expected/printDecimalTest.out create mode 100644 Tests/expected/printDigitTest.out create mode 100644 Tests/expected/printHexTest.out create mode 100644 Tests/expected/shiftTest.out diff --git a/Tests/expected/diagAlignOutside.out b/Tests/expected/diagAlignOutside.out new file mode 100644 index 0000000..857dcbb --- /dev/null +++ b/Tests/expected/diagAlignOutside.out @@ -0,0 +1,4 @@ +Error: #Align outside the Program or Data Segment. + There is nothing there for it to move along. + File: testPrograms/diagnostics/alignOutside.asm at line 6. +[exit 1] diff --git a/Tests/expected/diagBareAlign.out b/Tests/expected/diagBareAlign.out new file mode 100644 index 0000000..80c3d43 --- /dev/null +++ b/Tests/expected/diagBareAlign.out @@ -0,0 +1,3 @@ +Error: #Align without a number. + File: testPrograms/diagnostics/bareAlign.asm at line 11. +[exit 1] diff --git a/Tests/expected/diagBareInclude.out b/Tests/expected/diagBareInclude.out new file mode 100644 index 0000000..fd88371 --- /dev/null +++ b/Tests/expected/diagBareInclude.out @@ -0,0 +1,3 @@ +Error: #Include without a file name. + File: testPrograms/diagnostics/bareInclude.asm at line 11. +[exit 1] diff --git a/Tests/expected/diagBareSWI.out b/Tests/expected/diagBareSWI.out new file mode 100644 index 0000000..3024eac --- /dev/null +++ b/Tests/expected/diagBareSWI.out @@ -0,0 +1,4 @@ +Error: SWI without a vector to go to. +File: testPrograms/diagnostics/bareSWI.asm at line 9. +Token: SWI +[exit 1] diff --git a/Tests/expected/diagDuplicateLabel.out b/Tests/expected/diagDuplicateLabel.out new file mode 100644 index 0000000..c60a749 --- /dev/null +++ b/Tests/expected/diagDuplicateLabel.out @@ -0,0 +1,3 @@ +Error: Label "twice" is defined more than once. +File: testPrograms/diagnostics/duplicateLabel.asm at line 13. +[exit 1] diff --git a/Tests/expected/diagDuplicateVector.out b/Tests/expected/diagDuplicateVector.out new file mode 100644 index 0000000..ce94367 --- /dev/null +++ b/Tests/expected/diagDuplicateVector.out @@ -0,0 +1,3 @@ +Error: That vector already has a handler. +File: testPrograms/diagnostics/duplicateVector.asm at line 20. +[exit 1] diff --git a/Tests/expected/diagPinnedRange.out b/Tests/expected/diagPinnedRange.out new file mode 100644 index 0000000..03146a4 --- /dev/null +++ b/Tests/expected/diagPinnedRange.out @@ -0,0 +1,6 @@ +Error: 100 is not a vector number that can be given. + Numbers below 16 belong to the machine, and 64 and up are + handed out by the assembler. A vector two programs have to + agree about takes one from 16 to 63. +File: testPrograms/diagnostics/pinnedVectorRange.asm at line 19. +[exit 1] diff --git a/Tests/expected/diagPinnedTaken.out b/Tests/expected/diagPinnedTaken.out new file mode 100644 index 0000000..c33b43c --- /dev/null +++ b/Tests/expected/diagPinnedTaken.out @@ -0,0 +1,4 @@ +Error: Another vector already has that number. +File: testPrograms/diagnostics/pinnedVectorTaken.asm at line 28. +Token: second +[exit 1] diff --git a/Tests/expected/diagStringInProgram.out b/Tests/expected/diagStringInProgram.out new file mode 100644 index 0000000..96ef743 --- /dev/null +++ b/Tests/expected/diagStringInProgram.out @@ -0,0 +1,8 @@ +Error: A string cannot go in the Program Segment. + Strings live in Data Memory, which is the only memory an instruction + can read. A string in Program Memory could not be reached even by the + program holding it, except through the memory controller. + File: testPrograms/diagnostics/stringInProgram.asm at line 19. + The string: "this is not where this goes" + Move it below a #Data line. +[exit 1] diff --git a/Tests/expected/diagUnbasedSegment.out b/Tests/expected/diagUnbasedSegment.out new file mode 100644 index 0000000..5e91ded --- /dev/null +++ b/Tests/expected/diagUnbasedSegment.out @@ -0,0 +1,6 @@ +Error: The Program Segment is based at 0x2000, but the Data Segment + has 1 byte at 0x0000 and was never given a #Base. + Half a program loaded at zero lands on whatever is already there. + File: testPrograms/diagnostics/unbasedSegment.asm + Say "#Base 0x0000" in the Data Segment if that is what you meant. +[exit 1] diff --git a/Tests/expected/diagUnknownVector.out b/Tests/expected/diagUnknownVector.out new file mode 100644 index 0000000..f17402c --- /dev/null +++ b/Tests/expected/diagUnknownVector.out @@ -0,0 +1,4 @@ +Error: "neverDeclared" is not a vector. + Names used with SWI have to be given a handler in a #Vectors section. +File: testPrograms/diagnostics/unknownVector.asm at line 9. +[exit 1] diff --git a/Tests/expected/lib-print.out b/Tests/expected/lib-print.out new file mode 100644 index 0000000..dc88bc3 --- /dev/null +++ b/Tests/expected/lib-print.out @@ -0,0 +1,3 @@ +Error: Undefined label "start". +File: Libraries/print.asm at line 8. +[exit 1] diff --git a/Tests/expected/printDecimalTest.out b/Tests/expected/printDecimalTest.out new file mode 100644 index 0000000..29a4848 --- /dev/null +++ b/Tests/expected/printDecimalTest.out @@ -0,0 +1,3 @@ +Error: Undefined label "printByteDecimal". +File: testPrograms/printDecimalTest.asm at line 9. +[exit 1] diff --git a/Tests/expected/printDigitTest.out b/Tests/expected/printDigitTest.out new file mode 100644 index 0000000..d07301e --- /dev/null +++ b/Tests/expected/printDigitTest.out @@ -0,0 +1,3 @@ +Error: Undefined label "printDecimalDigit". +File: testPrograms/printDigitTest.asm at line 6. +[exit 1] diff --git a/Tests/expected/printHexTest.out b/Tests/expected/printHexTest.out new file mode 100644 index 0000000..17d91fc --- /dev/null +++ b/Tests/expected/printHexTest.out @@ -0,0 +1,3 @@ +Error: Undefined label "printByteHex". +File: testPrograms/printHexTest.asm at line 8. +[exit 1] diff --git a/Tests/expected/shiftTest.out b/Tests/expected/shiftTest.out new file mode 100644 index 0000000..46544ce --- /dev/null +++ b/Tests/expected/shiftTest.out @@ -0,0 +1,3 @@ +Error: Undefined label "printDecimal". +File: testPrograms/shiftTest.asm at line 12. +[exit 1] diff --git a/Tests/manifest b/Tests/manifest index 3325838..e167c95 100644 --- a/Tests/manifest +++ b/Tests/manifest @@ -12,8 +12,10 @@ # Modes: # run Assemble, execute, compare all output against Tests/expected/.out # assemble Assemble only. For library files that have no entry point to run -# xfail Assembly is expected to fail. Records a known breakage so that -# fixing one is noticed, and so an accidental new one is too +# xfail Assembly is expected to fail, and the assembler's diagnostic is recorded +# and compared like any other output. Catches a known breakage that stops +# being detected, an accidental new one, and a message that changes +# without anybody meaning it to # # limit is a cycle count, for programs that never halt on their own. It is passed # to the emulator as --cycles, which bounds the run by cycles rather than by wall diff --git a/Tests/run.sh b/Tests/run.sh index a89dcfc..3b41637 100755 --- a/Tests/run.sh +++ b/Tests/run.sh @@ -141,13 +141,27 @@ assemble() { # test programs outside it include. local name="$1" src="$2" local bin="$BUILD/$name.bin" - if ( cd "$PROGRAMS" && "$ASSEMBLER" -I Libraries -I CosmOS/Source -o "$bin" "$src" ) >"$BUILD/.assemble.log" 2>&1; then + ( cd "$PROGRAMS" && "$ASSEMBLER" -I Libraries -I CosmOS/Source -o "$bin" "$src" ) \ + >"$BUILD/.assemble.log" 2>&1 + # WRITTEN TO A FILE RATHER THAN A VARIABLE, because this function is called inside a + # command substitution below and that runs it in a subshell, where anything it set + # would be thrown away. A file is the one channel that survives either call site. + echo $? > "$BUILD/.assemble.status" + if [ "$(cat "$BUILD/.assemble.status")" -eq 0 ]; then echo "$bin" return 0 fi return 1 } +# Takes the colour out of a diagnostic, in place. The assembler paints its errors whether +# or not anything is a terminal, so a recorded refusal would otherwise carry escape +# sequences - and a recorded result full of them is unreadable in a diff, which is the one +# moment it has to be read. +uncolour() { + sed -i -E 's/\x1b\[[0-9;]*m//g' "$1" +} + while IFS='|' read -r name src mode stdin limit disk; do name="$(trim "$name")" [ -z "$name" ] && continue @@ -163,10 +177,20 @@ while IFS='|' read -r name src mode stdin limit disk; do if assemble "$name" "$src" >/dev/null; then FAIL=$((FAIL + 1)); FAILED_NAMES+=("$name") report "FAIL" "$name" "expected assembly to fail, but it succeeded" - else - PASS=$((PASS + 1)) - report "ok" "$name" "fails as recorded: $(head -1 "$BUILD/.assemble.log" | tr -d '\033' | sed 's/\[[0-9;]*m//g')" + continue fi + # WHAT IT SAID, not merely that it said something. Checking only that the assembler + # exited non zero passed for the intended error, for an unrelated error, for a + # changed message, and for the assembler falling over on its way to the point - all + # four look identical from outside. So the diagnostic is recorded and compared like + # any other output, which is what the Test Manual has always claimed happened here. + OUT="$BUILD/.out" + cp "$BUILD/.assemble.log" "$OUT" + uncolour "$OUT" + # The same last line every other recorded result carries, and for the same reason: + # which non zero status a refusal exits with is part of what it does. + printf '[exit %d]\n' "$(cat "$BUILD/.assemble.status")" >> "$OUT" + check "$name" "$OUT" continue fi