test: raise coverage and add CI coverage gates - #2
Merged
Conversation
Follow-up to the hardening pass — closes the coverage gaps the review surfaced, test-first throughout. Server (30 → 84 tests; line coverage 79% → 96%): - fs.ts (10%→96%): listDir/searchDir/searchPaths pickers and deleteOmpSession — the only function that destroys user files - omp.ts (8%→97%): CLI bridge via a generated fake omp binary (arg passing, stdout parsing, failure + timeout kill) and listOmpSessions header scanning with an injectable sessions root - sessions/manager.ts (88%→99%): delete data-safety (real file removal, resumedFromHistory protection), non-mock spawn identity handshake run against the bundled mock host as a fake omp binary, spawn-failure surfacing and start() retry semantics - rpc/process.ts (80%→87%): readiness timeout, close() SIGTERM→SIGKILL escalation, kill(), frame delivery - rpc/frame.ts (100% lines): chunk metadata/base64/sequence validation - app.ts (75%→88%): cross-site WS upgrade refusal, fs picker endpoints, WS refresh_session/stop_session control messages, non-mock omp CLI error surfacing; replay-pruning test de-flaked with a sentinel Web (19 → 57 tests; measured files now include api.ts at 80%): - SessionStore state machine driven by a fake transport (new injectable socket options on the constructor): hello/hydration, message upsert + history dedupe, immutable tool executions with late-joiner synthesis, extension dialog queue/cancel/answer, widget/status/editor surfaces, open_url http(s)-only gating, notice/output caps, registry refcounting - markdown unescape/tag-strip pipeline; format/content helpers Also fixes two real bugs the new tests exposed: - spawnOmpProcess threw synchronously on spawn failure, leaving its rejected `ready` promise unconsumed; it now reports failures through `ready` - start() could never retry a session whose spawn had failed (dead process handle blocked respawning) CI: test step now runs under `--coverage` with ratcheted thresholds (server ≥93%, web ≥90%) and a README badge.
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
1 participant
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.
Follow-up to #1: closes the coverage gaps surfaced by the pre-release analysis, test-first throughout.
Baseline → now
What's newly covered
Server
fs.ts(10%→96%): pickers +deleteOmpSession— the only user-file-destroying functionomp.ts(8%→97%): CLI bridge via a generated fake omp binary (arg passing, parsing, failure, timeout kill); session-history scanner with injectable rootsessions/manager.ts(88%→99%): delete data safety (real removal +resumedFromHistoryprotection), the full non-mock spawn identity handshake run against a fake omp binary (sh wrapper exec'ing the mock host), spawn-failure surfacing,start()retryrpc/process.ts: readiness timeout,close()SIGTERM→SIGKILL escalation (real child processes), kill(), frame deliveryrpc/frame.ts: chunk metadata/base64/sequence validation to 100% linesapp.ts: cross-site WS upgrade refusal, fs picker endpoints, WS refresh/stop control messages, non-mock omp CLI error surfacingWeb
Real bugs the new tests exposed (fixed)
spawnOmpProcessthrew synchronously on spawn failure — its rejectedreadypromise was left unconsumed. Failures now flow throughready.start()could never retry a session whose spawn had failed (a dead process handle blocked respawning) — errored sessions now restart from scratch.De-flake
The replay-pruning test no longer sleeps for the replay burst: it waits for the private auto-hydration response (queued after all buffered frames) before asserting absence.
CI
Test step now runs under
--coveragewith ratcheted thresholds (server ≥93%, web ≥90%) — both currently pass locally — plus a README badge.