mirror/firmware
1
0 Fork 0
mirror of https://github.com/meshtastic/firmware.git synced 2026-10-09 06:31:35 -04:00
Code Issues Packages Projects Releases Wiki Activity

test(harness): make run-tests.sh drivable by a caller that cannot see the terminal (#11862)

Browse Source 
* test(harness): make run-tests.sh drivable by a caller that cannot see the terminal

A tool call or a fresh session reads a captured output file, gets interrupted
mid-run, and starts cold. Two things in the wrapper tripped that caller: an
interrupt left pio's build tree running with no record of it (the next
invocation started a second build into the same .pio/build/, or pgrep'd and
matched itself); and pio's own "[PASSED]" / "N succeeded" lines made a
half-finished output file read as green.

run-tests.sh:
- Run record in .pio/runtests/current.tsv for the life of a run. A second
  invocation prints RESULT: BUSY and exits 4 without touching the build
  directory. Valid while the holder pid OR the recorded process group is
  alive, so a SIGKILLed wrapper with live scons children reads ORPHANED
  rather than clear.
- --status / --wait / --abort. --abort kills the whole tree by pgid.
- pio runs under setsid with its pgid recorded; INT/TERM/HUP kill the tree
  and record RESULT: ABORTED (exit 5), log kept.
- One result() for every verdict: prints to the stdout the script started
  with (a signal can arrive inside a redirected pio call) and writes
  .pio/runtests/last-result.tsv with head, args, env, finish time, kept log
  and a tree fingerprint (HEAD + working-tree diff + untracked files).
  --status marks the last verdict STALE when the tree has changed since.
- Banner naming the final RESULT: line as the only verdict.
- Non-Linux host: RESULT: UNSUPPORTED, exit 6, instead of the AMBER code.
- A failed build removes .pio/build/<env>/meshtasticd, which run bare would
  reprint the last good run.
- FILTERED lists the not-run count, not 77 suite names.

bin/run-tests.cmd forwards into WSL with the exit code passed through, so the
same command line works from cmd.exe and PowerShell; no logic is duplicated.

test/README.md, copilot-instructions.md and the mirrors document the new
codes and the rule.

* test(harness): one-suite warm-up, and name the build phase instead of freezing the counter

Measured on a full native run: the warm-up, `pio test --without-testing`
with no filter, builds AND links every suite - 78 links, 2320 s, 29.7 s
each, 39 minutes before the first test ran - and prints a "[PASSED]" line
for each program it merely linked. CI never did this; its warm-up is one
`platformio run`. The shared src objects are the same whichever suite links
them, so the warm-up now links one: the filtered suite when there is one,
else test_utf8. The run itself still builds every suite, as it must.

The heartbeat counted objects newer than its marker, which sits still through
PlatformIO's single-threaded scons dependency scan and through each link -
twelve minutes at "430/754 objs, ETA 17m" on that run, which reads as a hung
build to a caller who cannot run ps. It now names the phase from the
processes in the recorded group: [scons] / [compile] (with the ETA) / [link]
/ [test], and --status prints the same phase word.

* test(harness): define phase_of_run before --status can call it

* test(harness): bind --wait to the run it observed; serialize the run-record publish

Review findings on #11862, all three valid:

- --wait stored a state and then waited for any record to clear, so a run
  that finished and a second that started between polls would be followed to
  the second's verdict, and a run that turned ORPHANED mid-wait could print a
  stale last-result. Each run now has an id (pid-start) in current.tsv and
  last-result.tsv; --wait captures it and reports only a matching verdict,
  else ABORTED-without-verdict.
- run_state() then the current.tsv write was a check-then-act pair: two
  invocations in the same instant could both see IDLE. The pair is now one
  critical section under a short-lived flock, and both records are published
  by rename so no reader can see a partial file. The record stays the
  ownership token; the lock only serializes the handoff (a SIGKILLed holder
  releases flock but not the record, which is why flock alone was rejected).
- The FILTERED and AMBER examples in copilot-instructions.md carried a
  literal suite count, which the same document says never to do.

Verified on a live run: --status RUNNING, a concurrent start refused BUSY, a
--wait started before --abort reported the aborted run's own verdict with the
matching id, exit 5; no build process survived.

* fix(waypoints): do not create an empty store file on clear

clearAllWaypoints() wrote a two-byte empty store unconditionally, so test_waypoint_expiry left
Waypoints_default.wpts behind in every run and the suite has read AMBER (undeclared shared state)
since it landed. An existing file - stale or unreadable included - is still rewritten as empty, so
a reset after a failed load clears flash as before; a file that is not there is left not there.
This commit is contained in:
Tom authored and GitHub committed 2026-09-22 11:10:53 +00:00
1 parent 43479aa4e1
commit 08cd97ea2d
7 files changed
+404 -95
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

