Files
Tom bca7c0b480 Tom fiddles with the test suite - again (#11517)
* test: make every suite run its own binary, and fail the run when it does not

PlatformIO links every native test program to the one $BUILD_DIR/$PROGNAME path and
attributes Unity output by text alone, never checking that the source file a case came
from belongs to the suite it thinks it ran. Both harnesses had been split into a build
pass (--without-testing) and a run pass (--without-building), and for a non-embedded
platform the run pass never relinks - so all 57 suites executed whichever suite was
linked last, each reporting PASSED under its own name. Introduced for CI in 4906f8a6
and for bin/run-tests.sh in de6b2319; both ran fused, and correctly, before that.

Drop --without-building from both run passes. The --without-testing pass stays as a
warm-up so no single suite absorbs the whole src compile in its reported duration; with
the objects already cached the per-suite step is one test_main.cpp plus a link.

Add bin/check-test-attribution.py, which grades the JUnit reports both harnesses already
produce. It fails on a test case whose source file lies outside the suite that reported
it, and on a suite that was asked to run and produced no cases at all. Wired in three
places: bin/run-tests.sh as a RED verdict ahead of the softer ones, per area in CI so a
mismatch names its area, and once over the merged report so an area that never executed
cannot hide. Suite ownership is matched on whole path segments, so test_mesh does not
claim test_mesh_module, and the -f pattern is resolved against the canonical set rather
than taken as a literal suite name.

* fix(test): pin simradio off for the packet-signing PKI cases

[env:coverage] passes -s to the test binary (74e6723ad, #8251), which sets
portduino_config.force_simradio. wouldEncryptWithPKC() lists !force_simradio among its
preconditions, so perhapsEncode() takes the channel-crypto branch, returns NONE and leaves
pki_encrypted false - failing test_B11_normal_unicast_still_uses_pki and
test_B12_licensed_receiver_does_not_decrypt_pki, both of which assert the production PKI
path. [env:native] passes no such flag, which is the whole of the long-standing
"passes under native, fails under coverage" split; it was never gcov, ASan or a host.

Save and clear the flag in setUp, restore it in tearDown, so the suite asserts the encode
path it is named for under either env's invocation. Same binary, pristine $HOME: 77 tests
0 failures with -s and without, where before -s gave 2 failures.

Whether the unit-test binary should run with -s at all is a separate question - it means CI
exercises the simradio configuration for every suite - and is left alone here.

* fix(router): drive the admin-key fallback budget from the injectable clock

The budget is 8 tokens refilling one per 250ms of wall clock, and
test_admin_key_fallback_is_rate_limited drains it with eight PKI decodes before asserting the
ninth is refused. That gives the drain loop 31ms per iteration, each of which generates a
keypair and does three X25519 operations under gcov and ASan. This box runs them in ~4ms;
a GitHub runner takes ~38ms, so a token refills mid-drain and the packet the test expects to
be blocked decodes. Measured from both runs' own log timestamps, 9.5x apart.

Read the bucket through Time::getMillis() instead of millis(), and have the test set and
advance the virtual clock rather than sleeping. The subtraction was already wrap-correct, so
the deadline guard is unaffected. Restores the clock in tearDown so the rest of the suite is
untouched, and drops ~3s of real sleeping from the run.

* test: declare the event-channel suites' shared state

Both construct a NodeDB, whose constructor persists a default set into an empty prefs
directory, so each writes the five prefs protos. Neither was declared, because until suites
started running their own binaries nothing had ever observed them writing anything.

* test: add a repeat runner for order-independent flakes

A single green run says nothing about a real-time race or a slow-host margin: the rate-limit
budget above passes here with 7x headroom and still fails on a CI runner. Run one suite N
times against a fresh scratch $HOME each time, optionally against CPU contention, and print a
flake rate. Failing runs keep their log and their sandbox; passing runs leave nothing.

Simradio is taken from the env's own test_testing_command, so a stress run reproduces the
real invocation rather than inventing a third one.

* fix(test): keep a native test run off the host's radio

bin/pio-test-isolate.sh sandboxes $HOME, but portduinoSetup() looks for config in
./config.yaml and /etc/meshtasticd/config.yaml - the second absolute, so no $HOME sandbox
can hide it. On a machine running meshtasticd that config selects the real LoRa module and
the run continues into GPIO and SPI setup, so ./bin/run-tests.sh -e native would drive the
developer's own radio without saying so. -e native is also the faster of the two, and the
one reached for when iterating.

[env:coverage] already passes -s, which short-circuits ahead of the config search and returns
before hardware init. Pass it for [env:native] too. That closes the hazard and, incidentally,
makes the two envs invoke the binary identically - they did not, which is the whole of the
long-standing "green locally, red in CI" split.

* test: run every suite with PKC on, and assert it stays that way

force_simradio does two unrelated jobs. It keeps portduinoSetup() off the host's hardware,
which every test run wants, and it makes wouldEncryptWithPKC() return false, which no test
run wants: the encode path under test then falls back to channel crypto and any case
asserting PKI fails, or worse, passes while asserting the wrong thing.

Three suites had each worked this out separately and cleared the flag themselves -
test_admin_session_repro's comment describes the mechanism exactly. Clear it once in
initializeTestEnvironment() instead. By then portduinoSetup() has already skipped the config
search and chosen the simulated radio, and it never reconsults the flag, so clearing it
cannot bring hardware back; the only remaining readers are the PKC gate and an
exit_simulator intercept no test can reach. The per-suite copy added to test_packet_signing
for B11/B12 goes away with it.

Two asserts, because both invariants were true only by inspection:

- No listening sockets. main.cpp's setup()/loop() are compiled out under PIO_UNIT_TESTING, so
  the phone API, MQTT and the web server never start - but nothing checked. A suite that
  pulled in a service binding a port would open one on the developer's machine for the length
  of the run.
- force_simradio still clear, before every test rather than once per suite, since a case that
  restores a struct it snapshotted earlier puts it back and silently disables PKC for
  everything after it. Named per test, so the report points at the case after the culprit.

Both exit rather than TEST_FAIL: they run outside a Unity test frame, and silently repairing
either one would leave the suite that broke it passing. Verified by disabling the clear and
watching the guard fire on the first case instead of reporting two quiet failures.

* test: let the repeat runner vary suite order too

Repeating one binary finds races and slow-host margins; it cannot find state that leaks from
one suite into the next, because only one suite runs. --shuffle drives run-tests.sh --seed
with a fresh seed each iteration and reports which seeds went red, so the shuffle already in
the harness yields a flake rate rather than a single sample. Seeds are printed and replayable.

* fix(test): baseline the environment from whichever runs first

Clearing force_simradio in initializeTestEnvironment() missed the suites that never call it.
test_atak is one, and it also pulls in TestUtil.h, so it got the per-test assert without ever
getting the baseline and aborted on its first case - caught by CI, which is what the assert is
for. test_geocoord_distance, test_meshpacket_serializer and test_utf8 skip the init too, but
include no TestUtil.h at all, so nothing reached them either way.

Move the clear and the socket check into baselineEnvironment(), called from
initializeTestEnvironment() or from the first RUN_TEST, whichever comes first. Suites that
initialise are still asserted from their first case; the rest are baselined at case one and
asserted from case two.

Print the violation on stdout as well as stderr: bin/run-tests.sh filters the program's
stderr, so locally the message vanished and the run reported "exit-time abort (likely
sanitizer)" - the exit code read as a signal number again, with no sign of the real reason.

* test: drop the per-suite simradio exceptions

Three suites had each found that force_simradio disables PKC and cleared it themselves.
initializeTestEnvironment() now clears it once for every suite, so all six sites are dead
code - along with the PortduinoGlue.h include each pulled in for it.

test_event_channel_router's is the one worth removing rather than leaving: it snapshotted the
flag into SavedGlobals and restored it at teardown, which is exactly the shape the per-test
assert exists to catch. Harmless while the snapshot reads false, and a silent PKC-off for
every later case if that ever changed.

The three suites pass unchanged: 54 cases, attribution clean.

* test: tell a deliberate harness abort from a sanitizer fault

A guard in TestUtil.cpp that aborts on purpose - a listening socket, or force_simradio put
back - exits non-zero with no sanitizer report, so it fell through to the exit-time-abort
heuristic and was announced as "RED exit-time abort (tests passed; likely sanitizer)". That
is the same trap as the phantom SIGILL two checks above: a verdict line naming a cause it has
not established, sending the reader after a memory bug that does not exist. It cost hours in
the original investigation and it cost the first read of a test_atak failure today.

Match the FATAL line the guards print on stdout for exactly this purpose, and report the
reason they gave instead of guessing.

* test: say why three suites omit TestUtil.h

They are pure-function - no NodeDB, no router, no sockets, no PKC - so the harness-wide guards
in TestUtil.h would assert conditions they cannot reach, and initializeTestEnvironment()'s RTC
and OSThread setup would pull in portduino globals they otherwise never touch. Suite-level
state cleanliness still applies: bin/pio-test-isolate.sh fingerprints the sandbox from outside
and wraps every suite regardless.

Recorded at the top of each so the omission reads as a decision rather than an oversight - it
looked like the latter when the socket and simradio asserts landed.

* test(traffic): give every case a primary channel

resetTrafficConfig() zeroed channelFile and left channels_count at 0, so the 66 cases that do
not install a channel themselves ran against a device with none. Every router lookup then hit
Channels::getByIndex()'s out-of-range branch and logged, which is 12106 of the suite's 20088
ERROR lines and tests nothing - a real device always has a primary channel, and no case here
asserts channels-unset behaviour.

Install the well-known primary the suite already builds for its precision cases. All 85 pass
unchanged, and the suite's ERROR output drops to 7985, the remainder being decode failures
from test_tm_fuzz_nodenum_blitz's malformed payloads.

* test: budget each suite's LOG_ERROR output

A suite can pass while emitting six figures of ERROR, which buries a real failure and trains
everyone to skim. Count them per suite and grade the count as a second axis, alongside the
CLEAN/DIRTY verdict already computed from the same captured log.

Declared in the same manifest, as a RANGE rather than a ceiling, because for a fuzz suite the
floor is the half that matters: test_fuzz_decode logging ~100k rejections is the suite
working, and the same suite logging none means it stopped feeding malformed input while every
case still passes. Bounds are wide on purpose - they catch a path that has stopped running,
not a drift of a few hundred lines. Undeclared suites get 100, which 50 of 57 already meet.

AMBER, not RED. Three log sites - mesh-pb-constants.cpp:28, Channels.cpp:356, MQTT.cpp:92 -
account for nearly all the remaining volume, and landing this red before they are demoted
would buy exemptions rather than fixes.

* test: canary the attribution check, and run the state self-test in CI

check-test-attribution.py guards against the false green, and nothing guarded the guard. A
checker that has quietly stopped matching looks exactly like a codebase with no problem, which
is how the original went unnoticed for three weeks of green runs.

The canary reproduces the failure deliberately - two suites run with --without-building, so
PlatformIO does not relink and both execute the same leftover binary - and requires the
checker to catch it. It also fails if the reproduction stops reproducing: if PlatformIO ever
relinks per suite under that flag, the reason both harnesses stopped passing it no longer
holds, and the harness should be revisited rather than left on a stale assumption.

bin/test-state-check.sh already existed with fixtures asserting CLEAN/CLEAN/DIRTY/MISSING and
had never run in CI. Wire it in too - the shared-state checker had the same blind spot, and
somebody had already written the test for it.

* fix(ci): run the attribution canary where it cannot clobber the daemon

The canary relinks $BUILD_DIR/$PROGNAME, and in simulator-tests that replaced the daemon
binary with a test suite. The integration test then started it and waited for a listening
socket, which a test binary never opens - by assertion, since initializeTestEnvironment()
now fails a suite that holds one - so the step sat until its 20s timeout and the job exited
124. The canary itself had already passed.

Move it to platformio-tests, where the binary is per-suite already and nothing downstream
needs the daemon, and place it after the coverage capture so its extra runs stay out of the
numbers. The shared-state self-test stays in simulator-tests; it touches no binary.

Fitting failure mode for this branch: one shared program path, two consumers, and the second
one silently getting the first one's build.

* fix(ci): silence the XXE rule on the attribution checker

semgrep blocks xml.etree.ElementTree.parse as XXE-prone. The input here is the JUnit report
PlatformIO wrote moments earlier in the same run, and anything able to plant a hostile report
is already executing its own code in that job, so parsing it defused changes nothing it could
do. defusedxml is in the tree but only under bin/bump_metainfo with its own requirements, and
pulling it onto this path would add an install step to every native test job for no reachable
threat.

Suppressed with a reason at the call site, the same shape as the subprocess-shell-true
suppression in extra_scripts/nrf54l15_linker.py.

* fix(test): address the review findings on the harness guards

Two were real defects rather than style:

- state_count_errors() returned "0\n0" for a log with no ERROR lines, because grep -c prints 0
  and *then* exits 1, so the `|| printf 0` fallback appended a second one. The classifier threw
  a syntax error on it. Dormant only because every suite currently emits at least one ERROR
  line; the planned log-level demotions would have driven most suites to zero and tripped it
  everywhere, looking like the demotions broke the harness.
- check-test-attribution.py returned OK for a report whose cases carry no `file` attribute. It
  cannot prove ownership in that state, so a changed JUnit format would have restored the exact
  false green it exists to catch. Now its own finding, listed and fatal.

The rest: keep the sandbox when an error budget is breached, since that is the one outcome
whose evidence was being deleted; reject a missing or non-numeric option value in
stress-suite.sh instead of running an empty loop and reporting 0/0 as a pass; exit on INT/TERM
rather than cleaning up and carrying on; drive repetitions through pio-test-isolate.sh so a
stress run exercises the real invocation; require the canary to see MISATTRIBUTED rather than
any non-zero exit, so an unreadable report cannot read as a caught mismatch; and check for
listening sockets before every test, since a listener would be opened by the code under test.

resetAdminKeyFallbackBudget() is a new PIO_UNIT_TESTING hook, shaped like the neighbouring
resetRoutingAuthEvaluationCount(). The refill stamp is only meaningful against the clock that
produced it, so a suite switching timebases leaves a stamp from the other one and the next
unsigned subtraction reads as a near-infinite gap - silently refilling the bucket.

Also move the semgrep marker onto its own line: buried mid-sentence in a comment it was
ignored, and the XXE finding stayed blocking.
2026-08-16 11:34:02 +00:00

250 lines
7.4 KiB
C++

// 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 <cstring>
#include <unity.h>
void setUp(void) {}
void tearDown(void) {}
// --- Valid UTF-8 should pass through unchanged ---
void test_ascii_unchanged()
{
char buf[32] = "Hello World";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("Hello World", buf);
}
void test_valid_2byte_unchanged()
{
// "café" - é is C3 A9
char buf[16] = "caf\xC3\xA9";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("caf\xC3\xA9", buf);
}
void test_valid_3byte_unchanged()
{
// "€" is E2 82 AC
char buf[16] = "\xE2\x82\xAC";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("\xE2\x82\xAC", buf);
}
void test_valid_4byte_emoji_unchanged()
{
// 🌙 is F0 9F 8C 99
char buf[16] = "\xF0\x9F\x8C\x99";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("\xF0\x9F\x8C\x99", buf);
}
void test_valid_mixed_unchanged()
{
// "Hi 🌙!" - mix of ASCII and 4-byte
char buf[16] = "Hi \xF0\x9F\x8C\x99!";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("Hi \xF0\x9F\x8C\x99!", buf);
}
void test_empty_string()
{
char buf[8] = "";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("", buf);
}
// --- Invalid sequences observed in the wild ---
void test_truncated_4byte_at_end()
{
// Name with valid emoji 🌙 followed by a truncated 4-byte sequence + ASCII
char buf[32] = "Lunar Tower \xF0\x9F\x8C\x99\xF0\x9F\x97"
"4";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
// The 🌙 should be preserved; F0 9F 97 is an incomplete 4-byte sequence,
// '4' (0x34) is not a valid continuation byte
TEST_ASSERT_EQUAL_STRING("Lunar Tower \xF0\x9F\x8C\x99???4", buf);
}
void test_lone_lead_bytes_without_continuations()
{
// Mixed ASCII with stray multibyte lead bytes (E1, F3) lacking proper continuations
char buf[32] = "Mesht\xE1\xF3tic 37e2";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
// E1 expects 2 continuation bytes, but F3 is not a continuation → E1 replaced
// F3 expects 3 continuation bytes, 't','i','c' are not continuations → F3 replaced
TEST_ASSERT_EQUAL_STRING("Mesht??tic 37e2", buf);
}
// --- Edge cases ---
void test_bare_continuation_byte()
{
// 0x80 alone is invalid (continuation byte with no lead)
char buf[8] = "\x80";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("?", buf);
}
void test_overlong_2byte()
{
// C0 AF is an overlong encoding of U+002F '/'
char buf[8] = "\xC0\xAF";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
// C0 is a 2-byte lead, AF is valid continuation, but codepoint 0x2F < 0x80 → overlong
// C0 replaced, AF (now bare continuation) also replaced
TEST_ASSERT_EQUAL_STRING("??", buf);
}
void test_surrogate_half()
{
// ED A0 80 encodes U+D800 (surrogate half - invalid in UTF-8)
char buf[8] = "\xED\xA0\x80";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("???", buf);
}
void test_5byte_sequence_rejected()
{
// F8 80 80 80 80 - 5-byte sequence, not valid UTF-8
char buf[8] = "\xF8\x80\x80\x80\x80";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
// F8 is invalid lead (>= 0xF8), each 0x80 is bare continuation
TEST_ASSERT_EQUAL_STRING("?????", buf);
}
void test_truncated_3byte_at_buffer_end()
{
// Buffer is exactly 4 bytes: E2 82 then forced null at [3]
char buf[4];
buf[0] = '\xE2';
buf[1] = '\x82';
buf[2] = '\0'; // String ends before the 3-byte sequence completes
buf[3] = '\0';
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("??", buf);
}
void test_null_termination_enforced()
{
// Fill buffer completely with no null terminator
char buf[5];
memset(buf, 'A', sizeof(buf));
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
// Should be null-terminated and content preserved (all ASCII)
TEST_ASSERT_EQUAL_STRING("AAAA", buf);
}
void test_null_buffer()
{
TEST_ASSERT_FALSE(sanitizeUtf8(nullptr, 10));
}
void test_zero_size()
{
char buf[4] = "Hi";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, 0));
// Buffer should be untouched
TEST_ASSERT_EQUAL_STRING("Hi", buf);
}
void test_valid_max_codepoint()
{
// U+10FFFF = F4 8F BF BF (maximum valid Unicode codepoint)
char buf[8] = "\xF4\x8F\xBF\xBF";
TEST_ASSERT_FALSE(sanitizeUtf8(buf, sizeof(buf)));
TEST_ASSERT_EQUAL_STRING("\xF4\x8F\xBF\xBF", buf);
}
void test_above_max_codepoint()
{
// U+110000 = F4 90 80 80 (just above maximum valid Unicode)
char buf[8] = "\xF4\x90\x80\x80";
TEST_ASSERT_TRUE(sanitizeUtf8(buf, sizeof(buf)));
}
// --- clampLongName: local 24-byte cap over wider wire buffers ---
void test_clamp_long_name_short_unchanged()
{
char buf[40] = "Kevin Hester";
clampLongName(buf);
TEST_ASSERT_EQUAL_STRING("Kevin Hester", buf);
}
void test_clamp_long_name_exact_cap_unchanged()
{
char buf[40] = "abcdefghijklmnopqrstuvwx"; // exactly 24 bytes
clampLongName(buf);
TEST_ASSERT_EQUAL_STRING("abcdefghijklmnopqrstuvwx", buf);
}
void test_clamp_long_name_truncates_39_bytes()
{
char buf[40];
memset(buf, 'a', 39);
buf[39] = '\0';
clampLongName(buf);
TEST_ASSERT_EQUAL_INT(MAX_LONG_NAME_BYTES, (int)strlen(buf));
}
void test_clamp_long_name_fixes_partial_rune_at_cut()
{
// 22 ASCII then a 4-byte emoji straddling the 24-byte boundary
char buf[40];
memset(buf, 'a', 22);
buf[22] = '\xF0';
buf[23] = '\x9F';
buf[24] = '\x8C';
buf[25] = '\x99';
buf[26] = '\0';
clampLongName(buf);
TEST_ASSERT_EQUAL_INT(24, (int)strlen(buf));
TEST_ASSERT_EQUAL_INT('?', buf[22]);
TEST_ASSERT_EQUAL_INT('?', buf[23]);
}
void setup()
{
UNITY_BEGIN();
// Valid UTF-8 passthrough
RUN_TEST(test_ascii_unchanged);
RUN_TEST(test_valid_2byte_unchanged);
RUN_TEST(test_valid_3byte_unchanged);
RUN_TEST(test_valid_4byte_emoji_unchanged);
RUN_TEST(test_valid_mixed_unchanged);
RUN_TEST(test_empty_string);
// Invalid sequences observed in the wild
RUN_TEST(test_truncated_4byte_at_end);
RUN_TEST(test_lone_lead_bytes_without_continuations);
// Edge cases
RUN_TEST(test_bare_continuation_byte);
RUN_TEST(test_overlong_2byte);
RUN_TEST(test_surrogate_half);
RUN_TEST(test_5byte_sequence_rejected);
RUN_TEST(test_truncated_3byte_at_buffer_end);
RUN_TEST(test_null_termination_enforced);
RUN_TEST(test_null_buffer);
RUN_TEST(test_zero_size);
RUN_TEST(test_valid_max_codepoint);
RUN_TEST(test_above_max_codepoint);
// clampLongName
RUN_TEST(test_clamp_long_name_short_unchanged);
RUN_TEST(test_clamp_long_name_exact_cap_unchanged);
RUN_TEST(test_clamp_long_name_truncates_39_bytes);
RUN_TEST(test_clamp_long_name_fixes_partial_rune_at_cut);
exit(UNITY_END());
}
void loop() {}