[dhruv-joshi15]

Summary

Two test-only defects, each one where coverage that already exists in the tree
was not being executed. On current master the test suite does not build at all,
and one kernel regression test has been dead code since #97.

This PR changes test code and test build configuration only. No kernel,
network, driver, service, HAL or product source is touched, and no runtime
behaviour changes:

$ git diff upstream/master...HEAD --stat
 tests/CMakeLists.txt | 20 ++++++++++++++++++++
 tests/test_kernel.c  |  3 ++-
 tests/test_net.c     | 24 ++++++++++++------------
 3 files changed, 34 insertions(+), 13 deletions(-)

git diff upstream/master...HEAD --stat -- kernel/ net/ core/ hal/ services/ drivers/ power/ include/ products/ CMakeLists.txt is empty.

---

1. Understanding of the project I built before changing anything

Context I established first, so that the change I made would be the smallest one
that fits how this repository actually works.

What EoS is. A multi-platform embedded OS framework in pure C11 — not a
distro and not only an RTOS. One source tree provides a small RTOS kernel, a HAL
with host and bare-metal backends, a driver framework, and networking, power and
runtime-service layers, with 84 board descriptors and 48 product profiles
selected at configure time. Repository maturity is Experimental per
STATUS.md.

Component layout, each building its own static library:

Library	Path	Contents
eos_kernel	kernel/	tasks, mutex with priority inheritance, semaphores, queues (ipc.c), multicore, heap
eos_hal	hal/	33 peripherals; backend split hal_linux.c / hal_rtos.c
eos_drivers	drivers/	driver framework, devicetree parser
eos_net	net/	socket facade plus the POSIX backend net_posix.c
eos_power	power/	power management
core	core/	config parser, logging, dependency graph, cache, lockfile
services	services/	crypto, security, ota, sensor, motor, filesystem, gps, ui, init, compat

Execution paths. Two distinct ones. On a host it is an ordinary main()
linking the static libraries — no boot. On bare metal, kernel/src/arch/ startup
plus a board linker script runs eos_kernel_init() → eos_task_create() →
eos_kernel_start(), which enters the port layer. The kernel is cooperative:
blocking primitives end in eos_port_yield(). Host tests supply their own
eos_port_* mocks, which is what makes kernel tests runnable off-target — and it
is exactly the mechanism the restored kernel test depends on.

Configuration model. -DEOS_PRODUCT=<name> makes CMake define
EOS_PRODUCT_<UPPER>; include/eos/eos_config.h includes products/<name>.h,
which sets the EOS_ENABLE_* flags. Most service bodies and public types are
wrapped in #if EOS_ENABLE_X, so a header's types do not exist unless the right
product macro is defined. This is why EOS_BUILD_TESTS=ON requires
-DEOS_PRODUCT=vbox_test.

Platform abstraction subtleties that matter here. CMake, not the
preprocessor, selects the HAL backend and defines EOS_HAL_BACKEND_HOST/_RTOS.
Files needing POSIX headers are gated on NOT CMAKE_CROSSCOMPILING, deliberately
not on EOS_PLATFORM, because EOS_PLATFORM defaults to linux even for an
ARM cross build. That gate is what decides whether the code this PR touches is
compiled at all.

Error conventions. Three, by layer: EOS_KERN_* in the kernel, EosResult
in core, and 0/-1 in services and HAL.

