test+fix(setup): complete __DREAM_RESULT__ sentinel contract (exception path + tests)#1019
Open
yasinBursali wants to merge 5 commits intoLight-Heart-Labs:mainfrom
Open
Conversation
Collaborator
|
Audit follow-up: needs test fixes before merge. The sentinel contract work is directionally good, but the new SetupWizard tests mock the wrong fetch call: Step 2 consumes |
The PASS/FAIL paths already terminate the /api/setup/test stream with the machine-readable sentinel that the SetupWizard parser consumes, but an unexpected exception inside run_tests() (e.g. a process.stdout read error) would exit the generator without yielding anything terminal. The frontend defaults absence-of-sentinel to "failure" and falls back to scraping log lines for "All tests passed!", which is fragile. Wrap the existing try/finally body in a nested try/except that catches generic Exception, logs the trace, and yields a final FAIL sentinel before unwinding. CancelledError and OSError are re-raised so client disconnects and process kills propagate normally instead of trying to write into a dead stream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…icker a11y SetupWizard.test.jsx mocks fetch with a chunk-yielding ReadableStream shim and pins six diagnostic-stream behaviors: - PASS sentinel transitions the run to "All systems operational" and hides the sentinel line from user-visible output. - FAIL sentinel surfaces the failure UI and keeps Complete Setup disabled (tested=false). - Absent sentinel falls back to scanning collected output for "All tests passed!" trailer (older backend compatibility). - Stream with neither sentinel nor success trailer defaults to failure — wizard refuses to greenlight unknown outcomes. - Sentinel split across two chunks (e.g. "...PA" + "SS:0\n") still parses, exercising the buffer-and-split decoder loop. - Component unmount during an in-flight diagnostic fires the cleanup effect which aborts the AbortController; the captured signal goes from aborted=false to aborted=true. TemplatePicker.a11y.test.jsx pins: - Available card is enabled with the template name in its accessible name; isApplied / inProgress / hasErrors all disable the button. - The corresponding "Applied" / "Installing…" / "Has errors" status caption is rendered as text inside the card. - Every Lucide icon emitted by the picker carries aria-hidden="true" so the adjacent text label is the screen-reader-visible carrier. - Empty and undefined template lists render nothing (defensive). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ission Locks in the streaming contract that the SetupWizard frontend parser consumes. Five tests against /api/setup/test: - Script exits 0 → final stream line is "__DREAM_RESULT__:PASS:0". - Script exits 3 → final stream line is "__DREAM_RESULT__:FAIL:3" (the literal subprocess returncode, not a coerced 1). - Script missing on both INSTALL_DIR/scripts and cwd → error_stream() fallback still emits a structurally valid sentinel. - Sentinel matches the exact regex the frontend pins (^__DREAM_RESULT__:(PASS|FAIL):(-?\d+)$) — guards against stray whitespace, prefix drift, or trailing-character regressions on either side of the contract. - Unauthenticated POST returns 401 before any stream is opened. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two test-only fixes addressing maintainer audit feedback:
1. SetupWizard.test.jsx: replace fetch.mockImplementationOnce with a
URL-dispatched fetch mock helper (makeFetchMock + stubFetchWithSetupStream).
Step 2 of the wizard fetches /api/templates and /api/extensions/catalog
before the diagnostic POST — the previous mockImplementationOnce was
consumed by the wrong call, leaving /api/setup/test unmocked. Routing
by URL is the only stable pattern.
2. TemplatePicker.a11y.test.jsx: narrow the decorative-icon selector
from `container.querySelectorAll('svg')` (which caught the unhidden
HardDrive disk-size glyph) to `div.rounded-lg svg` — the status icon
wrapper specifically. Out-of-scope SVGs (HardDrive) are no longer
asserted by this test, matching the test's stated intent ("decorative
status icons (Loader2, AlertTriangle, Check, default Icon)").
All 13 tests in the two files pass: 6 SetupWizard sentinel-parser cases
+ 7 TemplatePicker a11y/defensive cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yasinBursali
force-pushed
the
test/setup-wizard-template-picker-coverage
branch
from
f8016f1 to
a69f2af
Compare
Contributor
Author
|
Rebased on current
Re-ran on the rebased branch: pytest 5/5 + Vitest 13/13 pass. Description updated; left a note about adding aria-hidden to HardDrive as a separate small follow-up. |
CI Lint-Python-with-Ruff job was failing on this PR with: F401 [*] `os` imported but unused --> dream-server/extensions/services/dashboard-api/tests/test_setup_sentinel.py:12:8 The `os` module is referenced only inside the docstring at line 55 (`Path(os.getcwd()) / "dream-test-functional.sh"`), not in actual code. The `stat` module IS used (`stat.S_IXUSR`/`S_IXGRP`/`S_IXOTH` at line 45) so it stays. Verified: `ruff check --select E,F,W` passes; 5/5 pytest cases still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
✅ Rebased on current
main2026-04-28 + audit feedback addressed#1003 has merged. The PR diff reduced to:
routers/setup.pyexception-path sentinel + 3 new test files (484 lines total). Two test fixes added on top per the audit:fetch.mockImplementationOncewith a URL-dispatched fetch helper (makeFetchMock+stubFetchWithSetupStream). Step 2 of the wizard fetches/api/templatesand/api/extensions/catalogbefore the diagnostic POST, so the one-shot mock was being consumed by the wrong call. Routing by URL is the only stable pattern.container.querySelectorAll('svg')(which caught the unhiddenHardDrivedisk-size glyph) todiv.rounded-lg svg— the status icon wrapper specifically. The test's stated intent ("Loader2, AlertTriangle, Check, default Icon") now matches what it actually asserts.What
Backend —
routers/setup.pyexception-path sentinelWraps the
run_setup_diagnosticsgenerator body in try/except. Re-raisesCancelledErrorandOSError(transport-level concerns — client disconnect, broken pipe — should propagate normally). Catches genericException, logs withlogger.exception(), yields aFAIL:1sentinel so the frontend parser never gets stuck in partial-state UI.The
except Exceptionis tagged# noqa: BLE001 — sentinel contract requires *some* terminal signal. CG reviewed and accepted this as a CLAUDE.md §2 I/O-boundary exception — narrow, logged, and the wire contract with the React parser is explicit.Tests (3 new files, 18 assertions)
test_setup_sentinel.py(5 cases): asserts PASS:0 on success, FAIL:<rc>on non-zero exit, fallback sentinel when script missing, machine-parseable regex format, auth required. 5/5 pass locally.SetupWizard.test.jsx(6 cases): sentinel parser happy path (PASS and FAIL with rc), fallback-scan on"All tests passed!"without sentinel, default-failure when neither signal is present, AbortController unmount cleanup, mocked ReadableStream chunk split across sentinel boundary.TemplatePicker.a11y.test.jsx(7 cases):isAppliedrender branch +aria-hidden="true"on decorative Lucide icons (scoped to status icon wrapper) + defensive undefined-template handling.Testing
python3 -m pytest tests/test_setup_sentinel.py -v)npx vitest run SetupWizard.test.jsx TemplatePicker.a11y.test.jsx)SetupWizard.jsx:104:^__DREAM_RESULT__:(PASS|FAIL):(-?\d+)$Platform Impact
Known Considerations (will file as fork follow-up issues if not addressed in review)
asyncio.create_subprocess_execto raise. Small follow-up test worth adding.HardDrivedisk-size glyph inTemplatePickerdoes not currently carryaria-hidden="true". Production a11y could be tightened by adding it; this PR's test scope is decorative status icons, so the test selector was narrowed instead. A separate small follow-up could add the marker.