Repository navigation
test: keep assertions live in test binaries - #104
Merged
srpatcha merged 9 commits intoSep 1, 2026
Merged
Conversation
…ckend
Which backend source CMake compiles and which registrar eos_hal_init() calls
are the same decision made in two places, with two different conditions.
CMakeLists.txt:
if(EOS_PLATFORM STREQUAL "linux" OR NOT CMAKE_CROSSCOMPILING)
target_sources(eos_hal PRIVATE hal/src/hal_linux.c)
else()
target_sources(eos_hal PRIVATE hal/src/hal_rtos.c)
hal_common.c:
#ifdef __linux__
eos_hal_linux_register();
#else
eos_hal_rtos_register();
They agree on Linux and on a cross build, and disagree on every other host. A
macOS host build compiles hal_linux.c, then references a symbol from the file
it did not compile:
Undefined symbols for architecture arm64:
"_eos_hal_rtos_register", referenced from: _eos_hal_init
CMake now sets EOS_HAL_BACKEND_HOST or EOS_HAL_BACKEND_RTOS next to the
target_sources() call that makes the choice, and hal_common.c keys off that.
__linux__ stays as a fallback for anyone compiling these files without CMake.
Two things had to move with it:
- hal.h declared the registrars under the same #ifdef, so hal_common.c could
not even name the one CMake had compiled. Both are declared unconditionally
now; exactly one is still defined per build, and an unreferenced declaration
costs nothing. The header's own comment already argued for this -- "an
undeclared registrar is one nobody can find".
- hal_linux.c wraps its whole body in #ifdef __linux__, so on macOS it compiled
to an empty translation unit and the build had no backend at all. It is the
host backend rather than the Linux one: the only platform calls in it are
clock_gettime() and nanosleep(), both POSIX. Guarded on the CMake define,
with __linux__ as the fallback. hal_rtos.c takes the mirror condition.
Linux behaviour is unchanged: EOS_HAL_BACKEND_HOST and __linux__ are both
defined there, and the same registrar is selected as before. Cross builds get
EOS_HAL_BACKEND_RTOS and are likewise unchanged. eos master CI is green on
Linux and stays that way.
Verified on macOS: the eos_hal link error is gone and ctest goes from 0/28 to
19/28.
This does NOT make macOS a supported host on its own. The remaining 9 failures
are services/compat/src/os_adapter_posix.c calling pthread_mutex_timedlock()
and sem_timedwait(), neither of which macOS provides. That is a separate piece
of work in a different subsystem and I have not attempted it here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With -DEBLDR_BOARD=qemu_arm64 actually taking effect (it was -DBOARD= before
and silently ignored), eBoot's configure now runs the board path and refuses:
EBLDR_VERIFY_STAGE1 is ON but board qemu_arm64 has no stage-1 linker
script, so there is no stage-1 image to hash. Configure with
-DEBLDR_VERIFY_STAGE1=OFF to build stage-0 without the integrity check.
That refusal is correct -- the board ships only board_qemu_arm64.c/.h, no
linker scripts, so there is no stage-1 image for stage-0 to verify -- and the
message names the flag. Passed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The QEMU step ran for ~20ms, left an empty serial log, and reported nothing -- because its stderr went to /dev/null and its exit code into `|| true`. The one message that says why the guest did not boot was the message being thrown away. Keep the run non-fatal only for exit 124 (timeout(1) killing a still-running guest, which is how a bare-metal image "finishes"); any other nonzero exit now fails the step loudly with QEMU's own stderr printed. Also list the artifact files before booting, so a missing or truncated eos_qemu.bin is visible. The TC validation gate below is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With stderr finally visible, the reason the guest never booted is one line:
qemu-system-aarch64: failed to find romfile "efi-virtio.rom"
The workflow installs qemu-system-arm with --no-install-recommends, which
skips ipxe-qemu -- the package that ships efi-virtio.rom. The virt machine
creates a default virtio-net device, that device wants its boot ROM, and QEMU
exits during machine init. This has been the failure since the job first ran;
it was just invisible behind 2>/dev/null.
The simulation uses no networking, so -nic none removes the device rather
than installing a ROM for a NIC nothing talks to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Build & Test (Linux x86_64) job hangs in "Run C unit tests" and holds the workflow open until GitHub's 6-hour limit. Which test hangs is currently unknowable from the outside: ctest runs with no per-test timeout, so the job never reaches the summary that would name it. --timeout 120 turns the invisible hang into a named failure. 120s is generous -- the slowest test in this suite takes 1.4s. Second, related: the concurrency group I added cancels in-progress runs on every push. Right for PR branches; wrong for master, where each merge cancelled the previous merge's run -- which is why this job has not been allowed to COMPLETE on master since 08-30 and the hang went unnoticed. cancel-in-progress is now conditional on the ref not being master. Verified locally (macOS): ctest 25/28 pass with the timeout flag; the 3 remaining are Not Run because test_os_adapter/test_os_services/test_ota do not build here (the known pthread_mutex_timedlock/sem_timedwait gap). Those three are Linux-only -- exactly the ones this timeout will name if one of them is the hang. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The test named by the new ctest timeout is test_net, and the hang is not a
networking bug. CI builds Release; Release defines NDEBUG; NDEBUG deletes
assert() together with the expression inside it. Half of test_net's checks
wrap the call under test:
assert(eos_net_connect(c, &addr) == 0);
assert(eos_net_send(c, "ping", 4) == 4);
With those gone, no client ever connects, the echo thread blocks in accept()
forever, and pthread_join() hangs the suite -- for exactly as long as nothing
bounds the test, which is what the previous commit fixed.
Reproduced locally before fixing: a Release build of the unmodified test hangs
on macOS too (this was never Linux-specific; my earlier local runs simply were
not Release). Replaced every assert() with a CHECK() macro that always
evaluates and exits nonzero on failure.
After: Release test_net passes 19/19 in under a second; full local Release
ctest 27/28 (the 1 is test_os_adapter, Not Run on macOS as known).
Same latent class elsewhere: other C tests also use assert() and silently
check nothing in Release builds -- they pass today because their expressions
have no needed side effects that block, but they are weaker than they look.
Worth a sweep in its own PR rather than burying it here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ARM Cortex-M4 job caught a regression in my backend-selection change:
hal/src/hal_linux.c:30:10: fatal error: sys/ioctl.h: No such file or directory
That cross build also compiles hal_linux.c -- EOS_PLATFORM defaults to
"linux", so CMake's host branch is taken even for arm-none-eabi -- and it only
ever built because `#ifdef __linux__` reduced the file to an empty translation
unit there. Widening the guard to EOS_HAL_BACKEND_HOST un-emptied it on a
toolchain with no POSIX headers.
The guard now asks the right question -- "is this a hosted POSIX build?" --
once, in hal.h:
#if defined(__linux__) || \
(defined(EOS_HAL_BACKEND_HOST) && (defined(__unix__) || defined(__APPLE__)))
#define EOS_HAL_HOSTED 1
and hal_linux.c, hal_rtos.c and hal_common.c all key off EOS_HAL_HOSTED
instead of each doing their own arithmetic. arm-none-eabi defines none of
those macros, so the empty-TU behavior the ARM job depends on is restored;
aarch64-linux-gnu defines __linux__, so the QEMU simulation lane is unchanged;
macOS defines __APPLE__, so the original fix stands.
Verified locally by preprocessing hal_linux.c both ways (clang -U__APPLE__
-U__unix__ -U__linux__ simulating bare metal: body absent; hosted: present)
and a full Release build + ctest: 26/28, the 2 remaining being the documented
macOS Not-Runs (pthread_mutex_timedlock/sem_timedwait).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI configures -DCMAKE_BUILD_TYPE=Release. Release defines NDEBUG. NDEBUG
deletes assert() *together with the expression inside it*. These suites express
nearly every check as assert():
684 assert() calls across 16 test files
All of them compile to nothing in CI, and the tests they belong to pass by
reaching the end of main(). Demonstrated with the exact flags CI uses:
$ clang -DNDEBUG -o t t.c # int main(void){ assert(1 == 2); ... }
REACHED END: the assert did nothing
exit=0
This is not theoretical. test_net wrapped the calls under test in assert():
assert(eos_net_connect(c, &addr) == 0);
assert(eos_net_send(c, "ping", 4) == 4);
Under Release no client ever connected, the echo thread blocked in accept()
forever, and the whole suite hung until ctest killed it -- the hang I fixed in
the previous commit. Every other assert() in the tree is the same construction;
that one was simply the only one whose deleted expression also blocked.
Stripping NDEBUG from test targets is a one-line change that makes all 684
meaningful without rewriting a single call site. It applies to the test
binaries only -- the libraries under test are still compiled exactly as CI
builds them for release, so this does not turn the suite into a debug-build
test of a debug-build library.
Verified on macOS (Release): every test that builds passes with assertions
live -- test_kernel (89 asserts), test_priority_inherit (49), test_power (26),
test_net (32), test_motor_ctrl (15) among them, roughly 211 assertions now
actually executing. The remaining targets do not link here for an unrelated
reason: services/compat/src/os_adapter_posix.c uses pthread_mutex_timedlock()
and sem_timedwait(), which macOS does not provide (that is embeddedos-org#96's subject, not
this one). Linux CI builds all 28 and is the real check.
Stacked on embeddedos-org#102, which repairs the macOS HAL link so this could be evaluated
locally at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 1, 2026
srpatcha
approved these changes
Sep 1, 2026
This was referenced Sep 1, 2026
srpatcha
added a commit
that referenced
this pull request
Sep 1, 2026
* fix: repair master — the default configuration and the test link Two independent breakages, both in "CI — eos" on master (9861a3b). net/src/net_posix.c did not compile in the default configuration. net.h wraps its declarations in `#if EOS_ENABLE_NET`, and nothing sets that flag without a product profile or a test build. CMake, however, adds net_posix.c to eos_net whenever the build is not cross-compiling — the flag never enters that decision. So the header expanded to nothing while the file still defined the API against it: net/src/net_posix.c:43:30: error: unknown type name 'eos_net_addr_t' net.c already guards its body the same way and survives; the POSIX mapping was the half that did not. Guarded so the two translation units appear and disappear together. This is the exact gap the "Default configuration" job was added to catch, and it caught it. tests/test_net.c failed to link: undefined reference to `assert' The file defines CHECK() precisely because CI builds Release, NDEBUG deletes assert() along with the expression inside it, and half these checks wrap the call under test — the comment above the macro says so. Ten call sites in the POSIX section still used bare assert(), including assert(eos_net_connect(...) == 0), the exact pattern named there. They were invisible until #104 added -UNDEBUG and stopped assert() being a no-op; the file never includes <assert.h>, so it resolved to an implicit function and failed at link. Converted to CHECK(), which the file already establishes as the right macro here. Verified locally on macOS, all three configurations CI builds: cmake -B build/host -DCMAKE_BUILD_TYPE=Release builds (fails on master) -DEOS_PRODUCT=medical / aerospace both build -DEOS_BUILD_TESTS=ON -DEOS_PRODUCT=vbox_test + ctest 34/34 passed Negative control: the same default-configuration build against unpatched origin/master reproduces the unknown-type errors verbatim. * test: build four suites that existed but were never compiled, and guard it Nine test_*.c files in tests/ are named by no add_executable(), so they are compiled by nothing. Nothing reports that: the file stays in the tree, ctest counts one fewer test, and the run is green. test_mpu_validate.c is the one that matters. It landed with the MPU fix it guards (#109) and a later merge kept the file while dropping the target that built it, so its six cases -- regions the MPU cannot program, and the divide-before-check that preceded them -- have not run since. Four of the nine build and pass as they are: test_mpu_validate 6 cases, registration restored test_hal_stm32f4 21 cases; it needed only the board header directory on the include path, which is the whole reason it never ran test_e2e kernel boot-to-task path test_net_mock eNet socket API against a mock transport ctest goes from 34 to 38. The other five stay out, each for a reason recorded next to its name in the new guard: one hangs because it asserts stub semantics against the real POSIX implementation; one cannot link because backends/ is in no add_subdirectory(); and three would pass without testing anything -- test_profiles reports 0/0 because its directory allowlist admits only "products", which holds no *.yaml; test_performance_benchmarks asserts a wall-clock threshold; test_emulation_simulation asserts a literal against itself one line after assigning it. tests/unit/test_cmake_test_registration.py makes the omission fail. It parses tests/CMakeLists.txt statically -- no cmake, no compiler -- and asserts every suite is either built or listed with a reason, that every built target is registered with ctest, that the exclusion list has no stale or non-existent entries, and, by name, that test_mpu_validate is built. Dropping that registration again fails two of the five. The eBoot repo has carried the same guard since a merge dropped test_fw_transport there; this is the same file adapted to this layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pkg): give eos_pkg a real trust anchor, or refuse to verify services/pkg/eos_pkg.c checked package signatures against static const uint8_t eos_pkg_public_key[32] = {0}; #99 stopped that key accepting every package. It did not make package signing work: the anchor is still all zeros, so eos_pkg now rejects every package, including correctly signed ones, and reports "signature verification failed" while doing it -- blaming the package for a key that was never provisioned. The key comes from the platform now. eos_pkg_set_trust_anchor() is the seam a keystore or a provisioning step fills, EOS_PKG_TRUST_ANCHOR_HEX is the build-time form, and with neither configured verification refuses and names the reason. EOS_ALLOW_UNSIGNED_PKG is the opt-in for tests and bring-up, matching EOS_ALLOW_STUB_CRYPTO in services/crypto and EOS_ALLOW_UNSIGNED_OTA in services/ota. An anchor is validated when it is installed rather than once per package, so an unusable key is reported as an unusable key. That needed #99's checks to be reachable, so ed25519_public_key_is_usable() exports them rather than duplicating the constants. services/pkg was in no CMakeLists.txt, so nothing compiled this file and no test could reach it -- which is why the all-zero anchor survived. It is now the eos_eapp library (pkg/ already owns the name eos_pkg for the build-recipe registry). Closes the second half of #98. * build: keep services/pkg out of cross-compiled builds Adding services/pkg to the build put it in the ARM Cortex-M4 leg too, where it does not belong and does not compile: eos_pkg.c lists directories, spawns processes and waits on them, so it needs <dirent.h>, <unistd.h> and <sys/wait.h>. Bare-metal newlib turns the first of those into a hard #error. Gated on CMAKE_CROSSCOMPILING, which is the switch net_posix.c already uses for the same reason, and for the same stated reason: EOS_PLATFORM defaults to "linux" and a cross toolchain file does not change it, so that condition is true even for a Cortex-M build. Verified with a toolchain file that sets CMAKE_SYSTEM_NAME while keeping the host compiler, so the CMake logic can be checked without arm-none-eabi installed: eos_eapp and test_pkg_trust_anchor are not generated under it, and both are present in the host build. Host ctest 39/39. * fix(tests): create the .eapp fixture 0600, not world-writable CodeQL flags this PR with one new high-severity alert: tests/test_pkg_trust_anchor.c:128 File created without restricting permissions A file may be created here with mode 0666, which would make it world-writable. fopen(path, "wb") creates with 0666 & ~umask, so on a permissive umask the fixture is writable by anyone on the machine. These fixtures are the signed packages the verifier is about to trust, so a world-writable one lets anything on the box rewrite the payload between the write here and the read under test -- which is the very substitution the suite exists to detect. Create through open(O_CREAT|O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR) and fdopen the descriptor. Verified the call yields 0600. 39/39. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>
Kartikey1306
added a commit
to Kartikey1306/eos
that referenced
this pull request
Sep 14, 2026
…ilds
master does not compile:
tests/test_net.c:212:5: error: call to undeclared function 'assert';
ISO C99 and later do not support implicit function declarations
test_net.c makes ten assert() calls and includes stdio, stdlib and string
but never assert.h. It built while every test target was compiled with
NDEBUG, and embeddedos-org#104 removed that from test targets so the assertions in
these suites are real again -- which is what turned the missing include
into an error.
The include is what the file always needed; nothing else changes. Swept
the rest of tests/*.c and this is the only file that calls assert()
without including the header.
Restores the build: 34/34, and the assertions are live in the binary
rather than compiled away.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI configures
-DCMAKE_BUILD_TYPE=Release. Release definesNDEBUG.NDEBUGdeletesassert()together with the expression inside it.These suites express nearly every check as
assert():All of them compile to nothing in CI, and the tests they belong to pass by reaching the end of
main().Demonstrated with the exact flags CI uses:
This already cost us a day
test_netwrapped the calls under test inassert():Under Release no client ever connected, the echo thread blocked in
accept()forever, and the whole suite hung until ctest killed it. That was theBuild & Test (Linux x86_64)hang — a job that had not completed onmastersince 08-30.Every other
assert()in the tree is the same construction. That one was simply the only one whose deleted expression also blocked rather than silently passing.The change
One line, applied to test targets only:
It makes all 684 assertions meaningful without rewriting a single call site. The libraries under test are still compiled exactly as CI builds them for release — this does not turn the suite into a debug-build test of a debug-build library, which would be a different and much weaker thing.
Verification
macOS, Release, assertions live — every test that builds passes:
test_kerneltest_priority_inherittest_sensor/test_power/test_net/test_motor_ctrlroughly 211 assertions actually running, all green.
What I could not check locally: the remaining targets do not link on macOS for an unrelated reason —
services/compat/src/os_adapter_posix.ccallspthread_mutex_timedlock()andsem_timedwait(), which macOS does not provide. That is #96's subject, not this one. Linux CI builds all 28 and is the real check here — if any of the previously-inert assertions was hiding a genuine failure, this PR is what surfaces it, and that is the point.Stacking
Stacked on #102, which repairs the macOS HAL link and is what made local evaluation possible at all.