ElixirForge/firmware
1
0
Fork 0
mirror of https://github.com/alexhopeoconnor/firmware.git synced 2026-10-04 03:18:10 +10:00
Code Releases Activity

Replace native-suite-count file with dynamic test discovery (#11413)

Browse Source 
* Derive the native suite count on the fly instead of registering it in a file

test/native-suite-count was a manually-maintained register of the test_*
directory count, reconciled against the actual directories by
bin/run-tests.sh (as an AMBER verdict) and by a dedicated suite-count-check
CI job. The reconciliation only ever guarded the file itself: the check
that matters - suites that actually ran vs. the test_* directories on
disk - already derives its expected count from a directory walk, so the
file added a bookkeeping step to every suite addition/removal without
adding signal.

Remove the file and everything that existed to keep it honest:

- bin/run-tests.sh: drop the canonical-count file read, the count-mismatch
  AMBER verdict, and the [canonical: x/y] suffix; the verdict lines already
  carry ran/expected from the directory walk. The shuffle seed suffix stays.
- test_native.yml: delete the suite-count-check job and its needs: edges.
- Docs (copilot-instructions.md, AGENTS.md, test/README.md) and the
  test-script comments now describe the count as derived from test/test_*
  at run time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APCEfNjd1X7ErDHEzT6Dqd

* Add suite-shrinkage-check: fail a PR that silently loses a test_* suite

With test/native-suite-count gone, nothing in CI noticed the suite set
shrinking: platformio test discovers and runs whatever test_* directories
exist, and bin/run-tests.sh derives its expected count from the same walk,
so a suite directory lost in a bad rebase or an overzealous cleanup just
means fewer suites run - every remaining check stays green.

Restore that tripwire git-aware instead of file-based: on pull_request
runs, compare the test_* directory list at the PR's merge base against the
PR result. A vanished suite fails the job unless its name appears in the
PR title, PR body, or a commit message in the PR's range - a deliberate
removal satisfies that by stating what it removes; an accidental loss
cannot. Other events skip: they have no natural base, and PRs are where
accidents arrive. No job depends on this one (a skipped job would skip
its dependents).

Incidentally: test/ currently holds 47 test_* directories while the
deleted count file said 46 - the manual register had already drifted,
which is exactly the bookkeeping failure mode this replaces.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APCEfNjd1X7ErDHEzT6Dqd

* Re-pad the verdict table after shortening the AMBER row

Shrinking the AMBER cell left the table's column padding inconsistent,
which trunk (prettier + markdownlint MD060) rejects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APCEfNjd1X7ErDHEzT6Dqd

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Jonathan Bennett
2026-08-11 16:16:56 -05:00
committed by GitHub
co-authored by Claude Fable 5
parent d765bd99ca
commit af56a11f00
8 changed files with 81 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/copilot-instructions.md
+13 -16
View File
@@ -313,7 +313,7 @@ firmware/
│ └── native/ # Linux/Portduino variants
├── protobufs/ # Protocol buffer definitions
├── boards/ # Custom PlatformIO board definitions
├── test/ # Native unit-test suites (count: test/native-suite-count)
├── test/ # Native unit-test suites (count = the test_* dirs, detected on the fly)
└── bin/ # Build and utility scripts
```
@@ -663,7 +663,7 @@ Most workflows can be triggered manually via `workflow_dispatch` for testing.
### Native unit tests (C++)
Unit tests in `test/` directory. The canonical suite count is in `test/native-suite-count`, cross-checked against `test/test_*` on every full run and by the `suite-count-check` CI job. **Never state the count as a literal anywhere else** - point at that file. The list below is a partial description of what suites cover, not an inventory:
Unit tests in `test/` directory. The canonical suite count is detected on the fly: the `test_*` directories under `test/` are the register, and `bin/run-tests.sh` cross-checks the suites that actually ran against them on every full run. **Never state the count as a literal anywhere** - it is whatever `test/test_*` contains right now. In CI, the `suite-shrinkage-check` job (`test_native.yml`) fails a PR that loses a `test_*` directory relative to its merge base unless the suite is named in the PR title, body, or a commit message - deleting a suite therefore requires saying so. The list below is a partial description of what suites cover, not an inventory:
- `test_admin_radio/` - LoRa region/config validation, AdminModule dispatch, node-DB metadata saves
- `test_fscommon_getfiles/` - bounded file-manifest walk (cap, depth, truncation reporting)
@@ -693,7 +693,7 @@ Unit tests in `test/` directory. The canonical suite count is in `test/native-su
- `test_utf8/` - UTF-8 utilities
- `test_warm_store/` - Warm-tier node store
**Preferred run command - `bin/run-tests.sh`** (defaults to the `coverage` env; emits a machine-readable verdict on the final line; update `test/native-suite-count` when adding or removing suites):
**Preferred run command - `bin/run-tests.sh`** (defaults to the `coverage` env; emits a machine-readable verdict on the final line; new `test_*` directories are picked up automatically):
```bash
./bin/run-tests.sh # all suites
@@ -712,18 +712,18 @@ Unit tests in `test/` directory. The canonical suite count is in `test/native-su
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`), `test/native-suite-count` disagrees with the `test/` directory count, 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 |
Examples - exact counts will vary by suite count and env:
```text
# GREEN: all suites ran and passed
RESULT: GREEN N/N suites passed [canonical: N/N]
RESULT: GREEN N/N suites passed, all CLEAN
# RED: real test failure
RESULT: RED 1 failed
@@ -731,14 +731,11 @@ RESULT: RED 1 failed
# RED: sanitizer exit-time abort (all tests passed but process aborted at exit)
RESULT: RED exit-time abort (tests passed; likely sanitizer - see hint above)
# AMBER: native-suite-count disagrees with test/ directory count (too low)
RESULT: AMBER test/ has 24 suite directories but native-suite-count says 5 - update test/native-suite-count after registering new suites
# AMBER: native-suite-count disagrees with test/ directory count (too high)
RESULT: AMBER test/ has 24 suite directories but native-suite-count says 99 - update test/native-suite-count after removing suites
# AMBER: a suite silently went missing on a full run
RESULT: AMBER 23/24 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 [canonical: 1/24]
RESULT: FILTERED 1/24 suites ran (not run: test_admin_radio test_atak …) - filtered: test_serial
```
> **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.
.github/workflows/test_native.yml
+41 -31
View File
@@ -23,13 +23,19 @@ env:
LCOV_CAPTURE_FLAGS: --quiet --capture --include "${PWD}/src/*" --exclude '*/src/mesh/generated/*' --directory .pio/build/coverage/src --base-directory "${PWD}"
jobs:
# Guard the registered native-suite total. `platformio test` discovers and runs whatever
# test_* directories exist, so it never notices when test/native-suite-count drifts from the
# actual directory count (a suite added without registering it, or the file left stale). That
# reconciliation only lives in bin/run-tests.sh, which CI does not invoke - so mirror the exact
# check here and fail the PR on a mismatch, keeping the manual count honest.
suite-count-check:
name: Native Suite Count
# Tripwire against the native suite set shrinking by accident. `platformio test` discovers and
# runs whatever test_* directories exist, and bin/run-tests.sh derives its expected count from
# the same walk - so a suite directory lost in a bad rebase or an overzealous cleanup just means
# fewer suites run, and every remaining check stays green. Compare the test_* directory list
# against the PR's merge base and fail when a suite vanished without the PR saying so: a removed
# suite's name must appear in the PR title, the PR body, or a commit message in the PR's range.
# A deliberate removal satisfies that by stating what it removes; an accidental loss cannot.
# Only pull_request runs have a base to compare against (and PRs are where accidents arrive);
# every other event skips. No job depends on this one: a skipped job would skip its dependents,
# and the expensive jobs should not wait on a full-history clone.
suite-shrinkage-check:
name: Native Suite Shrinkage
if: github.event_name == 'pull_request'
runs-on: ubuntu-slim
permissions:
contents: read
@@ -37,40 +43,45 @@ jobs:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with:
persist-credentials: false
# Full history: the merge base must be computed, not guessed from a possibly stale
# event payload, and the acknowledgment scan reads every commit message in the range.
fetch-depth: 0
- name: Reconcile native-suite-count with test/ directories
- name: Fail if a test_* suite vanished unacknowledged
shell: bash
# PR title/body are attacker-controlled text; they reach the script through env: only,
# never spliced into the shell source (same rule as the suite-order seed below).
env:
BASE_REF: ${{ github.base_ref }}
PR_TITLE: ${{ github.event.pull_request.title }}
PR_BODY: ${{ github.event.pull_request.body }}
run: |
set -euo pipefail
count_file="test/native-suite-count"
if [[ ! -f $count_file ]]; then
echo "::error title=Missing native-suite-count::$count_file not found - it must record the number of test_* suite directories."
exit 1
git fetch --quiet origin "$BASE_REF"
base=$(git merge-base FETCH_HEAD HEAD)
# Same canonical set every other consumer derives: directories named test_* directly
# under test/, read from the git trees so the comparison is exact at both endpoints.
list_suites() { git ls-tree -d --name-only "$1" test/ | sed 's#^test/##' | grep '^test_' | sort; }
removed=$(comm -23 <(list_suites "$base") <(list_suites HEAD))
if [[ -z $removed ]]; then
echo "No suite removed: $(list_suites HEAD | wc -l) test_* directories, none lost since merge base ${base:0:8}."
exit 0
fi
# Same canonical set as bin/run-tests.sh: directories named test_* directly under test/.
expected_count=$(find test -maxdepth 1 -type d -name 'test_*' -printf '%f\n' | wc -l)
canonical_count=$(tr -d '[:space:]' <"$count_file")
if ! [[ $canonical_count =~ ^[0-9]+$ ]]; then
echo "::error title=Invalid native-suite-count::$count_file must contain a single integer, got '$canonical_count'."
exit 1
fi
echo "test/ directories: $expected_count"
echo "native-suite-count: $canonical_count"
if [[ $expected_count -ne $canonical_count ]]; then
if [[ $expected_count -gt $canonical_count ]]; then
hint="a suite was added - bump $count_file to $expected_count"
messages=$(git log --format=%B "$base..HEAD")
fail=0
while IFS= read -r suite; do
if printf '%s\n%s\n%s\n' "$PR_TITLE" "$PR_BODY" "$messages" | grep -qF "$suite"; then
echo "Removed suite $suite is named in the PR title/body or a commit message - acknowledged."
else
hint="a suite was removed - lower $count_file to $expected_count"
echo "::error title=Native suite vanished::test/$suite exists on the merge base but is gone from this PR, and nothing in the PR title, body, or commit messages mentions it. If the removal is deliberate, name $suite in the PR description or a commit message; if not, restore the directory - platformio test would silently run without it."
fail=1
fi
echo "::error title=native-suite-count mismatch::test/ has $expected_count suite directories but $count_file says $canonical_count ($hint)."
exit 1
fi
echo "native-suite-count matches the $expected_count suite directories."
done <<<"$removed"
exit $fail
simulator-tests:
name: Native Simulator Tests
runs-on: ubuntu-24.04-arm
needs: suite-count-check
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with:
@@ -169,7 +180,6 @@ jobs:
platformio-tests:
name: Native PlatformIO Tests
runs-on: ubuntu-24.04-arm
needs: suite-count-check
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with: