diff --git a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts index fe0f7808..29615c72 100644 --- a/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts +++ b/apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts @@ -398,11 +398,12 @@ describe('Subprocess Spawn Integration', () => { // Wait for spawn to complete (ensures exit handlers are attached) await new Promise(resolve => setImmediate(resolve)); - // Both tasks share the same mock process, so emit exit once triggers both handlers + // Emit exit for task-1 (first task's handler) mockProcess.emit('exit', 0); - - // Wait for both promises to resolve await promise1; + + // Emit exit for task-2 (second task's handler replaces first due to shared mock process) + mockProcess.emit('exit', 0); await promise2; // Tasks should be removed from tracking after exit diff --git a/apps/frontend/src/renderer/components/settings/terminal-font-settings/TerminalFontSettings.tsx b/apps/frontend/src/renderer/components/settings/terminal-font-settings/TerminalFontSettings.tsx index fcd785e2..2c9cd792 100644 --- a/apps/frontend/src/renderer/components/settings/terminal-font-settings/TerminalFontSettings.tsx +++ b/apps/frontend/src/renderer/components/settings/terminal-font-settings/TerminalFontSettings.tsx @@ -1,5 +1,6 @@ import { Terminal } from 'lucide-react'; import { useTranslation } from 'react-i18next'; +import { useMemo } from 'react'; import { useToast } from '../../../hooks/use-toast'; import { SettingsSection } from '../SettingsSection'; import { useTerminalFontSettingsStore } from '../../../stores/terminal-font-settings-store'; @@ -29,18 +30,34 @@ export function TerminalFontSettings() { const { t } = useTranslation('settings'); const { toast } = useToast(); - // Get current settings from store using selector to exclude action functions - const settings = useTerminalFontSettingsStore((state) => ({ - fontFamily: state.fontFamily, - fontSize: state.fontSize, - fontWeight: state.fontWeight, - lineHeight: state.lineHeight, - letterSpacing: state.letterSpacing, - cursorStyle: state.cursorStyle, - cursorBlink: state.cursorBlink, - cursorAccentColor: state.cursorAccentColor, - scrollback: state.scrollback, - })); + // Get current settings from store using individual selectors to prevent infinite re-render loop + // Each selector only re-renders when its specific value changes + const fontFamily = useTerminalFontSettingsStore((state) => state.fontFamily); + const fontSize = useTerminalFontSettingsStore((state) => state.fontSize); + const fontWeight = useTerminalFontSettingsStore((state) => state.fontWeight); + const lineHeight = useTerminalFontSettingsStore((state) => state.lineHeight); + const letterSpacing = useTerminalFontSettingsStore((state) => state.letterSpacing); + const cursorStyle = useTerminalFontSettingsStore((state) => state.cursorStyle); + const cursorBlink = useTerminalFontSettingsStore((state) => state.cursorBlink); + const cursorAccentColor = useTerminalFontSettingsStore((state) => state.cursorAccentColor); + const scrollback = useTerminalFontSettingsStore((state) => state.scrollback); + + // Reconstruct settings object with stable reference using useMemo + // This prevents the infinite re-render loop caused by creating new object references + const settings = useMemo( + () => ({ + fontFamily, + fontSize, + fontWeight, + lineHeight, + letterSpacing, + cursorStyle, + cursorBlink, + cursorAccentColor, + scrollback, + }), + [fontFamily, fontSize, fontWeight, lineHeight, letterSpacing, cursorStyle, cursorBlink, cursorAccentColor, scrollback] + ); // Get action methods from store const updateSettings = useTerminalFontSettingsStore((state) => state.applySettings); diff --git a/apps/frontend/src/renderer/components/settings/terminal-font-settings/__tests__/TerminalFontSettings.test.tsx b/apps/frontend/src/renderer/components/settings/terminal-font-settings/__tests__/TerminalFontSettings.test.tsx new file mode 100644 index 00000000..8204a7dc --- /dev/null +++ b/apps/frontend/src/renderer/components/settings/terminal-font-settings/__tests__/TerminalFontSettings.test.tsx @@ -0,0 +1,342 @@ +/** + * @vitest-environment jsdom + */ + +/** + * Unit tests for TerminalFontSettings component + * Tests the infinite re-render loop fix using individual selectors + useMemo + * Verifies component renders without errors and maintains stable object references + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import '@testing-library/jest-dom/vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import { I18nextProvider } from 'react-i18next'; +import { act } from 'react'; +import { TerminalFontSettings } from '../TerminalFontSettings'; +import { useTerminalFontSettingsStore } from '../../../../stores/terminal-font-settings-store'; +import i18n from '../../../../../shared/i18n'; + +// Polyfill ResizeObserver for jsdom environment +class ResizeObserverMock { + observe = vi.fn(); + unobserve = vi.fn(); + disconnect = vi.fn(); +} +global.ResizeObserver = ResizeObserverMock; + +// Mock the toast hook +vi.mock('../../../../hooks/use-toast', () => ({ + useToast: () => ({ + toast: vi.fn(), + }), +})); + +// Mock xterm.js to prevent initialization errors in tests +// vi.mock calls are hoisted to the top, so we use function keyword +vi.mock('@xterm/xterm', () => ({ + Terminal: vi.fn().mockImplementation(function() { + return { + open: vi.fn(), + write: vi.fn(), + loadAddon: vi.fn(), + options: {}, + refresh: vi.fn(), + dispose: vi.fn(), + rows: 24, + }; + }), +})); + +vi.mock('@xterm/addon-fit', () => ({ + FitAddon: vi.fn().mockImplementation(function() { + return { + fit: vi.fn(), + }; + }), +})); + +function renderWithI18n(ui: React.ReactElement) { + return render({ui}); +} + +describe('TerminalFontSettings - Infinite Re-render Loop Fix', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Reset store to default state before each test + const store = useTerminalFontSettingsStore.getState(); + store.resetToDefaults(); + }); + + // Note: This fix addresses a React/Zustand selector issue that is platform-agnostic. + // The bug occurred on all platforms, so platform-specific mocking is not required. + + describe('Component Rendering', () => { + it('should render without throwing errors', () => { + expect(() => { + renderWithI18n(); + }).not.toThrow(); + }); + + it('should render all expected sections', () => { + renderWithI18n(); + + // Main sections - use getAllByText for text that may appear multiple times + expect(screen.getAllByText(/terminal fonts/i).length).toBeGreaterThan(0); + + // Import/Export buttons + expect(screen.getAllByText(/export json/i).length).toBeGreaterThan(0); + expect(screen.getAllByText(/import json/i).length).toBeGreaterThan(0); + expect(screen.getAllByText(/copy to clipboard/i).length).toBeGreaterThan(0); + + // Configuration sections + expect(screen.getAllByText(/font configuration/i).length).toBeGreaterThan(0); + expect(screen.getAllByText(/cursor configuration/i).length).toBeGreaterThan(0); + expect(screen.getAllByText(/performance settings/i).length).toBeGreaterThan(0); + expect(screen.getAllByText(/quick presets/i).length).toBeGreaterThan(0); + + // Preview section + expect(screen.getAllByText(/live preview/i).length).toBeGreaterThan(0); + }); + + it('should complete render cycle without hanging', async () => { + renderWithI18n(); + + // Wait for component to fully render + // The waitFor timeout provides the safety net for catching hangs/infinite loops + await waitFor( + () => { + expect(screen.getByText(/terminal fonts/i)).toBeInTheDocument(); + }, + { timeout: 2000 } + ); + }); + }); + + describe('Store Integration', () => { + it('should access all store properties without errors', () => { + renderWithI18n(); + + const state = useTerminalFontSettingsStore.getState(); + + // Verify all properties are accessible + expect(state.fontFamily).toBeDefined(); + expect(state.fontSize).toBeDefined(); + expect(state.fontWeight).toBeDefined(); + expect(state.lineHeight).toBeDefined(); + expect(state.letterSpacing).toBeDefined(); + expect(state.cursorStyle).toBeDefined(); + expect(state.cursorBlink).toBeDefined(); + expect(state.cursorAccentColor).toBeDefined(); + expect(state.scrollback).toBeDefined(); + }); + + it('should update store state when component is rendered', () => { + renderWithI18n(); + + // Update a single setting via store + act(() => { + useTerminalFontSettingsStore.getState().setFontSize(16); + }); + + // Verify store state updated + expect(useTerminalFontSettingsStore.getState().fontSize).toBe(16); + }); + }); + + describe('State Updates - No Infinite Loop', () => { + it('should handle rapid state changes without infinite loop', async () => { + renderWithI18n(); + + // Simulate rapid state changes (like dragging a slider) + const sizes = [14, 15, 16, 17, 18, 17, 16, 15, 14]; + + for (const size of sizes) { + act(() => { + useTerminalFontSettingsStore.getState().setFontSize(size); + }); + } + + // If we reach here without timeout, the infinite loop is fixed + expect(useTerminalFontSettingsStore.getState().fontSize).toBe(14); + }); + + it('should handle preset application without infinite loop', async () => { + renderWithI18n(); + + // Apply a preset (which updates multiple values at once) + await act(async () => { + useTerminalFontSettingsStore.getState().applyPreset('vscode'); + }); + + // Verify preset was applied + const state = useTerminalFontSettingsStore.getState(); + expect(state.fontFamily).toContain('Consolas'); + }); + + it('should handle reset to defaults without infinite loop', async () => { + // Capture defaults before mutating + const defaults = useTerminalFontSettingsStore.getState(); + const defaultFontSize = defaults.fontSize; + const defaultFontWeight = defaults.fontWeight; + const defaultFontFamily = defaults.fontFamily; + const defaultLineHeight = defaults.lineHeight; + + // First change some settings + act(() => { + useTerminalFontSettingsStore.getState().setFontSize(20); + useTerminalFontSettingsStore.getState().setFontWeight(700); + }); + + renderWithI18n(); + + // Verify settings changed + expect(useTerminalFontSettingsStore.getState().fontSize).toBe(20); + + // Get the OS-specific defaults to know what to expect + const store = useTerminalFontSettingsStore.getState(); + + // Reset to defaults - if there's an infinite loop, this will timeout + await act(async () => { + store.resetToDefaults(); + }); + + // Verify reset restored default values + const state = useTerminalFontSettingsStore.getState(); + expect(state.fontSize).toBe(defaultFontSize); + expect(state.fontWeight).toBe(defaultFontWeight); + expect(state.fontFamily).toEqual(defaultFontFamily); + expect(state.lineHeight).toBe(defaultLineHeight); + }); + + it('should handle concurrent updates without race conditions', async () => { + renderWithI18n(); + + // Simulate concurrent updates + const promises = [ + Promise.resolve().then(() => act(() => useTerminalFontSettingsStore.getState().setFontSize(16))), + Promise.resolve().then(() => act(() => useTerminalFontSettingsStore.getState().setFontWeight(500))), + Promise.resolve().then(() => act(() => useTerminalFontSettingsStore.getState().setLineHeight(1.5))), + ]; + + await Promise.all(promises); + + // Verify final state is consistent + const state = useTerminalFontSettingsStore.getState(); + expect(state.fontSize).toBe(16); + expect(state.fontWeight).toBe(500); + expect(state.lineHeight).toBe(1.5); + }); + }); + + describe('Import/Export Operations', () => { + it('should export settings without errors', () => { + renderWithI18n(); + + const exported = useTerminalFontSettingsStore.getState().exportSettings(); + + expect(exported).toBeTruthy(); + expect(typeof exported).toBe('string'); + + // Verify it's valid JSON + expect(() => JSON.parse(exported)).not.toThrow(); + + const parsed = JSON.parse(exported); + expect(parsed.fontFamily).toBeDefined(); + expect(parsed.fontSize).toBeDefined(); + }); + + it('should import settings and update store state', () => { + renderWithI18n(); + + const json = JSON.stringify({ + fontFamily: ['Fira Code', 'monospace'], + fontSize: 16, + fontWeight: 500, + lineHeight: 1.5, + letterSpacing: 0.5, + cursorStyle: 'underline', + cursorBlink: false, + cursorAccentColor: '#ff0000', + scrollback: 50000, + }); + + const success = useTerminalFontSettingsStore.getState().importSettings(json); + + expect(success).toBe(true); + + // Verify store state reflects imported settings + expect(useTerminalFontSettingsStore.getState().fontSize).toBe(16); + expect(useTerminalFontSettingsStore.getState().fontFamily).toEqual(['Fira Code', 'monospace']); + }); + }); + + describe('Child Component Integration', () => { + it('should render FontConfigPanel with current settings', () => { + renderWithI18n(); + + // Verify FontConfigPanel renders + expect(screen.getAllByText(/font size/i).length).toBeGreaterThan(0); + + // Verify the current font size value is accessible from store + const fontSize = useTerminalFontSettingsStore.getState().fontSize; + expect(fontSize).toBeGreaterThan(0); + expect(fontSize).toBeLessThanOrEqual(24); + }); + }); + + describe('Regression Prevention', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should not log React warnings about getSnapshot caching', () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + renderWithI18n(); + + // Check for getSnapshot-related warnings + const warnCalls = consoleWarnSpy.mock.calls.filter((call) => + call.some((arg) => typeof arg === 'string' && arg.includes('getSnapshot')) + ); + + expect(warnCalls.length).toBe(0); + }); + + it('should not cause "Maximum update depth exceeded" error', () => { + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + renderWithI18n(); + + // Check for infinite loop errors + const errorCalls = consoleErrorSpy.mock.calls.filter((call) => + call.some( + (arg) => + typeof arg === 'string' && + (arg.includes('Maximum update depth') || arg.includes('infinite loop')) + ) + ); + + expect(errorCalls.length).toBe(0); + }); + }); + + describe('Memoization - Stable References', () => { + it('should maintain stable component state across re-renders', () => { + // This test verifies useMemo provides stable references + // by checking that multiple re-renders don't break the component + + const { rerender } = renderWithI18n(); + + // Rerender multiple times without state changes + // If useMemo wasn't working correctly, this might cause issues + for (let i = 0; i < 5; i++) { + act(() => { + rerender(); + }); + } + + // Verify component still renders correctly after multiple re-renders + expect(screen.getAllByText(/terminal fonts/i).length).toBeGreaterThan(0); + }); + }); +});