Copilot commented on code in PR #35810:
URL: https://github.com/apache/superset/pull/35810#discussion_r2579158300
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ const columnsTab = screen.getByRole('tab', {
+ name: /settings/i,
});
+ await userEvent.click(columnsTab);
+
+ const extraField = screen.getAllByText(/extra/i);
+ expect(extraField.length).toBeGreaterThan(0);
+ expect(screen.getByText(/autocomplete query
predicate/i)).toBeInTheDocument();
+ expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+});
+
+test('Source Tab: edit mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- afterEach(() => {
- fetchMock.restore();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterAll(() => {
- (isFeatureEnabled as jest.Mock).mockRestore();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
});
- test('Source Tab: edit mode', async () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
+ expect(physicalRadioBtn).toBeEnabled();
+ expect(virtualRadioBtn).toBeEnabled();
+});
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+test('Source Tab: readOnly mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- expect(physicalRadioBtn).toBeEnabled();
- expect(virtualRadioBtn).toBeEnabled();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- test('Source Tab: readOnly mode', () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- expect(getLockBtn).toBeInTheDocument();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ expect(getLockBtn).toBeInTheDocument();
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
+ });
+
+ expect(physicalRadioBtn).toBeDisabled();
+ expect(virtualRadioBtn).toBeDisabled();
+});
+
+test('calls onChange with empty SQL when switching to physical dataset', async
() => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+ const testProps = createProps();
- expect(physicalRadioBtn).toBeDisabled();
- expect(virtualRadioBtn).toBeDisabled();
+ await asyncRender({
+ ...testProps,
+ datasource: {
+ ...testProps.datasource,
+ table_name: 'Vehicle Sales +',
+ type: DatasourceType.Query,
+ sql: 'SELECT * FROM users',
+ },
});
- test('calls onChange with empty SQL when switching to physical dataset',
async () => {
- // Clean previous render
- cleanup();
+ // Enable edit mode
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
- props.onChange.mockClear();
+ // Switch to physical dataset
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ await userEvent.click(physicalRadioBtn);
- await asyncRender({
- ...props,
- datasource: {
- ...props.datasource,
- table_name: 'Vehicle Sales +',
- type: DatasourceType.Query,
- sql: 'SELECT * FROM users',
- },
- });
-
- // Enable edit mode
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
-
- // Switch to physical dataset
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- await userEvent.click(physicalRadioBtn);
-
- // Assert that the latest onChange call has empty SQL
- expect(props.onChange).toHaveBeenCalled();
- const updatedDatasource = props.onChange.mock.calls[0];
- expect(updatedDatasource[0].sql).toBe('');
+ // Assert that the latest onChange call has empty SQL
+ expect(testProps.onChange).toHaveBeenCalled();
+ const updatedDatasource = testProps.onChange.mock.calls[0];
+ expect(updatedDatasource[0].sql).toBe('');
+});
+
+test('properly renders the metric information', async () => {
+ await asyncRender(props);
+
+ const metricButton = screen.getByTestId('collection-tab-Metrics');
+ await userEvent.click(metricButton);
+
+ const expandToggle = await screen.findAllByLabelText(/expand row/i);
+ // Metrics are sorted by ID descending, so metric with id=1 (which has
certification)
+ // is at position 6 (last). Expand that one.
+ await userEvent.click(expandToggle[6]);
+
+ // Wait for fields to appear
+ const certificationDetails = await screen.findByPlaceholderText(
+ /certification details/i,
+ );
+ const certifiedBy = await screen.findByPlaceholderText(/certified by/i);
+
+ expect(certificationDetails).toHaveValue('foo');
+ expect(certifiedBy).toHaveValue('someone');
+});
+
+test('properly updates the metric information', async () => {
+ await asyncRender(props);
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ const columnsTab = screen.getByRole('tab', {
+ name: /settings/i,
});
+ await userEvent.click(columnsTab);
+
+ const extraField = screen.getAllByText(/extra/i);
+ expect(extraField.length).toBeGreaterThan(0);
+ expect(screen.getByText(/autocomplete query
predicate/i)).toBeInTheDocument();
+ expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+});
+
+test('Source Tab: edit mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- afterEach(() => {
- fetchMock.restore();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ const columnsTab = screen.getByRole('tab', {
+ name: /settings/i,
});
+ await userEvent.click(columnsTab);
+
+ const extraField = screen.getAllByText(/extra/i);
+ expect(extraField.length).toBeGreaterThan(0);
+ expect(screen.getByText(/autocomplete query
predicate/i)).toBeInTheDocument();
+ expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+});
+
+test('Source Tab: edit mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- afterEach(() => {
- fetchMock.restore();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterAll(() => {
- (isFeatureEnabled as jest.Mock).mockRestore();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
});
- test('Source Tab: edit mode', async () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
+ expect(physicalRadioBtn).toBeEnabled();
+ expect(virtualRadioBtn).toBeEnabled();
+});
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+test('Source Tab: readOnly mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- expect(physicalRadioBtn).toBeEnabled();
- expect(virtualRadioBtn).toBeEnabled();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- test('Source Tab: readOnly mode', () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- expect(getLockBtn).toBeInTheDocument();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ expect(getLockBtn).toBeInTheDocument();
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
+ });
+
+ expect(physicalRadioBtn).toBeDisabled();
+ expect(virtualRadioBtn).toBeDisabled();
+});
+
+test('calls onChange with empty SQL when switching to physical dataset', async
() => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+ const testProps = createProps();
- expect(physicalRadioBtn).toBeDisabled();
- expect(virtualRadioBtn).toBeDisabled();
+ await asyncRender({
+ ...testProps,
+ datasource: {
+ ...testProps.datasource,
+ table_name: 'Vehicle Sales +',
+ type: DatasourceType.Query,
+ sql: 'SELECT * FROM users',
+ },
});
- test('calls onChange with empty SQL when switching to physical dataset',
async () => {
- // Clean previous render
- cleanup();
+ // Enable edit mode
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
- props.onChange.mockClear();
+ // Switch to physical dataset
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ await userEvent.click(physicalRadioBtn);
- await asyncRender({
- ...props,
- datasource: {
- ...props.datasource,
- table_name: 'Vehicle Sales +',
- type: DatasourceType.Query,
- sql: 'SELECT * FROM users',
- },
- });
-
- // Enable edit mode
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
-
- // Switch to physical dataset
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- await userEvent.click(physicalRadioBtn);
-
- // Assert that the latest onChange call has empty SQL
- expect(props.onChange).toHaveBeenCalled();
- const updatedDatasource = props.onChange.mock.calls[0];
- expect(updatedDatasource[0].sql).toBe('');
+ // Assert that the latest onChange call has empty SQL
+ expect(testProps.onChange).toHaveBeenCalled();
+ const updatedDatasource = testProps.onChange.mock.calls[0];
+ expect(updatedDatasource[0].sql).toBe('');
+});
+
+test('properly renders the metric information', async () => {
+ await asyncRender(props);
+
+ const metricButton = screen.getByTestId('collection-tab-Metrics');
+ await userEvent.click(metricButton);
+
+ const expandToggle = await screen.findAllByLabelText(/expand row/i);
+ // Metrics are sorted by ID descending, so metric with id=1 (which has
certification)
+ // is at position 6 (last). Expand that one.
+ await userEvent.click(expandToggle[6]);
+
+ // Wait for fields to appear
+ const certificationDetails = await screen.findByPlaceholderText(
+ /certification details/i,
+ );
+ const certifiedBy = await screen.findByPlaceholderText(/certified by/i);
+
+ expect(certificationDetails).toHaveValue('foo');
+ expect(certifiedBy).toHaveValue('someone');
+});
+
+test('properly updates the metric information', async () => {
+ await asyncRender(props);
+
+ const metricButton = screen.getByTestId('collection-tab-Metrics');
+ await userEvent.click(metricButton);
+
+ const expandToggle = await screen.findAllByLabelText(/expand row/i);
+ await userEvent.click(expandToggle[1]);
+
+ const certifiedBy = await screen.findByPlaceholderText(/certified by/i);
+ const certificationDetails = await screen.findByPlaceholderText(
+ /certification details/i,
+ );
+
+ // Use fireEvent.change for speed - we're testing wiring, not keystroke
behavior
+ fireEvent.change(certifiedBy, {
+ target: { value: 'I am typing a new name' },
});
+ fireEvent.change(certificationDetails, {
+ target: { value: 'I am typing something new' },
+ });
+
+ await waitFor(() => {
+ expect(certifiedBy).toHaveValue('I am typing a new name');
+ expect(certificationDetails).toHaveValue('I am typing something new');
+ });
+});
+
+test('shows the default datetime column', async () => {
+ await asyncRender(props);
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ const columnsTab = screen.getByRole('tab', {
+ name: /settings/i,
});
+ await userEvent.click(columnsTab);
+
+ const extraField = screen.getAllByText(/extra/i);
+ expect(extraField.length).toBeGreaterThan(0);
+ expect(screen.getByText(/autocomplete query
predicate/i)).toBeInTheDocument();
+ expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+});
+
+test('Source Tab: edit mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- afterEach(() => {
- fetchMock.restore();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterAll(() => {
- (isFeatureEnabled as jest.Mock).mockRestore();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
});
- test('Source Tab: edit mode', async () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
+ expect(physicalRadioBtn).toBeEnabled();
+ expect(virtualRadioBtn).toBeEnabled();
+});
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+test('Source Tab: readOnly mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- expect(physicalRadioBtn).toBeEnabled();
- expect(virtualRadioBtn).toBeEnabled();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- test('Source Tab: readOnly mode', () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- expect(getLockBtn).toBeInTheDocument();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ expect(getLockBtn).toBeInTheDocument();
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
+ });
+
+ expect(physicalRadioBtn).toBeDisabled();
+ expect(virtualRadioBtn).toBeDisabled();
+});
+
+test('calls onChange with empty SQL when switching to physical dataset', async
() => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+ const testProps = createProps();
- expect(physicalRadioBtn).toBeDisabled();
- expect(virtualRadioBtn).toBeDisabled();
+ await asyncRender({
+ ...testProps,
+ datasource: {
+ ...testProps.datasource,
+ table_name: 'Vehicle Sales +',
+ type: DatasourceType.Query,
+ sql: 'SELECT * FROM users',
+ },
});
- test('calls onChange with empty SQL when switching to physical dataset',
async () => {
- // Clean previous render
- cleanup();
+ // Enable edit mode
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
- props.onChange.mockClear();
+ // Switch to physical dataset
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ await userEvent.click(physicalRadioBtn);
- await asyncRender({
- ...props,
- datasource: {
- ...props.datasource,
- table_name: 'Vehicle Sales +',
- type: DatasourceType.Query,
- sql: 'SELECT * FROM users',
- },
- });
-
- // Enable edit mode
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
-
- // Switch to physical dataset
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- await userEvent.click(physicalRadioBtn);
-
- // Assert that the latest onChange call has empty SQL
- expect(props.onChange).toHaveBeenCalled();
- const updatedDatasource = props.onChange.mock.calls[0];
- expect(updatedDatasource[0].sql).toBe('');
+ // Assert that the latest onChange call has empty SQL
+ expect(testProps.onChange).toHaveBeenCalled();
+ const updatedDatasource = testProps.onChange.mock.calls[0];
+ expect(updatedDatasource[0].sql).toBe('');
+});
+
+test('properly renders the metric information', async () => {
+ await asyncRender(props);
Review Comment:
The shared `props` object from the test utils is used directly in multiple
tests without creating fresh copies. This can cause test pollution because the
underlying `mockDatasource['7__table']` object may be mutated by tests. While
spreading `props` creates a shallow copy, the nested `datasource` object is
still shared.
Consider using `createProps()` consistently in all tests instead of the
deprecated `props` export to prevent potential test pollution.
```suggestion
await asyncRender(createProps());
```
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ const columnsTab = screen.getByRole('tab', {
+ name: /settings/i,
});
+ await userEvent.click(columnsTab);
+
+ const extraField = screen.getAllByText(/extra/i);
+ expect(extraField.length).toBeGreaterThan(0);
+ expect(screen.getByText(/autocomplete query
predicate/i)).toBeInTheDocument();
+ expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+});
+
+test('Source Tab: edit mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- afterEach(() => {
- fetchMock.restore();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterAll(() => {
- (isFeatureEnabled as jest.Mock).mockRestore();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
});
- test('Source Tab: edit mode', async () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
+ expect(physicalRadioBtn).toBeEnabled();
+ expect(virtualRadioBtn).toBeEnabled();
+});
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+test('Source Tab: readOnly mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- expect(physicalRadioBtn).toBeEnabled();
- expect(virtualRadioBtn).toBeEnabled();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- test('Source Tab: readOnly mode', () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- expect(getLockBtn).toBeInTheDocument();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ expect(getLockBtn).toBeInTheDocument();
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
+ });
+
+ expect(physicalRadioBtn).toBeDisabled();
+ expect(virtualRadioBtn).toBeDisabled();
+});
+
+test('calls onChange with empty SQL when switching to physical dataset', async
() => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+ const testProps = createProps();
- expect(physicalRadioBtn).toBeDisabled();
- expect(virtualRadioBtn).toBeDisabled();
+ await asyncRender({
+ ...testProps,
+ datasource: {
+ ...testProps.datasource,
+ table_name: 'Vehicle Sales +',
+ type: DatasourceType.Query,
+ sql: 'SELECT * FROM users',
+ },
});
- test('calls onChange with empty SQL when switching to physical dataset',
async () => {
- // Clean previous render
- cleanup();
+ // Enable edit mode
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
- props.onChange.mockClear();
+ // Switch to physical dataset
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ await userEvent.click(physicalRadioBtn);
- await asyncRender({
- ...props,
- datasource: {
- ...props.datasource,
- table_name: 'Vehicle Sales +',
- type: DatasourceType.Query,
- sql: 'SELECT * FROM users',
- },
- });
-
- // Enable edit mode
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
-
- // Switch to physical dataset
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- await userEvent.click(physicalRadioBtn);
-
- // Assert that the latest onChange call has empty SQL
- expect(props.onChange).toHaveBeenCalled();
- const updatedDatasource = props.onChange.mock.calls[0];
- expect(updatedDatasource[0].sql).toBe('');
+ // Assert that the latest onChange call has empty SQL
+ expect(testProps.onChange).toHaveBeenCalled();
+ const updatedDatasource = testProps.onChange.mock.calls[0];
+ expect(updatedDatasource[0].sql).toBe('');
+});
+
+test('properly renders the metric information', async () => {
+ await asyncRender(props);
+
+ const metricButton = screen.getByTestId('collection-tab-Metrics');
+ await userEvent.click(metricButton);
+
+ const expandToggle = await screen.findAllByLabelText(/expand row/i);
+ // Metrics are sorted by ID descending, so metric with id=1 (which has
certification)
+ // is at position 6 (last). Expand that one.
+ await userEvent.click(expandToggle[6]);
+
+ // Wait for fields to appear
+ const certificationDetails = await screen.findByPlaceholderText(
+ /certification details/i,
+ );
+ const certifiedBy = await screen.findByPlaceholderText(/certified by/i);
+
+ expect(certificationDetails).toHaveValue('foo');
+ expect(certifiedBy).toHaveValue('someone');
+});
+
+test('properly updates the metric information', async () => {
+ await asyncRender(props);
+
+ const metricButton = screen.getByTestId('collection-tab-Metrics');
+ await userEvent.click(metricButton);
+
+ const expandToggle = await screen.findAllByLabelText(/expand row/i);
+ await userEvent.click(expandToggle[1]);
+
+ const certifiedBy = await screen.findByPlaceholderText(/certified by/i);
+ const certificationDetails = await screen.findByPlaceholderText(
+ /certification details/i,
+ );
+
+ // Use fireEvent.change for speed - we're testing wiring, not keystroke
behavior
+ fireEvent.change(certifiedBy, {
+ target: { value: 'I am typing a new name' },
});
+ fireEvent.change(certificationDetails, {
+ target: { value: 'I am typing something new' },
+ });
+
+ await waitFor(() => {
+ expect(certifiedBy).toHaveValue('I am typing a new name');
+ expect(certificationDetails).toHaveValue('I am typing something new');
+ });
+});
+
+test('shows the default datetime column', async () => {
+ await asyncRender(props);
+
+ const columnsButton = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsButton);
+
+ const dsDefaultDatetimeRadio = screen.getByTestId('radio-default-dttm-ds');
+ expect(dsDefaultDatetimeRadio).toBeChecked();
+
+ const genderDefaultDatetimeRadio = screen.getByTestId(
+ 'radio-default-dttm-gender',
+ );
+ expect(genderDefaultDatetimeRadio).not.toBeChecked();
+});
+
+test('allows choosing only temporal columns as the default datetime', async ()
=> {
+ await asyncRender(props);
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx:
##########
@@ -18,295 +18,367 @@
*/
import fetchMock from 'fetch-mock';
import {
- render,
+ fireEvent,
screen,
waitFor,
userEvent,
- cleanup,
+ within,
} from 'spec/helpers/testing-library';
-import mockDatasource from 'spec/fixtures/mockDatasource';
import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
-import type { DatasetObject } from 'src/features/datasets/types';
-import DatasourceEditor from '..';
+import {
+ props,
+ createProps,
+ DATASOURCE_ENDPOINT,
+ asyncRender,
+ fastRender,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
-/* eslint-disable jest/no-export */
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
isFeatureEnabled: jest.fn(),
}));
-interface DatasourceEditorProps {
- datasource: DatasetObject;
- addSuccessToast: () => void;
- addDangerToast: () => void;
- onChange: jest.Mock;
- columnLabels?: Record<string, string>;
- columnLabelTooltips?: Record<string, string>;
-}
-
-// Common setup for tests
-export const props: DatasourceEditorProps = {
- datasource: mockDatasource['7__table'],
- addSuccessToast: () => {},
- addDangerToast: () => {},
- onChange: jest.fn(),
- columnLabels: {
- state: 'State',
- },
- columnLabelTooltips: {
- state: 'This is a tooltip for state',
- },
-};
-
-export const DATASOURCE_ENDPOINT =
- 'glob:*/datasource/external_metadata_by_name/*';
-
-const routeProps = {
- history: {},
- location: {},
- match: {},
-};
-
-export const asyncRender = (renderProps: DatasourceEditorProps) =>
- waitFor(() =>
- render(<DatasourceEditor {...renderProps} {...routeProps} />, {
- useRedux: true,
- initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
- useRouter: true,
- }),
- );
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
+ setupDatasourceEditorMocks();
+ jest.clearAllMocks();
+});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor', () => {
- beforeAll(() => {
- jest.clearAllMocks();
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.restore();
+ // Reset module mock since jest.fn() doesn't support mockRestore()
+ jest.mocked(isFeatureEnabled).mockReset();
+});
+
+test('renders Tabs', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+});
+
+test('can sync columns from source', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterEach(() => {
- fetchMock.restore();
- // jest.clearAllMocks();
+ const columnsTab = screen.getByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const syncButton = screen.getByText(/sync columns from source/i);
+ expect(syncButton).toBeInTheDocument();
+
+ // Use a Promise to track when fetchMock is called
+ const fetchPromise = new Promise<string>(resolve => {
+ fetchMock.get(
+ DATASOURCE_ENDPOINT,
+ (url: string) => {
+ resolve(url);
+ return [];
+ },
+ { overwriteRoutes: true },
+ );
});
- test('renders Tabs', () => {
- expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
+ await userEvent.click(syncButton);
+
+ // Wait for the fetch to be called
+ const url = await fetchPromise;
+ expect(url).toContain('Vehicle+Sales%20%2B');
+});
+
+// to add, remove and modify columns accordingly
+test('can modify columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
+
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const getToggles = await screen.findAllByRole('button', {
+ name: /expand row/i,
});
+ await userEvent.click(getToggles[0]);
+
+ const getTextboxes = await screen.findAllByRole('textbox');
+ expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
+
+ const inputLabel = screen.getByPlaceholderText('Label');
+ const inputCertDetails = screen.getByPlaceholderText('Certification
details');
- test('can sync columns from source', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const syncButton = screen.getByText(/sync columns from source/i);
- expect(syncButton).toBeInTheDocument();
-
- // Use a Promise to track when fetchMock is called
- const fetchPromise = new Promise<string>(resolve => {
- fetchMock.get(
- DATASOURCE_ENDPOINT,
- (url: string) => {
- resolve(url);
- return [];
- },
- { overwriteRoutes: true },
- );
- });
-
- await userEvent.click(syncButton);
-
- // Wait for the fetch to be called
- const url = await fetchPromise;
- expect(url).toContain('Vehicle+Sales%20%2B');
+ // Clear onChange mock to track user action callbacks
+ limitedProps.onChange.mockClear();
+
+ // Use fireEvent.change for speed - testing wiring, not per-keystroke
behavior
+ fireEvent.change(inputLabel, { target: { value: 'test_label' } });
+ fireEvent.change(inputCertDetails, { target: { value: 'test_details' } });
+
+ // Verify the inputs were updated and onChange was triggered
+ await waitFor(() => {
+ expect(inputLabel).toHaveValue('test_label');
+ expect(inputCertDetails).toHaveValue('test_details');
+ expect(limitedProps.onChange).toHaveBeenCalled();
});
+});
- // to add, remove and modify columns accordingly
- test('can modify columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
- await userEvent.click(getToggles[0]);
-
- const getTextboxes = await screen.findAllByRole('textbox');
- expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
-
- const inputLabel = screen.getByPlaceholderText('Label');
- const inputDescription = screen.getByPlaceholderText('Description');
- const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
- const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
- const inputCertDetails = screen.getByPlaceholderText(
- 'Certification details',
- );
+test('can delete columns', async () => {
+ const limitedProps = {
+ ...props,
+ onChange: jest.fn(),
+ datasource: {
+ ...props.datasource,
+ table_name: 'Vehicle Sales +',
+ columns: props.datasource.columns
+ .slice(0, 1)
+ .map(column => ({ ...column })),
+ },
+ };
+
+ fastRender(limitedProps);
- // Clear onChange mock to track user action callbacks
- props.onChange.mockClear();
-
- await userEvent.type(inputLabel, 'test_label');
- await userEvent.type(inputDescription, 'test');
- await userEvent.type(inputDtmFormat, 'test');
- await userEvent.type(inputCertifiedBy, 'test');
- await userEvent.type(inputCertDetails, 'test');
-
- // Verify the inputs were updated with the typed values
- await waitFor(() => {
- expect(inputLabel).toHaveValue('test_label');
- expect(inputDescription).toHaveValue('test');
- expect(inputDtmFormat).toHaveValue('test');
- expect(inputCertifiedBy).toHaveValue('test');
- expect(inputCertDetails).toHaveValue('test');
- });
-
- // Verify that onChange was triggered by user actions
- await waitFor(() => {
- expect(props.onChange).toHaveBeenCalled();
- });
- }, 40000);
-
- test('can delete columns', async () => {
- const columnsTab = screen.getByTestId('collection-tab-Columns');
- await userEvent.click(columnsTab);
-
- const getToggles = screen.getAllByRole('button', {
- name: /expand row/i,
- });
-
- await userEvent.click(getToggles[0]);
-
- const deleteButtons = await screen.findAllByRole('button', {
- name: /delete item/i,
- });
- const initialCount = deleteButtons.length;
- expect(initialCount).toBeGreaterThan(0);
-
- await userEvent.click(deleteButtons[0]);
-
- await waitFor(() => {
- const countRows = screen.getAllByRole('button', { name: /delete item/i
});
- expect(countRows.length).toBe(initialCount - 1);
- });
- }, 60000); // 60 seconds timeout to avoid timeouts
-
- test('can add new columns', async () => {
- const calcColsTab = screen.getByTestId('collection-tab-Calculated
columns');
- await userEvent.click(calcColsTab);
-
- const addBtn = screen.getByRole('button', {
- name: /add item/i,
- });
- expect(addBtn).toBeInTheDocument();
-
- await userEvent.click(addBtn);
-
- // newColumn (Column name) is the first textbox in the tab
- await waitFor(() => {
- const newColumn = screen.getAllByRole('textbox')[0];
- expect(newColumn).toHaveValue('<new column>');
- });
- }, 60000);
-
- test('renders isSqla fields', async () => {
- const columnsTab = screen.getByRole('tab', {
- name: /settings/i,
- });
- await userEvent.click(columnsTab);
-
- const extraField = screen.getAllByText(/extra/i);
- expect(extraField.length).toBeGreaterThan(0);
+ await dismissDatasourceWarning();
+
+ const columnsTab = await screen.findByTestId('collection-tab-Columns');
+ await userEvent.click(columnsTab);
+
+ const columnsPanel = within(
+ await screen.findByRole('tabpanel', { name: /columns/i }),
+ );
+
+ const getToggles = await columnsPanel.findAllByRole('button', {
+ name: /expand row/i,
+ });
+
+ await userEvent.click(getToggles[0]);
+
+ const deleteButtons = await columnsPanel.findAllByRole('button', {
+ name: /delete item/i,
+ });
+ const initialCount = deleteButtons.length;
+ expect(initialCount).toBeGreaterThan(0);
+
+ // Clear onChange mock to track delete action
+ limitedProps.onChange.mockClear();
+
+ await userEvent.click(deleteButtons[0]);
+
+ await waitFor(() =>
expect(
- screen.getByText(/autocomplete query predicate/i),
- ).toBeInTheDocument();
- expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+ columnsPanel.queryAllByRole('button', { name: /delete item/i }),
+ ).toHaveLength(initialCount - 1),
+ );
+ await waitFor(() => expect(limitedProps.onChange).toHaveBeenCalled());
+});
+
+test('can add new columns', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
+ });
+
+ const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
+ await userEvent.click(calcColsTab);
+
+ const addBtn = screen.getByRole('button', {
+ name: /add item/i,
+ });
+ expect(addBtn).toBeInTheDocument();
+
+ await userEvent.click(addBtn);
+
+ // newColumn (Column name) is the first textbox in the tab
+ await waitFor(() => {
+ const newColumn = screen.getAllByRole('textbox')[0];
+ expect(newColumn).toHaveValue('<new column>');
});
});
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('DatasourceEditor Source Tab', () => {
- beforeAll(() => {
- (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
+test('renders isSqla fields', async () => {
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- beforeEach(async () => {
- fetchMock.get(DATASOURCE_ENDPOINT, [], { overwriteRoutes: true });
- await asyncRender({
- ...props,
- datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
- });
+ const columnsTab = screen.getByRole('tab', {
+ name: /settings/i,
});
+ await userEvent.click(columnsTab);
+
+ const extraField = screen.getAllByText(/extra/i);
+ expect(extraField.length).toBeGreaterThan(0);
+ expect(screen.getByText(/autocomplete query
predicate/i)).toBeInTheDocument();
+ expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
+});
+
+test('Source Tab: edit mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- afterEach(() => {
- fetchMock.restore();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
});
- afterAll(() => {
- (isFeatureEnabled as jest.Mock).mockRestore();
+ const getLockBtn = screen.getByRole('img', { name: /lock/i });
+ await userEvent.click(getLockBtn);
+
+ const physicalRadioBtn = screen.getByRole('radio', {
+ name: /physical \(table or view\)/i,
+ });
+ const virtualRadioBtn = screen.getByRole('radio', {
+ name: /virtual \(sql\)/i,
});
- test('Source Tab: edit mode', async () => {
- const getLockBtn = screen.getByRole('img', { name: /lock/i });
- await userEvent.click(getLockBtn);
+ expect(physicalRadioBtn).toBeEnabled();
+ expect(virtualRadioBtn).toBeEnabled();
+});
- const physicalRadioBtn = screen.getByRole('radio', {
- name: /physical \(table or view\)/i,
- });
- const virtualRadioBtn = screen.getByRole('radio', {
- name: /virtual \(sql\)/i,
- });
+test('Source Tab: readOnly mode', async () => {
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
- expect(physicalRadioBtn).toBeEnabled();
- expect(virtualRadioBtn).toBeEnabled();
+ await asyncRender({
+ ...props,
+ datasource: { ...props.datasource, table_name: 'Vehicle Sales +' },
Review Comment:
The shared `props` object is used directly without creating a fresh copy.
This can cause test pollution because the underlying
`mockDatasource['7__table']` object may be mutated by tests. While spreading
`props` creates a shallow copy, the nested `datasource` object is still shared.
Consider using `createProps()` consistently to prevent potential test
pollution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
