From 79f840ab6e669d5b5f5e178d71b085fd55e4c9f4 Mon Sep 17 00:00:00 2001 From: Anso Date: Thu, 25 Jun 2026 02:17:23 -0400 Subject: [PATCH] fix: clear structured log viewer rows on stack switch (#1448) The structured log viewer accumulated log rows across stack switches because the useEffect cleanup closed the old WebSocket but never cleared the committed rows state. Reset rows, row IDs, and auto-follow at the top of the effect before connecting to the new stack. The level filter is intentionally preserved across switches. Closes #1444 --- .../src/components/StructuredLogViewer.tsx | 7 + .../__tests__/StructuredLogViewer.test.tsx | 181 ++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 frontend/src/components/__tests__/StructuredLogViewer.test.tsx diff --git a/frontend/src/components/StructuredLogViewer.tsx b/frontend/src/components/StructuredLogViewer.tsx index a2ee33ee..d1db7df1 100644 --- a/frontend/src/components/StructuredLogViewer.tsx +++ b/frontend/src/components/StructuredLogViewer.tsx @@ -68,6 +68,13 @@ export default function StructuredLogViewer({ stackName }: StructuredLogViewerPr useEffect(() => { followingRef.current = following; }, [following]); useEffect(() => { + // Reset state accumulated from the previous stack before connecting. + // eslint-disable-next-line react-hooks/set-state-in-effect + setRows([]); + rowIdRef.current = 0; + setFollowing(true); + followingRef.current = true; + const cleanStackName = stackName.replace(/\.(yml|yaml)$/, ''); const wsProtocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:'; const activeNodeId = localStorage.getItem('sencho-active-node') || ''; diff --git a/frontend/src/components/__tests__/StructuredLogViewer.test.tsx b/frontend/src/components/__tests__/StructuredLogViewer.test.tsx new file mode 100644 index 00000000..478c5369 --- /dev/null +++ b/frontend/src/components/__tests__/StructuredLogViewer.test.tsx @@ -0,0 +1,181 @@ +/** + * Unit tests for StructuredLogViewer's log-row lifecycle, specifically that + * switching stacks clears the old stack's committed rows, closes the old + * WebSocket, resets auto-follow, and preserves the level filter. + */ +import { render, screen, cleanup, fireEvent, act } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import StructuredLogViewer from '../StructuredLogViewer'; + +class MockWS { + static instances: MockWS[] = []; + url: string; + onopen: (() => void) | null = null; + onmessage: ((e: { data: string }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: ((e?: unknown) => void) | null = null; + send = vi.fn(); + close = vi.fn(); + constructor(url: string) { this.url = url; MockWS.instances.push(this); } + static reset() { MockWS.instances = []; } +} + +beforeEach(() => { + MockWS.reset(); + vi.stubGlobal('WebSocket', MockWS); + vi.stubGlobal('cancelAnimationFrame', vi.fn()); + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { + cb(0); + return 0; + }); + localStorage.setItem('sencho-active-node', ''); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.removeItem('sencho-active-node'); + vi.clearAllMocks(); +}); + +describe('StructuredLogViewer', () => { + it('renders initial empty state and builds the correct WebSocket URL', () => { + const { container } = render(); + expect(container.textContent).toContain('Waiting for log output'); + expect(MockWS.instances).toHaveLength(1); + expect(MockWS.instances[0].url).toContain('/api/stacks/test-stack/logs'); + }); + + it('renders log lines with timestamp, level badge, and message', async () => { + const { container } = render(); + await act(async () => { + MockWS.instances[0].onopen?.(); + MockWS.instances[0].onmessage?.({ data: '2025-01-01T12:00:00Z ERROR something-failed' }); + }); + + expect(container.textContent).toContain('something-failed'); + // Level badge is lowercase in the DOM (CSS text-transform handles visual). + expect(container.textContent).toContain('err'); + + // Timestamp: computed in local time. + const ts = new Date('2025-01-01T12:00:00Z'); + const pad = (n: number) => String(n).padStart(2, '0'); + const expectedTs = `${pad(ts.getHours())}:${pad(ts.getMinutes())}:${pad(ts.getSeconds())}`; + expect(container.textContent).toContain(expectedTs); + }); + + it('clears committed rows when the stack changes', async () => { + const { rerender, container } = render(); + await act(async () => { + MockWS.instances[0].onopen?.(); + MockWS.instances[0].onmessage?.({ data: '2025-01-01T10:00:00Z INFO only-in-stack-a\n' }); + }); + + expect(container.textContent).toContain('only-in-stack-a'); + + rerender(); + await act(async () => { + MockWS.instances[1].onopen?.(); + MockWS.instances[1].onmessage?.({ data: '2025-01-01T11:00:00Z ERROR only-in-stack-b\n' }); + }); + + expect(container.textContent).not.toContain('only-in-stack-a'); + expect(container.textContent).toContain('only-in-stack-b'); + }); + + it('closes the old WebSocket on stack switch', () => { + const { rerender } = render(); + const oldWs = MockWS.instances[0]; + expect(oldWs.close).not.toHaveBeenCalled(); + + rerender(); + expect(oldWs.close).toHaveBeenCalled(); + }); + + it('downloads only the current stack rows after a switch', async () => { + let capturedBlob: Blob | null = null; + vi.stubGlobal('URL', { + ...URL, + createObjectURL: vi.fn((blob: Blob) => { + capturedBlob = blob; + return 'blob:fake'; + }), + revokeObjectURL: vi.fn(), + }); + + const { rerender } = render(); + await act(async () => { + MockWS.instances[0].onopen?.(); + MockWS.instances[0].onmessage?.({ data: '2025-01-01T10:00:00Z INFO from-stack-a\n' }); + }); + + rerender(); + await act(async () => { + MockWS.instances[1].onopen?.(); + MockWS.instances[1].onmessage?.({ data: '2025-01-01T11:00:00Z ERROR from-stack-b\n' }); + }); + + const downloadBtn = screen.getByLabelText('Download logs'); + await userEvent.click(downloadBtn); + + expect(capturedBlob).not.toBeNull(); + const text = await capturedBlob!.text(); + expect(text).toContain('from-stack-b'); + expect(text).not.toContain('from-stack-a'); + }); + + it('preserves the level filter across stack switches', async () => { + const { rerender, container } = render(); + + // Click the "warn" filter button. + await userEvent.click(screen.getByText('warn')); + + rerender(); + await act(async () => { + MockWS.instances[1].onopen?.(); + MockWS.instances[1].onmessage?.({ + data: '2025-01-01T10:00:00Z INFO should-be-hidden\n2025-01-01T11:00:00Z WARNING should-be-visible\n', + }); + }); + + expect(container.textContent).not.toContain('should-be-hidden'); + expect(container.textContent).toContain('should-be-visible'); + }); + + it('resets auto-follow when the stack changes', async () => { + const { rerender, container } = render(); + await act(async () => { + MockWS.instances[0].onopen?.(); + }); + + const scrollContainer = document.querySelector('.overflow-y-auto') as HTMLElement; + expect(scrollContainer).not.toBeNull(); + + Object.defineProperty(scrollContainer, 'scrollHeight', { configurable: true, value: 500 }); + Object.defineProperty(scrollContainer, 'clientHeight', { configurable: true, value: 200 }); + scrollContainer.scrollTop = 100; + await act(async () => { + fireEvent.scroll(scrollContainer); + }); + + expect(container.textContent).toContain('resume follow'); + + rerender(); + await act(async () => { + MockWS.instances[1].onopen?.(); + }); + + expect(container.textContent).toContain('following'); + }); + + it('strips .yml and .yaml suffixes from the WebSocket URL', () => { + const { rerender } = render(); + expect(MockWS.instances[0].url).toContain('/api/stacks/my-stack/logs'); + expect(MockWS.instances[0].url).not.toContain('.yml'); + + rerender(); + expect(MockWS.instances[1].url).toContain('/api/stacks/another-stack/logs'); + expect(MockWS.instances[1].url).not.toContain('.yaml'); + }); +});