From c583b510e09ddf9d58cca5b6132bf19a8f5a8091 Mon Sep 17 00:00:00 2001 From: Riddhi Dutta Date: Thu, 30 Oct 2025 12:48:45 +0530 Subject: [PATCH] Refactoring unit tests in packages/cli/src/ui (#12251) Co-authored-by: riddhi --- .../src/ui/components/SettingsDialog.test.tsx | 1366 ++++++----------- .../SettingsDialog.test.tsx.snap | 189 +-- .../src/ui/contexts/KeypressContext.test.tsx | 523 ++----- 3 files changed, 748 insertions(+), 1330 deletions(-) diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index f8577e6bb7..4baa2825df 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -223,10 +223,27 @@ const TOOLS_SHELL_FAKE_SCHEMA: SettingsSchemaType = { }, } as unknown as SettingsSchemaType; -describe('SettingsDialog', () => { - // Simple delay function for remaining tests that need gradual migration - const wait = (ms = 50) => new Promise((resolve) => setTimeout(resolve, ms)); +// Helper function to render SettingsDialog with standard wrapper +const renderDialog = ( + settings: LoadedSettings, + onSelect: ReturnType, + options?: { + onRestartRequest?: ReturnType; + availableTerminalHeight?: number; + }, +) => + render( + + + , + ); +describe('SettingsDialog', () => { beforeEach(() => { mockToggleVimEnabled.mockResolvedValue(true); }); @@ -242,127 +259,80 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); const output = lastFrame(); expect(output).toContain('Settings'); expect(output).toContain('Apply To'); - expect(output).toContain( - 'Use Enter to select, Tab to change focus, Esc to close', - ); + // Use regex for more flexible help text matching + expect(output).toMatch(/Enter.*select.*Esc.*close/); }); it('should accept availableTerminalHeight prop without errors', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect, { + availableTerminalHeight: 20, + }); const output = lastFrame(); // Should still render properly with the height prop expect(output).toContain('Settings'); - expect(output).toContain('Use Enter to select, Esc to close'); + // Use regex for more flexible help text matching + expect(output).toMatch(/Enter.*select.*Esc.*close/); }); - it('should show settings list with default values', () => { + it('should render settings list with visual indicators', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); const output = lastFrame(); - // Should show some default settings - expect(output).toContain('●'); // Active indicator - }); - - it('should highlight first setting by default', () => { - const settings = createMockSettings(); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - const output = lastFrame(); - // First item should be highlighted with green color and active indicator - expect(output).toContain('●'); + // Use snapshot to capture visual layout including indicators + expect(output).toMatchSnapshot(); }); }); describe('Settings Navigation', () => { - it('should navigate down with arrow key', async () => { + it.each([ + { + name: 'arrow keys', + down: TerminalKeys.DOWN_ARROW, + up: TerminalKeys.UP_ARROW, + }, + { + name: 'vim keys (j/k)', + down: 'j', + up: 'k', + }, + ])('should navigate with $name', async ({ down, up }) => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount, lastFrame } = render( - - - , - ); + const { stdin, unmount, lastFrame } = renderDialog(settings, onSelect); - // Press down arrow + const initialFrame = lastFrame(); + expect(initialFrame).toContain('Vim Mode'); + + // Navigate down act(() => { - stdin.write(TerminalKeys.DOWN_ARROW as string); // Down arrow + stdin.write(down as string); }); - expect(lastFrame()).toContain('● Disable Auto Update'); + await vi.waitFor(() => { + expect(lastFrame()).toContain('Disable Auto Update'); + }); - // The active index should have changed (tested indirectly through behavior) - unmount(); - }); + // Navigate up + act(() => { + stdin.write(up as string); + }); - it('should navigate up with arrow key', async () => { - const settings = createMockSettings(); - const onSelect = vi.fn(); - - const { stdin, unmount } = render( - - - , - ); - - // First go down, then up - stdin.write(TerminalKeys.DOWN_ARROW as string); // Down arrow - await wait(); - stdin.write(TerminalKeys.UP_ARROW as string); - await wait(); - - unmount(); - }); - - it('should navigate with vim keys (j/k)', async () => { - const settings = createMockSettings(); - const onSelect = vi.fn(); - - const { stdin, unmount } = render( - - - , - ); - - // Navigate with vim keys - stdin.write('j'); // Down - await wait(); - stdin.write('k'); // Up - await wait(); + await vi.waitFor(() => { + expect(lastFrame()).toContain('Vim Mode'); + }); unmount(); }); @@ -371,20 +341,17 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount, lastFrame } = render( - - - , - ); + const { stdin, unmount, lastFrame } = renderDialog(settings, onSelect); // Try to go up from first item act(() => { stdin.write(TerminalKeys.UP_ARROW); }); - await wait(); - - expect(lastFrame()).toContain('● Codebase Investigator Max Num Turns'); + await vi.waitFor(() => { + // Should wrap to last setting (without relying on exact bullet character) + expect(lastFrame()).toContain('Codebase Investigator Max Num Turns'); + }); unmount(); }); @@ -396,17 +363,12 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const component = ( - - - - ); - const { stdin, unmount, lastFrame } = render(component); + const { stdin, unmount, lastFrame } = renderDialog(settings, onSelect); // Wait for initial render and verify we're on Vim Mode (first setting) await vi.waitFor(() => { - expect(lastFrame()).toContain('● Vim Mode'); + expect(lastFrame()).toContain('Vim Mode'); }); // Navigate to Disable Auto Update setting and verify we're there @@ -414,7 +376,7 @@ describe('SettingsDialog', () => { stdin.write(TerminalKeys.DOWN_ARROW as string); }); await vi.waitFor(() => { - expect(lastFrame()).toContain('● Disable Auto Update'); + expect(lastFrame()).toContain('Disable Auto Update'); }); // Toggle the setting @@ -448,25 +410,35 @@ describe('SettingsDialog', () => { }); describe('enum values', () => { - it('toggles enum values with the enter key', async () => { + it.each([ + { + name: 'toggles to next value', + initialValue: undefined, + expectedValue: StringEnum.BAZ, + }, + { + name: 'loops back to first value when at end', + initialValue: StringEnum.BAZ, + expectedValue: StringEnum.FOO, + }, + ])('$name', async ({ initialValue, expectedValue }) => { vi.mocked(saveModifiedSettings).mockClear(); - vi.mocked(getSettingsSchema).mockReturnValue(ENUM_FAKE_SCHEMA); + const settings = createMockSettings(); + if (initialValue !== undefined) { + settings.setValue(SettingScope.User, 'ui.theme', initialValue); + } + const onSelect = vi.fn(); - const component = ( - - - - ); - const { stdin, unmount } = render(component); + const { stdin, unmount } = renderDialog(settings, onSelect); + + act(() => { + stdin.write(TerminalKeys.DOWN_ARROW as string); + stdin.write(TerminalKeys.ENTER as string); + }); - // Press Enter to toggle current setting - stdin.write(TerminalKeys.DOWN_ARROW as string); - await wait(); - stdin.write(TerminalKeys.ENTER as string); - await wait(); await vi.waitFor(() => { expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled(); }); @@ -475,44 +447,7 @@ describe('SettingsDialog', () => { new Set(['ui.theme']), expect.objectContaining({ ui: expect.objectContaining({ - theme: StringEnum.BAZ, - }), - }), - expect.any(LoadedSettings), - SettingScope.User, - ); - - unmount(); - }); - - it('loops back when reaching the end of an enum', async () => { - vi.mocked(saveModifiedSettings).mockClear(); - vi.mocked(getSettingsSchema).mockReturnValue(ENUM_FAKE_SCHEMA); - const settings = createMockSettings(); - settings.setValue(SettingScope.User, 'ui.theme', StringEnum.BAZ); - const onSelect = vi.fn(); - const component = ( - - - - ); - - const { stdin, unmount } = render(component); - - // Press Enter to toggle current setting - stdin.write(TerminalKeys.DOWN_ARROW as string); - await wait(); - stdin.write(TerminalKeys.ENTER as string); - await wait(); - await vi.waitFor(() => { - expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalled(); - }); - - expect(vi.mocked(saveModifiedSettings)).toHaveBeenCalledWith( - new Set(['ui.theme']), - expect.objectContaining({ - ui: expect.objectContaining({ - theme: StringEnum.FOO, + theme: expectedValue, }), }), expect.any(LoadedSettings), @@ -527,15 +462,12 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Press Space to toggle current setting - stdin.write(' '); // Space key - await wait(); + act(() => { + stdin.write(' '); // Space key + }); unmount(); }); @@ -544,16 +476,13 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Navigate to vim mode setting and toggle it // This would require knowing the exact position, so we'll just test that the mock is called - stdin.write(TerminalKeys.ENTER as string); // Enter key - await wait(); + act(() => { + stdin.write(TerminalKeys.ENTER as string); // Enter key + }); // The mock should potentially be called if vim mode was toggled unmount(); @@ -565,19 +494,14 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Switch to scope focus - stdin.write(TerminalKeys.TAB); // Tab key - await wait(); - - // Select different scope (numbers 1-3 typically available) - stdin.write('2'); // Select second scope option - await wait(); + act(() => { + stdin.write(TerminalKeys.TAB); // Tab key + // Select different scope (numbers 1-3 typically available) + stdin.write('2'); // Select second scope option + }); unmount(); }); @@ -586,11 +510,7 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame, unmount } = render( - - - , - ); + const { lastFrame, unmount } = renderDialog(settings, onSelect); // Wait for initial render await vi.waitFor(() => { @@ -598,8 +518,8 @@ describe('SettingsDialog', () => { }); // The UI should show the settings section is active and scope section is inactive - expect(lastFrame()).toContain('● Vim Mode'); // Settings section active - expect(lastFrame()).toContain(' Apply To'); // Scope section inactive + expect(lastFrame()).toContain('Vim Mode'); // Settings section active + expect(lastFrame()).toContain('Apply To'); // Scope section (don't rely on exact spacing) // This test validates the initial state - scope selection behavior // is complex due to keypress handling, so we focus on state validation @@ -613,19 +533,12 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onRestartRequest = vi.fn(); - const { unmount } = render( - - {}} - onRestartRequest={onRestartRequest} - /> - , - ); + const { unmount } = renderDialog(settings, vi.fn(), { + onRestartRequest, + }); // This test would need to trigger a restart-required setting change // The exact steps depend on which settings require restart - await wait(); unmount(); }); @@ -634,19 +547,14 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onRestartRequest = vi.fn(); - const { stdin, unmount } = render( - - {}} - onRestartRequest={onRestartRequest} - /> - , - ); + const { stdin, unmount } = renderDialog(settings, vi.fn(), { + onRestartRequest, + }); // Press 'r' key (this would only work if restart prompt is showing) - stdin.write('r'); - await wait(); + act(() => { + stdin.write('r'); + }); // If restart prompt was showing, onRestartRequest should be called unmount(); @@ -658,11 +566,7 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame, unmount } = render( - - - , - ); + const { lastFrame, unmount } = renderDialog(settings, onSelect); // Wait for initial render await vi.waitFor(() => { @@ -685,19 +589,13 @@ describe('SettingsDialog', () => { const settings = createMockSettings({ vimMode: true }); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); - // Switch to scope selector - stdin.write(TerminalKeys.TAB as string); // Tab - await wait(); - - // Change scope - stdin.write('2'); // Select workspace scope - await wait(); + // Switch to scope selector and change scope + act(() => { + stdin.write(TerminalKeys.TAB as string); // Tab + stdin.write('2'); // Select workspace scope + }); // Settings should be reloaded for new scope unmount(); @@ -711,11 +609,7 @@ describe('SettingsDialog', () => { ); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); // Should show user scope values initially const output = lastFrame(); @@ -730,15 +624,12 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Try to toggle a setting (this might trigger vim mode toggle) - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); + act(() => { + stdin.write(TerminalKeys.ENTER as string); // Enter + }); // Should not crash unmount(); @@ -750,21 +641,14 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); - // Toggle a setting - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); - - // Toggle another setting - stdin.write(TerminalKeys.DOWN_ARROW as string); // Down - await wait(); - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); + // Toggle a setting, then toggle another setting + act(() => { + stdin.write(TerminalKeys.ENTER as string); // Enter + stdin.write(TerminalKeys.DOWN_ARROW as string); // Down + stdin.write(TerminalKeys.ENTER as string); // Enter + }); // Should track multiple modified settings unmount(); @@ -774,17 +658,14 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Navigate down many times to test scrolling - for (let i = 0; i < 10; i++) { - stdin.write(TerminalKeys.DOWN_ARROW as string); // Down arrow - await wait(10); - } + act(() => { + for (let i = 0; i < 10; i++) { + stdin.write(TerminalKeys.DOWN_ARROW as string); // Down arrow + } + }); unmount(); }); @@ -805,8 +686,9 @@ describe('SettingsDialog', () => { // Navigate to and toggle vim mode setting // This would require knowing the exact position of vim mode setting - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); + act(() => { + stdin.write(TerminalKeys.ENTER as string); // Enter + }); unmount(); }); @@ -821,11 +703,7 @@ describe('SettingsDialog', () => { ); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); const output = lastFrame(); // Should contain settings labels @@ -836,15 +714,12 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Toggle a non-restart-required setting (like hideTips) - stdin.write(TerminalKeys.ENTER as string); // Enter - toggle current setting - await wait(); + act(() => { + stdin.write(TerminalKeys.ENTER as string); // Enter - toggle current setting + }); // Should save immediately without showing restart prompt unmount(); @@ -854,20 +729,17 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame, unmount } = render( - - - , - ); + const { lastFrame, unmount } = renderDialog(settings, onSelect); // This test would need to navigate to a specific restart-required setting // Since we can't easily target specific settings, we test the general behavior - await wait(); // Should not show restart prompt initially - expect(lastFrame()).not.toContain( - 'To see changes, Gemini CLI must be restarted', - ); + await vi.waitFor(() => { + expect(lastFrame()).not.toContain( + 'To see changes, Gemini CLI must be restarted', + ); + }); unmount(); }); @@ -876,11 +748,7 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { unmount } = render( - - - , - ); + const { unmount } = renderDialog(settings, onSelect); // Restart prompt should be cleared when switching scopes unmount(); @@ -896,11 +764,7 @@ describe('SettingsDialog', () => { ); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); const output = lastFrame(); // Settings should show inherited values @@ -915,11 +779,7 @@ describe('SettingsDialog', () => { ); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); const output = lastFrame(); // Should show settings with override indicators @@ -928,126 +788,83 @@ describe('SettingsDialog', () => { }); describe('Race Condition Regression Tests', () => { - it('should not reset sibling settings when toggling a nested setting multiple times', async () => { - vi.mocked(saveModifiedSettings).mockClear(); - - vi.mocked(getSettingsSchema).mockReturnValue(TOOLS_SHELL_FAKE_SCHEMA); - - const settings = createMockSettings({ - tools: { - shell: { - showColor: false, - enableInteractiveShell: true, - }, + it.each([ + { + name: 'not reset sibling settings when toggling a nested setting multiple times', + toggleCount: 5, + shellSettings: { + showColor: false, + enableInteractiveShell: true, }, - }); - - const onSelect = vi.fn(); - const component = ( - - - - ); - - const { stdin, unmount } = render(component); - - await wait(); - - // Toggle showColor 5 times to trigger race condition - for (let i = 0; i < 5; i++) { - act(() => { - stdin.write(TerminalKeys.ENTER as string); - }); - await wait(50); - } - - await vi.waitFor(() => { - expect( - vi.mocked(saveModifiedSettings).mock.calls.length, - ).toBeGreaterThan(0); - }); - - // Verify sibling settings are preserved - const calls = vi.mocked(saveModifiedSettings).mock.calls; - calls.forEach((call) => { - const [modifiedKeys, pendingSettings] = call; - - if (modifiedKeys.has('tools.shell.showColor')) { - expect(pendingSettings.tools?.shell?.enableInteractiveShell).toBe( - true, - ); - expect(modifiedKeys.has('tools.shell.enableInteractiveShell')).toBe( - false, - ); - } - }); - - expect(calls.length).toBeGreaterThan(0); - - unmount(); - }); - - it('should preserve multiple sibling settings in nested objects during rapid toggles', async () => { - vi.mocked(saveModifiedSettings).mockClear(); - - vi.mocked(getSettingsSchema).mockReturnValue(TOOLS_SHELL_FAKE_SCHEMA); - - const settings = createMockSettings({ - tools: { - shell: { - showColor: false, - enableInteractiveShell: true, - pager: 'less', - }, + expectedSiblings: { + enableInteractiveShell: true, }, - }); + }, + { + name: 'preserve multiple sibling settings in nested objects during rapid toggles', + toggleCount: 3, + shellSettings: { + showColor: false, + enableInteractiveShell: true, + pager: 'less', + }, + expectedSiblings: { + enableInteractiveShell: true, + pager: 'less', + }, + }, + ])( + 'should $name', + async ({ toggleCount, shellSettings, expectedSiblings }) => { + vi.mocked(saveModifiedSettings).mockClear(); - const onSelect = vi.fn(); - const component = ( - - - - ); + vi.mocked(getSettingsSchema).mockReturnValue(TOOLS_SHELL_FAKE_SCHEMA); - const { stdin, unmount } = render(component); - - await wait(); - - // Rapid toggles - for (let i = 0; i < 3; i++) { - act(() => { - stdin.write(TerminalKeys.ENTER as string); + const settings = createMockSettings({ + tools: { + shell: shellSettings, + }, }); - await wait(30); - } - await vi.waitFor(() => { - expect( - vi.mocked(saveModifiedSettings).mock.calls.length, - ).toBeGreaterThan(0); - }); + const onSelect = vi.fn(); - // Verify all siblings preserved - const calls = vi.mocked(saveModifiedSettings).mock.calls; - calls.forEach((call) => { - const [modifiedKeys, pendingSettings] = call; + const { stdin, unmount } = renderDialog(settings, onSelect); - if (modifiedKeys.has('tools.shell.showColor')) { - const shellSettings = pendingSettings.tools?.shell as - | Record - | undefined; - expect(shellSettings?.['enableInteractiveShell']).toBe(true); - expect(shellSettings?.['pager']).toBe('less'); - expect(modifiedKeys.size).toBe(1); - expect(modifiedKeys.has('tools.shell.enableInteractiveShell')).toBe( - false, - ); - expect(modifiedKeys.has('tools.shell.pager')).toBe(false); + for (let i = 0; i < toggleCount; i++) { + act(() => { + stdin.write(TerminalKeys.ENTER as string); + }); } - }); - unmount(); - }); + await vi.waitFor(() => { + expect( + vi.mocked(saveModifiedSettings).mock.calls.length, + ).toBeGreaterThan(0); + }); + + const calls = vi.mocked(saveModifiedSettings).mock.calls; + calls.forEach((call) => { + const [modifiedKeys, pendingSettings] = call; + + if (modifiedKeys.has('tools.shell.showColor')) { + const shellSettings = pendingSettings.tools?.shell as + | Record + | undefined; + + Object.entries(expectedSiblings).forEach(([key, value]) => { + expect(shellSettings?.[key]).toBe(value); + expect(modifiedKeys.has(`tools.shell.${key}`)).toBe(false); + }); + + expect(modifiedKeys.size).toBe(1); + } + }); + + expect(calls.length).toBeGreaterThan(0); + + unmount(); + }, + ); }); describe('Keyboard Shortcuts Edge Cases', () => { @@ -1055,74 +872,51 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Rapid navigation - for (let i = 0; i < 5; i++) { - stdin.write(TerminalKeys.DOWN_ARROW as string); - stdin.write(TerminalKeys.UP_ARROW as string); - } - await wait(100); + act(() => { + for (let i = 0; i < 5; i++) { + stdin.write(TerminalKeys.DOWN_ARROW as string); + stdin.write(TerminalKeys.UP_ARROW as string); + } + }); // Should not crash unmount(); }); - it('should handle Ctrl+C to reset current setting to default', async () => { - const settings = createMockSettings({ vimMode: true }); // Start with vimMode enabled - const onSelect = vi.fn(); + it.each([ + { key: 'Ctrl+C', code: '\u0003' }, + { key: 'Ctrl+L', code: '\u000C' }, + ])( + 'should handle $key to reset current setting to default', + async ({ code }) => { + const settings = createMockSettings({ vimMode: true }); + const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); - // Press Ctrl+C to reset current setting to default - stdin.write('\u0003'); // Ctrl+C - await wait(); + act(() => { + stdin.write(code); + }); - // Should reset the current setting to its default value - unmount(); - }); - - it('should handle Ctrl+L to reset current setting to default', async () => { - const settings = createMockSettings({ vimMode: true }); // Start with vimMode enabled - const onSelect = vi.fn(); - - const { stdin, unmount } = render( - - - , - ); - - // Press Ctrl+L to reset current setting to default - stdin.write('\u000C'); // Ctrl+L - await wait(); - - // Should reset the current setting to its default value - unmount(); - }); + // Should reset the current setting to its default value + unmount(); + }, + ); it('should handle navigation when only one setting exists', async () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Try to navigate when potentially at bounds - stdin.write(TerminalKeys.DOWN_ARROW as string); - await wait(); - stdin.write(TerminalKeys.UP_ARROW as string); - await wait(); + act(() => { + stdin.write(TerminalKeys.DOWN_ARROW as string); + stdin.write(TerminalKeys.UP_ARROW as string); + }); unmount(); }); @@ -1131,11 +925,7 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame, unmount } = render( - - - , - ); + const { lastFrame, unmount } = renderDialog(settings, onSelect); // Wait for initial render await vi.waitFor(() => { @@ -1143,8 +933,8 @@ describe('SettingsDialog', () => { }); // Verify initial state: settings section active, scope section inactive - expect(lastFrame()).toContain('● Vim Mode'); // Settings section active - expect(lastFrame()).toContain(' Apply To'); // Scope section inactive + expect(lastFrame()).toContain('Vim Mode'); // Settings section active + expect(lastFrame()).toContain('Apply To'); // Scope section (don't rely on exact spacing) // This test validates the rendered UI structure for tab navigation // Actual tab behavior testing is complex due to keypress handling @@ -1163,11 +953,7 @@ describe('SettingsDialog', () => { ); const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); // Should still render without crashing expect(lastFrame()).toContain('Settings'); @@ -1178,11 +964,7 @@ describe('SettingsDialog', () => { const onSelect = vi.fn(); // Should not crash even if some settings are missing definitions - const { lastFrame } = render( - - - , - ); + const { lastFrame } = renderDialog(settings, onSelect); expect(lastFrame()).toContain('Settings'); }); @@ -1193,11 +975,7 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { lastFrame, unmount } = render( - - - , - ); + const { lastFrame, unmount } = renderDialog(settings, onSelect); // Wait for initial render await vi.waitFor(() => { @@ -1206,12 +984,11 @@ describe('SettingsDialog', () => { // Verify the complete UI is rendered with all necessary sections expect(lastFrame()).toContain('Settings'); // Title - expect(lastFrame()).toContain('● Vim Mode'); // Active setting + expect(lastFrame()).toContain('Vim Mode'); // Active setting expect(lastFrame()).toContain('Apply To'); // Scope section expect(lastFrame()).toContain('User Settings'); // Scope options (no numbers when settings focused) - expect(lastFrame()).toContain( - '(Use Enter to select, Tab to change focus, Esc to close)', - ); // Help text + // Use regex for more flexible help text matching + expect(lastFrame()).toMatch(/Enter.*select.*Tab.*focus.*Esc.*close/); // This test validates the complete UI structure is available for user workflow // Individual interactions are tested in focused unit tests @@ -1223,27 +1000,16 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); - // Toggle first setting (should require restart) - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); - - // Navigate to next setting and toggle it (should not require restart - e.g., vimMode) - stdin.write(TerminalKeys.DOWN_ARROW as string); // Down - await wait(); - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); - - // Navigate to another setting and toggle it (should also require restart) - stdin.write(TerminalKeys.DOWN_ARROW as string); // Down - await wait(); - stdin.write(TerminalKeys.ENTER as string); // Enter - await wait(); + // Toggle multiple settings + act(() => { + stdin.write(TerminalKeys.ENTER as string); // Enter + stdin.write(TerminalKeys.DOWN_ARROW as string); // Down + stdin.write(TerminalKeys.ENTER as string); // Enter + stdin.write(TerminalKeys.DOWN_ARROW as string); // Down + stdin.write(TerminalKeys.ENTER as string); // Enter + }); // The test verifies that all changes are preserved and the dialog still works // This tests the fix for the bug where changing one setting would reset all pending changes @@ -1254,23 +1020,16 @@ describe('SettingsDialog', () => { const settings = createMockSettings({ vimMode: true }); const onSelect = vi.fn(); - const { stdin, unmount } = render( - - - , - ); + const { stdin, unmount } = renderDialog(settings, onSelect); // Multiple scope changes - stdin.write(TerminalKeys.TAB as string); // Tab to scope - await wait(); - stdin.write('2'); // Workspace - await wait(); - stdin.write(TerminalKeys.TAB as string); // Tab to settings - await wait(); - stdin.write(TerminalKeys.TAB as string); // Tab to scope - await wait(); - stdin.write('1'); // User - await wait(); + act(() => { + stdin.write(TerminalKeys.TAB as string); // Tab to scope + stdin.write('2'); // Workspace + stdin.write(TerminalKeys.TAB as string); // Tab to settings + stdin.write(TerminalKeys.TAB as string); // Tab to scope + stdin.write('1'); // User + }); // Should maintain consistent state unmount(); @@ -1280,19 +1039,14 @@ describe('SettingsDialog', () => { const settings = createMockSettings(); const onRestartRequest = vi.fn(); - const { stdin, unmount } = render( - - {}} - onRestartRequest={onRestartRequest} - /> - , - ); + const { stdin, unmount } = renderDialog(settings, vi.fn(), { + onRestartRequest, + }); // This would test the restart workflow if we could trigger it - stdin.write('r'); // Try restart key - await wait(); + act(() => { + stdin.write('r'); // Try restart key + }); // Without restart prompt showing, this should have no effect expect(onRestartRequest).not.toHaveBeenCalled(); @@ -1312,26 +1066,19 @@ describe('SettingsDialog', () => { , ); - // Wait for the dialog to render - await wait(); - // Navigate to the last setting - for (let i = 0; i < 20; i++) { - stdin.write('j'); // Down - await wait(10); - } + act(() => { + for (let i = 0; i < 20; i++) { + stdin.write('j'); // Down + } + }); - // Press Enter to start editing - stdin.write('\r'); - await wait(); - - // Type a new value - stdin.write('new value'); - await wait(); - - // Press Enter to commit - stdin.write('\r'); - await wait(); + // Press Enter to start editing, type new value, and commit + act(() => { + stdin.write('\r'); // Start editing + stdin.write('new value'); + stdin.write('\r'); // Commit + }); settings = createMockSettings( { 'a.string.setting': 'new value' }, @@ -1343,13 +1090,15 @@ describe('SettingsDialog', () => { , ); - await wait(); // Press Escape to exit - stdin.write('\u001B'); - await wait(60); + act(() => { + stdin.write('\u001B'); + }); - expect(onSelect).toHaveBeenCalledWith(undefined, 'User'); + await vi.waitFor(() => { + expect(onSelect).toHaveBeenCalledWith(undefined, 'User'); + }); unmount(); }); @@ -1358,331 +1107,226 @@ describe('SettingsDialog', () => { describe('Snapshot Tests', () => { /** * Snapshot tests for SettingsDialog component using ink-testing-library. - * These tests capture the visual output of the component in various states: - * - * - Default rendering with no custom settings - * - Various combinations of boolean settings (enabled/disabled) - * - Mixed boolean and number settings configurations - * - Different focus states (settings vs scope selector) - * - Different scope selections (User, System, Workspace) - * - Accessibility settings enabled - * - File filtering configurations - * - Tools and security settings - * - All settings disabled state - * + * These tests capture the visual output of the component in various states. * The snapshots help ensure UI consistency and catch unintended visual changes. */ - it('should render default state correctly', () => { - const settings = createMockSettings(); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with various boolean settings enabled', () => { - const settings = createMockSettings({ - general: { - vimMode: true, - disableAutoUpdate: true, - debugKeystrokeLogging: true, - enablePromptCompletion: true, - }, - ui: { - hideWindowTitle: true, - hideTips: true, - showMemoryUsage: true, - showLineNumbers: true, - showCitations: true, - accessibility: { - disableLoadingPhrases: true, - screenReader: true, - }, - }, - ide: { - enabled: true, - }, - context: { - loadMemoryFromIncludeDirectories: true, - fileFiltering: { - respectGitIgnore: true, - respectGeminiIgnore: true, - enableRecursiveFileSearch: true, - disableFuzzySearch: false, - }, - }, - tools: { - enableInteractiveShell: true, - autoAccept: true, - useRipgrep: true, - }, - security: { - folderTrust: { - enabled: true, - }, - }, - }); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with mixed boolean and number settings', () => { - const settings = createMockSettings({ - general: { - vimMode: false, - disableAutoUpdate: true, - }, - ui: { - showMemoryUsage: true, - hideWindowTitle: false, - }, - tools: { - truncateToolOutputThreshold: 50000, - truncateToolOutputLines: 1000, - }, - context: { - discoveryMaxDirs: 500, - }, - model: { - maxSessionTurns: 100, - skipNextSpeakerCheck: false, - }, - }); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render focused on scope selector', () => { - const settings = createMockSettings(); - const onSelect = vi.fn(); - - const { lastFrame, stdin } = render( - - - , - ); - - // Switch focus to scope selector with Tab - stdin.write('\t'); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with different scope selected (System)', () => { - const settings = createMockSettings( - {}, // userSettings - { - // systemSettings + it.each([ + { + name: 'default state', + userSettings: {}, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + { + name: 'various boolean settings enabled', + userSettings: { general: { vimMode: true, - disableAutoUpdate: false, + disableAutoUpdate: true, + debugKeystrokeLogging: true, + enablePromptCompletion: true, + }, + ui: { + hideWindowTitle: true, + hideTips: true, + showMemoryUsage: true, + showLineNumbers: true, + showCitations: true, + accessibility: { + disableLoadingPhrases: true, + screenReader: true, + }, + }, + ide: { + enabled: true, + }, + context: { + loadMemoryFromIncludeDirectories: true, + fileFiltering: { + respectGitIgnore: true, + respectGeminiIgnore: true, + enableRecursiveFileSearch: true, + disableFuzzySearch: false, + }, + }, + tools: { + enableInteractiveShell: true, + autoAccept: true, + useRipgrep: true, + }, + security: { + folderTrust: { + enabled: true, + }, + }, + }, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + { + name: 'mixed boolean and number settings', + userSettings: { + general: { + vimMode: false, + disableAutoUpdate: true, }, ui: { showMemoryUsage: true, - }, - }, - ); - const onSelect = vi.fn(); - - const { lastFrame, stdin } = render( - - - , - ); - - // Switch to scope selector - stdin.write('\t'); - // Navigate to System scope - stdin.write('ArrowDown'); - stdin.write('\r'); // Enter to select - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with different scope selected (Workspace)', () => { - const settings = createMockSettings( - {}, // userSettings - {}, // systemSettings - { - // workspaceSettings - general: { - vimMode: false, - debugKeystrokeLogging: true, + hideWindowTitle: false, }, tools: { + truncateToolOutputThreshold: 50000, + truncateToolOutputLines: 1000, + }, + context: { + discoveryMaxDirs: 500, + }, + model: { + maxSessionTurns: 100, + skipNextSpeakerCheck: false, + }, + }, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + { + name: 'focused on scope selector', + userSettings: {}, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: (stdin: { write: (data: string) => void }) => + stdin.write('\t'), + }, + { + name: 'accessibility settings enabled', + userSettings: { + ui: { + accessibility: { + disableLoadingPhrases: true, + screenReader: true, + }, + showMemoryUsage: true, + showLineNumbers: true, + }, + general: { + vimMode: true, + }, + }, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + { + name: 'file filtering settings configured', + userSettings: { + context: { + fileFiltering: { + respectGitIgnore: false, + respectGeminiIgnore: true, + enableRecursiveFileSearch: false, + disableFuzzySearch: true, + }, + loadMemoryFromIncludeDirectories: true, + discoveryMaxDirs: 100, + }, + }, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + { + name: 'tools and security settings', + userSettings: { + tools: { + enableInteractiveShell: true, + autoAccept: false, useRipgrep: true, - enableInteractiveShell: false, + truncateToolOutputThreshold: 25000, + truncateToolOutputLines: 500, + }, + security: { + folderTrust: { + enabled: true, + }, + }, + model: { + maxSessionTurns: 50, + skipNextSpeakerCheck: true, }, }, - ); - const onSelect = vi.fn(); - - const { lastFrame, stdin } = render( - - - , - ); - - // Switch to scope selector - stdin.write('\t'); - // Navigate to Workspace scope (down twice) - stdin.write('ArrowDown'); - stdin.write('ArrowDown'); - stdin.write('\r'); // Enter to select - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with accessibility settings enabled', () => { - const settings = createMockSettings({ - ui: { - accessibility: { - disableLoadingPhrases: true, - screenReader: true, + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + { + name: 'all boolean settings disabled', + userSettings: { + general: { + vimMode: false, + disableAutoUpdate: false, + debugKeystrokeLogging: false, + enablePromptCompletion: false, }, - showMemoryUsage: true, - showLineNumbers: true, - }, - general: { - vimMode: true, - }, - }); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with file filtering settings configured', () => { - const settings = createMockSettings({ - context: { - fileFiltering: { - respectGitIgnore: false, - respectGeminiIgnore: true, - enableRecursiveFileSearch: false, - disableFuzzySearch: true, + ui: { + hideWindowTitle: false, + hideTips: false, + showMemoryUsage: false, + showLineNumbers: false, + showCitations: false, + accessibility: { + disableLoadingPhrases: false, + screenReader: false, + }, }, - loadMemoryFromIncludeDirectories: true, - discoveryMaxDirs: 100, - }, - }); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with tools and security settings', () => { - const settings = createMockSettings({ - tools: { - enableInteractiveShell: true, - autoAccept: false, - useRipgrep: true, - truncateToolOutputThreshold: 25000, - truncateToolOutputLines: 500, - }, - security: { - folderTrust: { - enabled: true, - }, - }, - model: { - maxSessionTurns: 50, - skipNextSpeakerCheck: true, - }, - }); - const onSelect = vi.fn(); - - const { lastFrame } = render( - - - , - ); - - expect(lastFrame()).toMatchSnapshot(); - }); - - it('should render with all boolean settings disabled', () => { - const settings = createMockSettings({ - general: { - vimMode: false, - disableAutoUpdate: false, - debugKeystrokeLogging: false, - enablePromptCompletion: false, - }, - ui: { - hideWindowTitle: false, - hideTips: false, - showMemoryUsage: false, - showLineNumbers: false, - showCitations: false, - accessibility: { - disableLoadingPhrases: false, - screenReader: false, - }, - }, - ide: { - enabled: false, - }, - context: { - loadMemoryFromIncludeDirectories: false, - fileFiltering: { - respectGitIgnore: false, - respectGeminiIgnore: false, - enableRecursiveFileSearch: false, - disableFuzzySearch: false, - }, - }, - tools: { - enableInteractiveShell: false, - autoAccept: false, - useRipgrep: false, - }, - security: { - folderTrust: { + ide: { enabled: false, }, + context: { + loadMemoryFromIncludeDirectories: false, + fileFiltering: { + respectGitIgnore: false, + respectGeminiIgnore: false, + enableRecursiveFileSearch: false, + disableFuzzySearch: false, + }, + }, + tools: { + enableInteractiveShell: false, + autoAccept: false, + useRipgrep: false, + }, + security: { + folderTrust: { + enabled: false, + }, + }, }, - }); - const onSelect = vi.fn(); + systemSettings: {}, + workspaceSettings: {}, + stdinActions: undefined, + }, + ])( + 'should render $name correctly', + ({ userSettings, systemSettings, workspaceSettings, stdinActions }) => { + const settings = createMockSettings( + userSettings, + systemSettings, + workspaceSettings, + ); + const onSelect = vi.fn(); - const { lastFrame } = render( - - - , - ); + const { lastFrame, stdin } = renderDialog(settings, onSelect); - expect(lastFrame()).toMatchSnapshot(); - }); + if (stdinActions) { + stdinActions(stdin); + } + + expect(lastFrame()).toMatchSnapshot(); + }, + ); }); }); diff --git a/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap index 1555a01da5..b30599f965 100644 --- a/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/SettingsDialog.test.tsx.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`SettingsDialog > Snapshot Tests > should render default state correctly 1`] = ` +exports[`SettingsDialog > Initial Rendering > should render settings list with visual indicators 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ @@ -35,42 +35,7 @@ exports[`SettingsDialog > Snapshot Tests > should render default state correctly ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`SettingsDialog > Snapshot Tests > should render focused on scope selector 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Settings │ -│ │ -│ ▲ │ -│ ● Vim Mode false │ -│ │ -│ Disable Auto Update false │ -│ │ -│ Enable Prompt Completion false │ -│ │ -│ Debug Keystroke Logging false │ -│ │ -│ Session Retention undefined │ -│ │ -│ Enable Session Cleanup false │ -│ │ -│ Output Format Text │ -│ │ -│ Hide Window Title false │ -│ │ -│ ▼ │ -│ │ -│ │ -│ Apply To │ -│ ● User Settings │ -│ Workspace Settings │ -│ System Settings │ -│ │ -│ (Use Enter to select, Tab to change focus, Esc to close) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" -`; - -exports[`SettingsDialog > Snapshot Tests > should render with accessibility settings enabled 1`] = ` +exports[`SettingsDialog > Snapshot Tests > should render 'accessibility settings enabled' correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ @@ -105,7 +70,7 @@ exports[`SettingsDialog > Snapshot Tests > should render with accessibility sett ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`SettingsDialog > Snapshot Tests > should render with all boolean settings disabled 1`] = ` +exports[`SettingsDialog > Snapshot Tests > should render 'all boolean settings disabled' correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ @@ -140,77 +105,7 @@ exports[`SettingsDialog > Snapshot Tests > should render with all boolean settin ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`SettingsDialog > Snapshot Tests > should render with different scope selected (System) 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Settings │ -│ │ -│ ▲ │ -│ ● Vim Mode (Modified in System) false │ -│ │ -│ Disable Auto Update (Modified in System) false │ -│ │ -│ Enable Prompt Completion false │ -│ │ -│ Debug Keystroke Logging false │ -│ │ -│ Session Retention undefined │ -│ │ -│ Enable Session Cleanup false │ -│ │ -│ Output Format Text │ -│ │ -│ Hide Window Title false │ -│ │ -│ ▼ │ -│ │ -│ │ -│ Apply To │ -│ ● User Settings │ -│ Workspace Settings │ -│ System Settings │ -│ │ -│ (Use Enter to select, Tab to change focus, Esc to close) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" -`; - -exports[`SettingsDialog > Snapshot Tests > should render with different scope selected (Workspace) 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Settings │ -│ │ -│ ▲ │ -│ ● Vim Mode (Modified in Workspace) false │ -│ │ -│ Disable Auto Update false │ -│ │ -│ Enable Prompt Completion false │ -│ │ -│ Debug Keystroke Logging (Modified in Workspace) false │ -│ │ -│ Session Retention undefined │ -│ │ -│ Enable Session Cleanup false │ -│ │ -│ Output Format Text │ -│ │ -│ Hide Window Title false │ -│ │ -│ ▼ │ -│ │ -│ │ -│ Apply To │ -│ ● User Settings │ -│ Workspace Settings │ -│ System Settings │ -│ │ -│ (Use Enter to select, Tab to change focus, Esc to close) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" -`; - -exports[`SettingsDialog > Snapshot Tests > should render with file filtering settings configured 1`] = ` +exports[`SettingsDialog > Snapshot Tests > should render 'default state' correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ @@ -245,7 +140,77 @@ exports[`SettingsDialog > Snapshot Tests > should render with file filtering set ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`SettingsDialog > Snapshot Tests > should render with mixed boolean and number settings 1`] = ` +exports[`SettingsDialog > Snapshot Tests > should render 'file filtering settings configured' correctly 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Settings │ +│ │ +│ ▲ │ +│ ● Vim Mode false │ +│ │ +│ Disable Auto Update false │ +│ │ +│ Enable Prompt Completion false │ +│ │ +│ Debug Keystroke Logging false │ +│ │ +│ Session Retention undefined │ +│ │ +│ Enable Session Cleanup false │ +│ │ +│ Output Format Text │ +│ │ +│ Hide Window Title false │ +│ │ +│ ▼ │ +│ │ +│ │ +│ Apply To │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ +│ │ +│ (Use Enter to select, Tab to change focus, Esc to close) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" +`; + +exports[`SettingsDialog > Snapshot Tests > should render 'focused on scope selector' correctly 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Settings │ +│ │ +│ ▲ │ +│ ● Vim Mode false │ +│ │ +│ Disable Auto Update false │ +│ │ +│ Enable Prompt Completion false │ +│ │ +│ Debug Keystroke Logging false │ +│ │ +│ Session Retention undefined │ +│ │ +│ Enable Session Cleanup false │ +│ │ +│ Output Format Text │ +│ │ +│ Hide Window Title false │ +│ │ +│ ▼ │ +│ │ +│ │ +│ Apply To │ +│ ● User Settings │ +│ Workspace Settings │ +│ System Settings │ +│ │ +│ (Use Enter to select, Tab to change focus, Esc to close) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" +`; + +exports[`SettingsDialog > Snapshot Tests > should render 'mixed boolean and number settings' correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ @@ -280,7 +245,7 @@ exports[`SettingsDialog > Snapshot Tests > should render with mixed boolean and ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`SettingsDialog > Snapshot Tests > should render with tools and security settings 1`] = ` +exports[`SettingsDialog > Snapshot Tests > should render 'tools and security settings' correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ @@ -315,7 +280,7 @@ exports[`SettingsDialog > Snapshot Tests > should render with tools and security ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`SettingsDialog > Snapshot Tests > should render with various boolean settings enabled 1`] = ` +exports[`SettingsDialog > Snapshot Tests > should render 'various boolean settings enabled' correctly 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Settings │ diff --git a/packages/cli/src/ui/contexts/KeypressContext.test.tsx b/packages/cli/src/ui/contexts/KeypressContext.test.tsx index 7f60316ac8..c369f489b6 100644 --- a/packages/cli/src/ui/contexts/KeypressContext.test.tsx +++ b/packages/cli/src/ui/contexts/KeypressContext.test.tsx @@ -51,6 +51,21 @@ class MockStdin extends EventEmitter { } } +// Helper function to setup keypress test with standard configuration +const setupKeypressTest = (kittyProtocolEnabled = true) => { + const keyHandler = vi.fn(); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + const { result } = renderHook(() => useKeypressContext(), { wrapper }); + act(() => result.current.subscribe(keyHandler)); + + return { result, keyHandler }; +}; + describe('KeypressContext - Kitty Protocol', () => { let stdin: MockStdin; const mockSetRawMode = vi.fn(); @@ -77,19 +92,20 @@ describe('KeypressContext - Kitty Protocol', () => { }); describe('Enter key handling', () => { - it('should recognize regular enter key (keycode 13) in kitty protocol', async () => { - const keyHandler = vi.fn(); + it.each([ + { + name: 'regular enter key (keycode 13)', + sequence: '\x1b[13u', + }, + { + name: 'numpad enter key (keycode 57414)', + sequence: '\x1b[57414u', + }, + ])('should recognize $name in kitty protocol', async ({ sequence }) => { + const { keyHandler } = setupKeypressTest(true); - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: true }), - }); - - act(() => result.current.subscribe(keyHandler)); - - // Send kitty protocol sequence for regular enter: ESC[13u act(() => { - stdin.write(`\x1b[13u`); + stdin.write(sequence); }); expect(keyHandler).toHaveBeenCalledWith( @@ -103,117 +119,41 @@ describe('KeypressContext - Kitty Protocol', () => { ); }); - it('should recognize numpad enter key (keycode 57414) in kitty protocol', async () => { - const keyHandler = vi.fn(); + it.each([ + { + modifier: 'Shift', + sequence: '\x1b[57414;2u', + expected: { ctrl: false, meta: false, shift: true }, + }, + { + modifier: 'Ctrl', + sequence: '\x1b[57414;5u', + expected: { ctrl: true, meta: false, shift: false }, + }, + { + modifier: 'Alt', + sequence: '\x1b[57414;3u', + expected: { ctrl: false, meta: true, shift: false }, + }, + ])( + 'should handle numpad enter with $modifier modifier', + async ({ sequence, expected }) => { + const { keyHandler } = setupKeypressTest(true); - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: true }), - }); + act(() => stdin.write(sequence)); - act(() => result.current.subscribe(keyHandler)); - - // Send kitty protocol sequence for numpad enter: ESC[57414u - act(() => { - stdin.write(`\x1b[57414u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'return', - kittyProtocol: true, - ctrl: false, - meta: false, - shift: false, - }), - ); - }); - - it('should handle numpad enter with modifiers', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: true }), - }); - - act(() => result.current.subscribe(keyHandler)); - - // Send kitty protocol sequence for numpad enter with Shift (modifier 2): ESC[57414;2u - act(() => { - stdin.write(`\x1b[57414;2u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'return', - kittyProtocol: true, - ctrl: false, - meta: false, - shift: true, - }), - ); - }); - - it('should handle numpad enter with Ctrl modifier', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: true }), - }); - - act(() => result.current.subscribe(keyHandler)); - - // Send kitty protocol sequence for numpad enter with Ctrl (modifier 5): ESC[57414;5u - act(() => stdin.write(`\x1b[57414;5u`)); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'return', - kittyProtocol: true, - ctrl: true, - meta: false, - shift: false, - }), - ); - }); - - it('should handle numpad enter with Alt modifier', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: true }), - }); - - act(() => result.current.subscribe(keyHandler)); - - // Send kitty protocol sequence for numpad enter with Alt (modifier 3): ESC[57414;3u - act(() => { - stdin.write(`\x1b[57414;3u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'return', - kittyProtocol: true, - ctrl: false, - meta: true, - shift: false, - }), - ); - }); + expect(keyHandler).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'return', + kittyProtocol: true, + ...expected, + }), + ); + }, + ); it('should not process kitty sequences when kitty protocol is disabled', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: false }), - }); - - act(() => result.current.subscribe(keyHandler)); + const { keyHandler } = setupKeypressTest(false); // Send kitty protocol sequence for numpad enter act(() => { @@ -233,14 +173,7 @@ describe('KeypressContext - Kitty Protocol', () => { describe('Escape key handling', () => { it('should recognize escape key (keycode 27) in kitty protocol', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { - wrapper: ({ children }) => - wrapper({ children, kittyProtocolEnabled: true }), - }); - - act(() => result.current.subscribe(keyHandler)); + const { keyHandler } = setupKeypressTest(true); // Send kitty protocol sequence for escape: ESC[27u act(() => { @@ -257,174 +190,90 @@ describe('KeypressContext - Kitty Protocol', () => { }); describe('Tab and Backspace handling', () => { - it('should recognize Tab key in kitty protocol', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); + it.each([ + { + name: 'Tab key', + sequence: '\x1b[9u', + expected: { name: 'tab', shift: false }, + }, + { + name: 'Shift+Tab', + sequence: '\x1b[9;2u', + expected: { name: 'tab', shift: true }, + }, + { + name: 'Backspace', + sequence: '\x1b[127u', + expected: { name: 'backspace', meta: false }, + }, + { + name: 'Option+Backspace', + sequence: '\x1b[127;3u', + expected: { name: 'backspace', meta: true }, + }, + { + name: 'Ctrl+Backspace', + sequence: '\x1b[127;5u', + expected: { name: 'backspace', ctrl: true }, + }, + ])( + 'should recognize $name in kitty protocol', + async ({ sequence, expected }) => { + const { keyHandler } = setupKeypressTest(true); - act(() => { - stdin.write(`\x1b[9u`); - }); + act(() => { + stdin.write(sequence); + }); - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'tab', - kittyProtocol: true, - shift: false, - }), - ); - }); - - it('should recognize Shift+Tab in kitty protocol', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); - - // Modifier 2 is Shift - act(() => { - stdin.write(`\x1b[9;2u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'tab', - kittyProtocol: true, - shift: true, - }), - ); - }); - - it('should recognize Backspace key in kitty protocol', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); - - act(() => { - stdin.write(`\x1b[127u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'backspace', - kittyProtocol: true, - meta: false, - }), - ); - }); - - it('should recognize Option+Backspace in kitty protocol', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); - - // Modifier 3 is Alt/Option - act(() => { - stdin.write(`\x1b[127;3u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'backspace', - kittyProtocol: true, - meta: true, - }), - ); - }); - - it('should recognize Ctrl+Backspace in kitty protocol', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); - - // Modifier 5 is Ctrl - act(() => { - stdin.write(`\x1b[127;5u`); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'backspace', - kittyProtocol: true, - ctrl: true, - }), - ); - }); + expect(keyHandler).toHaveBeenCalledWith( + expect.objectContaining({ + ...expected, + kittyProtocol: true, + }), + ); + }, + ); }); describe('paste mode', () => { - it('should handle multiline paste as a single event', async () => { + it.each([ + { + name: 'handle multiline paste as a single event', + pastedText: 'This \n is \n a \n multiline \n paste.', + writeSequence: (text: string) => { + stdin.write(PASTE_START); + stdin.write(text); + stdin.write(PASTE_END); + }, + }, + { + name: 'handle paste start code split over multiple writes', + pastedText: 'pasted content', + writeSequence: (text: string) => { + stdin.write(PASTE_START.slice(0, 3)); + stdin.write(PASTE_START.slice(3)); + stdin.write(text); + stdin.write(PASTE_END); + }, + }, + { + name: 'handle paste end code split over multiple writes', + pastedText: 'pasted content', + writeSequence: (text: string) => { + stdin.write(PASTE_START); + stdin.write(text); + stdin.write(PASTE_END.slice(0, 3)); + stdin.write(PASTE_END.slice(3)); + }, + }, + ])('should $name', async ({ pastedText, writeSequence }) => { const keyHandler = vi.fn(); - const pastedText = 'This \n is \n a \n multiline \n paste.'; - - const { result } = renderHook(() => useKeypressContext(), { - wrapper, - }); - - act(() => result.current.subscribe(keyHandler)); - - // Simulate a bracketed paste event - act(() => { - stdin.write(PASTE_START); - stdin.write(pastedText); - stdin.write(PASTE_END); - }); - - await vi.waitFor(() => { - // Expect the handler to be called exactly once for the entire paste - expect(keyHandler).toHaveBeenCalledTimes(1); - }); - - // Verify the single event contains the full pasted text - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - paste: true, - sequence: pastedText, - }), - ); - }); - it('should paste start code split over multiple writes', async () => { - const keyHandler = vi.fn(); - const pastedText = 'pasted content'; const { result } = renderHook(() => useKeypressContext(), { wrapper }); act(() => result.current.subscribe(keyHandler)); - act(() => { - // Split PASTE_START into two parts - stdin.write(PASTE_START.slice(0, 3)); - stdin.write(PASTE_START.slice(3)); - stdin.write(pastedText); - stdin.write(PASTE_END); - }); - - await vi.waitFor(() => { - expect(keyHandler).toHaveBeenCalledTimes(1); - }); - - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - paste: true, - sequence: pastedText, - }), - ); - }); - - it('should paste end code split over multiple writes', async () => { - const keyHandler = vi.fn(); - const pastedText = 'pasted content'; - - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - - act(() => result.current.subscribe(keyHandler)); - - act(() => { - stdin.write(PASTE_START); - stdin.write(pastedText); - // Split PASTE_END into two parts - stdin.write(PASTE_END.slice(0, 3)); - stdin.write(PASTE_END.slice(3)); - }); + act(() => writeSequence(pastedText)); await vi.waitFor(() => { expect(keyHandler).toHaveBeenCalledTimes(1); @@ -666,9 +515,7 @@ describe('KeypressContext - Kitty Protocol', () => { describe('Double-tap and batching', () => { it('should emit two delete events for double-tap CSI[3~', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); + const { keyHandler } = setupKeypressTest(true); act(() => stdin.write(`\x1b[3~`)); act(() => stdin.write(`\x1b[3~`)); @@ -684,9 +531,7 @@ describe('KeypressContext - Kitty Protocol', () => { }); it('should parse two concatenated tilde-coded sequences in one chunk', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); + const { keyHandler } = setupKeypressTest(true); act(() => stdin.write(`\x1b[3~\x1b[5~`)); @@ -699,9 +544,7 @@ describe('KeypressContext - Kitty Protocol', () => { }); it('should ignore incomplete CSI then parse the next complete sequence', async () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); + const { keyHandler } = setupKeypressTest(true); // Incomplete ESC sequence then a complete Delete act(() => { @@ -749,93 +592,61 @@ describe('Drag and Drop Handling', () => { }); describe('drag start by quotes', () => { - it('should start collecting when single quote arrives and not broadcast immediately', async () => { - const keyHandler = vi.fn(); + it.each([ + { name: 'single quote', quote: SINGLE_QUOTE }, + { name: 'double quote', quote: DOUBLE_QUOTE }, + ])( + 'should start collecting when $name arrives and not broadcast immediately', + async ({ quote }) => { + const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); + const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); + act(() => result.current.subscribe(keyHandler)); - act(() => stdin.write(SINGLE_QUOTE)); + act(() => stdin.write(quote)); - expect(keyHandler).not.toHaveBeenCalled(); - }); - - it('should start collecting when double quote arrives and not broadcast immediately', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - - act(() => result.current.subscribe(keyHandler)); - - act(() => stdin.write(DOUBLE_QUOTE)); - - expect(keyHandler).not.toHaveBeenCalled(); - }); + expect(keyHandler).not.toHaveBeenCalled(); + }, + ); }); describe('drag collection and completion', () => { - it('should collect single character inputs during drag mode', async () => { + it.each([ + { + name: 'collect single character inputs during drag mode', + characters: ['a'], + expectedText: 'a', + }, + { + name: 'collect multiple characters and complete on timeout', + characters: ['p', 'a', 't', 'h'], + expectedText: 'path', + }, + ])('should $name', async ({ characters, expectedText }) => { const keyHandler = vi.fn(); const { result } = renderHook(() => useKeypressContext(), { wrapper }); act(() => result.current.subscribe(keyHandler)); - // Start by single quote act(() => stdin.write(SINGLE_QUOTE)); - // Send single character - act(() => stdin.write('a')); + characters.forEach((char) => { + act(() => stdin.write(char)); + }); - // Character should not be immediately broadcast expect(keyHandler).not.toHaveBeenCalled(); - // Fast-forward to completion timeout act(() => { vi.advanceTimersByTime(DRAG_COMPLETION_TIMEOUT_MS + 10); }); - // Should broadcast the collected path as paste (includes starting quote) expect(keyHandler).toHaveBeenCalledWith( expect.objectContaining({ name: '', paste: true, - sequence: `${SINGLE_QUOTE}a`, - }), - ); - }); - - it('should collect multiple characters and complete on timeout', async () => { - const keyHandler = vi.fn(); - - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - - act(() => result.current.subscribe(keyHandler)); - - // Start by single quote - act(() => stdin.write(SINGLE_QUOTE)); - - // Send multiple characters - act(() => stdin.write('p')); - act(() => stdin.write('a')); - act(() => stdin.write('t')); - act(() => stdin.write('h')); - - // Characters should not be immediately broadcast - expect(keyHandler).not.toHaveBeenCalled(); - - // Fast-forward to completion timeout - act(() => { - vi.advanceTimersByTime(DRAG_COMPLETION_TIMEOUT_MS + 10); - }); - - // Should broadcast the collected path as paste (includes starting quote) - expect(keyHandler).toHaveBeenCalledWith( - expect.objectContaining({ - name: '', - paste: true, - sequence: `${SINGLE_QUOTE}path`, + sequence: `${SINGLE_QUOTE}${expectedText}`, }), ); }); @@ -995,9 +806,7 @@ describe('Kitty Sequence Parsing', () => { }); it('should treat backslash as a regular keystroke', () => { - const keyHandler = vi.fn(); - const { result } = renderHook(() => useKeypressContext(), { wrapper }); - act(() => result.current.subscribe(keyHandler)); + const { keyHandler } = setupKeypressTest(true); act(() => stdin.write('\\')); @@ -1207,7 +1016,7 @@ describe('Kitty Sequence Parsing', () => { act(() => { stdin.emit('data', Buffer.from(char)); }); - await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setImmediate(resolve)); } // Should parse once complete