diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index 04e6b3a23..2167e2956 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -195,6 +195,13 @@ jobs: timeout-minutes: 5 run: ./bin/test-config-check.sh .pio/build/coverage/meshtasticd + - name: Shared-state checker self-test + # Fixtures that write nothing / exactly what they declare / something undeclared / + # a declared write they never make, asserting CLEAN / CLEAN / DIRTY / MISSING. A + # checker that has silently stopped matching looks identical to a clean codebase. + timeout-minutes: 5 + run: ./bin/test-state-check.sh + - name: Integration test # Cap the whole step: if the simulator ever fails to exit (e.g. the # exit_simulator admin path regresses again) the job must fail fast, @@ -273,9 +280,12 @@ jobs: restore-keys: | pio-coverage-tests- - - name: Build test programs once - # One shared build of src + every test program. This is the single source build; gcov then - # accumulates coverage counts into this shared .pio/build/coverage/src as the chunks run. + - name: Warm the shared test build + # Compiles src + every test program once so no single area absorbs the whole src build in + # its reported duration; gcov then accumulates counts into this shared + # .pio/build/coverage/src as the areas run. NOT a substitute for building in the run step: + # PlatformIO links every test program to the one .pio/build/coverage/meshtasticd path, so a + # --without-building run executes whichever suite was linked last under every suite's name. run: platformio test -e coverage --without-testing - name: Save PlatformIO cache @@ -368,12 +378,21 @@ jobs: echo "::group::area $a (${group[$a]# })" # Capture platformio's real exit status (not grep's) via a log file, then show the log # with the noisy per-variant SKIPPED rows filtered out. - if ! platformio test -e coverage --without-building -v ${group[$a]# } \ + if ! platformio test -e coverage -v ${group[$a]# } \ --junit-output-path "testreport-$a.xml" > "area-$a.log" 2>&1; then fail=1 echo "::error::area $a had test failures" fi + # Suites outside this area are reported SKIPPED by design (PlatformIO lists every suite + # in the env and marks the unselected ones finished), so those rows are noise here. The + # attribution check below is what catches a suite that was selected and did not run. grep -v "[[:space:]]SKIPPED$" "area-$a.log" || true + # Per area, so a mismatch names the area it happened in rather than the whole run. + if ! ./bin/check-test-attribution.py --label "area $a" \ + --expect "${group[$a]# }" "testreport-$a.xml"; then + fail=1 + echo "::error::area $a ran suites that did not match their own test binaries" + fi echo "::endgroup::" done exit $fail @@ -398,6 +417,18 @@ jobs: ET.ElementTree(out).write('testreport.xml', encoding='utf-8', xml_declaration=True) PY + - name: Verify every suite ran its own tests + # Whole-run gate over the merged report: every test_* directory must appear with at least + # one test case, and every case must come from the suite that reported it. The per-area + # check above cannot see an area that never executed - this can. + if: always() # a suite going missing is the finding; do not hide it behind an earlier failure + shell: bash + run: | + set -euo pipefail + mapfile -t suites < <(find test -maxdepth 1 -type d -name 'test_*' -printf '%f\n' | sort) + ./bin/check-test-attribution.py --label "coverage (all areas)" \ + --expect "${suites[*]}" testreport.xml + - name: Capture coverage information if: always() # run this step even if previous step failed run: | @@ -405,9 +436,31 @@ jobs: lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name tests --output-file coverage_tests.info sed -i -e "s#${PWD}#.#" coverage_tests.info # Make paths relative. + - name: Attribution canary + # Guards the guard above: runs two suites the broken way (--without-building, so PlatformIO + # does not relink and both execute the same leftover binary) and requires the checker to + # catch it. Fails if the checker regressed, or if the reproduction stops reproducing - in + # which case the reason both harnesses stopped passing that flag no longer holds. + # + # Lives in this job, not simulator-tests: it relinks $BUILD_DIR/$PROGNAME, and there that + # replaced the daemon binary with a test suite, so the integration test waited for a socket + # a test binary never opens. Here the binary is already per-suite and nothing later needs it. + timeout-minutes: 15 + run: ./bin/test-attribution-canary.sh -e coverage + - name: Event channel policy tests run: platformio test -e coverage-event-policy -v --junit-output-path event-policy-testreport.xml + - name: Verify the event-policy suites ran their own tests + # Expected set read through PlatformIO's own config parser, so it cannot drift from the + # env's test_filter the way a second hand-maintained list would. + run: | + set -euo pipefail + expect=$(python3 -c "from platformio.project.config import ProjectConfig; \ + print(' '.join(ProjectConfig().get('env:coverage-event-policy', 'test_filter', [])))") + ./bin/check-test-attribution.py --label coverage-event-policy \ + --expect "$expect" event-policy-testreport.xml + - name: Save test results if: always() # run this step even if previous step failed uses: actions/upload-artifact@v7 diff --git a/bin/check-test-attribution.py b/bin/check-test-attribution.py new file mode 100755 index 000000000..2d3d258e4 --- /dev/null +++ b/bin/check-test-attribution.py @@ -0,0 +1,165 @@ +#!/usr/bin/env python3 +"""Verify each PlatformIO JUnit report ran the suite it claims to have run. + +PlatformIO links every native test program to one path ($BUILD_DIR/$PROGNAME) and parses +Unity output textually, without checking that the reported source file belongs to the suite +it is running. Split a run into `--without-testing` then `--without-building` and every suite +executes whichever binary was linked last, all reporting PASSED. This reads the JUnit reports +that run already produces and fails on the two shapes that hides: + + MISATTRIBUTED - a test case whose source file lives outside the suite that reported it + EMPTY - a suite that was asked to run and produced no test cases at all + +Usage: + check-test-attribution.py [--expect "s1 s2"]... [--label TEXT] REPORT.xml... + +--expect names the suites the run was asked for (repeatable, whitespace- or `-f`-separated, +so a CI area string can be passed through verbatim). Omit it to check attribution only. +Exit: 0 clean, 1 findings, 2 bad usage / unreadable report. +""" + +import argparse +import glob +import sys +import xml.etree.ElementTree as ET + + +def parse_expect(values): + """Flatten repeated --expect values into a suite list, tolerating `-f suite` tokens.""" + suites = [] + for value in values or []: + for token in value.split(): + if token == "-f": + continue + suites.append(token.removeprefix("-f")) + return [s for s in suites if s] + + +def suite_of(testsuite_name): + """`coverage:test_foo` -> `test_foo`; a bare name is returned unchanged.""" + return testsuite_name.split(":", 1)[1] if ":" in testsuite_name else testsuite_name + + +def owns(suite, source_file): + """Report whether source_file sits inside the suite's own directory. + + Matched on a whole path segment so `test_mesh` does not claim `test_mesh_module`, and + with a leading separator so absolute and relative paths behave the same. + """ + normalized = "/" + source_file.replace("\\", "/").lstrip("/") + return f"/{suite}/" in normalized + + +def collect(paths): + """Map suite -> list of (case name, source file or None), merged across reports.""" + cases = {} + for path in paths: + try: + # The input is the JUnit report PlatformIO just wrote in this same run, not untrusted + # data, and defusedxml is not installed for this job. + # nosemgrep: python.lang.security.use-defused-xml-parse.use-defused-xml-parse + root = ET.parse(path).getroot() + except (ET.ParseError, OSError) as exc: + sys.stderr.write(f"check-test-attribution: cannot read {path}: {exc}\n") + sys.exit(2) + # PlatformIO nests under ; accept a bare too. + nodes = [root] if root.tag == "testsuite" else root.iter("testsuite") + for node in nodes: + suite = suite_of(node.get("name", "")) + if not suite: + continue + entries = cases.setdefault(suite, []) + for case in node.iter("testcase"): + entries.append((case.get("name", "?"), case.get("file"))) + return cases + + +def main(): + parser = argparse.ArgumentParser(add_help=True) + parser.add_argument("--expect", action="append", default=[]) + parser.add_argument("--label", default="") + parser.add_argument("reports", nargs="+") + args = parser.parse_args() + + # Expand globs ourselves: CI passes a pattern that may match nothing if a step was skipped, + # and a silent pass over zero reports is exactly the false green this script exists to stop. + paths = sorted({p for pattern in args.reports for p in glob.glob(pattern)}) + if not paths: + sys.stderr.write( + "check-test-attribution: no JUnit reports matched %s\n" + % " ".join(args.reports) + ) + return 2 + + cases = collect(paths) + expected = parse_expect(args.expect) + + misattributed = [] # (suite, case name, source file) + unsourced = [] # (suite, case name) + for suite, entries in sorted(cases.items()): + for name, source in entries: + if source is None: + unsourced.append((suite, name)) + elif not owns(suite, source): + misattributed.append((suite, name, source)) + + empty = [s for s in expected if not cases.get(s)] + + label = f" [{args.label}]" if args.label else "" + total = sum(len(v) for v in cases.values()) + print( + f"test attribution{label}: {len(paths)} report(s), " + f"{len([s for s, v in cases.items() if v])} suite(s) with cases, {total} case(s)" + ) + if unsourced: + print("") + print("UNSOURCED - these cases carry no source file, so ownership cannot be proved:") + for suite, name in unsourced[:20]: + print(f" {suite}: case '{name}'") + if len(unsourced) > 20: + print(f" ... +{len(unsourced) - 20} more") + print( + "A report without file attributes is not evidence that the suites ran their own" + ) + print( + "tests. Treat it as a finding rather than a pass: the JUnit format has changed, or" + ) + print("the runner emitted cases it could not attribute.") + + if misattributed: + print("") + print( + "MISATTRIBUTED - these suites reported test cases belonging to another suite." + ) + print( + "The run executed one suite's binary under another suite's name; the named" + ) + print( + "suites did NOT run. Check for --without-building in the test invocation." + ) + for suite, name, source in misattributed[:20]: + print(f" {suite}: case '{name}' came from {source}") + if len(misattributed) > 20: + print(f" ... +{len(misattributed) - 20} more") + + if empty: + print("") + print("EMPTY - these suites were asked to run and produced no test cases:") + for suite in empty: + print(f" {suite}") + + if misattributed or empty or unsourced: + print("") + print( + "RESULT: test attribution FAILED" + f"{label} ({len(misattributed)} misattributed, {len(empty)} empty," + f" {len(unsourced)} unsourced)" + ) + return 1 + + print(f"RESULT: test attribution OK{label}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/bin/lib/test-state.sh b/bin/lib/test-state.sh index a0c624455..ce77fdd2e 100644 --- a/bin/lib/test-state.sh +++ b/bin/lib/test-state.sh @@ -167,3 +167,53 @@ state_classify() { printf 'CLEAN\t\n' fi } + +# --- Error-line budget ------------------------------------------------------------------------- +# +# A second orthogonal axis, like CLEAN/DIRTY above: a suite can pass while emitting six figures of +# LOG_ERROR, which buries a real failure and trains everyone to skim. The budget is declared in the +# same manifest, as an `errors=` flag, and it is a RANGE rather than a ceiling - for a fuzz suite the +# floor is the load-bearing half. test_fuzz_decode logging ~100k rejections is it working; the same +# suite logging none means it stopped feeding malformed input, and every case would still pass. +# +# Undeclared suites get ERROR_BUDGET_DEFAULT. Declared forms: "N" (max), "MIN..MAX", "MIN.." (floor +# only). Everything is inclusive. +ERROR_BUDGET_DEFAULT=100 + +# Count LOG_ERROR lines in a suite's captured output. +state_count_errors() { + local log="$1" + [[ -f $log ]] || { + printf '0' + return 0 + } + # `|| true`, not `|| printf 0`: grep -c already prints 0 before exiting 1 on no match, so a + # fallback that prints appends a second line and the caller gets "0\n0" to do arithmetic on. + grep -cE '^ERROR +\|' "$log" 2>/dev/null || true +} + +# VERDICTDETAIL. WITHIN / OVER / UNDER, mirroring state_classify()'s shape. +state_classify_errors() { + local count="$1" declared="$2" min=0 max="$ERROR_BUDGET_DEFAULT" + + if [[ -n $declared ]]; then + if [[ $declared == *".."* ]]; then + min="${declared%%..*}" + max="${declared##*..}" + [[ -z $max ]] && max="" + else + max="$declared" + fi + fi + + if [[ -n $max ]] && ((count > max)); then + printf 'OVER\t%d error line(s), budget %s' "$count" "${declared:-$ERROR_BUDGET_DEFAULT}" + return 0 + fi + if ((count < min)); then + printf 'UNDER\t%d error line(s), expected at least %d - is it still exercising the path?' \ + "$count" "$min" + return 0 + fi + printf 'WITHIN\t%d' "$count" +} diff --git a/bin/pio-test-isolate.sh b/bin/pio-test-isolate.sh index bd58c73eb..bfc51cffd 100755 --- a/bin/pio-test-isolate.sh +++ b/bin/pio-test-isolate.sh @@ -92,6 +92,11 @@ GRANULARITY="$(state_flag_value state "$FLAGS")" IFS=$'\t' read -r VERDICT DETAIL <<<"$(state_classify "$CHANGED" "$DECLARED")" +# Error-line budget: same manifest, same declare-and-justify shape as the writes above. Counted from +# the captured log, so it costs nothing extra. +ERROR_COUNT="$(state_count_errors "$LOG")" +IFS=$'\t' read -r ERROR_VERDICT ERROR_DETAIL <<<"$(state_classify_errors "$ERROR_COUNT" "$(state_flag_value errors "$FLAGS")")" + # Per-test attribution, when the suite has not declared that it carries state across its own test # cases. For a state=per-suite suite every test after the first would be flagged by design - that # carry *is* the declared behaviour - so only the suite boundary is meaningful there. @@ -114,14 +119,14 @@ fi STATUS=$([[ $RC -eq 0 ]] && echo PASS || echo FAIL) mkdir -p "$(dirname "$SUMMARY")" 2>/dev/null -printf '%s\t%s\t%s\t%s\t%s\t%s\n' "$SUITE" "$STATUS" "$VERDICT" "${DETAIL-}" "${PER_TEST_DETAIL-}" \ - "${SURVIVORS-}" >>"$SUMMARY" +printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' "$SUITE" "$STATUS" "$VERDICT" "${DETAIL-}" "${PER_TEST_DETAIL-}" \ + "${SURVIVORS-}" "${ERROR_VERDICT-}" "${ERROR_DETAIL-}" >>"$SUMMARY" # Keep the sandbox when there is something to look at: on a failure it plus the built binary is a # complete, replayable reproduction, and on a DIRTY verdict the leftovers *are* the bug report. A # clean pass leaves nothing behind. KEEP="${MESHTASTIC_TEST_KEEP_STATE:-0}" -if [[ $RC -ne 0 || $VERDICT != CLEAN || -n ${SURVIVORS-} || $KEEP == 1 ]]; then +if [[ $RC -ne 0 || $VERDICT != CLEAN || $ERROR_VERDICT != WITHIN || -n ${SURVIVORS-} || $KEEP == 1 ]]; then DEST="$STATE_ROOT/$SUITE" rm -rf "$DEST" 2>/dev/null mv "$SCRATCH" "$DEST" 2>/dev/null || DEST="$SCRATCH" diff --git a/bin/run-tests.sh b/bin/run-tests.sh index dcc3a705f..f454d5c8f 100755 --- a/bin/run-tests.sh +++ b/bin/run-tests.sh @@ -38,7 +38,8 @@ # test/state-manifest.tsv. # FILTERED - a -f run completed cleanly; suites not in the filter were intentionally skipped. # Use this when iterating on a single suite; it is not a quality signal. -# RED - at least one failure, build error, or sanitizer fault. +# RED - at least one failure, build error, sanitizer fault, or a suite that reported +# another suite's test cases (bin/check-test-attribution.py). # # Two orthogonal axes: PASS/FAIL × CLEAN/DIRTY. Each suite runs in its own scratch $HOME # (bin/pio-test-isolate.sh), so leftovers are harmless; DIRTY means "undeclared", not "dangerous". @@ -59,6 +60,7 @@ # RESULT: AMBER N/M suites ran (missing: test_radio test_serial) - all that ran passed # RESULT: AMBER 3 test case(s) ignored # RESULT: FILTERED 1/N suites ran (not run: …) - filtered: test_utf8 +# RESULT: RED test attribution failed - suites did not run their own tests # RESULT: RED test_traffic_management: 1 failed (or: build/crash error) # RESULT: RED sanitizer fault - SUMMARY: AddressSanitizer: 1272 byte(s) leaked (tests may have # all passed; the coverage build aborts at exit on an ASan/LSan fault - often shown only @@ -163,6 +165,16 @@ export MESHTASTIC_TEST_STATE_SUMMARY="$STATE_SUMMARY" $KEEP_STATE && export MESHTASTIC_TEST_KEEP_STATE=1 $WRITE_MANIFEST && export MESHTASTIC_TEST_KEEP_STATE=1 +# --- Test attribution -------------------------------------------------------- +# PlatformIO parses Unity output textually and never checks that the source file a case came from +# belongs to the suite it thinks it ran, so one suite's binary running under another's name reads +# as a pass. The JUnit reports carry both halves (testsuite@name vs testcase@file), so collect them +# here and grade with bin/check-test-attribution.py below. Cleared first: a stale report from an +# earlier run would otherwise satisfy this run's expectations. +ATTRIB_DIR="$ROOT_DIR/.pio/test-attribution" +rm -rf "$ATTRIB_DIR" +mkdir -p "$ATTRIB_DIR" + # Canonical suite set = the directories in test/, detected on the fly. This is the sole source # of truth for "what should run"; a filtered run only expects its filtered suite. mapfile -t ALL_SUITES < <(find test -maxdepth 1 -type d -name 'test_*' -printf '%f\n' | sort) @@ -251,10 +263,15 @@ if $SHUFFLE; then echo "suite order: shuffled with --seed $SEED (${#RUN_ORDER[@]} suites)" fi -# Build every test program before running any of them, the way .github/workflows/test_native.yml +# Warm the shared src objects before running any suite, the way .github/workflows/test_native.yml # does. Fused build+run makes whichever suite PlatformIO's directory walk reaches first absorb the # whole src compile and report it as its own duration - that is how a 35s suite once reported 13 # minutes, and it hides the build cost from every timing the summary prints. +# +# This is a WARM-UP ONLY: the run below must still build. PlatformIO links every test program to +# the one $BUILD_DIR/$PROGNAME path, so a `--without-building` run executes whichever suite was +# linked last - every suite, under its own name, all PASSED. The warm-up keeps the src compile out +# of the suite timings; the per-suite step is then just one test_main.cpp plus a link. BUILD_SECS=0 build_started=$SECONDS if $QUIET; then @@ -289,19 +306,23 @@ if $SHUFFLE; then : >"$LOG" for suite in "${RUN_ORDER[@]}"; do if $QUIET; then - "$PIO" test -e "$ENV" -f "$suite" "${EXTRA_ARGS[@]}" --without-building >>"$LOG" 2>&1 + "$PIO" test -e "$ENV" -f "$suite" "${EXTRA_ARGS[@]}" \ + --junit-output-path "$ATTRIB_DIR/$suite.xml" >>"$LOG" 2>&1 rc=$? else - "$PIO" test -e "$ENV" -f "$suite" "${EXTRA_ARGS[@]}" --without-building 2>&1 | tee -a "$LOG" + "$PIO" test -e "$ENV" -f "$suite" "${EXTRA_ARGS[@]}" \ + --junit-output-path "$ATTRIB_DIR/$suite.xml" 2>&1 | tee -a "$LOG" rc=${PIPESTATUS[0]} fi ((rc != 0)) && PIO_RC=$rc done elif $QUIET; then - "$PIO" test -e "$ENV" "${PASSTHRU[@]}" --without-building >"$LOG" 2>&1 + "$PIO" test -e "$ENV" "${PASSTHRU[@]}" \ + --junit-output-path "$ATTRIB_DIR/all.xml" >"$LOG" 2>&1 PIO_RC=$? else - "$PIO" test -e "$ENV" "${PASSTHRU[@]}" --without-building 2>&1 | tee "$LOG" + "$PIO" test -e "$ENV" "${PASSTHRU[@]}" \ + --junit-output-path "$ATTRIB_DIR/all.xml" 2>&1 | tee "$LOG" PIO_RC=${PIPESTATUS[0]} fi @@ -426,6 +447,18 @@ verdict_red() { exit 1 fi + # A guard in test/TestUtil.cpp aborting on purpose - a listening socket, or force_simradio put + # back. It prints FATAL on stdout precisely so this can be told apart from a fault: otherwise its + # exit(EXIT_FAILURE) lands in the heuristic below and is reported as a sanitizer abort that never + # happened, which is the same wrong-cause-in-the-verdict trap as the phantom signal above. + if grep -qE '^FATAL: ' "$LOG"; then + grep -E '^FATAL: ' "$LOG" | head -3 | sed 's/^/ /' + echo " -> a harness guard aborted the suite deliberately. Not a crash and not a sanitizer" + echo " fault; the reason is the FATAL line above, and the suite's sandbox has the full log." + echo "RESULT: RED harness guard - $(grep -m1 -oE '^FATAL: .*' "$LOG")" + exit 1 + fi + # All tests passed but the process still aborted at EXIT (ERRORED/SIGHUP/SIGABRT) and the # sanitizer report was swallowed by the runner (often surfaced only as SIGHUP). Almost always a # sanitizer fault - point at how to surface it rather than calling it a generic crash. @@ -462,6 +495,34 @@ verdict_suffix() { echo "$rating" } +# --- Attribution axis --------------------------------------------------------- +# RED, and checked before every softer verdict: a suite that reported another suite's test cases +# did not run at all, so every count and state verdict below it is measuring the wrong thing. A +# filtered run expects only its own suite; a full run expects the canonical set. +# -f takes an fnmatch pattern, not necessarily a suite name, so resolve it against the canonical +# set rather than expecting a suite literally called "test_nodedb*". An unmatched pattern leaves +# the list empty, which checks attribution only - a filter that selects nothing is already RED +# above, for want of a pass summary. +ATTRIB_EXPECT="${ALL_SUITES[*]}" +if [[ -n $FILTER ]]; then + ATTRIB_EXPECT="" + for attrib_suite in "${ALL_SUITES[@]}"; do + # shellcheck disable=SC2053 # deliberate glob match: FILTER is a pattern, not a literal + [[ $attrib_suite == $FILTER ]] && ATTRIB_EXPECT+="$attrib_suite " + done +fi +ATTRIB_OUT="$("$SCRIPT_DIR/check-test-attribution.py" --expect "$ATTRIB_EXPECT" \ + --label "$ENV" "$ATTRIB_DIR"/*.xml 2>&1)" +ATTRIB_RC=$? +if ((ATTRIB_RC != 0)); then + echo "" + echo "$ATTRIB_OUT" | sed 's/^/ /' + preserve_run_log + echo "RESULT: RED test attribution failed - suites did not run their own tests $(verdict_suffix)" + exit 1 +fi +$QUIET || echo "$ATTRIB_OUT" | tail -1 + # --- Shared-state axis -------------------------------------------------------- # Read what the per-suite wrapper recorded. Reported after the count checks so a structural problem # still wins, and before the pass/fail verdict lines so the state summary always prints. @@ -472,6 +533,7 @@ if [[ -f $STATE_SUMMARY ]]; then mapfile -t DIRTY_SUITES < <(awk -F'\t' '$3 == "DIRTY" { print $1 " (" $4 ")" }' "$STATE_SUMMARY") mapfile -t MISSING_SUITES < <(awk -F'\t' '$3 == "MISSING" { print $1 " (" $4 ")" }' "$STATE_SUMMARY") mapfile -t SURVIVOR_SUITES < <(awk -F'\t' '$6 != "" { print $1 " (pid " $6 ")" }' "$STATE_SUMMARY") + mapfile -t ERROR_BUDGET_SUITES < <(awk -F'\t' '$7 == "OVER" || $7 == "UNDER" { print $1 " " tolower($7) " budget: " $8 }' "$STATE_SUMMARY") fi # Print the opt-out count on every run, so the number creeping upward is visible without anyone @@ -551,6 +613,21 @@ if ((${#DIRTY_SUITES[@]} > 0)); then exit 2 fi +# AMBER: a suite spent its LOG_ERROR budget, or came in under a declared floor. Over budget buries a +# real failure in noise - three log sites account for nearly all of today's volume, and until those +# are demoted this stays AMBER rather than RED so it does not land red on day one and get switched +# off. Under a floor is the more interesting half: a fuzz suite that stops logging rejections has +# stopped feeding malformed input, and every one of its cases still passes. +if ((${#ERROR_BUDGET_SUITES[@]} > 0)); then + echo "" + printf ' %s\n' "${ERROR_BUDGET_SUITES[@]}" + echo "" + echo " -> over: demote the log line if the condition is expected, or declare errors= in" + echo " test/state-manifest.tsv with a reason. Under: check the suite still exercises the path." + echo "RESULT: AMBER ${#ERROR_BUDGET_SUITES[@]} suite(s) outside their error budget $(verdict_suffix)" + exit 2 +fi + # AMBER: a suite was still running after PlatformIO reported it. A bare UNITY_END() ends the # reporting, not the process - the runtime goes on calling loop() - so the suite passes, the run goes # green, and the binary stays resident. The wrapper has already killed it, but the consequences do diff --git a/bin/stress-suite.sh b/bin/stress-suite.sh new file mode 100755 index 000000000..ec63612b1 --- /dev/null +++ b/bin/stress-suite.sh @@ -0,0 +1,202 @@ +#!/usr/bin/env bash +# +# Run one native test suite repeatedly and report how often it fails. +# +# For order-independent flakes - a real-time race, a slow-host margin, an uninitialised read - a +# single green run proves nothing. This runs the same built binary N times and prints a flake rate, +# so "passes here" becomes a measurement instead of an anecdote. +# +# ./bin/stress-suite.sh test_pki_admin_fallback # 20 runs, coverage, as CI invokes it +# ./bin/stress-suite.sh -n 200 test_packet_signing # 200 runs +# ./bin/stress-suite.sh -e native -n 50 test_admin_radio # the other env's invocation +# ./bin/stress-suite.sh -l 8 -n 50 test_pki_admin_fallback # 8 spinners of CPU contention +# ./bin/stress-suite.sh --no-simradio -n 50 test_packet_signing +# ./bin/stress-suite.sh --shuffle -n 5 # whole suite set, a new order each time +# +# --shuffle is the other axis and takes no suite name: it drives bin/run-tests.sh --seed with a fresh +# seed per iteration, so suite ORDER varies. Use it for state that leaks suite -> suite; use the +# single-suite mode above for races and slow-host margins, which order cannot expose. Every seed is +# printed, and a red one is replayable with ./bin/run-tests.sh --seed . +# +# Each run gets a fresh scratch $HOME, so no run inherits another's prefs. Failing runs keep their +# log and their $HOME; passing runs leave nothing behind. +# +# Exit: 0 = every run passed, 1 = at least one failed, 2 = usage/build error. + +set -uo pipefail + +REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +ENV_NAME=coverage +RUNS=20 +LOAD=0 +SIMRADIO=auto +SHUFFLE=false +SUITE="" + +usage() { + sed -n '3,20p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' + exit 2 +} + +# A missing or non-numeric value used to sail through and produce a loop that never ran, reporting +# "0/0 failed" as a pass. Reject it at parse time instead. +need_value() { + [[ -n ${2:-} && $2 != -* ]] || { + echo "$1 needs a value" >&2 + exit 2 + } +} +need_number() { + [[ $2 =~ ^[0-9]+$ ]] || { + echo "$1 needs a number, got '$2'" >&2 + exit 2 + } +} + +while [[ $# -gt 0 ]]; do + case "$1" in + -e | --environment) + need_value "$1" "${2:-}" + ENV_NAME="$2" + shift 2 + ;; + -n | --runs) + need_value "$1" "${2:-}" + need_number "$1" "$2" + RUNS="$2" + shift 2 + ;; + -l | --load) + need_value "$1" "${2:-}" + need_number "$1" "$2" + LOAD="$2" + shift 2 + ;; + --shuffle) + SHUFFLE=true + shift + ;; + --simradio) + SIMRADIO=yes + shift + ;; + --no-simradio) + SIMRADIO=no + shift + ;; + -h | --help) usage ;; + -*) + echo "unknown option: $1" >&2 + usage + ;; + *) + SUITE="$1" + shift + ;; + esac +done + +if $SHUFFLE; then + [[ -z $SUITE ]] || { + echo "--shuffle varies suite order across the whole set; drop the suite name" >&2 + exit 2 + } + fails=0 + reds=() + echo "running the full suite set x$RUNS on $ENV_NAME, reshuffled each time" + for ((run = 1; run <= RUNS; run++)); do + # Seeds from /dev/urandom, printed and recorded: an order you cannot replay is not evidence. + seed=$((RANDOM * 32768 + RANDOM)) + log="$REPO/.pio/build/$ENV_NAME/stress-shuffle.$seed.log" + mkdir -p "$(dirname "$log")" + printf 'run %d/%d seed %s ... ' "$run" "$RUNS" "$seed" + if "$REPO/bin/run-tests.sh" -e "$ENV_NAME" --seed "$seed" >"$log" 2>&1; then + echo "GREEN" + rm -f "$log" + else + rc=$? + fails=$((fails + 1)) + reds+=("$seed") + echo "$(grep -m1 '^RESULT:' "$log" || echo "exit $rc") - log $log" + fi + done + echo "RESULT: $fails/$RUNS runs not green" + [[ ${#reds[@]} -gt 0 ]] && echo "replay: ./bin/run-tests.sh --seed ${reds[0]}" + [[ $fails -eq 0 ]] || exit 1 + exit 0 +fi + +[[ -n $SUITE ]] || usage + +# Mirror what the env's test_testing_command passes, so a stress run reproduces the real invocation +# rather than a third one of its own. [env:coverage] adds -s (simradio); [env:native] does not. +if [[ $SIMRADIO == auto ]]; then + # Read to the next [section] header, not a fixed window: -s is the last line of the command block. + if awk "/^\\[env:$ENV_NAME\\]/{f=1;next} /^\\[/{f=0} f" \ + "$REPO/variants/native/portduino/platformio.ini" | grep -qE '^[[:space:]]+-s[[:space:]]*$'; then + SIMRADIO=yes + else + SIMRADIO=no + fi +fi +ARGS=() +[[ $SIMRADIO == yes ]] && ARGS+=(-s) + +PIO="$REPO/.pio_env/bin/pio" +[[ -x $PIO ]] || PIO="$(command -v pio)" || { + echo "pio not found" >&2 + exit 2 +} + +BIN="$REPO/.pio/build/$ENV_NAME/meshtasticd" +echo "building $SUITE for $ENV_NAME ..." +"$PIO" test -e "$ENV_NAME" -f "$SUITE" --without-testing >/dev/null 2>&1 || { + echo "build failed - rerun without --without-testing to see why" >&2 + exit 2 +} +[[ -x $BIN ]] || { + echo "no binary at $BIN" >&2 + exit 2 +} + +LOADPIDS=() +cleanup() { + [[ ${#LOADPIDS[@]} -gt 0 ]] && kill "${LOADPIDS[@]}" 2>/dev/null + return 0 +} +# EXIT cleans up; INT/TERM must also stop, or the loop keeps launching runs after a ^C. +trap cleanup EXIT +trap 'cleanup; exit 130' INT +trap 'cleanup; exit 143' TERM + +if [[ $LOAD -gt 0 ]]; then + echo "starting $LOAD spinner(s) against $(nproc) cpu(s)" + for ((i = 0; i < LOAD; i++)); do + (while :; do :; done) & + LOADPIDS+=($!) + done +fi + +OUT="$REPO/.pio/build/$ENV_NAME/stress" +mkdir -p "$OUT" +fails=0 +echo "running $SUITE x$RUNS on $ENV_NAME (simradio=$SIMRADIO)" +for ((run = 1; run <= RUNS; run++)); do + scratch=$(mktemp -d) + log="$OUT/$SUITE.$run.log" + # Through pio-test-isolate.sh, not the bare binary: that is what test_testing_command runs, so + # a repetition here exercises the sandboxing, survivor reaping and state verdict too. + if MESHTASTIC_TEST_STATE_DIR="$scratch/state" "$REPO/bin/pio-test-isolate.sh" "$BIN" "${ARGS[@]}" >"$log" 2>&1; then + rm -rf "$scratch" "$log" + printf '.' + else + fails=$((fails + 1)) + printf '\nRUN %d FAILED - log %s - state %s\n' "$run" "$log" "$scratch" + grep -E ':(FAIL|IGNORE)' "$log" | head -5 + fi +done +printf '\n' + +pct=$((fails * 100 / RUNS)) +echo "RESULT: $fails/$RUNS failed (${pct}%)" +[[ $fails -eq 0 ]] || exit 1 diff --git a/bin/test-attribution-canary.sh b/bin/test-attribution-canary.sh new file mode 100755 index 000000000..cb873e6a8 --- /dev/null +++ b/bin/test-attribution-canary.sh @@ -0,0 +1,82 @@ +#!/usr/bin/env bash +# Canary for bin/check-test-attribution.py: reproduce the false green on purpose and require the +# checker to catch it. +# +# The attribution check exists because both harnesses once ran every suite against whichever binary +# was linked last, so all 57 reported a pass while five test programs actually executed. A checker +# for that is only worth having if it still fires, and a checker that has quietly stopped firing +# looks exactly like a codebase with no problem. So: build two suites, run them the broken way +# (--without-building, which is what stops PlatformIO relinking on a non-embedded platform), and +# assert the checker reports a mismatch. +# +# It also fails if the reproduction stops reproducing - if PlatformIO ever relinks per suite under +# --without-building, the premise behind dropping that flag no longer holds and the harness should +# be revisited rather than left resting on a stale assumption. +# +# Not a Unity suite and not a test_* directory, so it stays outside the suite count run-tests.sh +# derives from test/ - same arrangement as bin/test-state-check.sh and bin/test-config-check.sh. +# +# Usage: ./bin/test-attribution-canary.sh [-e ] (default: coverage, as CI runs) + +set -uo pipefail + +REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO" || exit 2 + +ENV_NAME=coverage +[[ ${1-} == "-e" ]] && ENV_NAME="$2" + +PIO="$REPO/.pio_env/bin/pio" +[[ -x $PIO ]] || PIO="$(command -v pio)" || { + echo "canary: pio not found" >&2 + exit 2 +} + +# Two suites whose cases cannot be confused: different source files, different counts. Both are +# small and neither touches shared state, so the canary costs a link rather than a rebuild. +A=test_utf8 +B=test_breakout +REPORT="$(mktemp -d)/canary.xml" + +echo "canary: building $A and $B for $ENV_NAME" +"$PIO" test -e "$ENV_NAME" -f "$A" -f "$B" --without-testing >/dev/null 2>&1 || { + echo "canary: build failed" >&2 + exit 2 +} + +echo "canary: running them the broken way (--without-building)" +"$PIO" test -e "$ENV_NAME" -f "$A" -f "$B" --without-building --junit-output-path "$REPORT" >/dev/null 2>&1 + +[[ -s $REPORT ]] || { + echo "canary: no JUnit report at $REPORT - cannot judge the checker" >&2 + exit 2 +} + +# The checker must FAIL here, and fail for the RIGHT reason. Exit 1 is a finding; exit 2 is bad +# usage or an unreadable report, which would let a broken canary read as a caught mismatch. +OUT="$(./bin/check-test-attribution.py --label "canary" "$REPORT" 2>&1)" +RC=$? +if [[ $RC -eq 2 ]]; then + echo "" + echo "CANARY INCONCLUSIVE: the checker could not read the report it was given (exit 2)." + echo "$OUT" + echo "Report kept at: $REPORT" + exit 2 +fi +if [[ $RC -eq 0 ]] || ! grep -q 'MISATTRIBUTED' <<<"$OUT"; then + echo "" + echo "CANARY FAILED: the attribution check passed a run that mis-attributes its cases." + echo "" + echo "Two suites were run with --without-building, so PlatformIO did not relink and both" + echo "executed the same leftover binary. check-test-attribution.py is supposed to catch exactly" + echo "that and it did not, which means the guard against the whole false-green class is dead." + echo "" + echo "Either the checker regressed, or PlatformIO now relinks per suite under --without-building" + echo "- in which case the reason bin/run-tests.sh and CI stopped passing that flag has changed," + echo "and the harness should be revisited rather than left on a stale assumption." + echo "Report kept at: $REPORT" + exit 1 +fi + +echo "canary: OK - the attribution check caught the deliberate mis-attribution" +rm -rf "$(dirname "$REPORT")" diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index e4f52c779..c382e8576 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -809,6 +809,18 @@ RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p) static uint32_t adminKeyFallbackTokens = ADMIN_KEY_FALLBACK_BURST; static uint32_t adminKeyFallbackRefillMs = 0; +#ifdef PIO_UNIT_TESTING +// The refill stamp is a timestamp, so it is only meaningful against the clock that produced it. +// A suite that swaps between the real and the virtual clock leaves a stamp from the other +// timebase, and the next unsigned subtraction reads as a near-infinite gap: the bucket silently +// refills to full. Re-stamp when the clock changes. +void resetAdminKeyFallbackBudget() +{ + adminKeyFallbackTokens = ADMIN_KEY_FALLBACK_BURST; + adminKeyFallbackRefillMs = Time::getMillis(); +} +#endif + static bool adminKeyFallbackAllowed() { bool haveAdminKey = false; @@ -821,7 +833,8 @@ static bool adminKeyFallbackAllowed() if (!haveAdminKey) return false; // nothing to try, so do not spend a token - uint32_t now = millis(); + // Injectable clock so the budget can be tested without sleeping, and without racing a slow host. + uint32_t now = Time::getMillis(); if (adminKeyFallbackRefillMs == 0) adminKeyFallbackRefillMs = now; uint32_t elapsed = now - adminKeyFallbackRefillMs; diff --git a/src/mesh/Router.h b/src/mesh/Router.h index eb1213de3..9882a4b9c 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -265,6 +265,8 @@ RoutingAuthVerdict passesRoutingAuthGate(meshtastic_MeshPacket *p); #ifdef PIO_UNIT_TESTING uint32_t routingAuthEvaluationCount(); void resetRoutingAuthEvaluationCount(); +/** Refill the admin-key fallback budget and re-stamp it against the clock in use right now. */ +void resetAdminKeyFallbackBudget(); #endif /** Return 0 for success or a Routing_Error code for failure diff --git a/test/README.md b/test/README.md index d1dbd804c..ceb819206 100644 --- a/test/README.md +++ b/test/README.md @@ -33,6 +33,8 @@ Randomisation costs one `pio` invocation per suite (about 4.7s each), because Pl > **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. +**Never add `--without-building` to a test run.** PlatformIO links every native test program to the single `$BUILD_DIR/$PROGNAME` path and attributes Unity output by text alone, so a run that only builds beforehand executes whichever suite was linked last under _every_ suite's name - all reporting PASSED. Build once with `--without-testing` to warm the shared src objects if you like; the run itself must still build. `bin/check-test-attribution.py` grades the JUnit reports for exactly this and is wired into both `bin/run-tests.sh` (RED) and CI. + **Raw `pio test` (no sanitizers, no verdict logic)** - use when you need to override the env or inspect verbose Unity output: ```bash diff --git a/test/TestUtil.cpp b/test/TestUtil.cpp index 58cd34c15..9f36ee17a 100644 --- a/test/TestUtil.cpp +++ b/test/TestUtil.cpp @@ -18,21 +18,149 @@ // The state checkpoint needs a POSIX directory walk, and only the host builds run these suites. // Note ARDUINO *is* defined on portduino, so it is not the right guard here. #if ARCH_PORTDUINO +#include "platform/portduino/PortduinoGlue.h" #include #include #include #include #include #include +#include #include #include +#include #endif +#if ARCH_PORTDUINO +// A test binary must not be reachable from the network. main.cpp's setup()/loop() are compiled out +// under PIO_UNIT_TESTING, so the phone API, MQTT and the web server are never started - but that is +// a property of today's guards, not something anything checks. A suite that pulled in a service +// which binds a port would otherwise open one on the developer's machine, silently, for the length +// of the run. Assert the absence instead of trusting it. +// +// Listening sockets only: an outbound connection is a different (and louder) problem, and gethostby* +// opens transient sockets that would make an any-socket check flap. +static void assertNoListeningSockets() +{ + // Socket fds appear as "socket:[inode]"; a listening TCP row in /proc/self/net carries st 0A. + std::set ours; + if (DIR *fds = opendir("/proc/self/fd")) { + while (struct dirent *e = readdir(fds)) { + char path[64], target[128]; + snprintf(path, sizeof(path), "/proc/self/fd/%s", e->d_name); + ssize_t n = readlink(path, target, sizeof(target) - 1); + if (n <= 0) + continue; + target[n] = '\0'; + unsigned long inode = 0; + if (sscanf(target, "socket:[%lu]", &inode) == 1) + ours.insert(std::to_string(inode)); + } + closedir(fds); + } + if (ours.empty()) + return; + + std::string offenders; + for (const char *table : {"/proc/self/net/tcp", "/proc/self/net/tcp6"}) { + FILE *f = fopen(table, "r"); + if (!f) + continue; + char line[512]; + bool header = true; + while (fgets(line, sizeof(line), f)) { + if (header) { + header = false; + continue; + } + // sl local_address rem_address st tx:rx tr:when retrnsmt uid timeout inode + char local[128] = {0}; + unsigned st = 0, uid = 0; + unsigned long inode = 0; + if (sscanf(line, "%*d: %127s %*s %x %*s %*s %*s %u %*d %lu", local, &st, &uid, &inode) != 4) + continue; + if (st != 0x0A) // TCP_LISTEN + continue; + if (ours.count(std::to_string(inode)) == 0) + continue; + offenders += " "; + offenders += local; + } + fclose(f); + } + if (offenders.empty()) + return; + + // Before UNITY_BEGIN(), so there is no Unity failure to record - and a test binary that has + // opened a port is not a result worth collecting. Fail the suite outright and say why. + fprintf(stderr, + "FATAL: test binary is listening on%s\n" + "A unit-test run must not be reachable. Something started a network service - check what\n" + "the suite constructs, and whether it belongs behind main.cpp's PIO_UNIT_TESTING guard.\n", + offenders.c_str()); + fflush(stderr); + exit(EXIT_FAILURE); +} +#endif + +#if ARCH_PORTDUINO +static bool environmentBaselined = false; + +// -s is how the harness keeps a test run off the host's radio: it makes portduinoSetup() skip the +// /etc/meshtasticd/config.yaml search and return before GPIO/SPI init. That job is done by the time +// any of this runs, and the flag's only remaining readers are behaviour we do want under test - +// wouldEncryptWithPKC() disables PKC while it is set. Clear it so suites exercise the production +// encode path; the radio choice is already made and is not revisited. +static void baselineEnvironment() +{ + portduino_config.force_simradio = false; + assertNoListeningSockets(); + environmentBaselined = true; +} +#endif + +void testAssertEnvironmentIntact(const char *testName) +{ +#if ARCH_PORTDUINO + // Not every suite calls initializeTestEnvironment() - test_atak does not - so the baseline + // cannot live only there, or those suites run with PKC off and skip the socket check. Establish + // it at the first RUN_TEST for whoever has not, and hold it from then on. + if (!environmentBaselined) { + baselineEnvironment(); + return; + } + + // Per test, not once per suite: a service that binds a port is opened by the code under test, + // not by the harness, so checking only at startup would miss every case that starts one. + assertNoListeningSockets(); + + if (!portduino_config.force_simradio) + return; + + // Hard exit rather than TEST_FAIL: this runs between tests, outside any Unity test frame, so + // there is no failure to longjmp into. Repairing the flag silently would be worse - it would + // leave the suite that broke it passing. + for (FILE *out : {stdout, stderr}) + fprintf(out, + "FATAL: force_simradio was set back on before %s\n" + "PKC is disabled while it is set, so the encode path under test falls back to channel\n" + "crypto and every later case asserts the wrong thing. A test that needs simradio must\n" + "restore the flag before it returns.\n", + testName ? testName : "(unknown test)"); + fflush(stderr); + exit(EXIT_FAILURE); +#else + (void)testName; +#endif +} + void initializeTestEnvironment() { concurrency::hasBeenSetup = true; consoleInit(); #if ARCH_PORTDUINO + baselineEnvironment(); + struct timeval tv; tv.tv_sec = time(NULL); tv.tv_usec = 0; diff --git a/test/TestUtil.h b/test/TestUtil.h index bb56d1096..d2a145e52 100644 --- a/test/TestUtil.h +++ b/test/TestUtil.h @@ -17,6 +17,14 @@ void testDelay(unsigned long ms); // place instead of being spread across 40-odd suites. void testStateCheckpoint(const char *testName, const char *sourceFile); +// Checked before every test, because the environment a suite starts in is not the one it keeps. +// initializeTestEnvironment() clears force_simradio once, and a test that sets it - directly, or by +// restoring a struct it saved before the clear - silently disables PKC for every test after it. +// wouldEncryptWithPKC() would then return false and the encode path would quietly fall back to +// channel crypto, which is a passing test asserting the wrong thing. Named per test so the culprit +// is the test that follows the one that broke it. +void testAssertEnvironmentIntact(const char *testName); + // Every RUN_TEST becomes a checkpoint. An unintended write has no matching assertion *by // definition* - nobody wrote a TEST_ASSERT for the nodes.proto write that broke test_admin_radio, // because nobody knew it happened - so attribution has to come from outside the test body. @@ -27,6 +35,7 @@ void testStateCheckpoint(const char *testName, const char *sourceFile); #undef RUN_TEST #define RUN_TEST(func, ...) \ do { \ + testAssertEnvironmentIntact(#func); \ UnityDefaultTestRun(func, #func, __LINE__); \ testStateCheckpoint(#func, __FILE__); \ } while (0) diff --git a/test/state-manifest.tsv b/test/state-manifest.tsv index 7420504e8..02870c573 100644 --- a/test/state-manifest.tsv +++ b/test/state-manifest.tsv @@ -39,21 +39,30 @@ # add, for a human to paste and justify. It never applies them itself, and CI never applies them at # all - an auto-accepted baseline is the same rot as an auto-updated snapshot. # +# errors= | .. | .. caps a suite's LOG_ERROR lines, default 100. A range, not a +# ceiling: for a fuzz suite the floor is the half that matters. test_fuzz_decode logging ~100k +# rejections is the suite working; the same suite logging none means it stopped feeding malformed +# input, and every case would still pass. Bounds are wide on purpose - they catch a path that has +# stopped running, not a drift of a few hundred lines. +# # suite flags reason -test_admin_radio writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat,Messages_default.msgs per-test NodeDB fixture, and the admin handlers under test persist config, channels and node metadata +test_admin_radio writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat,Messages_default.msgs errors=400 per-test NodeDB fixture, and the admin handlers under test persist config, channels and node metadata test_admin_session_repro writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB, whose constructor persists a default set when the prefs directory is empty +test_event_channel_phone_api writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB, whose constructor persists a default set when the prefs directory is empty +test_event_channel_router writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto errors=200 subclasses NodeDB for the event-channel fixtures; the base constructor persists a default set when the prefs directory is empty test_firmware_edition writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto persists an event firmware_edition in devicestate, then reboots a NodeDB to prove a vanilla build resets it -test_fuzz_packets writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat,Messages_default.msgs drives decode of fuzzed packets through the real NodeDB and message store +test_fuzz_decode errors=20000..250000 fuzzes protobuf decode; every rejection logs. A collapse to near zero means the corpus stopped reaching the decoder +test_fuzz_packets writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat,Messages_default.msgs errors=5000..60000 drives decode of fuzzed packets through the real NodeDB and message store test_hop_scaling writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB to hold the hop-distance fixtures test_mesh_beacon writes=module.proto exercises the beacon's module-config save path test_mesh_module writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat module framework tests construct a NodeDB -test_mqtt writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB for node lookups in the MQTT paths +test_mqtt writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto errors=1000..12000 constructs a NodeDB for node lookups in the MQTT paths test_nexthop_routing writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto next-hop selection reads and updates the node DB test_nodedb_blocked state=per-suite writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat saturates the DB with MAX_NUM_NODES-2 favourited nodes to test the protected cap; a later test's removeNodeByNum() persists that state, and the cap test depends on the fill from the test before it -test_packet_signing writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat needs a NodeDB holding both peers' keys for the PKI encode/decode paths +test_packet_signing writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat errors=300 needs a NodeDB holding both peers' keys for the PKI encode/decode paths test_pki_admin_fallback writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto needs a NodeDB holding admin keys for the fallback paths test_stream_api writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto drives real PhoneAPI handshakes, which read and persist config and the node DB test_traceroute_nexthop writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto traceroute route selection reads the node DB -test_traffic_management writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat constructs a NodeDB for the per-node rate-limit and dedup state +test_traffic_management writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat errors=3000..12000 constructs a NodeDB for the per-node rate-limit and dedup state; test_tm_fuzz_nodenum_blitz feeds malformed payloads, and each rejection logs (measured 7985) test_transmit_history writes=transmit_history.dat persistence round-trip: what it asserts is that retransmission state survives a save/load test_warm_store writes=warm.dat persistence round-trip of the warm-tier snapshot, which is the tier's whole contract diff --git a/test/test_admin_session_repro/test_main.cpp b/test/test_admin_session_repro/test_main.cpp index c93c1fd94..36efc9a41 100644 --- a/test/test_admin_session_repro/test_main.cpp +++ b/test/test_admin_session_repro/test_main.cpp @@ -21,10 +21,6 @@ #include "support/MockMeshService.h" #include -#ifdef ARCH_PORTDUINO -#include "platform/portduino/PortduinoGlue.h" -#endif - static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; static constexpr NodeNum ADMIN_NODE = 0x0B0B0B0B; // authorized admin, sends remote admin to us static constexpr NodeNum QUERIED_NODE = 0x0C0C0C0C; // a remote we send admin requests to @@ -161,14 +157,6 @@ void setUp(void) nodeDB = mockNodeDB; myNodeInfo.my_node_num = LOCAL_NODE; -#ifdef ARCH_PORTDUINO - // The native test harness boots Portduino in simulated mode, and wouldEncryptWithPKC() - // hard-disables PKC whenever force_simradio is set. Left true, no outgoing admin request is - // ever key-pinned, so the pinning tests below cannot exercise what they are asserting. - // Model a real (non-sim) device instead. - portduino_config.force_simradio = false; -#endif - config = meshtastic_LocalConfig_init_zero; // A real device always holds a private key; without one perhapsEncode never picks PKC. config.security.private_key.size = 32; diff --git a/test/test_event_channel_router/test_main.cpp b/test/test_event_channel_router/test_main.cpp index c1f5e8bec..82700a795 100644 --- a/test/test_event_channel_router/test_main.cpp +++ b/test/test_event_channel_router/test_main.cpp @@ -10,9 +10,6 @@ #include "mesh/MeshService.h" #include "mesh/NodeDB.h" #include "mesh/Router.h" -#if ARCH_PORTDUINO -#include "platform/portduino/PortduinoGlue.h" -#endif #include #include #include @@ -121,9 +118,6 @@ struct SavedGlobals { MeshService *service; AirTime *airTime; concurrency::Lock *cryptLock; -#if ARCH_PORTDUINO - bool forceSimRadio; -#endif }; SavedGlobals saved; @@ -319,9 +313,6 @@ void setUp(void) saved.service = service; saved.airTime = airTime; saved.cryptLock = cryptLock; -#if ARCH_PORTDUINO - saved.forceSimRadio = portduino_config.force_simradio; -#endif testNodeDB = new TestNodeDB(); testNodeDB->clearTestNodes(); @@ -335,9 +326,6 @@ void setUp(void) memset(&myNodeInfo, 0, sizeof(myNodeInfo)); myNodeInfo.my_node_num = kLocalNode; service = nullptr; -#if ARCH_PORTDUINO - portduino_config.force_simradio = false; -#endif installChannels(); testAirTime = new AirTime(); @@ -379,9 +367,6 @@ void tearDown(void) router = saved.router; service = saved.service; airTime = saved.airTime; -#if ARCH_PORTDUINO - portduino_config.force_simradio = saved.forceSimRadio; -#endif } EVENT_ROUTER_TEST_ENTRY void setup() diff --git a/test/test_geocoord_distance/test_main.cpp b/test/test_geocoord_distance/test_main.cpp index de3430f1c..e54498a15 100644 --- a/test/test_geocoord_distance/test_main.cpp +++ b/test/test_geocoord_distance/test_main.cpp @@ -1,3 +1,8 @@ +// Deliberately does NOT include TestUtil.h. This suite is pure-function - no NodeDB, no router, no +// sockets, no PKC - so the harness-wide guards there (no listening sockets, force_simradio clear) +// would assert conditions it cannot reach, and initializeTestEnvironment()'s RTC and OSThread setup +// would add portduino globals it otherwise never touches. Suite-level state cleanliness is still +// checked from outside by bin/pio-test-isolate.sh, which wraps every suite regardless. #include "configuration.h" #include "gps/GeoCoord.h" #include diff --git a/test/test_meshpacket_serializer/test_serializer.cpp b/test/test_meshpacket_serializer/test_serializer.cpp index db863ca3c..0ddb4ca0b 100644 --- a/test/test_meshpacket_serializer/test_serializer.cpp +++ b/test/test_meshpacket_serializer/test_serializer.cpp @@ -1,3 +1,8 @@ +// Deliberately does NOT include TestUtil.h. This suite is pure-function - no NodeDB, no router, no +// sockets, no PKC - so the harness-wide guards there (no listening sockets, force_simradio clear) +// would assert conditions it cannot reach, and initializeTestEnvironment()'s RTC and OSThread setup +// would add portduino globals it otherwise never touches. Suite-level state cleanliness is still +// checked from outside by bin/pio-test-isolate.sh, which wraps every suite regardless. #include "test_helpers.h" #include #include diff --git a/test/test_pki_admin_fallback/test_main.cpp b/test/test_pki_admin_fallback/test_main.cpp index 5c3b408c9..127d4f999 100644 --- a/test/test_pki_admin_fallback/test_main.cpp +++ b/test/test_pki_admin_fallback/test_main.cpp @@ -9,6 +9,7 @@ // The whole feature is compiled out when PKI is excluded. #if !(MESHTASTIC_EXCLUDE_PKI) +#include "UptimeClock.h" #include "mesh/Channels.h" #include "mesh/CryptoEngine.h" #include "mesh/NodeDB.h" @@ -148,6 +149,11 @@ void setUp(void) void tearDown(void) { + // The rate-limit case drives a virtual timebase; leave the real clock for everyone else, and + // re-stamp the budget so the next case does not measure a virtual stamp against real millis. + Time::useRealClock(); + resetAdminKeyFallbackBudget(); + delete mockNodeDB; mockNodeDB = nullptr; nodeDB = nullptr; @@ -205,8 +211,10 @@ void test_wrong_admin_key_does_not_decode(void) // The fallback is budget-limited against flooding; see Router.cpp for why the budget is global. void test_admin_key_fallback_is_rate_limited(void) { - // Start from a full bucket regardless of what earlier tests consumed (8 tokens, one per 250ms). - delay(2500); + // Drive the virtual clock: on the wall clock the eight decodes below have to beat the 250ms + // refill, which is ~31ms each - CI misses that and the bucket refills mid-drain. + Time::setTestMillis(1000000); + resetAdminKeyFallbackBudget(); // re-stamp against the virtual clock we just switched to uint8_t otherPub[32], otherPriv[32]; crypto->generateKeyPair(otherPub, otherPriv); @@ -225,7 +233,7 @@ void test_admin_key_fallback_is_rate_limited(void) TEST_ASSERT_NOT_EQUAL_MESSAGE(DECODE_SUCCESS, perhapsDecode(&blocked), "fallback should be budget-limited"); // The budget refills, so the throttle is not a permanent lockout. - delay(600); + Time::advanceTestMillis(600); meshtastic_MeshPacket allowed = makePkiPacket(ADMIN_NODE, meshtastic_PortNum_PRIVATE_APP, 16, adminPriv); TEST_ASSERT_EQUAL_MESSAGE(DECODE_SUCCESS, perhapsDecode(&allowed), "budget should refill over time"); assertDecodedAndLearned(&allowed, adminPub); diff --git a/test/test_position_precision/test_main.cpp b/test/test_position_precision/test_main.cpp index fae50e87f..7497b42f6 100644 --- a/test/test_position_precision/test_main.cpp +++ b/test/test_position_precision/test_main.cpp @@ -8,10 +8,6 @@ #include #include #include -#if ARCH_PORTDUINO -#include "platform/portduino/PortduinoGlue.h" -#endif - static meshtastic_Position makePosition() { meshtastic_Position position = meshtastic_Position_init_default; @@ -332,9 +328,6 @@ static void test_eventCoordinatePolicy_coversPortsAndExcludesPki() waypoint.to = 0x12345678; config.security.private_key.size = 32; owner.is_licensed = false; -#if ARCH_PORTDUINO - portduino_config.force_simradio = false; -#endif TEST_ASSERT_TRUE(willUsePki(&waypoint)); TEST_ASSERT_FALSE(isBlockedEventCoordinatePacket(&waypoint)); #else diff --git a/test/test_traffic_management/test_main.cpp b/test/test_traffic_management/test_main.cpp index cfe06e7f5..d7d947d5a 100644 --- a/test/test_traffic_management/test_main.cpp +++ b/test/test_traffic_management/test_main.cpp @@ -211,6 +211,8 @@ class TrafficManagementModuleTestShim : public TrafficManagementModule MockNodeDB *mockNodeDB = nullptr; +static void installWellKnownPrimaryChannel(); // defined below, next to the other channel fixtures + static void resetTrafficConfig() { moduleConfig = meshtastic_LocalModuleConfig_init_zero; @@ -220,7 +222,9 @@ static void resetTrafficConfig() config = meshtastic_LocalConfig_init_zero; config.device.role = meshtastic_Config_DeviceConfig_Role_CLIENT; - channelFile = meshtastic_ChannelFile_init_zero; + // A real device always has a primary channel; leaving channels_count at 0 made every router + // lookup log "Invalid channel index", 12k lines of it, without testing anything. + installWellKnownPrimaryChannel(); owner.is_licensed = false; myNodeInfo.my_node_num = kLocalNode; diff --git a/test/test_utf8/test_main.cpp b/test/test_utf8/test_main.cpp index 7ce90f250..ebf47be9b 100644 --- a/test/test_utf8/test_main.cpp +++ b/test/test_utf8/test_main.cpp @@ -1,3 +1,8 @@ +// Deliberately does NOT include TestUtil.h. This suite is pure-function - no NodeDB, no router, no +// sockets, no PKC - so the harness-wide guards there (no listening sockets, force_simradio clear) +// would assert conditions it cannot reach, and initializeTestEnvironment()'s RTC and OSThread setup +// would add portduino globals it otherwise never touches. Suite-level state cleanliness is still +// checked from outside by bin/pio-test-isolate.sh, which wraps every suite regardless. #include "meshUtils.h" #include #include diff --git a/variants/native/portduino/platformio.ini b/variants/native/portduino/platformio.ini index 37d5bf2a0..81b96f3e8 100644 --- a/variants/native/portduino/platformio.ini +++ b/variants/native/portduino/platformio.ini @@ -32,9 +32,16 @@ build_flags = ${native_base.build_flags} ; assertions. Registered here rather than only in bin/run-tests.sh so a bare `pio test` and CI get ; the same boundary. See bin/pio-test-isolate.sh. ; https://docs.platformio.org/en/latest/projectconf/sections/env/options/test/test_testing_command.html +; -s matches [env:coverage]. The sandbox above only covers $HOME, but portduinoSetup() searches +; ./config.yaml and /etc/meshtasticd/config.yaml - absolute, so no $HOME sandbox can hide it. On a +; host running meshtasticd that config selects the real LoRa module and the run proceeds into GPIO +; and SPI setup, so a test run would drive the developer's own radio. -s short-circuits ahead of the +; config search and returns before hardware init. Suites asserting behaviour that simradio changes +; (PKC selection) clear the flag themselves in setUp, after the radio choice is already made. test_testing_command = ${platformio.src_dir}/../bin/pio-test-isolate.sh ${platformio.build_dir}/${this.__env__}/meshtasticd + -s [env:native-tft] extends = native_base