Test infrastructure. 44 C suites driven by ctest, plus 10 pytest tests. The
suites express nearly every check as a bare assert(), so tests/CMakeLists.txt
carries add_compile_options(-UNDEBUG) (added by #104) to keep those assertions
alive in the Release build CI uses. Understanding that line was central to
choosing the fix in this PR.

---

2. Tests performed before fixing — the baseline

Measured on a clean checkout of master at 9861a3b, in both Debug and
Release, with the project's documented commands and CI's own ctest flags.
Results were identical in both build types.

cmake -B build -DCMAKE_BUILD_TYPE=<Debug|Release> -DEOS_BUILD_TESTS=ON -DEOS_PRODUCT=vbox_test
cmake --build build --parallel
ctest --test-dir build --output-on-failure --no-tests=error --timeout 120 --parallel
python3 -m pytest tests/ -q

Check	Baseline result
configure	PASS, exit 0
build	FAIL, exit 2, 3 errors
build warnings	2
ctest	FAIL, exit 8 — 97% tests passed, 1 tests failed out of 34, test_net (Not Run)
test_net binary	never produced
test_kernel	12/12 PASS, queue-overflow case 0 occurrences
pytest	10 passed

The build failure, verbatim:

tests/test_net.c:212:5: error: call to undeclared function 'assert';
  ISO C99 and later do not support implicit function declarations
tests/test_net.c:232:5: error: ... (same)
tests/test_net.c:249:5: error: ... (same)
make[2]: *** [tests/CMakeFiles/test_net.dir/test_net.c.o] Error 1

Both baseline warnings turned out to be the two defects this PR fixes, sitting in
plain sight in every build log:

net/src/net.c:15:13:        warning: unused variable 'net_initialized'          [-Wunused-variable]   <- unrelated, out of scope
tests/test_kernel.c:159:13: warning: unused function 'test_queue_send_waiter_overflow' [-Wunused-function]

I also ran the clang static analyzer over all 54 library translation units to
make sure I was not missing a larger problem in the same area. It produced 50
warnings, of which 24 are the normal deadcode.DeadStores idiom in the Ed25519
ref10 code and the rest are unused. Nothing in kernel/src/ipc.c or
net/src/net_posix.c.

---

3. How I identified the bugs, and the code walkthrough

Neither defect came from guessing. Both were found by reading the two compiler
diagnostics the build was already emitting, then following them into git history.

Bug 1 — tests/test_net.c calls assert() with no <assert.h>

Reading the file, the includes are <stdio.h>, <stdlib.h>, <string.h>, and
inside the EOS_NET_POSIX block <arpa/inet.h>, <pthread.h>, <time.h>. None
declares assert. So the ten assert() calls in that block compile as an
implicit call to a function that exists in no libc.

The file already contains the right mechanism, with a comment explaining why:

#define CHECK(cond) do { \
    if (!(cond)) { \
        fprintf(stderr, "[FAIL] %s:%d: %s\n", __FILE__, __LINE__, #cond); \
        exit(1); \
    } \
} while (0)

So this was not a missing include — it was ten call sites that had fallen out of
the file's own convention.

I confirmed the defect two independent ways rather than trusting the one
diagnostic. Compile error on Clang ≥ 15, and — because gcc-12, which the CI
test job uses, only warns about an implicit declaration — I demoted the
diagnostic to reproduce gcc's behaviour and showed it fails at link instead:

$ cc -std=c11 -Wno-implicit-function-declaration -c tests/test_net.c -o t.o
$ nm -u t.o | grep -i assert
_assert
$ cc t.o libeos_net.a libeos_hal.a -lpthread
Undefined symbols for architecture arm64:
  "_assert", referenced from:
      _test_net_tcp_round_trip in t.o        (x5)
      _test_net_resolve_localhost in t.o     (x4)
      _test_net_connect_refused in t.o       (x1)

[srpatcha]

Review — eos#123 "fix(tests): restore POSIX network tests and kernel regression coverage"

head: 1bb5914 author: dhruv-joshi15 ci: none — no checks have run

Verdict: Test-only, scoped honestly, and the -Werror=implicit-function-declaration
guard is the durable version of a fix the maintainer's own repair commit says was needed.
The problem is evidence, not design: checks.txt is empty — zero status checks on this
head — and the PR body stops mid-walkthrough with a thorough baseline and no post-fix
result at all. Both changes carry a specific way to fail, and neither the body nor CI
shows that they don't.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	checks.txt (empty)	No CI has run on this head. Every other PR in this batch reports 26–27 statuses; this one reports none. The likely cause is procedural rather than technical — dhruv-joshi15 is a different author from the rest of the batch and GitHub holds workflow runs for a first-time contributor until a maintainer approves them. The effect is what matters: nothing independent has compiled this branch, so findings 2–4 have no answer from any source.	Maintainer action, before further review: Approve and run workflows on this PR. It is also the cheapest way to close findings 3 and 4 outright.
2	Medium	PR body, §3	The body has a complete baseline and no result. §2 measures master@9861a3b in Debug and Release with configure/build/ctest/pytest and exit codes. §3 then ends mid-sentence after the nm output: there is no "Bug 2" walkthrough, no after-the-fix table, no ctest line. .ai/reviewer.md: "Every PASS should point at command output." The only figure this PR asserts about its own change is 13/13, and that lives inside a printf format string at tests/test_kernel.c:324 — it is a label, not a measurement.	Add an "after" table in the same shape as §2's baseline: build exit code and warning count, the ctest summary line, test_kernel's stdout showing [PASS] queue send waiter overflow, and pytest. The baseline is genuinely well done; it just needs its other half.
3	Medium	tests/test_kernel.c:323	A test that has never run is being switched on, and nothing shows it passes. test_queue_send_waiter_overflow is not a smoke case: it creates EOS_MAX_TASKS - 2 waiters, drives them through q7_yield_hook, and asserts both q7_enqueued > 0 and q7_overflow_reported (tests/test_kernel.c:183-184) — it depends on the queue's waiter list overflowing at a particular point. It has been dead since #97, and #97 is the change that reworked priority inheritance, so the odds that its assumptions still hold are not obviously good. If it fails, this PR turns test_kernel red.	Paste the run. If it does fail, that is a real kernel finding and a better PR than this one — but it needs to be reported as a finding, not registered as green.
4	Medium	tests/CMakeLists.txt:36-38	Same gap, other direction. add_compile_options(-Werror=implicit-function-declaration) is directory-scoped, so it applies to all 44 C suites declared below it. That is the correct control — and it is by construction capable of breaking targets that compile today, since an implicit declaration is currently a gcc-12 warning that nothing fails on. The PR does not show the suite still building under it.	One build log covering the whole tests/ directory. The compiler-family guard (MATCHES "GNU|Clang") and the MSVC note about nightly.yml on windows-latest are the right care to have taken; this just needs the evidence that goes with it.
5	Medium	PR body, §2	The premise is stale. The baseline is master@9861a3b; master is now e37ef4a, "fix(tests): add the missing assert.h include that broke master", which added #include <assert.h> at tests/test_net.c:5. "On current master the test suite does not build at all" is no longer true, and the 3 errors / test_net (Not Run) figures no longer describe the base. The PR is still worth having — e37ef4a's own message diagnoses the cause as "nothing built the merge result" and "no required status check", which is what finding 4's guard addresses and e37ef4a's include does not — but the framing has to be rewritten to say that.	Rebase, re-measure, and reframe: the change is now "make the class of defect impossible" rather than "unbreak the build". Note also that once the last assert() leaves test_net.c, the <assert.h> include added by e37ef4a is unused and should go with it.
6	Low	tests/test_net.c:212-249	Four open PRs convert the same ten assert() calls to CHECK(): #119, #121, #122 and this one. Three of the four will conflict on identical hunks. This is the best version of them — it is the only one that also repairs the comment at tests/test_net.c:12-16, which on master names the wrong macro twice ("Not CHECK():", "NDEBUG deletes CHECK()") where both mean assert().	Keep the hunk here and drop it from #119, #121 and #122, which carry it only as a consequence of being stacked on #116. Worth a merge-order note on all four, as was done for #118/#115.
7	Low	tests/test_kernel.c:325	The file still ends without a trailing newline (\ No newline at end of file sits on the added line, so the PR reproduces it). This PR rewrites that exact line.	Add the newline while you are on it.

What is right here, since it bears on whether the claims are trustworthy. The
diagnosis of bug 1 was confirmed two independent ways rather than from one diagnostic —
the compile error on Clang ≥ 15 and, because CI's gcc-12 only warns, a demonstration
that it then fails at link, with the nm -u output showing the undefined _assert and
the exact call counts per function. That is the standard .ai/reviewer.md asks for and it
is rare to see. The scope claim is stated in a checkable form (git diff --stat over
kernel/ net/ core/ hal/ services/ drivers/ power/ include/ products/ CMakeLists.txt
empty) rather than asserted. The static-analyzer sweep characterises its own noise (24 of
50 being the ref10 deadcode.DeadStores idiom) instead of reporting a bare count. None of
that is undone by findings 2–4; the missing piece is specifically the after.

No check is weakened by this PR: it registers a test rather than removing one, converts
assert to a macro that cannot be compiled out, and promotes a warning to an error.
Nothing is disabled, no assertion dropped, no permission widened.

Architecture conformance

§21 — conforms. Test code and test build configuration only; tests/ is where
.ai/architect.md's target shape puts unit, functional and regression suites, and the
diff does not leave that directory. No dependency direction is touched, no tier crossed,
nothing added to a public header.

§28 Status, evidence and claims — this is the section the PR sits against. §28 makes
"Implemented" require "code and functional tests" and "Validated" require "CI/test
reports". Restoring a suite that ctest reported as Not Run and a case that was dead code
is directly in service of that. Findings 2–4 are the same policy applied to the PR itself.

No master-design proposal from this PR; nothing here shows the design to be wrong, stale
or silent.

Proposed changes

1. Rebase onto e37ef4a and re-measure; rewrite §2 and the summary to the current base
   (finding 5).
2. Run the suite and add the "after" table, including test_kernel's stdout (findings
   2–4). If test_queue_send_waiter_overflow fails, split it out and report it as a
   kernel finding rather than shipping it registered.
