summaryrefslogtreecommitdiff
path: root/test
diff options
context:
space:
mode:
authorhathach <[email protected]>2026-08-20 16:43:49 +0700
committerhathach <[email protected]>2026-08-20 16:43:49 +0700
commit6905639b07c69fec68e9ebc77f7d27ac2775ee41 (patch)
tree2d46b0b6ae45b27df4874583c5830997df5829bd /test
parent7800876bf151a239046521232dc4603b156061be (diff)
hil: run every board in one hil_test.py and hand results across as JSON
hil-validate ran one hil-operator per board. That parallelizes at the wrong layer: hil_test.py already schedules boards across host controllers and budgets concurrent flashes and usbtest batteries per controller (FLASH_PARALLEL/USBTEST_PARALLEL), and those permits live in one process - N parallel runs multiply the budget onto the same uPD720201 cards for no wall-clock gain over one run that already parallelizes. The workflow now spawns ONE operator with every board as repeated -b. The operator no longer retypes the report table. Four consecutive max-effort review rounds found ~15 defects in this file and every one was in reconstructing board identity from transcribed prose: report rows are named per VARIANT (nanoch32v203 only ever produces -fsdev/-usbfs rows), a variant need not start with its board's name, lock contention is a `board-locked` cell rather than a phrase, and each fix introduced the next round's bug - including a fake-green test that asserted an invariant with the one input shape that could not break it. The new helper test/hil/helper/hil_summary.py does the join where the roster lives and emits one machine verdict per board ({board, ran, pass, locked, detail}); the operator returns that JSON verbatim plus `wedged`, the only field it authors, and the workflow reads fields, never parses a string. Its cell classifier mirrors hil_test.py's own tally exactly: failures are always marked ('fail' or a ❌ prefix, TestFail's contract), everything unmarked is a pass - a passing test may return a plain metric cell like '13443 KB/s', and the mirrored rule is what keeps a green table from becoming a red verdict. hil_ci.sh kept only the LAST -b, so multi-board remote runs staged one board's binaries and every other board died on the rig after its lock and flash slot were spent. It now parses every -b spelling argparse accepts (with the -bt arms ordered first, longest-match, so the <config>.failed retry form is never read as a board named "t..."), pre-flights roster membership and build dirs for ALL boards before anything is wiped or staged, warns per declared variant with no build dir (which hil_test.py would silently green-skip), forwards HIL_* knobs as export lines in one %q word the remote evals ('; '-joined so it round-trips under dash - an authorized HIL_NO_BOARD_LOCK force must not silently no-op), keeps HIL_REPORT_DIR local because the copy-backs look in REMOTE_DIR, and copies hil_report.json and the .failed re-run spec back beside the markdown, deleting stale local copies first so a green run cannot leave last run's spec looking current. Retries preserve the fleet: the documented path is the <config>.failed spec, which already begins with --accumulate; a fresh scoped re-run would unlink the report and collapse the whole-fleet table to the retried boards alone. The risky logic is executable, not argued about: .claude/workflows/test-hil-validate.mjs pins the lookup/verdict helpers and runs in pre-commit (hil-validate-logic); nine staging tests drive hil_ci.sh through an ssh stub that models the real thing (argv joined into one string the remote re-splits, heredoc on stdin - the naive echo-stub passed while the feature was broken); and deliberate mutations of the verdict logic are all caught. Validated on the rig: a 2-board run (usbtest 30/30 on both; the pre-fix classifier, replayed against that run's real report, fails the fully-green stm32f723disco on its two passing '13443 KB/s' cells), the .failed --accumulate retry (merged report kept every earlier row), and a 10-run soak over random subsets of a 22-board pool - 43 board-slots, every failure signature matched pre-existing CI state or known flake, zero tooling failures, no locks left behind.
Diffstat (limited to 'test')
-rw-r--r--test/hil/helper/hil_summary.py115
-rw-r--r--test/hil/hil_ci.sh250
-rw-r--r--test/hil/test/test_hil_bounded.py317
3 files changed, 626 insertions, 56 deletions
diff --git a/test/hil/helper/hil_summary.py b/test/hil/helper/hil_summary.py
new file mode 100644
index 000000000..e566bead0
--- /dev/null
+++ b/test/hil/helper/hil_summary.py
@@ -0,0 +1,115 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: MIT
+"""Fold hil_report.json into one machine-readable verdict per BOARD.
+
+A workflow driving hil_test.py through an operator agent has no filesystem access, so the
+agent has to carry the results across. It must carry them, not retype them: the previous
+design asked the agent to transcribe the markdown table, and every defect found in four
+review rounds came from re-parsing that prose -- variant row names vs board names,
+`board locked` vs `board-locked`, folding several variant rows into one verdict, rows that
+matched no board. All of it is a join, and the join belongs here, where the roster is.
+
+Report rows are named per VARIANT (hil_test.py builds them from `vname`), and a variant name
+is not required to start with the board name -- nanoch32v203 produces only `-fsdev`/`-usbfs`,
+ch32v307v_r1_1v0 only `-usbhs`/`-usbfs`. The config is what maps them back.
+
+Emits, on stdout:
+ {"results": [{"board", "ran", "pass", "locked", "detail"}...], "banner": str}
+
+`locked` is a field, not a prefix to grep for. `ran` false means the board produced no row at
+all, which is not the same as failing.
+
+Usage: hil_summary.py <config.json> [-b BOARD]... [--report-dir DIR]
+"""
+import argparse
+import json
+import sys
+from pathlib import Path
+
+FAIL_ICON, SKIP_ICON = '❌', '⚪' # a pass needs no icon: unmarked = pass
+LOCKED_CELL = 'board-locked'
+
+
+def cell_state(v: str) -> str:
+ """'pass' | 'fail' | 'skip' -- the EXACT classifier hil_test.py's own tally uses
+ (cell_kind in render_matrix): 'fail' or a ❌ prefix is a failure, 'skip' or a ⚪
+ prefix is a skip, and EVERYTHING ELSE is a pass. That last arm is load-bearing: a
+ passing test may return a plain metric string ('480.0 MBps') that lands in the cell
+ unprefixed, while failures are guaranteed marked -- TestFail's docstring pins that its
+ metric is icon-prefixed precisely so render/tally treat it as a failure. Classifying
+ unknown shapes as fail here would publish a green table as a red verdict."""
+ if v == 'fail' or v.startswith(FAIL_ICON):
+ return 'fail'
+ if v == 'skip' or v.startswith(SKIP_ICON):
+ return 'skip'
+ return 'pass'
+
+
+def variants_of(cfg: dict, board: str) -> list:
+ for b in cfg.get('boards', []):
+ if b['name'] == board:
+ return [v['name'] for v in (b.get('variant') or [])] or [board]
+ return [board]
+
+
+def summarize(cfg: dict, boards: list, report: dict) -> dict:
+ rows = {r['board']: r.get('cells') or {} for r in report.get('rows', [])}
+ owner = {v['name']: b['name'] for b in cfg.get('boards', [])
+ for v in (b.get('variant') or [])}
+ results = []
+ for board in boards:
+ names = variants_of(cfg, board)
+ mine = {n: rows[n] for n in names if n in rows}
+ # a variant name that is neither declared nor prefixed cannot be attributed; the
+ # `<board>-` fallback only helps ad-hoc builds, it is not the primary path. It must
+ # also never steal a row DECLARED by another board: a declared variant need not start
+ # with its own board's name, so it may happen to start with this board's name plus '-'.
+ mine.update({n: c for n, c in rows.items()
+ if n.startswith(f'{board}-') and n not in mine
+ and owner.get(n, board) == board})
+ if not mine:
+ results.append({'board': board, 'ran': False, 'pass': False, 'locked': False,
+ 'detail': 'no report row for this board'})
+ continue
+ locked = any(LOCKED_CELL in cells for cells in mine.values())
+ bad = []
+ for vname, cells in sorted(mine.items()):
+ for test, val in sorted(cells.items()):
+ if test == LOCKED_CELL:
+ continue
+ if cell_state(str(val)) == 'fail':
+ bad.append(f'{vname} {test}: {val}')
+ ok = not bad and not locked
+ if locked:
+ detail = 'held by another holder; not flashed'
+ elif bad:
+ detail = '; '.join(bad)
+ else:
+ detail = f'{len(mine)} variant(s), {sum(len(c) for c in mine.values())} cell(s) ok'
+ results.append({'board': board, 'ran': True, 'pass': ok, 'locked': locked,
+ 'detail': detail})
+ return {'results': results, 'banner': report.get('banner', '')}
+
+
+def main() -> int:
+ ap = argparse.ArgumentParser()
+ ap.add_argument('config_file')
+ ap.add_argument('-b', '--board', action='append', default=[],
+ help='boards to report on; default: every board in the config')
+ ap.add_argument('--report-dir', default='.', help='where hil_report.json lives (default: cwd)')
+ a = ap.parse_args()
+
+ cfg = json.loads(Path(a.config_file).read_text())
+ boards = a.board or [b['name'] for b in cfg.get('boards', [])]
+ jpath = Path(a.report_dir) / 'hil_report.json'
+ if not jpath.is_file():
+ print(f'error: {jpath} not found -- did hil_test.py run in this directory?',
+ file=sys.stderr)
+ return 1
+ json.dump(summarize(cfg, boards, json.loads(jpath.read_text())), sys.stdout, indent=2)
+ print()
+ return 0
+
+
+if __name__ == '__main__':
+ sys.exit(main())
diff --git a/test/hil/hil_ci.sh b/test/hil/hil_ci.sh
index c7dfa95df..f474c3055 100644
--- a/test/hil/hil_ci.sh
+++ b/test/hil/hil_ci.sh
@@ -1,8 +1,9 @@
#!/usr/bin/env bash
# Run HIL test remotely on ci.lan
-# Usage: test/hil/hil_ci.sh [-b BOARD] [-t TEST] [extra hil_test.py args...]
+# Usage: test/hil/hil_ci.sh [-b BOARD]... [-t TEST] [extra hil_test.py args...]
# Example:
# test/hil/hil_ci.sh -b stm32f723disco
+# test/hil/hil_ci.sh -b stm32f723disco -b raspberry_pi_pico
# test/hil/hil_ci.sh -b stm32f723disco -t host/cdc_msc_hid -r 1
#
# Env overrides: REMOTE, REMOTE_DIR, CONFIG (path to HIL config json),
@@ -44,17 +45,46 @@ for a in "$@"; do
exit 1
done
-# Parse -b BOARD from arguments to know which build to copy
-BOARD=""
+# Parse -b BOARD from arguments to know which builds to copy. Repeatable: hil_test.py
+# takes the whole board set in ONE run (it schedules them across host controllers and
+# budgets the flashes itself), so every -b needs its binaries staged, not just the last.
+BOARDS=()
ARGS=()
while [[ $# -gt 0 ]]; do
case "$1" in
- -b)
- [[ $# -ge 2 ]] || { echo "error: -b requires a BOARD argument" >&2; exit 1; }
- BOARD="$2"
+ # hil_test.py declares `-b, --board` with action='append', so argparse also accepts
+ # --board=X and -bX. Recognising only the bare `-b X` forwarded the others to the rig
+ # while never staging them: the board ran with no firmware and reported a green row.
+ -b|--board)
+ [[ $# -ge 2 ]] || { echo "error: $1 requires a BOARD argument" >&2; exit 1; }
+ BOARDS+=("$2")
ARGS+=("$1" "$2")
shift 2
;;
+ --board=*)
+ BOARDS+=("${1#--board=}")
+ ARGS+=("$1")
+ shift
+ ;;
+ # -bt (--board-test) BEFORE the glued -b?* arm, mirroring argparse's longest-match: it is
+ # the form <config>.failed uses, and a bare -b?* would register a board named "t..." that
+ # the roster check below rejects -- killing every documented retry.
+ -bt|--board-test)
+ [[ $# -ge 2 ]] || { echo "error: $1 requires NAME:tests" >&2; exit 1; }
+ ARGS+=("$1" "$2")
+ shift 2
+ ;;
+ -bt?*|--board-test=*)
+ ARGS+=("$1")
+ shift
+ ;;
+ # glued short form: argparse resolves -bNAME to --board NAME, so staging must too --
+ # unparsed it fell through to the all-boards branch and silently staged everything built
+ -b?*)
+ BOARDS+=("${1#-b}")
+ ARGS+=("$1")
+ shift
+ ;;
*)
ARGS+=("$1")
shift
@@ -62,6 +92,107 @@ while [[ $# -gt 0 ]]; do
esac
done
+# Resolve a board to its build dirs: its own dir, the cmake-build-<board>-* glob (ad-hoc
+# local builds), and the variant dirs named in $CONFIG -- variant names are NOT required to
+# be prefixed with the board name, so the glob alone is not enough. Prints one dir per line.
+variant_names() {
+ python3 -c '
+import json, sys
+cfg = json.load(open(sys.argv[1]))
+for b in cfg.get("boards", []):
+ if b["name"] == sys.argv[2]:
+ for v in b.get("variant") or []:
+ print(v["name"])
+' "$CONFIG" "$1"
+}
+
+resolve_build_dirs() {
+ local board="$1" d v
+ declare -A seen=()
+ shopt -s nullglob
+ for d in "$ROOT_DIR"/examples/cmake-build-"$board" "$ROOT_DIR"/examples/cmake-build-"$board"-*; do
+ [[ -d $d && -z ${seen[$d]:-} ]] && { seen[$d]=1; printf '%s\n' "$d"; }
+ done
+ shopt -u nullglob
+ # to a file, not a process substitution: `set -e`/pipefail cannot see the exit status of
+ # the latter, so a malformed roster silently yielded zero variant dirs
+ local vf; vf=$(mktemp)
+ variant_names "$board" > "$vf" || { rm -f "$vf"; echo "Error: could not read variants for $board from $CONFIG" >&2; exit 1; }
+ while IFS= read -r v; do
+ d="$ROOT_DIR/examples/cmake-build-$v"
+ [[ -d $d && -z ${seen[$d]:-} ]] && { seen[$d]=1; printf '%s\n' "$d"; }
+ done < "$vf"
+ rm -f "$vf"
+}
+
+# Pre-flight: EVERY board must resolve to at least one build dir before anything is wiped or
+# copied. This check used to live in the copy loop, so an unbuilt board late in the list
+# aborted the run after the remote tree had been rm -rf'd and earlier boards fully rsynced --
+# zero coverage, a half-staged rig, and a stale local hil_report.md left in place. Report all
+# missing boards at once so one build round fixes them.
+MANIFEST=$(mktemp)
+trap 'rm -f "$MANIFEST"' EXIT
+# Roster membership first: hil_test.py rejects an unknown -b with sys.exit(1) for the WHOLE
+# run (hil_test.py:2297), and it does so AFTER this script has wiped REMOTE_DIR and staged
+# every board -- one typo then costs the entire batch. We already parse $CONFIG here, so
+# catch it before anything is touched. Note -b matches board names only, never variant names.
+if [ ${#BOARDS[@]} -gt 0 ]; then
+ROSTER=$(python3 -c '
+import json, sys
+print("\n".join(b["name"] for b in json.load(open(sys.argv[1])).get("boards", [])))
+' "$CONFIG") || { echo "error: could not read the board roster from $CONFIG" >&2; exit 1; }
+notinroster=()
+for b in ${BOARDS[@]+"${BOARDS[@]}"}; do
+ grep -qxF -- "$b" <<< "$ROSTER" || notinroster+=("$b")
+done
+if [ ${#notinroster[@]} -gt 0 ]; then
+ echo "error: not in $(basename "$CONFIG"): ${notinroster[*]}" >&2
+ echo " (-b takes board names, not variant names)" >&2
+ exit 1
+fi
+fi # BOARDS non-empty: nothing to validate for an all-boards run
+
+missing=()
+for b in ${BOARDS[@]+"${BOARDS[@]}"}; do
+ dirs=$(resolve_build_dirs "$b")
+ if [ -z "$dirs" ]; then
+ missing+=("$b")
+ else
+ while IFS= read -r d; do printf '%s\t%s\n' "$b" "$d" >> "$MANIFEST"; done <<< "$dirs"
+ # A declared variant with no build dir is NOT an error -- no cmake preset is
+ # variant-suffixed, so this is the normal state for e.g. the -DMA variants. It is worth
+ # saying out loud: hil_test.py logs `Skip (no binary)` and counts zero errors for it, so
+ # the run exits 0 and the operator reads a green table for cells that never ran.
+ while IFS= read -r v; do
+ [ -z "$v" ] && continue
+ # whole lines: a substring match lets cmake-build-<v>-DMA silence the warning for <v>
+ grep -qxF -- "$ROOT_DIR/examples/cmake-build-$v" <<< "$dirs" \
+ || echo "warning: $b variant '$v' has no build dir -- its cells will be skipped, not tested" >&2
+ done < <(variant_names "$b")
+ fi
+done
+if [ ${#missing[@]} -gt 0 ]; then
+ echo "Error: no build directory under $ROOT_DIR/examples/ for: ${missing[*]}" >&2
+ for b in "${missing[@]}"; do
+ echo " cd examples && cmake --preset $b && cmake --build --preset $b" >&2
+ done
+ exit 1
+fi
+
+# The all-boards form needs its emptiness check HERE too: below the setup ssh it fired after
+# the remote tree was already rm -rf'd, destroying the previous run's report and re-run spec
+# on the rig before deciding there was nothing to do.
+if [ ${#BOARDS[@]} -eq 0 ]; then
+ shopt -s nullglob
+ allbuilds=("$ROOT_DIR"/examples/cmake-build-*/)
+ shopt -u nullglob
+ if [ ${#allbuilds[@]} -eq 0 ]; then
+ echo "error: no examples/cmake-build-* directories under $ROOT_DIR -- nothing to test" >&2
+ echo " build first, e.g.: cd examples && cmake --preset <board> && cmake --build --preset <board>" >&2
+ exit 1
+ fi
+fi
+
# Setup remote directory. `bash -s` + heredoc so REMOTE_DIR arrives as a positional
# parameter, keeping the `rm -rf` target out of the command string the heredoc runs.
echo "==> Setting up remote $REMOTE:$REMOTE_DIR"
@@ -89,6 +220,7 @@ scp -q "$ROOT_DIR/test/hil/helper/__init__.py" \
"$ROOT_DIR/test/hil/helper/hil_util.py" \
"$ROOT_DIR/test/hil/helper/hil_health.py" \
"$ROOT_DIR/test/hil/helper/hil_lock.py" \
+ "$ROOT_DIR/test/hil/helper/hil_summary.py" \
"$ROOT_DIR/test/hil/helper/hil_select.py" \
"$REMOTE:$REMOTE_DIR/test/hil/helper/"
@@ -103,52 +235,23 @@ copy_board_binaries() {
"$src" "$REMOTE:$REMOTE_DIR/examples/"
}
-if [ -n "$BOARD" ]; then
- # Copy the board's build dir plus its variant dirs. Variant names come from
- # $CONFIG (they are not required to be prefixed with the board name); the
- # cmake-build-<BOARD>-* glob is kept as a fallback for ad-hoc local builds.
- # Collect only dirs that actually exist, deduplicated.
- declare -A SEEN_DIRS=()
- BUILD_DIRS=()
- add_build_dir() {
- [[ -d "$1" && -z "${SEEN_DIRS[$1]:-}" ]] || return 0
- SEEN_DIRS[$1]=1
- BUILD_DIRS+=("$1")
- }
- shopt -s nullglob
- for d in "$ROOT_DIR"/examples/cmake-build-"$BOARD" "$ROOT_DIR"/examples/cmake-build-"$BOARD"-*; do
- add_build_dir "$d"
- done
- shopt -u nullglob
- # to a file, not a process substitution: `set -e`/pipefail cannot see the exit
- # status of the latter, so a malformed roster silently yielded zero variant dirs
- VARIANTS_FILE=$(mktemp)
- python3 -c '
-import json, sys
-cfg = json.load(open(sys.argv[1]))
-for b in cfg.get("boards", []):
- if b["name"] == sys.argv[2]:
- for v in b.get("variant") or []:
- print(v["name"])
-' "$CONFIG" "$BOARD" > "$VARIANTS_FILE" || {
- echo "Error: could not read variants for $BOARD from $CONFIG"
- rm -f "$VARIANTS_FILE"
- exit 1
- }
- while IFS= read -r v; do
- add_build_dir "$ROOT_DIR/examples/cmake-build-$v"
- done < "$VARIANTS_FILE"
- rm -f "$VARIANTS_FILE"
- if [ ${#BUILD_DIRS[@]} -eq 0 ]; then
- echo "Error: no build directory found for $BOARD under $ROOT_DIR/examples/"
- echo "Build first with: cd examples && cmake --preset $BOARD && cmake --build --preset $BOARD"
- exit 1
- fi
- echo "==> Copying binaries for $BOARD (${#BUILD_DIRS[@]} build dir(s))"
- for d in "${BUILD_DIRS[@]}"; do
- copy_board_binaries "$d"
+if [ ${#BOARDS[@]} -gt 0 ]; then
+ # Replay the pre-flight manifest: the dirs were already resolved and proved non-empty
+ # for every board, so nothing here can abort mid-staging. Plain reads of the manifest --
+ # a process substitution would hide a reader failure from set -e (the comment in
+ # resolve_build_dirs is about exactly that trap).
+ for b in "${BOARDS[@]}"; do
+ dirs=()
+ while IFS=$'\t' read -r bb d; do
+ [ "$bb" = "$b" ] && [ -n "$d" ] && dirs+=("$d")
+ done < "$MANIFEST"
+ echo "==> Copying binaries for $b (${#dirs[@]} build dir(s))"
+ for d in ${dirs[@]+"${dirs[@]}"}; do
+ copy_board_binaries "$d"
+ done
done
else
+ # emptiness was already refused in pre-flight, before the remote wipe
echo "==> Copying all built binaries"
# Use `%/` parameter expansion to strip the trailing slash from the glob —
# rsync needs the bare dir name so the per-board cmake-build-<BOARD>/ subdir
@@ -170,14 +273,38 @@ for a in ${ARGS[@]+"${ARGS[@]}"}; do ARGS_Q+=("$(printf '%q' "$a")"); done
CONFIG_Q="$(printf '%q' "test/hil/$(basename "$CONFIG")")"
echo "==> Running HIL test on $REMOTE"
rc=0
-# --retry 1 FIRST, before the user's args: this targets the same shared rig CI uses, and
-# the pool guard is a flat constant that does not scale with max_retry -- argparse's
-# default of 3 lets a few flaky boards re-pay 510s each until the 3600s guard fires,
-# abandoning the pool and holding board flocks against concurrent CI. Placed first, not
-# appended, so argparse's last-wins means `hil_ci.sh -r 3` still gets 3.
-ssh "$REMOTE" bash -s -- "$REMOTE_DIR" --retry 1 ${ARGS_Q[@]+"${ARGS_Q[@]}"} "$CONFIG_Q" <<'REMOTE' || rc=$?
+# --retry 1 FIRST, before the user's args: this targets the same shared rig CI uses, and the
+# pool guard is a flat constant that does not scale with max_retry, so a few flaky boards can
+# re-pay ~510s each until the 3600s guard fires, abandoning the pool and holding board flocks
+# against concurrent CI. hil_test.py's own default is already 1; passing it explicitly keeps
+# that true if the default ever moves. Placed first, not appended, so argparse's last-wins
+# means `hil_ci.sh -r 3` still gets 3.
+# Forward the HIL_* knobs (HIL_NO_BOARD_LOCK for an authorized force, the parallel widths,
+# HIL_POOL_TIMEOUT). ssh passes no environment and joins its argv into one string the remote
+# shell re-splits, so a bare NAME=value element would arrive as a positional argument to
+# hil_test.py and argparse would exit 2. Build `export` lines instead and hand them over as a
+# single %q-quoted word for the remote to eval.
+# Joined with '; ', NOT newlines: %q renders a newline as bash-only $'...' quoting, which the
+# remote LOGIN shell must parse from the joined command string -- under dash the force arrives
+# as garbage and silently does nothing. Backslash escaping round-trips in both shells.
+# HIL_REPORT_DIR stays local: where the report lands on the rig is this script's contract
+# (REMOTE_DIR, where all three copy-backs below look), so forwarding it would relocate the
+# report and every copy-back would come home empty.
+HIL_EXPORTS=""
+while IFS= read -r v; do
+ [ -z "$v" ] && continue
+ HIL_EXPORTS+="export $(printf '%s=%q' "$v" "${!v}"); "
+done < <(compgen -v | grep -x 'HIL_[A-Z0-9_]*' | grep -vxE 'HIL_EXPORTS|HIL_REPORT_DIR' || true)
+[ -n "$HIL_EXPORTS" ] && echo "==> Forwarding: $HIL_EXPORTS"
+# One %q-quoted word, so ssh's argv join and the remote shell's re-split hand it back
+# byte-for-byte, and the remote evals it. Empty stays `''` -- a real, shiftable argument --
+# rather than vanishing from the joined string and shifting the run's own flags out of place.
+HIL_EXPORTS_Q=$(printf '%q' "$HIL_EXPORTS")
+
+ssh "$REMOTE" bash -s -- "$REMOTE_DIR" "$HIL_EXPORTS_Q" --retry 1 ${ARGS_Q[@]+"${ARGS_Q[@]}"} "$CONFIG_Q" <<'REMOTE' || rc=$?
cd -- "$1"
shift
+eval "$1"; shift # HIL_* exports, %q-quoted locally into one word
# Flasher CLIs live in the user bin dirs on ci.lan (esptool/idf in ~/.local/bin,
# STM32CubeProgrammer's STM32_Programmer_CLI in ~/bin); the non-interactive shell
# subprocess used for flashing doesn't source profile/rc, so add them explicitly.
@@ -191,4 +318,15 @@ scp -q "$REMOTE:$REMOTE_DIR/hil_report.md" "$ROOT_DIR/hil_report.md" \
&& echo "==> Report copied to $ROOT_DIR/hil_report.md" \
|| echo "==> warning: no hil_report.md copied back" >&2
+# The re-run spec and the JSON sidecar live in the run's cwd on the rig (REMOTE_DIR), and the
+# next invocation rm -rf's it. Without copying them back, the `--accumulate` retry every doc on
+# this branch prescribes has nothing to read and nothing to merge onto. Delete the local copies
+# FIRST: a green run writes no .failed, so a silent no-op scp would leave last run's spec in
+# the checkout looking current, and "retry from the spec" would re-flash boards that passed.
+for extra in "$(basename "$CONFIG").failed" hil_report.json; do
+ rm -f "$ROOT_DIR/$extra"
+ scp -q "$REMOTE:$REMOTE_DIR/$extra" "$ROOT_DIR/$extra" 2>/dev/null \
+ && echo "==> $extra copied to $ROOT_DIR/$extra" || true
+done
+
exit $rc
diff --git a/test/hil/test/test_hil_bounded.py b/test/hil/test/test_hil_bounded.py
index 908a142d5..c6d454f0e 100644
--- a/test/hil/test/test_hil_bounded.py
+++ b/test/hil/test/test_hil_bounded.py
@@ -1364,7 +1364,14 @@ class RemoteDirIsScreened(unittest.TestCase):
rc = 0 if keep_going else 77
for tool in ('ssh', 'scp', 'rsync'):
write_script(Path(td) / tool, f'echo "stub-{tool} $*" >&2; exit {rc}')
+ # hil_ci.sh now refuses an all-boards run with nothing built, so this arg-quoting
+ # test needs a checkout stub with one build dir to reach the run invocation
+ root = Path(td) / 'root'
+ (root / 'test' / 'hil').mkdir(parents=True)
+ (root / 'test' / 'hil' / 'hil_test.py').touch()
+ (root / 'examples' / 'cmake-build-alpha').mkdir(parents=True)
env = {**os.environ, 'REMOTE_DIR': remote_dir, 'REMOTE': 'stub',
+ 'ROOT_DIR': str(root),
'PATH': td + os.pathsep + os.environ['PATH']}
return subprocess.run(
['bash', str(Path(TEST_DIR).parents[0] / 'hil_ci.sh'), *args],
@@ -1414,6 +1421,316 @@ class RemoteDirIsScreened(unittest.TestCase):
self.assertIn(r'host/cdc\ msc', run_line)
+class EveryBoardIsStaged(unittest.TestCase):
+ """One hil_test.py run takes several `-b` flags, and hil-operator hands it the whole board
+ set that way. The `-b` parse loop kept a single BOARD, so only the LAST board's binaries
+ were rsynced and every other board died on the rig with a missing firmware path -- after
+ its flash slot and lock were already spent.
+
+ Three of these five fail against the pre-fix script (the discriminating unbuilt case
+ puts the board FIRST, because the old single-BOARD parse happened to handle a trailing
+ one correctly); the run-line and variant-dir tests are characterization -- the old script
+ already forwarded ARGS whole and read variants from the config for its one board."""
+
+ def _run(self, boards, cfg_boards=None, variants=None):
+ import json
+ import subprocess
+ td = TemporaryDirectory()
+ self.addCleanup(td.cleanup)
+ root = Path(td.name) / 'root'
+ (root / 'test' / 'hil').mkdir(parents=True)
+ (root / 'test' / 'hil' / 'hil_test.py').touch()
+ built = cfg_boards if cfg_boards is not None else boards
+ (root / 'examples').mkdir(parents=True, exist_ok=True)
+ for b in built:
+ (root / 'examples' / f'cmake-build-{b}').mkdir(parents=True)
+ roster = [{'name': b} for b in boards]
+ for entry in roster:
+ for v in (variants or {}).get(entry['name'], []):
+ entry.setdefault('variant', []).append({'name': v})
+ cfg = root / 'test' / 'hil' / 'cfg.json'
+ cfg.write_text(json.dumps({'boards': roster}))
+ stubs = Path(td.name) / 'bin'
+ stubs.mkdir()
+ # real ssh/scp/rsync would reach the rig; these just record the argv
+ for tool in ('ssh', 'scp', 'rsync'):
+ write_script(stubs / tool, f'echo "stub-{tool} $*" >&2; exit 0')
+ env = {**os.environ, 'REMOTE': 'stub', 'ROOT_DIR': str(root), 'CONFIG': str(cfg),
+ 'PATH': str(stubs) + os.pathsep + os.environ['PATH']}
+ args = [a for b in boards for a in ('-b', b)]
+ r = subprocess.run(['bash', str(Path(TEST_DIR).parents[0] / 'hil_ci.sh'), *args],
+ capture_output=True, text=True, timeout=60, env=env)
+ r.rsyncs = [l for l in r.stderr.splitlines() if l.startswith('stub-rsync')]
+ # the RUN ssh is the one carrying hil_test.py's args; the setup ssh is not
+ r.run_lines = [l for l in r.stderr.splitlines() if '--retry 1' in l]
+ return r
+
+ def test_binaries_for_every_requested_board_are_copied(self):
+ r = self._run(['alpha', 'beta', 'gamma'])
+ self.assertEqual(r.returncode, 0, r.stderr)
+ for b in ('alpha', 'beta', 'gamma'):
+ self.assertTrue(any(f'cmake-build-{b} ' in l for l in r.rsyncs),
+ f'{b} binaries never staged: {r.rsyncs}')
+
+ def test_every_board_reaches_hil_test(self):
+ r = self._run(['alpha', 'beta'])
+ self.assertEqual(len(r.run_lines), 1, r.stderr)
+ self.assertIn('-b alpha', r.run_lines[0])
+ self.assertIn('-b beta', r.run_lines[0])
+
+ def test_an_unbuilt_board_aborts_before_anything_is_staged(self):
+ """The discriminating case: the unbuilt board is FIRST. The pre-fix script kept only
+ the last -b, found it built, and ran happily while silently testing one board. It also
+ has to fail BEFORE staging -- the old in-loop check fired after the remote tree was
+ wiped and earlier boards were rsynced, costing a run and leaving a half-staged rig."""
+ r = self._run(['alpha', 'beta'], cfg_boards=['beta'])
+ self.assertNotEqual(r.returncode, 0, 'unbuilt first board was accepted')
+ self.assertIn('alpha', r.stdout + r.stderr)
+ self.assertEqual(r.rsyncs, [], f'staged despite an unbuilt board: {r.rsyncs}')
+ self.assertEqual(r.run_lines, [], 'reached the run despite an unbuilt board')
+
+ def test_a_board_whose_firmware_is_only_a_variant_dir_is_accepted(self):
+ """Variant names are not required to be prefixed with the board name, so a board can
+ own no `cmake-build-<board>` dir at all. A pre-flight that only globs the board name
+ rejects it and tells the user to build firmware that is already there."""
+ r = self._run(['alpha', 'beta'], cfg_boards=['alpha', 'odd-name-v'],
+ variants={'beta': ['odd-name-v']})
+ self.assertEqual(r.returncode, 0, r.stderr)
+ self.assertTrue(any('cmake-build-odd-name-v ' in l for l in r.rsyncs),
+ f"beta's variant dir never staged: {r.rsyncs}")
+
+ def test_all_unbuilt_boards_are_named_at_once(self):
+ """One build round should fix every complaint, so the guard reports the whole set."""
+ r = self._run(['alpha', 'beta', 'gamma'], cfg_boards=['beta'])
+ self.assertNotEqual(r.returncode, 0)
+ out = r.stdout + r.stderr
+ self.assertIn('alpha', out)
+ self.assertIn('gamma', out)
+
+
+class StagingCoversEveryBoardForm(unittest.TestCase):
+ """hil_test.py declares `-b, --board` with action='append', so argparse accepts --board X,
+ --board=X and -bX too. Staging only the bare form sent boards to the rig with no firmware,
+ where every test logs `Skip (no binary)` and counts zero errors -- a green row for a board
+ that was never flashed. Also covers the roster check, which has to fire BEFORE the remote
+ tree is wiped, since hil_test.py rejects an unknown -b for the whole run."""
+
+ def _run(self, argv, built, roster=None, variants=None, env_extra=None, stale=None):
+ import json
+ import subprocess
+ td = TemporaryDirectory()
+ self.addCleanup(td.cleanup)
+ root = Path(td.name) / 'root'
+ (root / 'test' / 'hil').mkdir(parents=True)
+ (root / 'test' / 'hil' / 'hil_test.py').touch()
+ (root / 'examples').mkdir(parents=True, exist_ok=True)
+ for b in built:
+ (root / 'examples' / f'cmake-build-{b}').mkdir(parents=True)
+ entries = [{'name': b} for b in (roster if roster is not None else built)]
+ for e in entries:
+ for v in (variants or {}).get(e['name'], []):
+ e.setdefault('variant', []).append({'name': v})
+ cfg = root / 'test' / 'hil' / 'cfg.json'
+ cfg.write_text(json.dumps({'boards': entries}))
+ stubs = Path(td.name) / 'bin'
+ stubs.mkdir()
+ # ssh joins its argv into ONE string that the REMOTE shell re-splits, and feeds the
+ # heredoc on stdin. A stub that echoes "$*" hides exactly that, which is how a
+ # completely broken env-forwarding change once passed its own test -- so this stub
+ # re-splits like the real thing and reports the script body separately.
+ write_script(stubs / 'ssh', 'shift; printf "REMOTE-ARGV: %s\\n" "$*" >&2; '
+ 'body=$(cat); printf "REMOTE-BODY: %s\\n" "$body" >&2; exit 0')
+ for tool in ('scp', 'rsync'):
+ write_script(stubs / tool, f'echo "stub-{tool} $*" >&2; exit 0')
+ for name, content in (stale or {}).items():
+ (root / name).write_text(content)
+ env = {**os.environ, 'REMOTE': 'stub', 'ROOT_DIR': str(root), 'CONFIG': str(cfg),
+ 'PATH': str(stubs) + os.pathsep + os.environ['PATH'], **(env_extra or {})}
+ r = subprocess.run(['bash', str(Path(TEST_DIR).parents[0] / 'hil_ci.sh'), *argv],
+ capture_output=True, text=True, timeout=60, env=env)
+ r.rsyncs = [l for l in r.stderr.splitlines() if l.startswith('stub-rsync')]
+ r.run_lines = [l for l in r.stderr.splitlines() if '--retry 1' in l]
+ r.body = '\n'.join(l for l in r.stderr.splitlines() if l.startswith('REMOTE-BODY'))
+ r.stale_left = {name: (root / name).exists() for name in (stale or {})}
+ return r
+
+ def test_long_board_forms_are_staged_and_only_that_board(self):
+ """Two boards are built so the pre-fix 'copy all built binaries' else-branch cannot
+ stage the right one by accident -- that is what made the first version of this test
+ pass against master while the feature was broken. -balpha is the glued short form
+ argparse resolves to --board alpha; unparsed it fell through to the all-boards branch
+ and silently staged everything built with no roster check."""
+ for argv in (['--board', 'alpha'], ['--board=alpha'], ['-balpha']):
+ with self.subTest(argv=argv):
+ r = self._run(argv, built=['alpha', 'beta'], roster=['alpha', 'beta'])
+ self.assertEqual(r.returncode, 0, r.stderr)
+ self.assertTrue(any('cmake-build-alpha ' in l for l in r.rsyncs),
+ f'{argv} never staged: {r.rsyncs}')
+ self.assertFalse(any('cmake-build-beta ' in l for l in r.rsyncs),
+ f'{argv} staged an unrequested board: {r.rsyncs}')
+
+ def test_board_test_flag_is_not_mistaken_for_a_board(self):
+ """-bt is hil_test.py's --board-test and is exactly what <config>.failed contains, so
+ a glued -b?* pattern turns the documented retry into 'not in the roster: t'."""
+ for argv in (['-b', 'alpha', '-bt', 'alpha:device/cdc_msc'],
+ ['-b', 'alpha', '-btalpha:device/cdc_msc']):
+ with self.subTest(argv=argv):
+ r = self._run(argv, built=['alpha'])
+ self.assertEqual(r.returncode, 0, r.stderr)
+ self.assertNotIn('not in', r.stderr)
+ self.assertTrue(any('alpha:device/cdc_msc' in l for l in r.run_lines),
+ f'-bt never reached the rig: {r.run_lines}')
+
+ def test_a_board_outside_the_roster_is_refused_before_staging(self):
+ r = self._run(['-b', 'alpha', '-b', 'ghost'], built=['alpha', 'ghost'], roster=['alpha'])
+ self.assertNotEqual(r.returncode, 0)
+ self.assertIn('ghost', r.stdout + r.stderr)
+ self.assertEqual(r.rsyncs, [], 'staged despite an unknown board')
+ self.assertEqual(r.run_lines, [], 'reached the run despite an unknown board')
+
+ def test_a_variant_with_no_build_dir_warns_instead_of_passing_silently(self):
+ r = self._run(['-b', 'alpha'], built=['alpha'], variants={'alpha': ['alpha-DMA']})
+ self.assertEqual(r.returncode, 0, r.stderr)
+ self.assertIn('alpha-DMA', r.stderr)
+ self.assertIn('skipped, not tested', r.stderr)
+
+ def test_no_build_dirs_at_all_aborts_the_all_boards_form(self):
+ """`hil_ci.sh` with no -b stages everything built. With nothing built it used to wipe
+ the rig, stage nothing, and return a green all-skip table."""
+ r = self._run([], built=[], roster=['alpha'])
+ self.assertNotEqual(r.returncode, 0)
+ self.assertIn('nothing to test', r.stdout + r.stderr)
+ self.assertEqual(r.run_lines, [], 'reached the run with nothing staged')
+ self.assertNotIn('Setting up remote', r.stdout + r.stderr,
+ 'the guard fired only after the remote tree was already wiped')
+
+ def test_hil_env_reaches_the_rig_as_environment_not_argv(self):
+ """An authorized force is HIL_NO_BOARD_LOCK=1. Passed through ssh's argv it arrives as a
+ positional argument and argparse exits 2, so it has to travel in the script body."""
+ r = self._run(['-b', 'alpha'], built=['alpha'], env_extra={'HIL_NO_BOARD_LOCK': '1'})
+ self.assertEqual(r.returncode, 0, r.stderr)
+ # one %q-quoted word of `export NAME=value; ` fragments, evaluated by the remote —
+ # NOT a bare NAME=value element, which hil_test.py's argparse takes as a positional.
+ # %q backslash-escapes the spaces, so match the pieces rather than the plain phrase.
+ run = '\n'.join(r.run_lines)
+ self.assertIn('HIL_NO_BOARD_LOCK=1', run)
+ self.assertIn('export', run)
+ self.assertFalse(any(' HIL_NO_BOARD_LOCK=1 ' in l for l in r.run_lines),
+ 'env reached argv unquoted, where hil_test.py sees a positional')
+
+ def test_a_value_with_spaces_survives_forwarding(self):
+ r = self._run(['-b', 'alpha'], built=['alpha'],
+ env_extra={'HIL_SCRATCH': '/tmp/my scratch'})
+ self.assertEqual(r.returncode, 0, r.stderr)
+ run = '\n'.join(r.run_lines)
+ self.assertIn('HIL_SCRATCH', run)
+ self.assertIn('scratch', run)
+
+ def test_hil_report_dir_is_never_forwarded(self):
+ """Where the report lands on the rig is this script's contract (REMOTE_DIR, where all
+ three copy-backs look); forwarding a local HIL_REPORT_DIR relocates it there and every
+ copy-back comes home empty -- two of the three silently."""
+ r = self._run(['-b', 'alpha'], built=['alpha'],
+ env_extra={'HIL_REPORT_DIR': '/tmp/elsewhere'})
+ self.assertEqual(r.returncode, 0, r.stderr)
+ self.assertFalse(any('HIL_REPORT_DIR' in l for l in r.run_lines),
+ f'HIL_REPORT_DIR reached the rig: {r.run_lines}')
+
+ def test_a_stale_local_failed_spec_does_not_survive_a_green_run(self):
+ """A green run writes no .failed on the rig, so the copy-back scp no-ops; the local
+ spec from a previous FAILED run must not survive it looking current -- a later
+ "retry from the spec" would re-flash boards that already passed."""
+ r = self._run(['-b', 'alpha'], built=['alpha'],
+ stale={'cfg.json.failed': '--accumulate -b alpha'})
+ self.assertEqual(r.returncode, 0, r.stderr)
+ self.assertFalse(r.stale_left['cfg.json.failed'],
+ "last run's re-run spec survived a green run")
+
+
+class SummaryFoldsReportToBoards(unittest.TestCase):
+ """hil_summary.py replaces the agent retyping the markdown table. Report rows are named per
+ VARIANT and a variant need not start with the board name, so the config is what maps them
+ back -- the previous string-matching design produced a defect in each of four review rounds."""
+
+ def _sum(self, boards, rows, cfg_boards=None, banner=''):
+ import json
+ import subprocess
+ td = TemporaryDirectory()
+ self.addCleanup(td.cleanup)
+ d = Path(td.name)
+ (d / 'hil_report.json').write_text(json.dumps(
+ {'rows': [{'board': b, 'cells': c, 'duration': '1s'} for b, c in rows],
+ 'banner': banner}))
+ cfg = d / 'cfg.json'
+ cfg.write_text(json.dumps({'boards': cfg_boards or [{'name': b} for b in boards]}))
+ args = [a for b in boards for a in ('-b', b)]
+ r = subprocess.run(['python3', str(Path(TEST_DIR).parents[0] / 'helper' / 'hil_summary.py'),
+ str(cfg), *args, '--report-dir', str(d)],
+ capture_output=True, text=True, timeout=60)
+ self.assertEqual(r.returncode, 0, r.stderr)
+ return json.loads(r.stdout)['results']
+
+ def test_variant_rows_fold_onto_their_board(self):
+ """nanoch32v203 never produces a row named after the board."""
+ got = self._sum(['nanoch32v203'],
+ [('nanoch32v203-fsdev', {'usbtest': 'pass'}),
+ ('nanoch32v203-usbfs', {'usbtest': 'pass'})],
+ cfg_boards=[{'name': 'nanoch32v203',
+ 'variant': [{'name': 'nanoch32v203-fsdev'},
+ {'name': 'nanoch32v203-usbfs'}]}])
+ self.assertEqual([r['board'] for r in got], ['nanoch32v203'])
+ self.assertTrue(got[0]['pass'])
+ self.assertTrue(got[0]['ran'])
+
+ def test_one_failing_variant_fails_the_board(self):
+ got = self._sum(['nano'],
+ [('nano-a', {'usbtest': 'pass'}), ('nano-b', {'usbtest': '❌ 29/30'})],
+ cfg_boards=[{'name': 'nano', 'variant': [{'name': 'nano-a'},
+ {'name': 'nano-b'}]}])
+ self.assertFalse(got[0]['pass'])
+ self.assertIn('29/30', got[0]['detail'])
+
+ def test_lock_contention_is_a_field_not_a_prefix(self):
+ got = self._sum(['alpha'], [('alpha', {'board-locked': 'fail'})])
+ self.assertTrue(got[0]['locked'])
+ self.assertFalse(got[0]['pass'])
+
+ def test_a_board_with_no_row_is_marked_not_run(self):
+ got = self._sum(['alpha', 'beta'], [('alpha', {'usbtest': 'pass'})])
+ self.assertTrue(got[0]['ran'])
+ self.assertFalse(got[1]['ran'])
+ self.assertFalse(got[1]['pass'])
+
+ def test_a_metric_cell_counts_by_its_icon(self):
+ got = self._sum(['a', 'b'], [('a', {'cdc_msc_throughput': '✅ C 1.2 M 3.4'}),
+ ('b', {'cdc_msc_throughput': '❌ C 0.0 M 0.0'})])
+ self.assertTrue(got[0]['pass'])
+ self.assertFalse(got[1]['pass'])
+
+ def test_skipped_cells_do_not_fail_a_board(self):
+ got = self._sum(['a'], [('a', {'usbtest': 'skip', 'cdc_msc': 'pass'})])
+ self.assertTrue(got[0]['pass'])
+
+ def test_a_plain_metric_cell_is_a_pass(self):
+ """Mirrors hil_test.py's own tally (cell_kind): failures are ALWAYS marked -- 'fail'
+ or a ❌ prefix, per TestFail's docstring -- while a passing test may return a plain
+ metric string that lands in the cell unprefixed. Treating unknown shapes as fail
+ would publish a green table as a red verdict."""
+ got = self._sum(['a'], [('a', {'device_speed': '480.0 MBps'})])
+ self.assertTrue(got[0]['pass'])
+
+ def test_a_declared_variant_of_another_board_is_not_stolen(self):
+ """A declared variant need not start with its own board's name, so it may start with
+ a DIFFERENT board's name plus '-'. The prefix fallback must not attribute it twice."""
+ got = self._sum(['alpha', 'beta'],
+ [('beta-x', {'usbtest': 'fail'})],
+ cfg_boards=[{'name': 'alpha', 'variant': [{'name': 'beta-x'}]},
+ {'name': 'beta'}])
+ self.assertTrue(got[0]['ran'])
+ self.assertFalse(got[0]['pass'])
+ self.assertFalse(got[1]['ran'], "beta must not inherit alpha's row")
+
+
class CaveatSurvivesAccumulate(unittest.TestCase):
"""CI reruns with --accumulate: the sidecar keeps every earlier attempt's cells, but the
banner was recomputed per attempt. A first attempt on a degraded rig and a clean rerun