From df2d5c349204c950fbb3b24fec536f74fe6a9fa4 Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Fri, 7 Nov 2025 06:22:49 +0530 Subject: [PATCH] Fix issue with logs page infinite scroll (#24216) Co-authored-by: Ashish Gupta --- .../LogsViewerPage.component.test.tsx | 392 +++++++++++++++++- .../pages/LogsViewerPage/LogsViewerPage.tsx | 9 +- 2 files changed, 396 insertions(+), 5 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.component.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.component.test.tsx index 03a0a11ae14..464f79825b2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.component.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.component.test.tsx @@ -171,6 +171,7 @@ jest.mock('@melloware/react-logviewer', () => ({ { text, onScroll, + loading, }: { text: string; onScroll: (args: { @@ -178,6 +179,7 @@ jest.mock('@melloware/react-logviewer', () => ({ scrollHeight: number; clientHeight: number; }) => void; + loading?: boolean; }, ref ) => { @@ -196,7 +198,7 @@ jest.mock('@melloware/react-logviewer', () => ({ } return ( -
+
{text}
({ })); describe('LogsViewerPage.component', () => { + beforeEach(() => { + // Reset mocks before each test + const { getIngestionPipelineByFqn, getIngestionPipelineLogById } = + jest.requireMock('../../rest/ingestionPipelineAPI'); + + (getIngestionPipelineByFqn as jest.Mock).mockClear(); + (getIngestionPipelineLogById as jest.Mock).mockClear(); + + // Set default implementations + (getIngestionPipelineByFqn as jest.Mock).mockResolvedValue( + mockIngestionPipeline + ); + (getIngestionPipelineLogById as jest.Mock).mockResolvedValue({ + data: mockLogsData, + }); + }); + it('On initial, component should render', async () => { render(); @@ -660,4 +679,375 @@ describe('LogsViewerPage.component', () => { expect(showErrorToast).toHaveBeenCalledWith(mockError); }); }); + + describe('fetchMoreLogs dependency', () => { + it('should include fetchLogs in fetchMoreLogs useCallback dependencies to prevent stale closure', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + // Set up mock to return data on second call + (getIngestionPipelineLogById as jest.Mock).mockResolvedValueOnce({ + data: mockLogsData, + }); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect(screen.getByTestId('scroll-container')).toBeInTheDocument(); + }); + + // Set up mock for the next fetch + (getIngestionPipelineLogById as jest.Mock).mockResolvedValueOnce({ + data: { + ...mockLogsData, + after: '2', + }, + }); + + mockScrollPosition = { + scrollTop: 80, + scrollHeight: 100, + clientHeight: 20, + }; + + const scrollContainer = screen.getByTestId('scroll-container'); + + // Trigger scroll event to call fetchMoreLogs + await act(async () => { + fireEvent.click(scrollContainer); + }); + + // Verify fetchLogs was called with correct parameters (proving dependency is working) + await waitFor(() => { + expect(getIngestionPipelineLogById).toHaveBeenLastCalledWith( + 'c379d75a-43cd-4d93-a799-0bba4a22c690', + '1' + ); + }); + }); + }); + + describe('Skeleton display logic', () => { + it('should only show skeleton when isLoading is true, not isLogsLoading', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + const { getIngestionPipelineByFqn } = jest.requireMock( + '../../rest/ingestionPipelineAPI' + ); + + // Mock to keep isLoading true for longer + (getIngestionPipelineByFqn as jest.Mock).mockImplementationOnce( + () => + new Promise((resolve) => { + setTimeout(() => resolve(mockIngestionPipeline), 100); + }) + ); + + await act(async () => { + render(); + }); + + // Should show skeleton while isLoading is true + expect(screen.getByTestId('skeleton-container')).toBeInTheDocument(); + + // Wait for loading to complete + await waitFor( + () => { + expect( + screen.queryByTestId('skeleton-container') + ).not.toBeInTheDocument(); + }, + { timeout: 3000 } + ); + + // After isLoading is false, should show content even if isLogsLoading is true + expect(screen.getByTestId('jump-to-end-button')).toBeInTheDocument(); + }); + + it('should show skeleton only during initial data fetch', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + await act(async () => { + render(); + }); + + // Wait for skeleton to disappear + await waitFor(() => { + expect( + screen.queryByTestId('skeleton-container') + ).not.toBeInTheDocument(); + }); + + // Content should be visible - check for logs content or jump button + expect(screen.getByTestId('jump-to-end-button')).toBeInTheDocument(); + }); + }); + + describe('Empty logs with loading state', () => { + it('should not show empty logs message when isLogsLoading is true', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + // Mock empty logs response but with loading state + (getIngestionPipelineLogById as jest.Mock).mockImplementationOnce( + () => + new Promise((resolve) => { + setTimeout( + () => + resolve({ + data: { + ...mockLogsData, + ingestion_task: '', + }, + }), + 100 + ); + }) + ); + + await act(async () => { + render(); + }); + + // While loading, should not show "no logs available" message + expect( + screen.queryByText('label.no-entity-available') + ).not.toBeInTheDocument(); + + // Wait for loading to complete + await waitFor( + () => { + // After loading completes with empty logs, should show empty state + expect( + screen.getByText('label.no-entity-available') + ).toBeInTheDocument(); + }, + { timeout: 3000 } + ); + }); + + it('should show empty logs message only when logs are empty AND isLogsLoading is false', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + (getIngestionPipelineLogById as jest.Mock).mockImplementationOnce(() => + Promise.resolve({ + data: { + ...mockLogsData, + ingestion_task: '', + }, + }) + ); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect( + screen.getByText('label.no-entity-available') + ).toBeInTheDocument(); + }); + }); + + it('should not show empty message when logs are present', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect(screen.getByTestId('jump-to-end-button')).toBeInTheDocument(); + }); + + // Should not show empty state when logs exist + expect( + screen.queryByText('label.no-entity-available') + ).not.toBeInTheDocument(); + }); + }); + + describe('LazyLog loading prop', () => { + it('should pass loading prop to LazyLog component to show loading indicator', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + // Mock delayed response to keep isLogsLoading true + (getIngestionPipelineLogById as jest.Mock).mockImplementationOnce( + () => + new Promise((resolve) => { + setTimeout(() => resolve({ data: mockLogsData }), 500); + }) + ); + + await act(async () => { + render(); + }); + + // LazyLog should receive loading prop + // The component should be rendered but in loading state + await waitFor( + () => { + expect(screen.getByTestId('mocked-lazy-log')).toBeInTheDocument(); + }, + { timeout: 1000 } + ); + }); + + it('should set loading to false when logs have finished loading', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect(screen.getByTestId('mocked-lazy-log')).toBeInTheDocument(); + // Logs should be displayed (not loading) + expect( + screen.getByText(mockLogsData.ingestion_task) + ).toBeInTheDocument(); + }); + }); + }); + + describe('Race conditions and state management', () => { + it('should handle rapid scroll events without making duplicate API calls', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect(screen.getByTestId('scroll-container')).toBeInTheDocument(); + }); + + // After initial render, paging.after will be '1' from mockLogsData + // Set up a delayed mock to simulate loading time + (getIngestionPipelineLogById as jest.Mock).mockImplementation( + () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + data: { + ...mockLogsData, + after: '2', + }, + }); + }, 100); + }) + ); + + mockScrollPosition = { + scrollTop: 80, + scrollHeight: 100, + clientHeight: 20, + }; + + const scrollContainer = screen.getByTestId('scroll-container'); + + // Trigger first scroll event + await act(async () => { + fireEvent.click(scrollContainer); + }); + + // Immediately try to trigger more scroll events while first is loading + // These should be ignored because isLogsLoading should be true + fireEvent.click(scrollContainer); + fireEvent.click(scrollContainer); + + // Wait for the first call to complete + await waitFor(() => { + const calls = (getIngestionPipelineLogById as jest.Mock).mock.calls; + const scrollCalls = calls.filter((call) => call[1] === '1'); + + // Should only have 1 call with after='1' despite multiple rapid clicks + expect(scrollCalls).toHaveLength(1); + }); + }); + + it('should properly reset isLogsLoading after fetch completes', async () => { + (useParams as jest.Mock).mockReturnValue({ + logEntityType: 'TestSuite', + ingestionName: 'ingestion_123456', + }); + + // Ensure mocks are set up correctly for this test + const { getIngestionPipelineByFqn, getIngestionPipelineLogById } = + jest.requireMock('../../rest/ingestionPipelineAPI'); + + (getIngestionPipelineByFqn as jest.Mock).mockResolvedValueOnce( + mockIngestionPipeline + ); + (getIngestionPipelineLogById as jest.Mock).mockResolvedValueOnce({ + data: mockLogsData, + }); + + await act(async () => { + render(); + }); + + await waitFor(() => { + expect(screen.getByTestId('mocked-lazy-log')).toBeInTheDocument(); + }); + + // Verify logs are loaded and component is not in loading state + expect(screen.getByText(mockLogsData.ingestion_task)).toBeInTheDocument(); + + // Set up mock for the next fetch + (getIngestionPipelineLogById as jest.Mock).mockResolvedValueOnce({ + data: { + ...mockLogsData, + after: '2', + }, + }); + + mockScrollPosition = { + scrollTop: 80, + scrollHeight: 100, + clientHeight: 20, + }; + + const scrollContainer = screen.getByTestId('scroll-container'); + + await act(async () => { + fireEvent.click(scrollContainer); + }); + + // Should make API call since loading is complete + await waitFor(() => { + expect(getIngestionPipelineLogById).toHaveBeenLastCalledWith( + 'c379d75a-43cd-4d93-a799-0bba4a22c690', + '1' + ); + }); + }); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.tsx index 824d9af9c67..d9ddfd5b976 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/LogsViewerPage/LogsViewerPage.tsx @@ -224,7 +224,7 @@ const LogsViewerPage = () => { const fetchMoreLogs = useCallback(() => { fetchLogs(ingestionDetails?.id, ingestionDetails?.pipelineType); - }, [ingestionDetails]); + }, [ingestionDetails, fetchLogs]); const handleScroll = useCallback( (scrollValues: { @@ -349,7 +349,7 @@ const LogsViewerPage = () => { const logsSkeleton = useMemo( () => ( - + @@ -369,7 +369,7 @@ const LogsViewerPage = () => { ); const logsContainer = useMemo(() => { - if (isLoading || isLogsLoading) { + if (isLoading) { return logsSkeleton; } @@ -438,7 +438,7 @@ const LogsViewerPage = () => { })} - {isEmpty(logs) ? ( + {isEmpty(logs) && !isLogsLoading ? ( { enableSearch selectableLines extraLines={1} // 1 is to be add so that linux users can see last line of the log + loading={isLogsLoading} ref={lazyLogRef} text={logs} onScroll={handleScroll}