Tom
f5314148c2
* Copy airtime reports into a caller buffer instead of exposing the array
airtimeReport() returned a pointer into the rotating bucket arrays, so the
caller held a handle to state that logAirtime() and every accessor mutate
underneath it. Copy into a caller-supplied buffer instead, and report failure
for a null buffer, a count past the log depth, or an unknown report type.
ContentHandler owns its buffer and hoists getPeriodsToLog() out of the three
calls that repeated it.
* Cover the AirTime report API and log-dispatch contract
Half of AirTime's surface had no tests: which store each report type feeds,
what airtimeReport() does when misused, how the first sync seeds itself, and
whether calling several entry points in one interval compounds the rotation.
Eighteen tests, asserted through the public API rather than the public bucket
arrays - those arrays are meant to become private, and a test that reads them
would have to be rewritten rather than pinning a contract.
Two of them state a convention that was never written down: the report arrays
are shift-ordered with slot 0 newest, and slot 0 covers only the time since the
last rotation. channelUtilization and utilizationTX use the opposite convention
- a modular ring indexed by uptime phase - and reading one as if it were the
other is a defect that has already happened once.
* Characterise AirTime window decay, TX gates, and sleep behaviour
Thirty-three tests in three kinds. Invariants must hold forever; boundaries pin
off-by-ones a refactor would move; five characterisations encode today's wrong
numbers, each tagged with the phase that will flip it.
Readings are asserted against an event-log oracle - airtime physically on air
inside (now - window, now], computed from a list of completed packets - rather
than against hand-worked constants, so a test states "this matches the
definition" instead of "this looked right when I wrote it".
The characterisations, all measured rather than assumed:
- the window covers (N-1)p + phase but divides by Np, so a steady 10% load
reads 8.33% right after a bucket boundary -> phase 5
- the same load sweeps across bucket phase instead of holding -> phase 5
- the hour window carries the same defect, 10x smaller -> phase 5
- a packet longer than its bucket is credited whole to the bucket
it completed in, so a saturated LONG_SLOW channel reads >100% -> phase 4b
- getSilentMinutes() reads a modular ring as if the index were an
age, so identical airtime gives different answers by phase -> phase 6
Two tests needed correcting during the write, both my expectations rather than
the code: a six-bucket ring sheds whole buckets, so a 30s gap drops three of
five survivors and not "half"; and the oracle sees 59 completions in a 60s
window, not 60, because the one on the lower edge is outside it.
Not written: the planned RX_LOG/RX_ALL_LOG disjointness test. That is a
property of the two radio drivers, which choose one or the other per packet -
it is not observable from AirTime, which records what it is told. The
AirTime-side half is already covered by the routing tests.
* Drop write-only and undefined AirTime members
None of this was reachable:
air_period_tx / air_period_rx file-scope mirrors of airtimes.periodTX/RX,
accumulated, rotated and memset in lockstep
with them but never read out or serialised.
Orphaned when #2552 re-pointed the writes at
bare globals instead of deleting them.
lastUtilPeriod, lastUtilPeriodTX written on every sync, read nowhere
airtimes.lastPeriodIndex written on every rotation, read nowhere
currentPeriodIndex() computes (secs / 3600) % 8 - a modular-ring
index for the one array that is shift-ordered
rather than a ring. Its only two uses were the
dead field above and a log line. It is the
fossil of the same confusion that makes
getSilentMinutes() wrong.
UtilizationPercentTX() declared, never defined
free logAirtime()/airtimeReport() declared, never defined; the latter still
carried the array-returning signature the
previous commit removed, so it actively misled
Also fixes the rotation log line, which read currentPeriodIndex() from inside
the loop although the index is advanced before it - on a multi-hour wake it
printed the same final value once per rotation. It now reports which of the
crossed hours is being rotated.
airtimeRotatePeriod() is kept: it has no caller in the tree either, but unlike
the above it is a defined public method, so out-of-tree callers are plausible.
Measured, not estimated: sizeof(AirTime) 464 -> 456 B, plus 64 B of globals, so
-72 B of static RAM. Padding accounts for the difference from the 66 B the plan
predicted by counting declared bytes.
The whole point of writing the tests first: the suite is green here with zero
test changes.
* Document what the AirTime figures measure and how they are stored
Comments only, but four of the things they replace were false.
The header's example analytics claimed RX_ALL_LOG was "all received lora
packets" and offered "RX_ALL_LOG - RX_LOG = other lora radios". Both radio
drivers pick exactly one of the two per packet, so they are disjoint: RX_ALL_LOG
is airtime we could not parse, the subtraction can go negative, and the total is
TX + RX + RX_ALL. Replaced with the actual contract - four inputs, eight
outputs, the window each spans, and the fact that the three thresholds are
hard-coded members rather than the settings they look like.
Names the two storage conventions on their declarations, because mixing them up
is what makes getSilentMinutes() wrong: channelUtilization and utilizationTX are
modular rings indexed by uptime phase, where the oldest bucket is (current + 1)
% N; airtimes.period* is shift-ordered with slot 0 newest, where the index IS an
age and slot 0 is a partial hour.
Defines the measurement as wall time rather than awake time, and says why: a
sleeping node still hears traffic, and per-node redefinition would make two
broadcast readings incomparable. Records that the 60s figure is published to the
mesh at >= 1h cadence, so what other nodes see is a snapshot - at LONG_FAST and
1% occupancy it reads exactly 0 in about 44% of reports - and that the contention
window it feeds moves in 20-percentage-point steps, so small errors never reach
the backoff.
Finally, states that rotation happens on access rather than on the scheduler
tick, names the test that enforces it, and leaves a TODO pointing at the plan
phases that fix the characterised accuracy defects.
* Serialise AirTime behind a lock proven by a private token
Two mechanisms solving different halves. A lock-free inner core (Windows) holds
all state and all logic; it has no lock and no way to reach one, so nesting is
impossible by construction. A private Held token takes the lock in its own
constructor and is the only thing that can be passed where a core method demands
one, so the lock cannot be forgotten either.
The rule is now uniform with no exceptions to remember: every public method
takes the lock once and delegates. In particular isTxAllowed*() lock like
everything else - before the split they could not, because they called the
public accessors and the lock is not recursive. That asymmetry was the foot-gun
the previous design documented in prose and hoped nobody would trip.
getPeriodsToLog()/getSecondsPerPeriod() still take no lock; they return
compile-time constants and touch no state.
channelUtilization[] and utilizationTX[] were public, so the lock was bypassable
at compile time. They move into the private core. Four test sites reached in;
all four now use logAirtime() plus the virtual clock, and no new test seam was
needed. Nothing in src/ was affected.
The re-entry assert is guarded on PIO_UNIT_TESTING, so it exists in test builds
only. The design sketched #ifdef DEBUG, but nothing in this tree defines DEBUG
or NDEBUG, so either spelling ships the assert to every board - and
nrf52_promicro_diy_tcxo has ~128 bytes of headroom under its 0xEA000 warm-store
cap, which the assert's strings and abort path overrun. It would have worked on
hardware, since the check runs in Held's owner initialiser and so precedes the
blocking take; the objection is that abort()ing a live mesh node is a poor trade
for a bug never seen in the field. Native tests are where it earns its keep
anyway: Portduino compiles Lock::lock() to an empty body, so a nested take there
succeeds silently and nothing else would notice.
Also comments out ScopedBusyAirTime in test_traffic_management. It is inert
twice over: the module holds no reference to airTime at all since hop exhaustion
was shelved, and the fixture never worked anyway - writing the buckets on a
fresh AirTime is undone by the first accessor call, which takes the firstTime
branch and memsets them. It reported 0%, not the 100% it claimed. Left in place,
commented, with both reasons recorded.
Cost on the tightest board in the tree, nrf52_promicro_diy_tcxo: the six phases
together add 96 bytes of flash, leaving it 32 bytes clear of the warm-store
guard. RAM is 72 bytes lower from the dead-state removal. Suite green at 47/47,
with test_airtime unedited apart from the added nesting test.
* Count rotations with the loop variable, not a separate tally
LOG_DEBUG compiles to nothing under DEBUG_MUTE, so the counter's only read
disappeared with it and the tally became write-only. It does not warn today -
this build has -Wunused-but-set-variable on, and it fires for other locals, but
not for one that is only initialised and never read - so it was latent rather
than broken: a stricter flag or -Werror would have failed muted builds only.
Using the loop variable removes the class of problem, since the loop condition
reads it, and drops the elapsedAirtimePeriods-- mutation as a side benefit.
Same iteration count, same output.
Found by compiling nrf52_promicro_diy_tcxo with -D DEBUG_MUTE, which is worth
recording for its own sake: muting logs takes that image from 802 784 to
673 416 bytes, 98.5% to 82.6% of flash. Logging is 16% of the largest nrf52
image, and its 32 bytes of warm-store headroom are a logging-verbosity question
rather than a code-size one.
* Tighten the comments added by this branch
Comment-only: with comments stripped, all five files are byte-identical to the
previous commit.
Removed the references to the planning notes. Those documents are working
material and will go stale; the code should not depend on them. The five
CHARACTERISATION tags now describe the defect they pin and stop there, and the
accuracy TODO names the four defects and points at the tests instead of a plan
file.
Also removed, as noise rather than information:
- comparisons against pre-#11291 behaviour, which nobody reading this needs
- a comment describing the lock restructure as future work, written before it
landed
- speculation ("plausible", "worth pinning so a future...")
- an aside arguing with an arithmetic slip made while writing the test
Kept the mechanical facts that are slow to re-derive: the two storage orderings
and which array uses which, RX_LOG/RX_ALL_LOG disjointness, the locking rule and
the addSpanned() constraint that protects it, why the re-entry assert is
test-only, and the concrete numbers - (N-1)p + phase, 14 164 ms, the 20 pp
contention-window steps.
Net 16 comment lines out of src/, 33 out of test/.
* Gate the AirTime re-entry check on the host, not on testing
PIO_UNIT_TESTING is injected by PlatformIO purely on BUILD_TYPE, with no
platform check, so it is defined on an on-target `pio test` run too. The
check arms before the lock is taken - a nested take blocks forever, so a
later check would never run - which under preemption false-positives on
legitimate contention and races on its own write.
Derive AIRTIME_REENTRY_CHECK once from PIO_UNIT_TESTING && !HAS_FREE_RTOS
and use it at all three sites. Had the three conditions ever diverged, an
on-target test build would fail to compile on a member the header no
longer declares.
* Log AirTime outside the lock it serialises
DEBUG_PORT.log() blocks on a UART write, and `lock` is a plain binary
semaphore with no priority inheritance, so holding it across a log call
lets the main thread stall the radio thread in getTxDelayMsec().
Move logAirtime()'s LOG_DEBUG into the shell, after the Held scope
closes; the shell already has both arguments, so nothing has to be
passed back out of the core. isTxAllowed{ChannelUtil,AirUtil} read into
a local under the lock and warn after it. The log bodies are braced
because LOG_DEBUG compiles away under DEBUG_MUTE and a bare `if (x) ;`
trips -Wempty-body.
Fold the two doubled index calls into `+=` while touching the lines.
* Give each airtime report its own buffer
handleReport() reused one array across the three airtimeReport() calls
and ignored the bool. A failed report would have left the previous
type's data in place and emitted it under the next type's key. Build
each through a lambda whose buffer is zeroed per call, so a failure
emits zeros.
Unreachable today - the count is always PERIODS_TO_LOG and the type is
always valid - but the old shape only read as correct by accident.
* Drop a stray semicolon from the inert-guard comment
* Address external review: name the race, tighten the claims and the tests
The header sold the lock as mechanism without naming a second thread, which
invites the reasonable objection that this is a cooperative OSThread codebase.
There is a real race and it is nRF52-only: NRF52Bluetooth registers its ToRadio
write callback with defer == false, so a phone's packet runs handleToRadio ->
sendToMesh -> Router::send on the Bluefruit BLE task, reading
utilizationTXPercent() and getSilentMinutes() while loopTask may be inside
logAirtime(). ESP32 hands BLE work to the main task and does not have it.
Three claims in the header were wrong or overstated:
- "nesting is impossible by construction" - Windows is a nested class with an
enclosing class's access rights, and `extern AirTime *airTime` is in the
same header, so airTime->anyPublicMethod() from inside it is well-formed
and would hang. Nothing does it; the assert is the backstop. Say that
instead, because the comment below instructs contributors to add helpers
to Windows on the strength of the guarantee.
- "every public method takes the lock exactly once" - two constant accessors
take none and isTxAllowedAirUtil() takes it zero or one times. State the
exceptions where the invariant is stated, not only at the definitions.
- "both radio drivers pick exactly one per packet" - five drop paths log
neither. At most one. Recorded against plan4 rather than fixed here: it
changes a telemetry value.
getPeriodsToLog()/getSecondsPerPeriod() become static constexpr, which removes
them from the locking claim structurally and lets ContentHandler size its
buffer and its count from one constant.
Tests:
- C14's saturated AirTime is installed by a helper and restored in tearDown.
Unity's TEST_ABORT() is longjmp and does not run destructors of automatic
objects, so the scoped guard it replaces would leave airTime dangling into
an abandoned frame on any assertion failure - and the same commit that
added it removed the tearDown reset that did cover that.
- test_getSilentMinutes_counts_minutes_until_enough_ages_out asserted only
`mins <= 60`, which neither return path can violate. The answer is 59.
- test_backwards_uptime_degrades_safely stepped 600s -> 60s, which leaves
elapsedAirtimePeriods at 0, so it never reached the hourly-report branch
its own comment describes. Step by the wrap instead and assert the exact
figures.
- test_airtime leaked EU_868 out of the duty-cycle case into every later one,
and the reentry test's isTxAllowedAirUtil() coverage depended on it.
Restore the region in tearDown and set it explicitly where it is wanted.
- Rename that test to what it can actually check: no single method takes the
lock twice. The calls are sequential, so it cannot catch two methods
nesting.
* trunk: suppress trufflehog/Lob false positives in test_airtime
* Address CodeRabbit review: the rotate trace, the cap warn, the backoff
Four findings from the CodeRabbit pass. Two were introduced by this branch,
one is a real inconsistency it inherited, one is a naming slip.
The rotate trace was the one that mattered. "Log AirTime outside the lock it
serialises" moved the per-packet lines and the two TX-gate warnings out to the
shell, but missed LOG_DEBUG("Rotate airtimes, crossed hour %u") because it does
not sit in the shell at all: it is inside Windows::syncNow(), the lock-free
core, which by construction only ever runs under Held. Nothing at that line
looks like a lock, which is why it survived.
The exposure is smaller than the review suggests - runOnce() syncs at 1 Hz, so
in steady state this is one line an hour, and the PERIODS_TO_LOG - 1 burst
needs an hour of light sleep with no intervening sync - but a UART write under
a plain binary semaphore with no priority inheritance is exactly what the
comment above logAirtime() says this code does not do. syncNow() now
accumulates crossings in rotationsPendingLog and runOnce() drains it inside the
Held scope, then logs after release. Any caller can cross an hour; only that
thread reports it, so a crossing raised elsewhere is traced at most one tick
late. The `if (rotations > 0)` guard keeps the drained value read under
DEBUG_MUTE, where LOG_DEBUG expands to nothing - the write-only tally that
"Count rotations with the loop variable" removed.
addFromContact()'s favorite fallback stamped silently when the protected cap
refused it. The stamp is new on this branch; the two sibling refusals (ignore,
verify) both emit PROTECTED_CAP_WARN_FMT, so the operator lost the only signal
that the cap was hit on the one path that has a fallback.
lfs_assert() mixed clocks: Throttle read Time::getMillis(), the remainder was
computed from a second, bare millis(). The review's stated failure mode - a
native test overriding the clock - cannot happen, since the hook is behind
PIO_UNIT_TESTING and this file is nRF52-only. The real defect is the second
read: a tick landing on the 20-minute boundary between the check and the
subtraction underflows the remainder into delay(~50 days), on a device that has
just found its flash corrupt. One read, clamped, and preFSBegin() stores from
the same clock.
The eviction test is renamed to
test_eviction_prefersCurrentBootStampOverPost2038Epoch. The finding is right
that it was snake_case, but the suggested testEvictionPrefers... does not match
this file either, which is test_<area>_<camelCase> throughout.
Not taken, both pre-existing and out of scope for a rollover branch:
- t5s3_epaper's touchResumeAtMs/suppressFromMs read an active suppression as
inactive if the wake lands in the 1 ms where millis() is 0. Consequence is
one skipped 150 ms touch-settle window per 49.7-day wrap.
- NRF52Bluetooth::onPairingPasskey() busy-waits 30 s in a BLE callback. Worth
saying plainly that this branch makes it more visible: the old
`millis() < start_time + 30000` overflowed at the wrap and cut the wait
short, so the correct Throttle form is what lets it run the full 30 s.
Reworking it into an OSThread is its own change.
Native suite GREEN, 48/48, 672 cases.
Native Unit Tests - Authoring Guide
This directory contains C++ unit tests that run on the host machine via PlatformIO's native environment. Tests use the Unity framework.
Running Tests
Preferred: use bin/run-tests.sh - it defaults to the coverage env, cross-checks the number of suites that actually ran, and emits an unambiguous RED/AMBER/GREEN verdict:
./bin/run-tests.sh # all suites
./bin/run-tests.sh -f test_traffic_management # single suite
./bin/run-tests.sh -f test_traffic_management > /tmp/test_out.txt 2>&1; tail -5 /tmp/test_out.txt
Exit codes: 0 = GREEN, 1 = RED, 2 = AMBER, 3 = FILTERED.
The harness is Linux-only, by choice. bin/run-tests.sh and the per-suite isolation it drives need bash 4+ and GNU coreutils/find (find -printf, md5sum), and the script refuses to start anywhere else rather than degrade quietly - a shared-state check that silently mis-hashes a sandbox still prints a verdict, and that verdict would be worthless. The native-macos PlatformIO env is a build target for meshtasticd, not a test host; the isolation wrapper is registered for env:native and env:coverage only. On macOS or Windows, run the suite in a container: ./bin/test-native-docker.sh.
-f is not a gate. A filtered run can pass while a full run fails, because filtering removes the suites that create the state a later suite trips over. Iterate with -f; gate on a full run.
Sanitizers are per env. coverage (the default) has ASan/LSan; native has none, verified. -e native runs are not sanitized.
A signal name in the output is not a crash. exit(UNITY_END()) returns the failure count and PlatformIO renders it as a signal number (4 -> SIGILL, 5 -> SIGTRAP), reporting the suite [ERRORED]. Match it against the failure count before assuming a fault.
Suite order is randomisable, and reproducible. --shuffle runs the suites in a seeded random order; --seed <n> replays an exact one. The seed defaults to the commit SHA - one order per commit, so a red is replayable and attributable rather than flaky - and is printed at the start of the run and on the RESULT: line. On failure the full order is printed, because for an order-dependent failure the order is the diagnostic. A single green seed is not evidence of order independence; vary it.
./bin/run-tests.sh --shuffle # seed from HEAD, printed
./bin/run-tests.sh --seed 2855893161 # replay that exact order
Randomisation costs one pio invocation per suite (about 4.7s each), because PlatformIO orders suites by its own directory walk and -f only selects.
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 when you need to override the env or inspect verbose Unity output:
# All test suites
pio test -e native
# Single suite
pio test -e native -f test_your_module
# Verbose (shows build errors in detail)
pio test -e native -f test_your_module -vvv
Never pipe through | tail -N to shorten output. PlatformIO prints build errors at the top of output and test results at the bottom; tail will show stale cached results from a prior successful build while hiding the compile error that caused the current run to fail.
Preferred pattern for raw pio - redirect to file, then grep:
# Redirect all output to a file; grep for errors and results after it exits
pio test -e native -f test_your_module > /tmp/test_out.txt 2>&1
echo "exit: $?"
grep -E 'error:|PASS|FAIL|succeeded|failed' /tmp/test_out.txt
tail -15 /tmp/test_out.txt
Why: piping through | grep line-buffers the output and suppresses all progress until the process exits, making it look hung. The redirect approach lets the build stream normally while still giving you filtered results afterwards.
Viewing verbose test output without truncation (e.g. TEST_MESSAGE group headers):
/tmp/meshtastic-pio-venv/bin/python -m platformio test -e coverage --filter test_mesh_beacon -vv 2>&1 | grep -v "[[:space:]]SKIPPED$"
The -vv flag makes Unity emit INFO: lines from TEST_MESSAGE calls; piping through grep -v SKIPPED removes the noise from platform feature gates while keeping all PASS/FAIL/INFO lines visible.
externally-managed-environment error on Ubuntu/Debian:
If pio test fails immediately with error: externally-managed-environment, the system pio binary is using the OS Python which newer distros lock down. Use PlatformIO's own venv instead:
~/.platformio/penv/bin/python -m platformio test -e native -f test_your_module > /tmp/test_out.txt 2>&1
grep -E 'error:|PASS|FAIL|succeeded|failed' /tmp/test_out.txt
tail -15 /tmp/test_out.txt
Helper Scripts (Useful Shortcuts)
These wrappers are handy when local host dependencies are missing or when you want repeatable commands.
# Run native tests in Docker (recommended on macOS / non-Linux hosts)
./bin/test-native-docker.sh
# Pass normal PlatformIO test args through to Dockerized test run
./bin/test-native-docker.sh -f test_your_module
# Force Docker image rebuild (after dependency changes)
./bin/test-native-docker.sh --rebuild
# Run simulator integration check (build native first)
pio run -e native && ./bin/test-simulator.sh
# Build and run meshtasticd natively
./bin/native-run.sh
# Build and run under gdbserver on localhost:2345
./bin/native-gdbserver.sh
# Build native release artifact into ./release/
./bin/build-native.sh native
Notes:
- The repository script name is
./bin/test-simulator.sh(there is notest-native-simulator.sh). ./bin/test-native-docker.shis the closest match to CI behavior for native tests and avoids host package setup.
System Dependencies (Ubuntu/Debian)
The native build requires several system libraries. Install them all at once:
sudo apt-get install -y \
libbluetooth-dev libgpiod-dev libyaml-cpp-dev libjsoncpp-dev openssl libssl-dev \
libulfius-dev liborcania-dev libusb-1.0-0-dev libi2c-dev libuv1-dev
See .github/actions/setup-native/action.yml for the canonical list.
Creating a New Test Suite
1. Directory Structure
test/test_your_module/test_main.cpp
One file per suite. No per-test platformio.ini is needed - tests build under the [env:native] environment defined in the root platformio.ini.
2. File Skeleton
#include "MeshTypes.h" // Include BEFORE TestUtil.h (provides NodeNum, etc.)
#include "TestUtil.h" // initializeTestEnvironment(), testDelay()
#include <unity.h>
#if YOUR_FEATURE_GUARD // Same #if guard as the module under test
#include "FSCommon.h"
#include "gps/RTC.h"
#include "mesh/NodeDB.h"
#include "modules/YourModule.h"
#include <cstdio> // required for printf() - used for blank-line group separators
#include <cstring>
#include <memory>
// --- Test output helpers ---
// printf() writes directly to stdout and appears in -vv output as a plain line (no prefix).
// Use it for blank-line group separators: printf("\n");
// TEST_MESSAGE() emits a "file:line:INFO: <text>" line - visible at -vv and above.
// Use TEST_MSG_FMT for formatted diagnostic lines inside tests.
#define MSG_BUF_LEN 200
#define TEST_MSG_FMT(fmt, ...) do { \
char _buf[MSG_BUF_LEN]; \
snprintf(_buf, sizeof(_buf), fmt, __VA_ARGS__); \
TEST_MESSAGE(_buf); \
} while(0)
// --- Tests ---
void test_example()
{
TEST_MESSAGE("=== Example test ===");
TEST_ASSERT_TRUE(true);
}
// --- Unity lifecycle ---
void setUp(void) { /* runs before every test */ }
void tearDown(void) { /* runs after every test */ }
void setup()
{
initializeTestEnvironment(); // MUST call - sets up RTC, OSThread, console
UNITY_BEGIN();
printf("\n=== Example group ===\n"); // header line to help find tests
RUN_TEST(test_example);
exit(UNITY_END()); // REQUIRED - a bare UNITY_END() leaves the process running
}
void loop() {}
#else // !YOUR_FEATURE_GUARD
void setUp(void) {}
void tearDown(void) {}
void setup()
{
initializeTestEnvironment();
UNITY_BEGIN();
exit(UNITY_END());
}
void loop() {}
#endif
3. Terminate with exit(UNITY_END()), on every branch
A bare UNITY_END() does not end the suite - it ends the reporting. setup() returns, the runtime goes on calling loop(), and the process runs forever. PlatformIO does not notice: it reads the Unity summary off stdout, reports the suite PASSED and moves to the next one, so the run is green while the binary is still resident. Nothing surfaces it, and the leak is one process per suite per run.
The consequences are worse than an idle process:
- The per-suite sandbox is deleted underneath a live process, so its CLEAN/DIRTY verdict says what the suite had written by the time the harness stopped looking, not what it left behind.
.gcdacoverage data and LeakSanitizer's report are both flushed byatexithandlers, so a suite that never exits contributes no coverage and gets no leak check - silently.- Each survivor pins its own deleted binary on disk (~94 MB), which
ducannot see.
So: exit(UNITY_END()) in every setup() branch, including the #else of a feature or architecture guard where the suite does nothing. The empty-suite branch is the easiest one to get wrong, because it looks like there is nothing to clean up.
4. Feature Guard
Wrap the entire test body in the same #if guard the module uses (e.g. #if HAS_VARIABLE_HOPS, #if !MESHTASTIC_EXCLUDE_GPS). When the feature is disabled, the #else branch produces an empty passing suite.
Common Patterns
MockNodeDB
Most module tests need to inject nodes with controlled hop distances and ages:
class MockNodeDB : public NodeDB
{
public:
void clearTestNodes()
{
testNodes.clear();
numMeshNodes = 0;
}
void addTestNode(NodeNum num, uint8_t hopsAway, bool hasHops,
uint32_t ageSecs, bool viaMqtt = false)
{
meshtastic_NodeInfoLite node = meshtastic_NodeInfoLite_init_zero;
node.num = num;
node.has_hops_away = hasHops;
node.hops_away = hopsAway;
nodeInfoLiteSetBit(&node, NODEINFO_BITFIELD_VIA_MQTT_MASK, viaMqtt);
node.last_heard = getTime() - ageSecs;
testNodes.push_back(node);
meshNodes = &testNodes;
numMeshNodes = testNodes.size();
}
std::vector<meshtastic_NodeInfoLite> testNodes;
};
static MockNodeDB *mockNodeDB = nullptr;
Set nodeDB = mockNodeDB; in setUp().
Test Shim (Exposing Protected/Private Members)
Subclass the module under test to make protected methods callable and private members writable:
class YourModuleTestShim : public YourModule
{
public:
// Pull protected methods into public scope via using.
// IMPORTANT: using requires the method to be protected (or public) in the base -
// friend alone does NOT satisfy this. See pitfall #6.
using YourModule::runOnce;
using YourModule::someProtectedMethod;
// Wrap private members with setter methods (friend grants direct access here).
void setPrivateField(int x) { privateField = x; }
};
For methods you want to expose via using, use the conditional access-specifier pattern in the header - not plain friend:
// In YourModule.h, inside the class body:
#ifdef PIO_UNIT_TESTING
protected:
#else
private:
#endif
bool someMethod();
For private member variables that a shim setter needs to touch directly, friend is sufficient (no using involved):
// In YourModule.h, inside the class body:
#ifdef PIO_UNIT_TESTING
friend class YourModuleTestShim;
#endif
Global Singleton Lifecycle
Most modules use a global pointer (extern YourModule *yourModule;). Manage it carefully:
void setUp(void) {
// ... setup ...
}
void tearDown(void) {
yourModule = nullptr; // prevent dangling pointer between tests
}
void test_something() {
auto shim = std::unique_ptr<YourModuleTestShim>(new YourModuleTestShim());
yourModule = shim.get();
// ... test ...
yourModule = nullptr;
}
Pitfalls and How to Avoid Them
1. Persisted Filesystem State
You are handed a clean sandbox. Declare what you write.
Each suite runs inside its own scratch $HOME (bin/pio-test-isolate.sh), so state cannot reach the next suite. The files in play are wider than module state, and all but the last live under ~/.portduino/default/prefs/:
| File | Written by |
|---|---|
nodes.proto |
any NodeDB save - including incidental ones from removeNodeByNum(), resetNodes(), nodeDBSelfCare(), and the constructor itself when the file is absent |
config.proto, module.proto, channels.proto, device.proto |
config/channel saves, admin handlers |
warm.dat |
WarmNodeStore::saveIfDirty(), on the node-DB save cadence |
transmit_history.dat |
retransmission tracking |
/prefs/<module>.bin |
per-module saveState() |
NodeDB's constructor calls loadFromDisk(), so any suite that constructs one inherits whatever is there.
What you have to do:
-
Nothing, if your suite is self-contained. That is the default and what almost every suite wants.
-
If your suite mutates persisted state on purpose, add a line to
test/state-manifest.tsvwith a reason:test_nodedb_blocked state=per-suite writes=nodes.proto,warm.dat saturates the DB to test the protected-node capAn undeclared write is reported as DIRTY and grades the run AMBER. A declared write that never happens is reported as MISSING - a warning, and a useful one: it catches persistence that silently stopped working.
-
Use
state=per-suiteonly if a test genuinely needs to observe the previous test's write (persistence round-trips, migration ladders). It relaxes per-test checking to the suite boundary, so make it a deliberate choice rather than an accident ofsetUp().
Deleting your own state in setUp() is still fine and still a good habit for intra-suite isolation - it is just no longer what stands between you and the next suite:
void setUp(void) {
// ...
#ifdef FSCom
FSCom.remove("/prefs/your_module.bin");
#endif
}
2. A Shared Fixture Is Not a Fixture
If your suite touches globals the code under test writes - nodeDB, config, owner, devicestate, channelFile - build and restore them in setUp/tearDown for every test, not just the ones that seem to need it. An opt-in fixture that only some tests arm leaves the rest sharing one never-reset object, and "the other tests set their own state and are unaffected" is a claim that quietly stops being true as tests are added.
test/test_admin_radio/test_main.cpp is the worked example:
void setUp(void) {
// ...
replaceAdminRadioGlobals(); // saves the globals, installs a fresh NodeDB
}
void tearDown(void) {
restoreAdminRadioGlobals(); // restores them, deletes the NodeDB, re-runs initRegion()
// ...
}
A fresh NodeDB per test costs real time (loadFromDisk() plus, when the region is set, key generation) - in that suite roughly 7% of a ~7½-minute run. Pay it. If a test genuinely needs to observe the previous test's state, that is what state=per-suite in test/state-manifest.tsv is for; say so there rather than achieving it by omission.
3. File-Scope Mutable Globals Persist Across Tests
Variables like static uint8_t someDenominator = 8; in the module .cpp file retain mutations from previous tests. This is distinct from member variables - it affects all instances.
Fix: Add a static void resetGlobal() method to the module and call it in setUp().
4. Randomness Breaks Determinism
If the module uses rand() for jitter or similar, test results become non-reproducible.
Fix: Add a static enable/disable flag:
// Module header:
static void setJitter(bool enabled) { s_jitterEnabled = enabled; }
// Test setUp:
YourModule::setJitter(false);
// Test tearDown:
YourModule::setJitter(true);
5. Time-Dependent Logic Produces Zeros
Rolling averages weighted by elapsedMs / ONE_HOUR_MS collapse to zero when tests complete in microseconds. Sample windows, EMA alphas, and interval-based accumulators all suffer from this.
Fix: Expose the timestamp via friend access and simulate realistic elapsed time:
// In test shim:
void setWindowStartMs(uint32_t ms) { windowStartMs = ms; }
// In test:
shim.setWindowStartMs(millis() - 3600000UL); // pretend 1 hour elapsed
6. Capacity Limits Cause Cascading Failures
Fixed-size data structures (hash sets, ring buffers) overflow when tests inject more data than fits. This triggers early flushes with near-zero time fractions, compounding the time-dependent-zeros problem.
Fix: Simulate multiple realistic time windows rather than one massive burst. Let adaptive mechanisms (if any) self-tune over several rolls.
7. Granting test access to private/protected members
PlatformIO defines PIO_UNIT_TESTING during pio test builds. Several production headers (TransmitHistory.h, CryptoEngine.h, MQTT.h, RTC.h) use this to gate test-only visibility changes. PlatformIO also defines UNIT_TEST in the same builds for backward compatibility, but that spelling is deprecated - always use PIO_UNIT_TESTING in new code. The established pattern for exposing a private method to a test shim without widening production visibility:
#ifdef PIO_UNIT_TESTING
protected:
#else
private:
#endif
bool myMethod();
Critical C++ rule: a using declaration in a derived class (e.g. using Base::myMethod) requires myMethod to be protected or public in the base - friend alone does not satisfy this. Adding friend class TestShim while leaving the method private will still fail to compile. Use the conditional access-specifier pattern above, not friend.
setUp/tearDown Checklist
- Create and clear MockNodeDB (if needed)
- Zero global configs:
config,moduleConfig,myNodeInfo - Set
nodeDB = mockNodeDB - Delete your own persisted state files (
FSCom.remove(...)) for intra-suite isolation - cross-suite isolation is already guaranteed, see Pitfall 1 - Declare deliberate writes to shared state in
test/state-manifest.tsv, with a reason - Reset file-scope mutable globals
- Reset mock clock to a safe base value (e.g.
mockTime = ONE_HOUR_MS) - prevents unsigned subtraction underflow in time-dependent logic - Disable randomness/jitter flags
- In
tearDown: null the global singleton pointer, restore flags
Test Organization
A well-structured test suite follows this pattern:
- Topology/scenario builders - static helper functions that set up specific test conditions
- Injection helpers - simulate realistic traffic, time, or event patterns
- Scenario tests - each builds a scenario, runs the module, asserts on outcomes
- Lifecycle tests - state persistence, startup from blank, restart recovery
- Summary test (optional) - emits a scenario table into the log for quick CI review
Not a Unity suite: bin/test-config-check.sh
Portduino YAML validation is tested by driving a built meshtasticd rather than by a
Unity suite, because what it asserts - the exit status and printed report of
meshtasticd --check, and the fact that a normal run still refuses a bad config - are
properties of the process, not of a linkable function. Fixtures live in
test/fixtures/portduino-config/ (see the README there); CI runs it in
test_native.yml. It is not a test_* directory, so it sits outside the suite count the
harness derives from test/.
pio run -e native && ./bin/test-config-check.sh
Existing Test Suites
This table is a description, not an inventory. The canonical suite total is the number of
test_* directories under test/, detected on the fly by bin/run-tests.sh on every full run
and cross-checked against the suites that actually ran. That derived count is the only number
that should be trusted or quoted. Entries below carry per-suite descriptions the count cannot;
do not infer completeness from the row count.
| Suite | Module Under Test |
|---|---|
test_admin_radio |
Admin + LoRa region config |
test_fscommon_getfiles |
Bounded file-manifest walk |
test_atak |
ATAK integration |
test_crypto |
CryptoEngine |
test_default |
Default configuration helpers |
test_hop_scaling |
Hop scaling algorithm |
test_http_content_handler |
HTTP handling |
test_mac_from_string |
MAC address parsing |
test_mesh_module |
Module framework |
test_meshpacket_serializer |
Packet serialization |
test_mqtt |
MQTT integration |
test_packet_history |
Packet history tracking |
test_position_precision |
Position precision helpers |
test_radio |
Radio interface |
test_serial |
Serial communication |
test_module_config |
AdminModule module config |
test_tak_config |
TAK (ATAK) team/role values |
test_traffic_management |
Traffic management |
test_transmit_history |
Retransmission tracking |
test_type_conversions |
NodeDB v25 type conversions |
test_utf8 |
UTF-8 utilities |