test: direct suite for the settings preview builders#583
Conversation
lib/codex-manager/settings-preview.ts renders the preview-first settings UX but was reachable only through the settings-hub panels. These 12 cases pin: statusline-field normalization (defaults, first-occurrence dedup, defensive copy), summary composition (default parts, cooldown toggle, field ordering, the two nothing-visible explanations incl. the status-badge dependency), the account-list row (badge toggles, layout text from the real resolveMenuLayoutMode incl. the legacy boolean), and highlightPreviewToken TTY gating with ANSI-stripped equality against the plain render. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
More reviews will be available in 12 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review follow-ups: setStdoutTty now sets writable: true (matching how tty streams expose isTTY) and documents that the TTY-sensitive tests must stay sequential; the undefined input path is now also guarded against returning the shared DEFAULT_STATUSLINE_FIELDS reference. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Summary
test/settings-preview.test.ts(12 cases) forlib/codex-manager/settings-preview.ts— the preview-first settings UX renderer, previously reachable only through the interactive settings-hub panels.What Changed
One new test file pinning, with the real
resolveMenuLayoutModefromsettings-hub/shared.ts(no mocks):normalizeStatuslineFields: undefined/empty fall back to the documented defaults (and the empty-input path returns a defensive copy, not the shared default array); duplicates collapse to first-occurrence order.buildSummaryPreviewText: default composition (last-used + limits with cooldowns; status text only when status badges are hidden — the inverse dependency the preview exists to explain); the cooldown toggle; part ordering followingmenuStatuslineFields; and both nothing-visible explanations (the status-badge-dependency note vs the generic message when no status field is configured).buildAccountListPreview: the demo row label with both badges, individual badge toggles, and the layout text driven by the real resolver — explicitexpanded-rowsmode and the legacymenuShowDetailsForUnselectedRowsboolean both produce "details shown on all rows".highlightPreviewToken: returns plain text on non-TTY stdout; on a forced TTY it ANSI-wraps the token (stripping escapes recovers the exact plain text), and in the composed summary only the focused part carries escapes — the trailing limits segment stays clean.TTY forcing uses
Object.definePropertyonprocess.stdout.isTTYwith the original descriptor restored infinally.Validation
npm run typecheck(also runs in the pre-commit hook)npx eslint test/settings-preview.test.ts --max-warnings=0npm test -- test/settings-preview.test.ts— 12/12Docs and Governance Checklist
Risk and Rollback
Additional Notes
main. Complements test: cover dashboard settings clone defaults and equality semantics #569's clone/equality suite, which owns the settings data model while this one owns the preview rendering.https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
Generated by Claude Code
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
adds
test/settings-preview.test.ts— a 12-case direct suite for the settings-preview builders inlib/codex-manager/settings-preview.ts. no source changes; the three issues flagged in the previous round (sequential-execution comment,writable: trueon the defineProperty call, and thefromUndefinedreference-identity assertion) are all present in this version.normalizeStatuslineFields(2 cases): undefined/empty fall back to defaults returning a defensive copy; duplicates deduplicate to first-occurrence order.buildSummaryPreviewText(4 cases): default composition, cooldown toggle, field ordering, and both nothing-visible messages (status-badge-dependency note vs. generic message).buildAccountListPreview(3 cases): demo row with/without badges, expanded-rows layout via both the explicitmenuLayoutModefield and the legacymenuShowDetailsForUnselectedRowsboolean.highlightPreviewToken(3 cases): non-TTY plain text, TTY ANSI wrapping with strip-and-recover assertion, focused-part isolation in the composed summary.Confidence Score: 5/5
test-only pr with no source changes; all previous review issues have been addressed in the submitted version.
the change is a single new test file that adds 12 assertions against an existing renderer module. the setStdoutTty helper correctly handles nested calls and restores the original descriptor in finally. the fromUndefined reference-identity guard is present. the sequential-execution comment is in place. no production code is touched.
no files require special attention.
Important Files Changed
Reviews (2): Last reviewed commit: "test: harden TTY helper and pin undefine..." | Re-trigger Greptile