This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch feat-chart-list-rtl-tests in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3ccc71df094ca67f1fddb8ae4d525497caa7f2d2 Author: Joe Li <[email protected]> AuthorDate: Fri Feb 6 14:07:14 2026 -0800 refactor(chart-list): consolidate filters, remove redundant tests, rename - Consolidate 8 individual filter rendering tests into 1 comprehensive test - Remove 2 redundant loading state tests (ChartList.test.tsx) - Remove 3 redundant tests: empty dataset column, schema-less dataset name, and duplicate bulk select exit path (ChartList.listview.test.tsx) - Remove duplicate bulk select exit path (ChartList.cardview.test.tsx) - Rename all test names to drop verbose "ChartList list view" prefix Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../pages/ChartList/ChartList.cardview.test.tsx | 29 ---- .../pages/ChartList/ChartList.listview.test.tsx | 151 ++--------------- .../src/pages/ChartList/ChartList.test.tsx | 181 +++------------------ 3 files changed, 38 insertions(+), 323 deletions(-) diff --git a/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx index e6420432939..d6369376925 100644 --- a/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx +++ b/superset-frontend/src/pages/ChartList/ChartList.cardview.test.tsx @@ -489,35 +489,6 @@ describe('ChartList Card View Tests', () => { }); }); - test('exit bulk select by clicking bulk select button again', async () => { - renderChartList(mockUser); - - // Wait for cards to load - await screen.findByTestId('chart-list-view'); - await waitFor(() => { - expect(screen.getByText(mockCharts[0].slice_name)).toBeInTheDocument(); - }); - - // Enable bulk select mode - const bulkSelectButton = screen.getByTestId('bulk-select'); - fireEvent.click(bulkSelectButton); - - // Wait for bulk select controls - await waitFor(() => { - expect(screen.getByTestId('bulk-select-controls')).toBeInTheDocument(); - }); - - // Click bulk select button again to exit - fireEvent.click(bulkSelectButton); - - // Verify bulk select controls are gone - await waitFor(() => { - expect( - screen.queryByTestId('bulk-select-controls'), - ).not.toBeInTheDocument(); - }); - }); - test('card click behavior changes in bulk select mode', async () => { renderChartList(mockUser); diff --git a/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx index 43827328b6b..f444f3534fb 100644 --- a/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx +++ b/superset-frontend/src/pages/ChartList/ChartList.listview.test.tsx @@ -72,7 +72,7 @@ afterEach(() => { mockIsFeatureEnabled.mockReset(); }); -test('ChartList list view renders correctly', async () => { +test('renders table in list view', async () => { renderChartList(mockUser); // Wait for component to load @@ -91,7 +91,7 @@ test('ChartList list view renders correctly', async () => { }); }); -test('ChartList list view correctly displays dataset names with and without schema', async () => { +test('displays dataset names with and without schema prefix', async () => { // Create custom mock data with different datasource_name_text formats const customMockCharts = [ { @@ -167,7 +167,7 @@ test('ChartList list view correctly displays dataset names with and without sche expect(dotsLink).toHaveTextContent('table.with.dots'); }); -test('ChartList list view switches from list view to card view', async () => { +test('switches from list view to card view', async () => { renderChartList(mockUser); await waitFor(() => { @@ -188,7 +188,7 @@ test('ChartList list view switches from list view to card view', async () => { expect(cards).toHaveLength(mockCharts.length); }); -test('ChartList list view renders all required column headers', async () => { +test('renders all required column headers', async () => { renderChartList(mockUser); await waitFor(() => { @@ -218,7 +218,7 @@ test('ChartList list view renders all required column headers', async () => { }); }); -test('ChartList list view sorts table when clicking column headers', async () => { +test('sorts table when clicking column headers', async () => { renderChartList(mockUser); await waitFor(() => { @@ -275,7 +275,7 @@ test('ChartList list view sorts table when clicking column headers', async () => }); }); -test('ChartList list view displays chart data correctly', async () => { +test('displays chart data correctly in table rows', async () => { /** * @todo Implement test logic for tagging. * If TAGGING_SYSTEM is ever deprecated to always be on, @@ -354,7 +354,7 @@ test('ChartList list view displays chart data correctly', async () => { expect(within(chartRow).getByTestId('edit-alt')).toBeInTheDocument(); }); -test('ChartList list view export chart api called when export button is clicked', async () => { +test('calls export API when export button is clicked', async () => { renderChartList(mockUser); await waitFor(() => { @@ -381,7 +381,7 @@ test('ChartList list view export chart api called when export button is clicked' }); }); -test('ChartList list view opens edit properties modal when edit button is clicked', async () => { +test('opens edit properties modal on edit button click', async () => { renderChartList(mockUser); await waitFor(() => { @@ -405,7 +405,7 @@ test('ChartList list view opens edit properties modal when edit button is clicke }); }); -test('ChartList list view opens delete confirmation when delete button is clicked', async () => { +test('opens delete confirmation on delete button click', async () => { renderChartList(mockUser); await waitFor(() => { @@ -429,7 +429,7 @@ test('ChartList list view opens delete confirmation when delete button is clicke }); }); -test('ChartList list view displays certified badge only for certified charts', async () => { +test('displays certified badge only for certified charts', async () => { // Test certified chart (mockCharts[1] has certification) const certifiedChart = mockCharts[1]; // Test uncertified chart (mockCharts[0] has no certification) @@ -468,7 +468,7 @@ test('ChartList list view displays certified badge only for certified charts', a ).not.toBeInTheDocument(); }); -test('ChartList list view displays info icon only for charts with descriptions', async () => { +test('displays info icon only for charts with descriptions', async () => { // Test chart with description (mockCharts[0] has description) const chartWithDesc = mockCharts[0]; // Test chart without description (mockCharts[2] has description: null) @@ -506,47 +506,7 @@ test('ChartList list view displays info icon only for charts with descriptions', ).not.toBeInTheDocument(); }); -test('ChartList list view displays chart with empty dataset column', async () => { - renderChartList(mockUser); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - await waitFor(() => { - expect(screen.getByText(mockCharts[2].slice_name)).toBeInTheDocument(); - }); - - const table = screen.getByTestId('listview-table'); - const chartNameElement = within(table).getByText(mockCharts[2].slice_name); - const chartRow = chartNameElement.closest( - '[data-test="table-row"]', - ) as HTMLElement; - - // Chart name should be visible - expect( - within(chartRow).getByText(mockCharts[2].slice_name), - ).toBeInTheDocument(); - - // Find dataset column index by header - const headers = within(table).getAllByRole('columnheader'); - const datasetHeaderIndex = headers.findIndex(header => - header.textContent?.includes('Dataset'), - ); - expect(datasetHeaderIndex).toBeGreaterThan(-1); // Ensure column exists - - // Since mockCharts[2] has datasource_name_text: null, verify dataset cell is empty - const datasetCell = within(chartRow).getAllByRole('cell')[datasetHeaderIndex]; - expect(datasetCell).toBeInTheDocument(); - - // Verify dataset cell is empty for charts with no dataset - expect(datasetCell).toHaveTextContent(''); - // There's a link element but with empty href - const datasetLink = within(datasetCell).getByRole('link'); - expect(datasetLink).toHaveAttribute('href', ''); -}); - -test('ChartList list view displays chart with empty on dashboards column', async () => { +test('renders empty dashboard column for charts without dashboards', async () => { renderChartList(mockUser); await waitFor(() => { @@ -605,7 +565,7 @@ test('renders dashboard crosslinks as navigable links', async () => { expect(links[2]).toHaveAttribute('href', '/superset/dashboard/4'); }); -test('ChartList list view shows tag info when TAGGING_SYSTEM is enabled', async () => { +test('shows tag column when TAGGING_SYSTEM is enabled', async () => { // Enable tagging system feature flag mockIsFeatureEnabled.mockImplementation( feature => feature === 'TAGGING_SYSTEM', @@ -645,7 +605,7 @@ test('ChartList list view shows tag info when TAGGING_SYSTEM is enabled', async expect(tagLink).toHaveAttribute('target', '_blank'); }); -test('ChartList list view can bulk select and deselect all charts', async () => { +test('supports bulk select and deselect all', async () => { renderChartList(mockUser); await waitFor(() => { @@ -705,7 +665,7 @@ test('ChartList list view can bulk select and deselect all charts', async () => expect(screen.queryByTestId('bulk-select-action')).not.toBeInTheDocument(); }); -test('ChartList list view can bulk export selected charts', async () => { +test('supports bulk export of selected charts', async () => { renderChartList(mockUser); await waitFor(() => { @@ -751,7 +711,7 @@ test('ChartList list view can bulk export selected charts', async () => { }); }); -test('ChartList list view can bulk delete selected charts', async () => { +test('supports bulk delete of selected charts', async () => { renderChartList(mockUser); await waitFor(() => { @@ -797,7 +757,7 @@ test('ChartList list view can bulk delete selected charts', async () => { }); }); -test('ChartList list view can bulk add tags to selected charts', async () => { +test('supports bulk add tags to selected charts', async () => { // Enable tagging system feature flag mockIsFeatureEnabled.mockImplementation( feature => feature === 'TAGGING_SYSTEM', @@ -848,51 +808,7 @@ test('ChartList list view can bulk add tags to selected charts', async () => { }); }); -test('ChartList list view exit bulk select by hitting x on bulk select bar', async () => { - renderChartList(mockUser); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - await waitFor(() => { - expect(screen.getByText(mockCharts[0].slice_name)).toBeInTheDocument(); - expect(screen.getByText(mockCharts[1].slice_name)).toBeInTheDocument(); - }); - - const bulkSelectButton = screen.getByTestId('bulk-select'); - await userEvent.click(bulkSelectButton); - - await waitFor(() => { - // Expect header checkbox + one checkbox per chart - expect(screen.getAllByRole('checkbox')).toHaveLength(mockCharts.length + 1); - }); - - const table = screen.getByTestId('listview-table'); - // Target first data row specifically (not header row) - const dataRows = within(table).getAllByTestId('table-row'); - const firstRowCheckbox = within(dataRows[0]).getByRole('checkbox'); - await userEvent.click(firstRowCheckbox); - - await waitFor(() => { - expect(screen.getByTestId('bulk-select-copy')).toHaveTextContent( - '1 Selected', - ); - }); - - // Find and click the close button (x) on the bulk select bar - const closeIcon = document.querySelector( - '.ant-alert-close-icon', - ) as HTMLButtonElement; - await userEvent.click(closeIcon); - - await waitFor(() => { - expect(screen.queryAllByRole('checkbox')).toHaveLength(0); - expect(screen.queryByTestId('bulk-select-copy')).not.toBeInTheDocument(); - }); -}); - -test('ChartList list view exit bulk select by clicking bulk select button again', async () => { +test('exits bulk select on button toggle', async () => { renderChartList(mockUser); await waitFor(() => { @@ -931,34 +847,3 @@ test('ChartList list view exit bulk select by clicking bulk select button again' expect(screen.queryByTestId('bulk-select-copy')).not.toBeInTheDocument(); }); }); - -test('ChartList list view displays dataset name without schema prefix correctly', async () => { - // Test just name case - should display the full name when no schema prefix - renderChartList(mockUser); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - const table = screen.getByTestId('listview-table'); - - // Wait for chart with simple dataset name to load - await waitFor(() => { - expect( - within(table).getByText(mockCharts[1].slice_name), - ).toBeInTheDocument(); - }); - - // Test mockCharts[1] which has 'sales_data' (no schema prefix) - const chart1Row = within(table) - .getByText(mockCharts[1].slice_name) - .closest('[data-test="table-row"]') as HTMLElement; - const chart1DatasetLink = within(chart1Row).getByTestId('internal-link'); - - // Should display the full name when there's no schema prefix - expect(chart1DatasetLink).toHaveTextContent('sales_data'); - expect(chart1DatasetLink).toHaveAttribute( - 'href', - mockCharts[1].datasource_url, - ); -}); diff --git a/superset-frontend/src/pages/ChartList/ChartList.test.tsx b/superset-frontend/src/pages/ChartList/ChartList.test.tsx index a515fcc9059..6f9dedc5a09 100644 --- a/superset-frontend/src/pages/ChartList/ChartList.test.tsx +++ b/superset-frontend/src/pages/ChartList/ChartList.test.tsx @@ -87,7 +87,7 @@ describe('ChartList', () => { expect(screen.getByText('Charts')).toBeInTheDocument(); }); - test('verify New Chart button existence and functionality', async () => { + test('navigates to /chart/add on New Chart button click', async () => { renderChartList(mockUser); await screen.findByTestId('chart-list-view'); @@ -108,7 +108,7 @@ describe('ChartList', () => { ); }); - test('verify Import button existence and functionality', async () => { + test('opens import modal on Import button click', async () => { renderChartList(mockUser); await screen.findByTestId('chart-list-view'); @@ -168,72 +168,6 @@ describe('ChartList', () => { }); }); - test('shows loading state while API calls are in progress', async () => { - // Mock delayed API responses - // fetchMock.removeRoute(API_ENDPOINTS.CHARTS_INFO) - fetchMock.removeRoutes(); - fetchMock.get( - API_ENDPOINTS.CHARTS_INFO, - new Promise(resolve => - setTimeout( - () => resolve({ permissions: ['can_read', 'can_write'] }), - 100, - ), - ), - { name: API_ENDPOINTS.CHARTS_INFO }, - ); - - // fetchMock.removeRoute(API_ENDPOINTS.CHARTS) - fetchMock.get( - API_ENDPOINTS.CHARTS, - new Promise(resolve => - setTimeout(() => resolve({ result: mockCharts, chart_count: 3 }), 150), - ), - { name: API_ENDPOINTS.CHARTS }, - ); - - renderChartList(mockUser); - - // Main container should render immediately - expect(screen.getByTestId('chart-list-view')).toBeInTheDocument(); - - // Eventually data should load - await waitFor( - () => { - const infoCalls = fetchMock.callHistory.calls(/chart\/_info/); - const dataCalls = fetchMock.callHistory.calls(/chart\/\?q/); - - expect(infoCalls).toHaveLength(1); - expect(dataCalls).toHaveLength(1); - }, - { timeout: 1000 }, - ); - }); - - test('maintains component structure during loading', async () => { - renderChartList(mockUser); - - // Core structure should be available immediately - expect(screen.getByTestId('chart-list-view')).toBeInTheDocument(); - expect(screen.getByText('Charts')).toBeInTheDocument(); - - // View toggles should be available during loading - expect(screen.getByRole('img', { name: 'appstore' })).toBeInTheDocument(); - expect( - screen.getByRole('img', { name: 'unordered-list' }), - ).toBeInTheDocument(); - - // Wait for permissions to load, then action buttons should appear - expect( - await screen.findByRole('button', { name: 'Bulk select' }), - ).toBeInTheDocument(); - - // Wait for data to eventually load - expect( - await screen.findByText(mockCharts[0].slice_name), - ).toBeInTheDocument(); - }); - test('displays Matrixify tag for charts with matrixify enabled', async () => { renderChartList(mockUser); @@ -278,7 +212,7 @@ describe('ChartList', () => { expect(screen.getByTestId('chart-list-view')).toBeInTheDocument(); }); - test('handles empty results', async () => { + test('renders controls when chart list is empty', async () => { // Mock empty chart data (not permissions) fetchMock.removeRoute(API_ENDPOINTS.CHARTS); fetchMock.get( @@ -319,107 +253,32 @@ describe('ChartList - Global Filter Interactions', () => { ).mockReset(); }); - test('renders search filter correctly', async () => { + test('renders all standard filters', async () => { renderChartList(mockUser); await screen.findByTestId('chart-list-view'); - await waitFor(() => { expect(screen.getByTestId('listview-table')).toBeInTheDocument(); }); - // Verify search filter renders correctly + // Search filter expect(screen.getByTestId('filters-search')).toBeInTheDocument(); expect(screen.getByPlaceholderText(/type a value/i)).toBeInTheDocument(); - }); - test('renders Type filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); + // All standard select filters + const standardFilters = [ + 'Type', + 'Dataset', + 'Owner', + 'Certified', + 'Favorite', + 'Dashboard', + 'Modified by', + ]; + standardFilters.forEach(filterLabel => { + const filter = findFilterByLabel(filterLabel); + expect(filter).toBeVisible(); + expect(filter).toBeEnabled(); }); - - const typeFilter = findFilterByLabel('Type'); - expect(typeFilter).toBeVisible(); - expect(typeFilter).toBeEnabled(); - }); - - test('renders Dataset filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - const datasetFilter = findFilterByLabel('Dataset'); - expect(datasetFilter).toBeVisible(); - expect(datasetFilter).toBeEnabled(); - }); - - test('renders Owner filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - const ownerFilter = findFilterByLabel('Owner'); - expect(ownerFilter).toBeVisible(); - expect(ownerFilter).toBeEnabled(); - }); - - test('renders Certified filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - const certifiedFilter = findFilterByLabel('Certified'); - expect(certifiedFilter).toBeVisible(); - expect(certifiedFilter).toBeEnabled(); - }); - - test('renders Favorite filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - const favoriteFilter = findFilterByLabel('Favorite'); - expect(favoriteFilter).toBeVisible(); - expect(favoriteFilter).toBeEnabled(); - }); - - test('renders Dashboard filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - const dashboardFilter = findFilterByLabel('Dashboard'); - expect(dashboardFilter).toBeVisible(); - expect(dashboardFilter).toBeEnabled(); - }); - - test('renders Modified by filter correctly', async () => { - renderChartList(mockUser); - await screen.findByTestId('chart-list-view'); - - await waitFor(() => { - expect(screen.getByTestId('listview-table')).toBeInTheDocument(); - }); - - const modifiedByFilter = findFilterByLabel('Modified by'); - expect(modifiedByFilter).toBeVisible(); - expect(modifiedByFilter).toBeEnabled(); }); test('renders Tags filter when TAGGING_SYSTEM is enabled', async () => { @@ -476,7 +335,7 @@ describe('ChartList - Global Filter Interactions', () => { expect(filterLabels).not.toContain('Tag'); }); - test('allows filters to be reset correctly', async () => { + test('resets search filter value on clear', async () => { renderChartList(mockUser); await screen.findByTestId('chart-list-view');