.github/copilot-instructions.md
+19 -14
View File
@@ -2,12 +2,12 @@
> **TL;DR**
>
> | | |
> | -------------- | ---------------------------------------------------------------------------------------------------------------------- |
> | Local tests | `./bin/run-tests.sh` (exit 0 GREEN · 1 RED · 2 AMBER · 3 FILTERED) |
> | Hardware tests | [meshtastic/meshtastic-mcp](https://github.com/meshtastic/meshtastic-mcp) (`MESHTASTIC_FIRMWARE_ROOT` → this checkout) |
> | Format | `trunk fmt` |
> | Mirror docs | `AGENTS.md` (short pointer for agents that don't read this file) · `CLAUDE.md` (Claude Code) |
> | | |
> | -------------- | ------------------------------------------------------------------------------------------------------------------------- |
> | Local tests | `./bin/run-tests.sh` (exit 0 GREEN · 1 RED · 2 AMBER · 3 FILTERED · 4 BUSY · 5 ABORTED · 6 UNSUPPORTED); `--status` first |
> | Hardware tests | [meshtastic/meshtastic-mcp](https://github.com/meshtastic/meshtastic-mcp) (`MESHTASTIC_FIRMWARE_ROOT` → this checkout) |
> | Format | `trunk fmt` |
> | Mirror docs | `AGENTS.md` (short pointer for agents that don't read this file) · `CLAUDE.md` (Claude Code) |
>
> **Need this? It's here.**
>
@@ -772,12 +772,15 @@ Unit tests in `test/` directory. The canonical suite count is detected on the fl
Exit codes and verdicts (exact counts will vary; examples below are illustrative):
| Exit | Verdict | Meaning |
| ---- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 0 | `GREEN` | All canonical suites ran, all passed, no ignored test cases |
| 1 | `RED` | At least one failure, build error, or sanitizer fault |
| 2 | `AMBER` | All that ran passed, but something was lost or unexplained: a suite silently went missing on a full run, individual test cases were skipped (`TEST_IGNORE`), or a suite left behind shared state it does not declare |
| 3 | `FILTERED` | A `-f` run completed cleanly; suites outside the filter were intentionally not run |
| Exit | Verdict | Meaning |
| ---- | ------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| 0 | `GREEN` | All canonical suites ran, all passed, no ignored test cases |
| 1 | `RED` | At least one failure, build error, or sanitizer fault |
| 2 | `AMBER` | All that ran passed, but something was lost or unexplained: a suite silently went missing on a full run, individual test cases were skipped (`TEST_IGNORE`), or a suite left behind shared state it does not declare |
| 3 | `FILTERED` | A `-f` run completed cleanly; suites outside the filter were intentionally not run |
| 4 | `BUSY` | A run is already in progress (this or another session); nothing was started. `--status` to see it, `--wait` to attach, `--abort` to stop it |
| 5 | `ABORTED` | The run was stopped by a signal or `--abort`; its log is kept. `--status` shows it |
| 6 | `UNSUPPORTED` | Not a Linux host; nothing ran. Use WSL (`bin\run-tests.cmd` forwards) or `./bin/test-native-docker.sh` |
Examples - exact counts will vary by suite count and env:
@@ -792,12 +795,14 @@ RESULT: RED 1 failed
RESULT: RED exit-time abort (tests passed; likely sanitizer - see hint above)
# AMBER: a suite silently went missing on a full run
RESULT: AMBER 23/24 suites ran (missing: test_radio) - all that ran passed
RESULT: AMBER N-1/N suites ran (missing: test_radio) - all that ran passed
# FILTERED: single suite run completed cleanly
RESULT: FILTERED 1/24 suites ran (not run: test_admin_radio test_atak …) - filtered: test_serial
RESULT: FILTERED 1/N suites ran (N-1 not run) - filtered: test_serial
```
The script is written to be driven by a caller that cannot see the terminal: the final `RESULT:` line is the only verdict (pio's own `[PASSED]` and `N succeeded` lines precede it and mean nothing on their own); a second invocation while a run is in progress is refused with `BUSY` rather than started; the last verdict is kept in `.pio/runtests/last-result.tsv` with its log, and `./bin/run-tests.sh --status` prints it, marking it **STALE** when the tree has changed since. Never `pgrep` for a run - ask `--status`.
> **Copilot interface note:** When running tests via the Copilot chat interface, edits made through the chat may not be reflected in the on-disk files that the test binary reads. If tests pass in chat but fail locally (or vice versa), verify the files on disk match what you expect before trusting the result. Always confirm with a local terminal run.
Raw `pio test` (no sanitizers, no verdict logic) - use only when you need to override the env: