mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-10 17:29:41 +02:00
docs(tests): finish the ephemeral-port wording (#570)
- docs/browser-testing-guide.md still described the shrink-only legacy list and the mobile suite's fixed ports; both are gone. - CLAUDE.md: name the one fixed port left (codex-predictive-echo's separate lab server on 3222), keep "Never 3000", and say the guard refuses a fixed port rather than that it checks boundPort is read. - The tui-client comment described the old "port + 1" dead port. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -438,7 +438,7 @@ One module per domain in `src/web/routes/` (plus a barrel; `ls src/web/routes/`
|
||||
- **App setting**: decide per-device vs synced first. Per-device keys go in the `displayKeys` set in settings-ui.js and must NOT be added to `SettingsUpdateSchema` (it is `.strict()`). ⚠️ Anything in `PUT /api/settings` that acts on a setting (the `toggleService` watcher calls) must resolve from **`merged`** (persisted + incoming), never from the raw request body: a partial PUT omits keys it doesn't intend to change, and `body.x ?? default` turns every omission into "apply the default" and silently resets live services. Pinned by `test/routes/system-routes-settings-partial-put.test.ts`.
|
||||
- **Hook event**: Add to `HookEventType`, add hook in `hooks-config.ts:generateHooksConfig()`, update `HookEventSchema`
|
||||
- **Mobile feature**: Add to relevant singleton, guard with `MobileDetection.isMobile()`. New header buttons must stay off phones (`test/mobile-header-buttons-policy.test.ts`).
|
||||
- **New test**: a test that constructs `WebServer` itself binds port 0 (`new WebServer(0, …)`, then `server.boundPort`), and a raw `http`/`net`/Fastify/`ws` server listens on `0` and reads `address().port`; mobile tests get theirs from `createTestServer()` and read `boundPort` too. Both enforced by `test/test-ports-guard.test.ts`. Route tests use `app.inject()` (no port needed); see `test/routes/_route-test-utils.ts`.
|
||||
- **New test**: a test that constructs `WebServer` itself binds port 0 (`new WebServer(0, …)`, then `server.boundPort`), and a raw `http`/`net`/Fastify/`ws` server listens on `0` and reads `address().port`; mobile tests get theirs from `createTestServer()` and read `boundPort` too. `test/test-ports-guard.test.ts` refuses a fixed port in either. Route tests use `app.inject()` (no port needed); see `test/routes/_route-test-utils.ts`.
|
||||
|
||||
**Validation**: Zod v4 (different API from v3). Define schemas in `schemas.ts`, use `.parse()`/`.safeParse()`.
|
||||
|
||||
@@ -477,7 +477,7 @@ Raw `npx vitest` skips the config (and with it `setup.ts`); always use `npm test
|
||||
|
||||
**Tmux safety**: under vitest (`VITEST`), `TmuxManager` no-ops ALL shell commands (`IS_TEST_MODE` in `src/tmux-manager.ts`), docker IO is no-op'd likewise, and `Session` spawns an echo PTY (`TEST_PTY_SCRIPT`) instead of attaching tmux. `test/setup.ts` gives each file a temp `HOME`/`USERPROFILE` and strips `CODEMAN_PASSWORD`/`CODEMAN_USERNAME`, `CODEMAN_GESTURE` and `CODEMAN_INSTANCE`/`CODEMAN_DATA_DIR`/`CODEMAN_TMUX_SOCKET` (pinned by `test/test-env-isolation.test.ts`). ⚠️ `CODEMAN_DATA_DIR` overrides the temp HOME, so never drop its strip; strip `CODEMAN_INSTANCE` in the setup file, never in a hook (captured at first import). ⚠️ Delete case trees only via `safeRmHomeTree()`. ⚠️ Raw `npx vitest` without `--config` skips `setup.ts` and its isolation. → [architecture-invariants#test-isolation-tmux-docker-and-home](docs/architecture-invariants.md#test-isolation-tmux-docker-and-home)
|
||||
|
||||
**Ports**: every test binds port 0. A test that constructs `WebServer` reads `server.boundPort` (mobile tests: `createTestServer()`, then `server.boundPort`); a raw `http`/`net`/Fastify/`ws` server listens on `0` and reads `address().port`. `test/test-ports-guard.test.ts` fails a `WebServer` built on any other port and a raw `listen` on a number or `…PORT` constant. A test that needs a port nothing listens on binds 0, reads it and closes (`closedPort()` in `test/daemon-control.test.ts`), never "the server's port + 1".
|
||||
**Ports**: every in-process test server binds port 0 (the one exception, `test/codex-predictive-echo.test.ts`, starts a separate lab server process on 3222). A test that constructs `WebServer` reads `server.boundPort` (mobile tests: `createTestServer()`, then `server.boundPort`); a raw `http`/`net`/Fastify/`ws` server listens on `0` and reads `address().port`. `test/test-ports-guard.test.ts` fails a `WebServer` built on any other port and a raw `listen` on a number or `…PORT` constant. A test that needs a port nothing listens on binds 0, reads it and closes (`closedPort()` in `test/daemon-control.test.ts`), never "the server's port + 1". Never 3000 (the live instance).
|
||||
|
||||
⚠️ **Browser tests can pass vacuously on mobile input paths.** Two traps, both hit on 2026-07-27 while fixing the phone Enter button: **(1)** driving input with `app.sendInput('…')` writes PAST the `LocalEchoOverlay`, so `pendingText` stays empty and any overlay bug is invisible — type with `page.keyboard.type()` instead; **(2)** headless Chromium reports `MobileDetection.isTouchDevice()` **false even with `hasTouch: true`**, so `_localEchoEnabled` is off and the local-echo branch never executes. Force it (`app._localEchoEnabled = true`) or the test proves nothing. Assert on real state (`app._localEchoOverlay.pendingText`, plus `tmux -L codeman capture-pane -p -t <pane>` for what actually reached the PTY), not on HTTP 200.
|
||||
|
||||
|
||||
@@ -77,11 +77,13 @@ A test that starts a server binds an ephemeral port, never a fixed one:
|
||||
|
||||
- `WebServer`: `new WebServer(0, false, true)`, then read the port the OS handed out from
|
||||
`server.boundPort` after `await server.start()`. `test/test-ports-guard.test.ts` fails
|
||||
any `WebServer` built under `test/` on a non-zero port (a shrink-only legacy list
|
||||
excepted).
|
||||
- A raw Fastify or `ws` server: `listen({ port: 0 })`, then `address().port`.
|
||||
- The mobile suite (`test/mobile/**`, via `createTestServer(PORT)`) keeps the fixed-port
|
||||
convention in `test/mobile/README.md` for now.
|
||||
any `WebServer` built under `test/` on a non-zero port.
|
||||
- A raw `http`, `net`, Fastify or `ws` server: `listen({ port: 0 })`, then
|
||||
`address().port`. The guard also fails a raw listen on a number or a `…PORT` constant.
|
||||
- The mobile suite (`test/mobile/**`) gets its server from `createTestServer()`, which
|
||||
binds an ephemeral port too; read it from `server.boundPort`.
|
||||
- The one exception is `test/codex-predictive-echo.test.ts`, which starts a separate lab
|
||||
server process on port 3222 that the guard cannot see.
|
||||
- Never port 3000: that is the live instance.
|
||||
|
||||
`scripts/browser-comparison.mjs` is a standalone script outside the guard and still uses
|
||||
|
||||
@@ -9,8 +9,8 @@
|
||||
*
|
||||
* Two rules, both over every file under test/:
|
||||
* - A `WebServer` is built with the literal `0` as its port — `new WebServer(…)`, a
|
||||
* class declared `extends WebServer`, or a destructured alias (`{ WebServer: T }`) —
|
||||
* and the test reads `server.boundPort`.
|
||||
* class declared `extends WebServer`, or a destructured alias (`{ WebServer: T }`).
|
||||
* The test then reads `server.boundPort`, which this guard does not check.
|
||||
* - A raw server (`http`/`net`/Fastify `listen`, `new WebSocketServer`) never listens
|
||||
* on a number or a `…PORT` constant: `listen(0, …)` / `{ port: 0 }`, then
|
||||
* `address().port`.
|
||||
|
||||
@@ -46,7 +46,7 @@ async function closedPort(): Promise<number> {
|
||||
}
|
||||
|
||||
let port: number;
|
||||
/** Nothing listens one above the bound port: the "no server" path. */
|
||||
/** A port nothing listens on (`closedPort()`): the "no server" path. */
|
||||
let deadPort: number;
|
||||
let baseUrl: string;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user