This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch fix-app-prefix-export in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1b960082ca9208a700622ecab494aff777c44e3f Author: Joe Li <[email protected]> AuthorDate: Thu Dec 18 21:31:30 2025 -0800 test(sqllab): add tests for streaming export URL prefix handling Add tests to validate streaming export behavior with application root prefixes: ResultSet.test.tsx: - Table-driven tests for URL prefix scenarios (no prefix, /superset, /my-app/superset) - Test for non-streaming export fallback when rows below threshold useStreamingExport.test.ts: - Test retryExport reuses original URL (important for prefixed deployments) - Test error handling sets ERROR status and calls onError callback - Fix test isolation: reset global.fetch mock in beforeEach These tests document the expected behavior for subdirectory deployments. Two prefix tests intentionally fail until the fix is applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --- .../SqlLab/components/ResultSet/ResultSet.test.tsx | 142 +++++++++++++++++++++ .../useStreamingExport.test.ts | 83 ++++++++++++ 2 files changed, 225 insertions(+) diff --git a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx index 29e2fd162f..ad9a39f6e1 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx @@ -47,6 +47,19 @@ jest.mock('src/components/ErrorMessage', () => ({ ErrorMessageWithStackTrace: () => <div data-test="error-message">Error</div>, })); +// Mock useStreamingExport to capture startExport calls +const mockStartExport = jest.fn(); +const mockResetExport = jest.fn(); +const mockCancelExport = jest.fn(); +jest.mock('src/components/StreamingExportModal/useStreamingExport', () => ({ + useStreamingExport: () => ({ + startExport: mockStartExport, + resetExport: mockResetExport, + cancelExport: mockCancelExport, + progress: { status: 'streaming', rowsProcessed: 0 }, + }), +})); + jest.mock( 'react-virtualized-auto-sizer', () => @@ -160,6 +173,7 @@ describe('ResultSet', () => { beforeEach(() => { applicationRootMock.mockReturnValue(''); + mockStartExport.mockClear(); }); // Add cleanup after each test @@ -657,4 +671,132 @@ describe('ResultSet', () => { const resultsCalls = fetchMock.calls('glob:*/api/v1/sqllab/results/*'); expect(resultsCalls).toHaveLength(1); }); + + test('should use non-streaming export (href) when rows below threshold', async () => { + // This test validates that when rows < CSV_STREAMING_ROW_THRESHOLD, + // the component uses the direct download href instead of streaming export. + const appRoot = '/superset'; + applicationRootMock.mockReturnValue(appRoot); + + // Create a query with rows BELOW the threshold + const smallQuery = { + ...queries[0], + rows: 500, // Below the 1000 threshold + limitingFactor: 'NOT_LIMITED', + }; + + const { getByTestId } = setup( + mockedProps, + mockStore({ + ...initialState, + user: { + ...user, + roles: { + sql_lab: [['can_export_csv', 'SQLLab']], + }, + }, + sqlLab: { + ...initialState.sqlLab, + queries: { + [smallQuery.id]: smallQuery, + }, + }, + common: { + conf: { + CSV_STREAMING_ROW_THRESHOLD: 1000, + }, + }, + }), + ); + + await waitFor(() => { + expect(getByTestId('export-csv-button')).toBeInTheDocument(); + }); + + const exportButton = getByTestId('export-csv-button'); + + // Non-streaming export should have href attribute with prefixed URL + expect(exportButton).toHaveAttribute( + 'href', + expect.stringMatching(new RegExp(`^${appRoot}/api/v1/sqllab/export/`)), + ); + + // Click should NOT trigger startExport for non-streaming + fireEvent.click(exportButton); + expect(mockStartExport).not.toHaveBeenCalled(); + }); + + test.each([ + { + name: 'no prefix (default deployment)', + appRoot: '', + expectedUrl: '/api/v1/sqllab/export_streaming/', + }, + { + name: 'with subdirectory prefix', + appRoot: '/superset', + expectedUrl: '/superset/api/v1/sqllab/export_streaming/', + }, + { + name: 'with nested subdirectory prefix', + appRoot: '/my-app/superset', + expectedUrl: '/my-app/superset/api/v1/sqllab/export_streaming/', + }, + ])( + 'streaming export URL should be correctly prefixed: $name', + async ({ appRoot, expectedUrl }) => { + // This test validates that streaming export startExport receives the correct URL + // based on the applicationRoot configuration. + // Without the fix, all cases will receive '/api/v1/sqllab/export_streaming/' without prefix. + applicationRootMock.mockReturnValue(appRoot); + + // Create a query with enough rows to trigger streaming export (>= threshold) + const largeQuery = { + ...queries[0], + rows: 5000, // Above the default 1000 threshold + limitingFactor: 'NOT_LIMITED', + }; + + const { getByTestId } = setup( + mockedProps, + mockStore({ + ...initialState, + user: { + ...user, + roles: { + sql_lab: [['can_export_csv', 'SQLLab']], + }, + }, + sqlLab: { + ...initialState.sqlLab, + queries: { + [largeQuery.id]: largeQuery, + }, + }, + common: { + conf: { + CSV_STREAMING_ROW_THRESHOLD: 1000, + }, + }, + }), + ); + + await waitFor(() => { + expect(getByTestId('export-csv-button')).toBeInTheDocument(); + }); + + const exportButton = getByTestId('export-csv-button'); + fireEvent.click(exportButton); + + // Verify startExport was called exactly once + expect(mockStartExport).toHaveBeenCalledTimes(1); + + // The URL should match the expected prefixed URL + expect(mockStartExport).toHaveBeenCalledWith( + expect.objectContaining({ + url: expectedUrl, + }), + ); + }, + ); }); diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts index 03241082dd..7fe03cd621 100644 --- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts +++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts @@ -17,6 +17,7 @@ * under the License. */ import { renderHook, act } from '@testing-library/react-hooks'; +import { waitFor } from '@testing-library/react'; import { useStreamingExport } from './useStreamingExport'; import { ExportStatus } from './StreamingExportModal'; @@ -35,6 +36,7 @@ global.fetch = jest.fn(); beforeEach(() => { jest.clearAllMocks(); + global.fetch = jest.fn(); }); test('useStreamingExport initializes with default progress state', () => { @@ -124,3 +126,84 @@ test('useStreamingExport cleans up on unmount', () => { // Cleanup should not throw errors expect(true).toBe(true); }); + +test('retryExport reuses the same URL from the original startExport call', async () => { + // This test ensures that retryExport uses the exact same URL that was passed to startExport, + // which is important for subdirectory deployments where the URL is already prefixed. + const originalUrl = '/superset/api/v1/sqllab/export_streaming/'; + const mockFetch = jest.fn().mockResolvedValue({ + ok: true, + headers: new Headers({ + 'Content-Disposition': 'attachment; filename="export.csv"', + }), + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, + }); + global.fetch = mockFetch; + + const { result } = renderHook(() => useStreamingExport()); + + // First call with startExport + act(() => { + result.current.startExport({ + url: originalUrl, + payload: { client_id: 'test-id' }, + exportType: 'csv', + expectedRows: 100, + }); + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + expect(mockFetch).toHaveBeenCalledWith(originalUrl, expect.any(Object)); + + // Reset mock to track retry call + mockFetch.mockClear(); + + // Reset the export state so we can retry + act(() => { + result.current.resetExport(); + }); + + // Call retryExport - should reuse the same URL + act(() => { + result.current.retryExport(); + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + // Retry should use the exact same URL that was passed to startExport + expect(mockFetch).toHaveBeenCalledWith(originalUrl, expect.any(Object)); +}); + +test('sets ERROR status and calls onError when fetch rejects', async () => { + const errorMessage = 'Network error'; + const mockFetch = jest.fn().mockRejectedValue(new Error(errorMessage)); + global.fetch = mockFetch; + + const onError = jest.fn(); + const { result } = renderHook(() => useStreamingExport({ onError })); + + act(() => { + result.current.startExport({ + url: '/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', + expectedRows: 100, + }); + }); + + // Wait for fetch to be called and error to be processed + await waitFor(() => { + expect(result.current.progress.status).toBe(ExportStatus.ERROR); + }); + + // Verify onError was called exactly once with the error message + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith(errorMessage); +});
