test(daemon): add F1/F2 process-lifecycle integration tests; fix clobbered start error
Links the real EmbeddedDaemon into the ObsidianDragonTests target (its deps were already present) and adds two POSIX integration tests that exercise the actual fork/exec/waitpid fixes headlessly: - testExecFailureReported (F2): start() against a non-executable file must fail with a precise "not executable or wrong architecture" reason. - testDaemonCrashDetected (F1): a short-lived child that exits abnormally is still detected (crash_count_ increments) while isRunning() is hammered from the test thread — a regression test for the reap race. Writing the F2 test surfaced a real bug: start()'s failure branch called setState(State::Error, "Failed to start dragonxd process"), and setState stores the Error message into last_error_ — clobbering the precise message startProcess() had just set, so getLastError()/the UI only ever saw the generic string. Fixed to pass the preserved detail to setState, so the precise reason survives and now also reaches the state callback (crash panel / status). ctest 1/1, green including the two new integration tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -24,9 +24,9 @@ back-fill applied additively to `res/lang/*.json` (42 keys — all 6 for es/de/f
|
||||
entries whose glyphs aren't in the current `NotoSansCJK-Subset.ttf` were left on English fallback
|
||||
rather than render as tofu).
|
||||
|
||||
**Still owed before release:** the F1/F2 manual repros (`kill -SEGV` / non-executable binary — not
|
||||
unit-testable), and a **CJK subset-font rebuild** (`scripts/build_cjk_subset.py`, needs the Noto
|
||||
CJK source font) to cover those 6 deferred zh/ja/ko strings.
|
||||
**Still owed before release:** a **CJK subset-font rebuild** (`scripts/build_cjk_subset.py`, needs
|
||||
the Noto CJK source font) to cover the 6 deferred zh/ja/ko strings. *(F1 and F2 now have headless
|
||||
integration-test coverage — see the progress log — so their GUI repros are optional, not blocking.)*
|
||||
|
||||
---
|
||||
|
||||
@@ -537,6 +537,8 @@ design record; see the shared-helper table above.
|
||||
|
||||
## Progress log
|
||||
|
||||
- **F1/F2 integration tests** — ☑ added `testExecFailureReported` (F2) and `testDaemonCrashDetected` (F1) to `test_phase4.cpp`, driving the **real** `EmbeddedDaemon` fork/exec/waitpid code headlessly (POSIX; required linking `embedded_daemon.cpp` into the test target — its deps were already there). The F1 test hammers `isRunning()` from the test thread while the child exits, so it's a genuine regression test for the reap race. **The F2 test caught a real bug:** `start()`'s failure branch overwrote `startProcess()`'s precise `last_error_` ("…not executable or wrong architecture") with a generic "Failed to start dragonxd process" (because `setState(Error, …)` stores its message into `last_error_`), so the precise reason never reached `getLastError()`/the UI — **fixed** to preserve the detail (now also surfaced via the state callback / crash panel). Build-clean; `ctest` 1/1.
|
||||
|
||||
- **F1** — ☑ landed: `isRunning()` (POSIX) now reads the atomic `state_` (predicate `Running || Stopping`) instead of calling `waitpid`, leaving `monitorProcess()` the sole reaper. Clean build (all targets link); `ctest` 1/1 passing. Not unit-testable — needs the manual `kill -SEGV` repro before release.
|
||||
- **F2** — ☑ landed: `startProcess()` (POSIX) now creates a `FD_CLOEXEC` self-pipe before `fork()`; the child writes `errno` to it on `execv` failure, the parent reads EOF-vs-errno and, on failure, reaps the zombie + sets a precise `last_error_` ("not executable or wrong architecture") + returns `false` (so `start()` no longer reports `Running` for a daemon that never started). Parent-side `setpgid` is now best-effort with a `DEBUG_LOGF` on failure. Clean build; `ctest` 1/1 passing. Not unit-testable — needs the manual non-executable / wrong-arch-binary repro before release.
|
||||
- **F8** — ☑ landed: `isLocalHost()` tightened to exact loopback via `isExactIPv4Loopback` (a `127.`-prefixed *hostname* like `127.evil.com` is no longer misclassified as local). `tryConnect()` now **refuses** a plaintext connection to a remote host instead of warn-and-proceeding — a local-network MITM can no longer capture `rpcuser:rpcpassword` — unless the user opts in with `rpcallowplaintext=1` in `DRAGONX.conf` (new `ConnectionConfig::allow_plaintext_remote` + `allowsPlaintextRemote()` policy). The refusal surfaces via status line + a one-time notification. New `testIsLocalHost` (12 assertions) + `testAllowsPlaintextRemote` (5). Clean build; `ctest` 1/1 passing. **Breaking — needs release notes; Settings-toggle UI deferred (see as-built note).**
|
||||
|
||||
Reference in New Issue
Block a user