3. Remove #include <assert.h> from tests/test_net.c:5 once the last assert() is
   gone.
4. Trailing newline on tests/test_kernel.c.
5. Coordinate with #119/#121/#122 so only one of the four carries the CHECK conversion
   (finding 6).

Nothing here requires changing what the PR does — only rebasing it and showing the result.

Not checked

• I did not build this branch and I did not run ctest. Building inside
  /home/srpatcha/eos/eos would leave the repository dirty, which this run is required
  not to do. So findings 3 and 4 are stated as unverified risks, not as failures: I do
  not know whether test_queue_send_waiter_overflow passes, and I do not know whether all
  44 suites survive -Werror=implicit-function-declaration. With checks.txt empty,
  no other source answers either.
• The %s of §2's baseline — the 3 build errors, exit 8, 97% tests passed, the 50
  analyzer warnings — I did not reproduce. They are consistent with e37ef4a's commit
  message describing the same breakage, which is corroboration but not verification.
• I read test_queue_send_waiter_overflow (tests/test_kernel.c:159-187) and its hook
  (:148-156) to judge how load-bearing it is. I did not analyse whether its assumptions
  about the waiter list survived #97 — that question is what running it answers.
• Whether any existing test target currently relies on an implicit declaration. I did not
  grep the other 43 suites for one.

---

Automated architecture review of 1bb5914c5485 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.