Python Part II · Module 36 · Python for VLSI Design Review
Reviewing the Contract: Inputs, Outputs, Failure, Exit Status
A design review does not begin with the implementation, and neither does this one. Before anybody reads a parser, there are four questions about the script as a black box — what goes in, what comes out, what counts as failure, and whether the process says so — and the last of them is the only one a reviewer can settle in ten seconds with no knowledge of the code at all. Run it and look at the exit code. Of eight specimen scripts measured here, four get it wrong, and each gets it wrong by a different mechanism.
Intermediate25 min readPythonPart IIAutomationCode ReviewExit Codes
Part II · Module 36 · Chapter 36.1 · Reviewing the Contract: Inputs, Outputs, Failure, Exit Status
1. Reviewing Automation Like RTL
RTL_REVIEW = [
("what are the ports?", "the inputs", "declared in the module header"),
("what does it drive?", "the outputs", "declared, and typed"),
("what is illegal?", "the assertions", "written down"),
("how does it say so?", "the error signal", "a port you can connect to"),
]
SCRIPT_REVIEW = [
("what are the inputs?", "argv, env, files, the tool", "mostly undeclared"),
("what is the output?", "stdout, files, the exit code", "mostly undeclared"),
("what counts as failure?", "a judgement in the code", "rarely written down"),
("how does it say so?", "the exit code", "checkable in 10 seconds"),
]
print(f"{'reviewing RTL':<26} {'reviewing a script':<30} ")
for (rq, _, rwhere), (sq, _, swhere) in zip(RTL_REVIEW, SCRIPT_REVIEW):
print(f"{rq:<26} {sq:<30}")
print(f"{' ' + rwhere:<26} {' ' + swhere:<30}")
print()
print("RTL questions answerable from the header:", 4)
print("script questions answerable from the code:", 1)
print("and the one that is :", "the exit code")
print("which is why the review starts there :", True)reviewing RTL reviewing a script
what are the ports? what are the inputs?
declared in the module header mostly undeclared
what does it drive? what is the output?
declared, and typed mostly undeclared
what is illegal? what counts as failure?
written down rarely written down
how does it say so? how does it say so?
a port you can connect to checkable in 10 seconds
RTL questions answerable from the header: 4
script questions answerable from the code: 1
and the one that is : the exit code
which is why the review starts there : TrueA module header declares its contract and a script does not. That asymmetry is the reason automation review feels harder than RTL review and it is also the reason it is easier than it feels: three of the four questions need reading, and the fourth needs a terminal.
2. The Annotation
# run it, then check: does `echo $?` tell the truth?That is the whole first move of a review, and it is deliberately not "read the code". A reviewer who begins with the exit code learns something no amount of reading would have told them in the same time:
MOVES = [
("read the code looking for bugs", "hours", "you find what you look for"),
("read the docs", "minutes", "they may be aspirational — 34.6 §5"),
("run it and check $?", "seconds", "it either lies or it does not"),
]
print(f"{'first move':<34} {'cost':<10} what it tells you")
for move, cost, tells in MOVES:
print(f"{move:<34} {cost:<10} {tells}")
print()
print("the cheapest move :", "running it")
print("the only one with a yes/no answer:", "the exit code")
print("and a lying exit code means :",
"every wrapper in the flow is wrong")
print()
print("so it is checked FIRST because :",
"a failure here invalidates the rest of the review")first move cost what it tells you
read the code looking for bugs hours you find what you look for
read the docs minutes they may be aspirational — 34.6 §5
run it and check $? seconds it either lies or it does not
the cheapest move : running it
the only one with a yes/no answer: the exit code
and a lying exit code means : every wrapper in the flow is wrong
so it is checked FIRST because : a failure here invalidates the rest of the reviewA lying exit code invalidates the rest of the review. If the process says 0 when it failed, then every conclusion about the parser, the reporting and the tests is about a tool nobody can use in a flow — so the finding that matters most is also the one that takes the least time to establish.
3. Eight Specimens, Four of Them Lying
import subprocess
import sys
from pathlib import Path
SPECIMENS = {
"honest": 'import sys\nprint("FAIL: 2 errors")\nsys.exit(1)\n',
"no sys.exit": 'print("FAIL: 2 errors")\n',
"inverted bool": ('import sys\nok = False\n'
'print("FAIL: 2 errors")\nsys.exit(not ok)\n'),
"success bool": 'import sys\nok = True\nprint("PASS")\nsys.exit(ok)\n',
"exit the count": ('import sys\nn = 3\n'
'print(f"FAIL: {n} errors")\nsys.exit(n)\n'),
"count mod 256": ('import sys\nn = 256\n'
'print(f"FAIL: {n} errors")\nsys.exit(n)\n'),
"crash": 'print("starting")\nraise KeyError("offset")\n',
"exit in finally": ('import sys\ntry:\n print("FAIL: 2 errors")\n'
' sys.exit(1)\nfinally:\n sys.exit(0)\n'),
}
def claims_failure(stdout, stderr):
"""What the script's own output asserts about the outcome."""
first = (stdout.strip().splitlines() or [""])[0]
if "Traceback" in stderr:
return True
if first.startswith("FAIL"):
return True
if first.startswith("PASS"):
return False
return True # no verdict printed: assume the worst
print(f"{'specimen':<17} {'rc':>4} {'stdout says':<22} {'claims':<8} honest?")
lying = []
for name, src in SPECIMENS.items():
Path("s.py").write_text(src, encoding="utf-8")
r = subprocess.run([sys.executable, "s.py"], stdout=subprocess.PIPE,
stderr=subprocess.PIPE, text=True)
first = (r.stdout.strip().splitlines() or ["(nothing)"])[0]
failed_per_output = claims_failure(r.stdout, r.stderr)
honest = (r.returncode != 0) == failed_per_output
if not honest:
lying.append(name)
print(f"{name:<17} {r.returncode:>4} {first[:22]:<22} "
f"{str(failed_per_output):<8} {honest}")
print()
print("specimens :", len(SPECIMENS))
print("that lie :", len(lying))
print("which ones :", lying)
print()
print("each lies by a different mechanism:", True)
print("and all four would pass a code review:", True)specimen rc stdout says claims honest?
honest 1 FAIL: 2 errors True True
no sys.exit 0 FAIL: 2 errors True False
inverted bool 1 FAIL: 2 errors True True
success bool 1 PASS False False
exit the count 3 FAIL: 3 errors True True
count mod 256 0 FAIL: 256 errors True False
crash 1 starting True True
exit in finally 0 FAIL: 2 errors True False
specimens : 8
that lie : 4
which ones : ['no sys.exit', 'success bool', 'count mod 256', 'exit in finally']
each lies by a different mechanism: True
and all four would pass a code review: TrueFour of eight lie, each by a different mechanism, and all four read as correct code. The next four sections take one each, because the mechanisms are what a reviewer has to recognise — the symptom is identical in every case.
import subprocess
import sys
from pathlib import Path
print("=== mechanism 1: no sys.exit at all")
Path("a.py").write_text('print("FAIL: 2 errors")\n', encoding="utf-8")
r = subprocess.run([sys.executable, "a.py"], stdout=subprocess.DEVNULL)
print(" rc:", r.returncode, "| a script that ends normally exits:", 0)
print()
print("=== mechanism 2: a bool IS an int")
for value in (True, False):
Path("b.py").write_text(f"import sys\nsys.exit({value})\n",
encoding="utf-8")
r = subprocess.run([sys.executable, "b.py"])
print(f" sys.exit({value!s:<5}) -> rc {r.returncode}")
print(" so sys.exit(ok) with ok=True exits:", 1)
print(" which means a SUCCESS flag reports :", "failure")
print(" and isinstance(True, int) is :", isinstance(True, int))
print()
print("=== mechanism 3: the count is truncated mod 256")
for n in (3, 255, 256, 257, 512):
Path("c.py").write_text(f"import sys\nsys.exit({n})\n", encoding="utf-8")
r = subprocess.run([sys.executable, "c.py"])
print(f" sys.exit({n:>4}) -> rc {r.returncode:>3}"
f"{' <- reports SUCCESS' if r.returncode == 0 else ''}")
print()
print("=== mechanism 4: sys.exit in a finally")
Path("d.py").write_text(
"import sys\ntry:\n print('FAIL')\n sys.exit(1)\n"
"finally:\n sys.exit(0)\n", encoding="utf-8")
r = subprocess.run([sys.executable, "d.py"], stdout=subprocess.DEVNULL)
print(" the try block asked for :", 1)
print(" the finally block asked :", 0)
print(" the process exited :", r.returncode)
print(" which is 33.13 §5's bug :", "a finally swallowing a propagating exit")=== mechanism 1: no sys.exit at all
rc: 0 | a script that ends normally exits: 0
=== mechanism 2: a bool IS an int
sys.exit(True ) -> rc 1
sys.exit(False) -> rc 0
so sys.exit(ok) with ok=True exits: 1
which means a SUCCESS flag reports : failure
and isinstance(True, int) is : True
=== mechanism 3: the count is truncated mod 256
sys.exit( 3) -> rc 3
sys.exit( 255) -> rc 255
sys.exit( 256) -> rc 0 <- reports SUCCESS
sys.exit( 257) -> rc 1
sys.exit( 512) -> rc 0 <- reports SUCCESS
=== mechanism 4: sys.exit in a finally
the try block asked for : 1
the finally block asked : 0
the process exited : 0
which is 33.13 §5's bug : a finally swallowing a propagating exitEvery mechanism is measured and none is exotic. sys.exit(True) returning
1 is the one that catches people hardest, because ok = True means success
everywhere else in the program and sys.exit is the one place where truth is
failure — bool is a subclass of int, True is 1, and 1 means something
went wrong.
The mod-256 rows are Chapter 32.7 Section 5's truncation arriving as a review
finding: exiting the error count works for 1 to 255 errors and reports success
on exactly 256 and 512. A reviewer who sees sys.exit(error_count) has found a
defect without needing to know how many errors are plausible.
4. The Four Questions
Four surfaces, and only one is checkable without reading
The order in that diagram is the review order and it is chosen by cost. The exit status takes seconds and can invalidate everything; the input audit can be mechanised and takes a minute; the output audit needs judgement about consumers; and the failure definition needs the code. Starting with the code — which is what reviewers do by default — spends the expensive attention first.
5. Inputs: What Is Declared Against What Is Read
import ast
SPECIMEN = '''"""run_regress.py - run a suite.
Configuration:
FLOW_RUN_ROOT where run directories go
"""
import argparse
import os
import subprocess
import sys
from pathlib import Path
def main(argv=None):
p = argparse.ArgumentParser()
p.add_argument("--suite")
p.add_argument("--jobs", type=int, default=4)
args = p.parse_args(argv)
root = Path(os.environ.get("FLOW_RUN_ROOT", "runs"))
sim = os.environ["FLOW_SIM"]
licence = os.environ.get("LM_LICENSE_FILE", "")
filelist = Path("rtl/files.f").read_text(encoding="utf-8")
baseline = Path(os.path.expanduser("~/.flow/baseline.json"))
subprocess.run([sim, "-f", "rtl/files.f"], cwd=root)
return 0
'''
def declared_flags(tree):
out = set()
for node in ast.walk(tree):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr == "add_argument":
for a in node.args:
if isinstance(a, ast.Constant) and isinstance(a.value, str):
out.add(a.value)
return out
def env_reads(tree):
out = set()
for node in ast.walk(tree):
# os.environ["X"] and os.environ.get("X", ...)
if isinstance(node, ast.Subscript) and isinstance(node.slice,
ast.Constant):
if "environ" in ast.dump(node.value):
out.add(node.slice.value)
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr == "get" and "environ" in ast.dump(node.func):
if node.args and isinstance(node.args[0], ast.Constant):
out.add(node.args[0].value)
return out
def path_literals(tree):
out = set()
for node in ast.walk(tree):
if isinstance(node, ast.Constant) and isinstance(node.value, str):
v = node.value
if "/" in v and not v.startswith("-") and " " not in v:
out.add(v)
return out
tree = ast.parse(SPECIMEN)
doc = ast.get_docstring(tree) or ""
flags = declared_flags(tree)
envs = env_reads(tree)
paths = path_literals(tree)
documented_env = {w for w in doc.split() if w.isupper() and "_" in w}
print("declared on the command line:", sorted(flags))
print("read from the environment :", sorted(envs))
print("documented in the docstring :", sorted(documented_env))
print("path literals in the source :", sorted(paths))
print()
print("env vars read but NOT documented:", sorted(envs - documented_env))
print("env vars documented but not read:", sorted(documented_env - envs))
print()
print("inputs the --help tells you about:", len(flags))
print("inputs the script actually has :",
len(flags) + len(envs) + len(paths))
print("undeclared inputs :", len(envs | paths)
- len(documented_env & envs))declared on the command line: ['--jobs', '--suite']
read from the environment : ['FLOW_RUN_ROOT', 'FLOW_SIM', 'LM_LICENSE_FILE']
documented in the docstring : ['FLOW_RUN_ROOT']
path literals in the source : ['rtl/files.f', '~/.flow/baseline.json']
env vars read but NOT documented: ['FLOW_SIM', 'LM_LICENSE_FILE']
env vars documented but not read: []
inputs the --help tells you about: 2
inputs the script actually has : 7
undeclared inputs : 4Two declared inputs and seven actual ones. --help names --suite and
--jobs; the script also depends on three environment variables and two
hard-coded paths, and the docstring mentions one of the five.
That is a mechanised review finding rather than a judgement, and it is worth running before reading anything. Three of the undeclared inputs have different severities:
UNDECLARED = [
("FLOW_SIM", "os.environ['FLOW_SIM']", "KeyError if unset", "high"),
("LM_LICENSE_FILE", "os.environ.get(..., '')", "silently empty", "medium"),
("rtl/files.f", "a path literal", "assumes the cwd — 35.7 §4", "high"),
("~/.flow/baseline.json", "a path literal", "per-user state", "high"),
]
print(f"{'undeclared input':<24} {'how it is read':<26} {'if absent':<26} "
f"severity")
for name, how, absent, sev in UNDECLARED:
print(f"{name:<24} {how:<26} {absent:<26} {sev}")
print()
print("the one that raises :", "FLOW_SIM — at least it is loud")
print("the one that is silently empty :", "LM_LICENSE_FILE")
print("the two that depend on the cwd ")
print(" or the user :", 2)
print()
print("the worst of the four :", "~/.flow/baseline.json")
print("because :",
"the tool's answer depends on who ran it")undeclared input how it is read if absent severity
FLOW_SIM os.environ['FLOW_SIM'] KeyError if unset high
LM_LICENSE_FILE os.environ.get(..., '') silently empty medium
rtl/files.f a path literal assumes the cwd — 35.7 §4 high
~/.flow/baseline.json a path literal per-user state high
the one that raises : FLOW_SIM — at least it is loud
the one that is silently empty : LM_LICENSE_FILE
the two that depend on the cwd
or the user : 2
the worst of the four : ~/.flow/baseline.json
because : the tool's answer depends on who ran it~/.flow/baseline.json is the worst of the four and the one a code review
is least likely to flag, because the line reads as sensible caching. It means
two engineers running the identical command on the identical inputs get
different answers, which is Chapter 33.14's undeclared input in its purest form
— and no amount of reading the parser finds it.
6. Outputs: Who Reads Each One
OUTPUTS = [
("a per-run line on stdout", "a human watching", "no", "reword freely"),
("summary.csv", "another script", "yes", "34.7 §6 applies"),
("summary.html", "a browser", "no", "cosmetic"),
("the exit code", "a wrapper", "yes", "frozen — 34.7 §7"),
("run_*/sim.log", "whoever debugs it", "yes", "33.13's evidence"),
("a line on stderr", "a log reader", "no", "33.10"),
]
print(f"{'output':<26} {'read by':<22} {'contract':<10} review note")
for what, who, contract, note in OUTPUTS:
print(f"{what:<26} {who:<22} {contract:<10} {note}")
contracts = sum(1 for *_, c, _ in OUTPUTS if c == "yes")
print()
print("outputs :", len(OUTPUTS))
print("that are a contract :", contracts)
print("that can change freely :", len(OUTPUTS) - contracts)
print()
print("the review question is not 'what does it print'")
print("it is :", "'who breaks if this changes'")
print()
print("and the answer must be written down somewhere:", "34.6 §7's non-goals")output read by contract review note
a per-run line on stdout a human watching no reword freely
summary.csv another script yes 34.7 §6 applies
summary.html a browser no cosmetic
the exit code a wrapper yes frozen — 34.7 §7
run_*/sim.log whoever debugs it yes 33.13's evidence
a line on stderr a log reader no 33.10
outputs : 6
that are a contract : 3
that can change freely : 3
the review question is not 'what does it print'
it is : 'who breaks if this changes'
and the answer must be written down somewhere: 34.6 §7's non-goalsThree of six outputs are a contract and three are not, and the reviewer's job is to establish which is which before commenting on any of them. A comment asking for nicer wording on a stdout line is free; the same comment about a CSV column header breaks every wrapper, and Chapter 35.9 measured that a rename breaks the careful consumers and spares the careless ones.
run_*/sim.log is on the contract list for a reason that is easy to miss: it is
not read by a program, but Chapter 33.13 established it as the evidence a
failing run leaves behind, so a change that stops writing it — or writes it only
on success — removes the only artefact a debugger has.
7. Failure: What Counts
import io
import re
LOG_CLEAN = "UVM_INFO start\nTEST DONE\n"
LOG_ERRORS = "UVM_ERROR a.sv(1) x\nUVM_ERROR : 1\nTEST DONE\n"
LOG_TRUNC = "UVM_INFO start\n"
LOG_FATAL = "UVM_FATAL env.sv(3) no iface\n"
LOG_EMPTY = ""
SPECIMEN_DEFINITIONS = {
"errors only": lambda t: sum(
1 for l in t.splitlines()
if l.startswith("UVM_ERROR") and "UVM_ERROR :" not in l) > 0,
"errors or fatals": lambda t: any(
l.startswith(("UVM_ERROR", "UVM_FATAL")) and " :" not in l[:12]
for l in t.splitlines()),
"errors, fatals, or no DONE": lambda t: (
any(l.startswith(("UVM_ERROR", "UVM_FATAL")) and " :" not in l[:12]
for l in t.splitlines())
or not any(l.strip() == "TEST DONE" for l in t.splitlines())),
}
LOGS = [("a clean run", LOG_CLEAN), ("errors", LOG_ERRORS),
("truncated", LOG_TRUNC), ("a fatal", LOG_FATAL), ("empty", LOG_EMPTY)]
print(f"{'log':<14}", end="")
for name in SPECIMEN_DEFINITIONS:
print(f" {name:>28}", end="")
print()
for label, text in LOGS:
print(f"{label:<14}", end="")
for fn in SPECIMEN_DEFINITIONS.values():
print(f" {str(fn(text)):>28}", end="")
print()
print()
rows = []
for label, text in LOGS:
rows.append([fn(text) for fn in SPECIMEN_DEFINITIONS.values()])
disagreements = sum(1 for r in rows if len(set(r)) > 1)
print("logs tried :", len(LOGS))
print("logs where the three")
print(" definitions DISAGREE :", disagreements)
print()
print("the empty log is a failure under:",
sum(1 for fn in SPECIMEN_DEFINITIONS.values() if fn(LOG_EMPTY)), "of 3")
print("the truncated log :",
sum(1 for fn in SPECIMEN_DEFINITIONS.values() if fn(LOG_TRUNC)), "of 3")log errors only errors or fatals errors, fatals, or no DONE
a clean run False False False
errors True True True
truncated False False True
a fatal False True True
empty False False True
logs tried : 5
logs where the three
definitions DISAGREE : 3
the empty log is a failure under: 1 of 3
the truncated log : 1 of 3Three plausible definitions of failure disagree on three of five logs. Each
is defensible in isolation and the differences are exactly the cases that
matter: a UVM_FATAL is not a failure under the first, and an empty or
truncated log is a pass under the first two.
That is the review question stated precisely. "What counts as failure" is not a philosophical question — it is a function, it is in the code, and a reviewer can tabulate it against five logs in a minute. The finding to write is not "the failure definition is unclear" but "an empty log is reported as PASS", which is a defect with a reproduction.
8. The Review, Run as a Script
import ast
import subprocess
import sys
from dataclasses import dataclass
from pathlib import Path
@dataclass(frozen=True)
class Finding:
surface: str
severity: str
detail: str
def review_exit_status(path):
"""Run it on inputs that should fail, and compare rc with the output."""
findings = []
src = Path(path).read_text(encoding="utf-8")
tree = ast.parse(src)
if "sys.exit" not in src:
findings.append(Finding("exit status", "blocker",
"no sys.exit anywhere: every outcome exits 0"))
for node in ast.walk(tree):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) \
and node.func.attr == "exit":
arg = node.args[0] if node.args else None
if isinstance(arg, ast.Constant) and isinstance(arg.value, bool):
findings.append(Finding(
"exit status", "blocker",
f"sys.exit({arg.value}) - a bool is an int, so "
f"True exits 1"))
if isinstance(arg, ast.Name) and arg.id in {"ok", "passed",
"success"}:
findings.append(Finding(
"exit status", "blocker",
f"sys.exit({arg.id}) - if {arg.id} is a success flag "
f"the code is inverted"))
if isinstance(arg, ast.Name) and "count" in arg.id:
findings.append(Finding(
"exit status", "blocker",
f"sys.exit({arg.id}) - a count is truncated mod 256"))
# a sys.exit inside a finally handler
for node in ast.walk(tree):
if isinstance(node, ast.Try) and node.finalbody:
for sub in ast.walk(ast.Module(body=node.finalbody,
type_ignores=[])):
if isinstance(sub, ast.Call) \
and isinstance(sub.func, ast.Attribute) \
and sub.func.attr == "exit":
findings.append(Finding(
"exit status", "blocker",
"sys.exit inside a finally: it overrides the "
"propagating exit"))
return findings
def review_inputs(path):
findings = []
src = Path(path).read_text(encoding="utf-8")
tree = ast.parse(src)
doc = ast.get_docstring(tree) or ""
documented = {w for w in doc.split() if w.isupper() and "_" in w}
for node in ast.walk(tree):
if isinstance(node, ast.Call) \
and isinstance(node.func, ast.Attribute) \
and node.func.attr == "get" and "environ" in ast.dump(node.func) \
and node.args and isinstance(node.args[0], ast.Constant):
name = node.args[0].value
if name not in documented:
findings.append(Finding("inputs", "major",
f"${name} is read and undocumented"))
if isinstance(node, ast.Constant) and isinstance(node.value, str) \
and node.value.startswith("~"):
findings.append(Finding("inputs", "major",
f"per-user path literal {node.value!r}"))
return findings
SPECIMEN = '''"""tool.py - check a log."""
import os
import sys
def main(argv=None):
root = os.environ.get("FLOW_RUN_ROOT", "runs")
baseline = "~/.flow/baseline.json"
errors = 2
ok = errors == 0
print("FAIL" if errors else "PASS")
try:
sys.exit(ok)
finally:
sys.exit(0)
'''
Path("tool.py").write_text(SPECIMEN, encoding="utf-8")
findings = review_exit_status("tool.py") + review_inputs("tool.py")
ORDER = {"blocker": 0, "major": 1, "minor": 2}
print("review of tool.py")
for f in sorted(findings, key=lambda f: (ORDER[f.severity], f.surface,
f.detail)):
print(f" [{f.severity:<7}] {f.surface:<12} {f.detail}")
print()
print("findings :", len(findings))
print("blockers :", sum(1 for f in findings if f.severity == "blocker"))
print("surfaces covered:", sorted({f.surface for f in findings}))
print()
print("lines of specimen:", len(SPECIMEN.strip().splitlines()))
print("defects found :", len(findings))
print("and none of them ")
print(" needed the ")
print(" parser read :", True)review of tool.py
[blocker] exit status sys.exit inside a finally: it overrides the propagating exit
[blocker] exit status sys.exit(ok) - if ok is a success flag the code is inverted
[major ] inputs $FLOW_RUN_ROOT is read and undocumented
[major ] inputs per-user path literal '~/.flow/baseline.json'
findings : 4
blockers : 2
surfaces covered: ['exit status', 'inputs']
lines of specimen: 15
defects found : 4
and none of them
needed the
parser read : TrueFour defects in a fourteen-line specimen, two of them blockers, and none required reading the parsing logic. Both blockers are on the exit status and both would make every wrapper in the flow wrong.
What makes this worth automating is the last line. A reviewer's attention is finite and the contract surfaces are where the cheapest defects live, so a script that reports them leaves the human free to read the parts that need judgement — which is what the rest of this module is about.
The checker deliberately reports sys.exit(ok) as a blocker with a
condition rather than as a certainty: it cannot know that ok is a success
flag, and a finding phrased "if ok is a success flag the code is inverted"
is one the author can confirm or dismiss in a second. A checker that claimed
certainty here would be wrong sometimes and would be switched off.
9. What One Run Can Actually Settle
Section 4's diagram makes a claim worth testing against a real tool: the exit status is the only surface a reviewer can settle from outside the program. The claim is nearly right, and the way it is wrong changes the review order.
Here is a wrapper of the shape Section 3's specimens take, exercised four ways:
import os
import subprocess
import sys
TOOL = "\n".join([
"import sys, os",
"",
"def main():",
" if not os.path.isfile('files.f'):",
" open('error.txt', 'w').write('no filelist\\n')",
" return 0",
" log = sys.argv[1]",
" if not os.path.isfile(log):",
" open('error.txt', 'w').write('no log\\n')",
" return 0",
" text = open(log).read()",
" bad = [l for l in text.splitlines() if l.startswith('UVM_ERROR')]",
" open('report.txt', 'w').write('errors=%d\\n' % len(bad))",
" return 0",
"",
"sys.exit(main())",
])
open("tool.py", "w").write(TOOL)
open("files.f", "w").write("rtl/a.sv\n")
open("clean.log", "w").write("UVM_INFO a\nTEST DONE\n")
open("fail.log", "w").write("UVM_ERROR drv.sv(88) x\nTEST DONE\n")
ARTEFACTS = ("report.txt", "error.txt")
def run(args, hide=None):
"""One black-box run: no reading, just arguments and observation."""
if hide:
os.rename(hide, hide + ".bak")
for name in ARTEFACTS:
if os.path.exists(name):
os.remove(name)
proc = subprocess.run([sys.executable, "tool.py"] + args,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT, text=True)
made = sorted(n for n in ARTEFACTS if os.path.exists(n))
if hide:
os.rename(hide + ".bak", hide)
return proc.returncode, made, proc.stdout
RUNS = [("a clean log", ["clean.log"], None),
("a FAILING log", ["fail.log"], None),
("no filelist", ["clean.log"], "files.f"),
("a missing log", ["nope.log"], None)]
print(f"{'what the reviewer runs':<18} {'rc':>3} {'stdout':<8} outputs written")
seen = set()
codes = set()
for label, args, hide in RUNS:
rc, made, out = run(args, hide)
codes.add(rc)
seen.update(made)
print(f"{label:<18} {rc:>3} {str(bool(out)):<8} {made}")
print()
print("distinct exit codes across all four runs:", sorted(codes))
print("output files discovered :", sorted(seen))
print("discovered after the FIRST run only :", ["report.txt"])
print()
print("a failing log exits :", 0)
print("so one clean run settles the exit status:", False)
print("and two runs settle it as :", "a blocker")what the reviewer runs rc stdout outputs written
a clean log 0 False ['report.txt']
a FAILING log 0 False ['report.txt']
no filelist 0 False ['error.txt']
a missing log 0 False ['error.txt']
distinct exit codes across all four runs: [0]
output files discovered : ['error.txt', 'report.txt']
discovered after the FIRST run only : ['report.txt']
a failing log exits : 0
so one clean run settles the exit status: False
and two runs settle it as : a blockerEvery run exits 0, including the one on a failing log and the two that could
not do their job at all. That is the blocker from Section 3 again, and the
measurement adds the part the diagram leaves out: a single run settles
nothing. rc 0 on a clean log is exactly what a correct tool does. It becomes
evidence only beside a second run whose status should have differed.
So the refinement to the review order is one sentence: the exit status is the only surface settleable from outside, and it takes the pair of runs, not one. A reviewer who runs a tool once on a working input has confirmed nothing about any of the four surfaces.
The other three surfaces are sampled by a run rather than settled by it, and the output column shows how:
SURFACES = [
("exit status", "yes", "2", "run it on an input that must fail"),
("outputs", "partly", "4+", "each run reveals only the paths it took"),
("inputs", "partly", "4+", "it dies on the first missing one, in order"),
("failure definition", "no", "-", "the set of conditions is not observable"),
]
print(f"{'surface':<20} {'black-box?':<11} {'runs':<5} why")
for surface, settles, runs, why in SURFACES:
print(f"{surface:<20} {settles:<11} {runs:<5} {why}")
print()
print("surfaces one run can settle :", 0)
print("surfaces two runs can settle :", 1)
print("surfaces no number of runs can:", 1)
print()
print("report.txt was visible after run:", 1)
print("error.txt only after run :", 3)
print("so the output list after one run:", "incomplete and looked complete")surface black-box? runs why
exit status yes 2 run it on an input that must fail
outputs partly 4+ each run reveals only the paths it took
inputs partly 4+ it dies on the first missing one, in order
failure definition no - the set of conditions is not observable
surfaces one run can settle : 0
surfaces two runs can settle : 1
surfaces no number of runs can: 1
report.txt was visible after run: 1
error.txt only after run : 3
so the output list after one run: incomplete and looked completeerror.txt does not exist until the third run. A reviewer who ran the tool
once and wrote down its outputs would have recorded one file and been wrong,
with nothing in the run to suggest it — which is why Section 6's output audit
reads the code instead of watching the tool, and why Section 5's input audit
walks the syntax tree rather than deleting files to see what breaks.
That is also the honest limit on black-box review of the failure definition. You can discover that this input is treated as a failure; you cannot discover the set. Enumerating conditions means reading them, which is what Chapter 36.2's battery does from the other direction — it fixes the parser and then lists the inputs the parser's own behaviour must be checked against.
10. Predict, Run, Observe
import subprocess
import sys
from pathlib import Path
def rc(src):
Path("x.py").write_text(src, encoding="utf-8")
return subprocess.run([sys.executable, "x.py"],
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL).returncode
# A
print("A:", rc("import sys\nsys.exit(True)\n"),
rc("import sys\nsys.exit(False)\n"))
# B
print("B:", rc("import sys\nsys.exit(256)\n"),
rc("import sys\nsys.exit(255)\n"))
# C
print("C:", rc("print('FAIL')\n"))
# D
print("D:", rc("import sys\ntry:\n sys.exit(1)\nfinally:\n sys.exit(0)\n"))The verified output:
A: 1 0
B: 0 255
C: 0
D: 0A — True exits 1 and False exits 0, so a variable meaning success must
never be handed to sys.exit. The one-character fix is sys.exit(not ok), and
the better fix is an explicit code.
B — 256 exits 0 and 255 exits 255, which is the whole of the mod-256 argument: a script exiting its error count is correct for 1 to 255 errors and claims success on 256.
C — printing FAIL and ending normally exits 0. There is no mechanism by
which a print affects the exit status, and that sentence is the single most
useful thing a reviewer can keep in mind.
D — the finally wins. All four of these are one line each and all four
would survive a reading of the code.
11. Exercises
Exercise 1 — four surfaces. For a script your team owns, write down what is declared about each of the four.
Exercise 2 — the asymmetry. Explain why an RTL module header makes review easier than a script's first line.
Exercise 3 — run it first. Take a script you have never read, run it on
input that should fail, and record $?.
Exercise 4 — what a lie invalidates. Say what follows for the rest of the review if the exit code is wrong.
Exercise 5 — eight specimens. Build them and measure how many lie.
Exercise 6 — no sys.exit. Search your own tools for one with none.
Exercise 7 — a bool is an int. Confirm sys.exit(True) exits 1 and explain
the consequence for a success flag.
Exercise 8 — the count. Find a sys.exit(n) where n is a count, and say
what happens at 256.
Exercise 9 — exit in a finally. Write the four-line specimen and confirm which code wins.
Exercise 10 — the review order. Justify checking the exit status before reading anything.
Exercise 11 — declared against actual. Run the input audit on one of your scripts and record both counts.
Exercise 12 — undocumented variables. List the environment variables yours reads and does not document.
Exercise 13 — a per-user path. Search for a ~ literal and say what it
does to reproducibility.
Exercise 14 — bracket or get. Explain why os.environ.get(x, "") is the
worse smell.
Exercise 15 — who reads each output. Classify your outputs as contract or free.
Exercise 16 — the free comment. Find a review comment you could make for free, and one that would break a wrapper.
Exercise 17 — the log is a contract. Explain why sim.log is on the
contract list.
Exercise 18 — three definitions. Implement three failure definitions and tabulate them against five logs.
Exercise 19 — where they disagree. Name the logs that separate them.
Exercise 20 — the empty log. Check what your tool reports for "".
Exercise 21 — a finding with a reproduction. Rewrite "the failure definition is unclear" as a defect.
Exercise 22 — automate the cheap part. Write the exit-status checker for your own repository.
Exercise 23 — a conditional finding. Explain why sys.exit(ok) is reported
with an if.
Exercise 24 — certainty gets switched off. Say what happens to a checker that over-claims.
Exercise 25 — four defects, no parser. Review a real script using only the contract surfaces and count what you find.
Exercise 26 — one run proves nothing. Run a wrapper of your own on a working input and list what you now know about its four surfaces.
Exercise 27 — the pair. Run it again on an input that must fail, and compare the two exit codes.
Exercise 28 — the blocker. State the finding in one sentence that names both runs.
Exercise 29 — a hidden output. Find a tool of yours that writes a file only on the error path, and say how many runs it took to see it.
Exercise 30 — incomplete and looking complete. Explain why the first run's output list is the dangerous kind of wrong.
Exercise 31 — inputs in order. Delete the inputs of a tool one at a time and record how many runs it takes to enumerate them.
Exercise 32 — why the AST instead. Say why Section 5 walks the syntax tree rather than deleting files.
Exercise 33 — the surface you cannot reach. Explain why the failure definition is not observable from outside at any number of runs.
12. Summary
- An RTL module declares its contract and a script does not, which is why three of the four review questions need reading and the fourth needs a terminal.
- Check the exit status first. It costs seconds, and a defect there means every wrapper built on the tool is already wrong — so it invalidates the rest of the review.
- Four of eight specimens lie about their own verdict, each by a different mechanism, and all four read as correct code.
- No
sys.exitat all leaves rc 0. Aprintcannot affect the exit status. sys.exit(True)exits 1 —boolis a subclass ofint— so handing a success flag tosys.exitinverts the contract.sys.exit(256)exits 0. Exiting an error count is correct for 1 to 255 errors and claims success on exactly 256 and 512.- A
sys.exitin afinallyoverrides the propagating one, which is Chapter 33.13'sreturn-in-finallybug applied to the exit code. - The input audit can be mechanised. One specimen declared two inputs and actually had seven: three environment variables and two path literals, with one of five documented.
- A per-user path literal is the worst undeclared input and the least likely to be flagged, because the line reads as sensible caching while making the tool's answer depend on who ran it.
os.environ.get(x, "")is a worse smell thanos.environ[x]— the first hides a dependency, the second declares it with a bad error message.- Three of six outputs are a contract, and establishing which before commenting is what separates a free review comment from one that breaks every wrapper.
- Three plausible failure definitions disagreed on three of five logs, and the disagreements are the cases that matter: an empty or truncated log is a PASS under two of them.
- Write the finding as a defect with a reproduction, not as "the definition is unclear".
- Four defects in fourteen lines with the parser unread, two of them blockers — which is where the cheapest findings live.
- Report an inferred finding with an
if. A checker that claims certainty - A single run settles nothing.
rc 0on a working input is exactly what a correct tool does; it is evidence only beside a run whose status should have differed. - The exit status is still the only black-box surface — Section 4's claim holds, with the refinement that it takes a pair of runs.
- All four runs of the specimen exit 0, including the failing log and the two that could not do the job at all.
error.txtappeared only on the third run, so a one-run output list is incomplete and looks complete — which is why Section 6 reads the code and Section 5 walks the syntax tree.- The failure definition is not observable from outside at any number of runs: a run shows that one input is treated as a failure, never the set. about intent is wrong sometimes and gets switched off.
The next chapter takes the surface this one deliberately skipped — the parser — and asks the question that makes it the module's most critical: can malformed input pass silently?
Where this fits
Part of the Python curriculum.
