You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2025/10/10 21:55:57 UTC
(superset) 01/01: refactor(frontend): convert DatasourceEditor tests to TypeScript
This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch convert-DatasourceEditor-test-to-tsx
in repository https://gitbox.apache.org/repos/asf/superset.git
commit f30cc657eaffc72a518a6cf9568cee390b8c054a
Author: Joe Li <jo...@preset.io>
AuthorDate: Fri Oct 10 14:53:59 2025 -0700
refactor(frontend): convert DatasourceEditor tests to TypeScript
Convert DatasourceEditor test files from JSX to TSX, adding proper TypeScript
types and eliminating implicit `any` usage.
**Changes:**
- Convert 3 test files: DatasourceEditor.test, DatasourceEditorCurrency.test, DatasourceEditorRTL.test
- Convert mockDatasource fixture from JS to TS with DatasourceType enum
- Add proper TypeScript interfaces (DatasourceEditorProps, MetricType)
- Fix all TypeScript compilation errors by adding required properties to mock data
- Improve test reliability with consistent async/await usage for all userEvent.click() calls
- Add onChange mock clearing and callback verification in column modification test
**Test improvements:**
- All 9 DatasourceEditor.test.tsx tests passing (100%)
- 42/44 total tests passing (95.5%)
- Enhanced assertions verify both state changes and UI updates
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <no...@anthropic.com>
---
.../{mockDatasource.js => mockDatasource.ts} | 79 +++++++++++++--------
...ceEditor.test.jsx => DatasourceEditor.test.tsx} | 80 +++++++++++++++-------
....test.jsx => DatasourceEditorCurrency.test.tsx} | 44 ++++++++----
...orRTL.test.jsx => DatasourceEditorRTL.test.tsx} | 8 +--
4 files changed, 140 insertions(+), 71 deletions(-)
diff --git a/superset-frontend/spec/fixtures/mockDatasource.js b/superset-frontend/spec/fixtures/mockDatasource.ts
similarity index 79%
rename from superset-frontend/spec/fixtures/mockDatasource.js
rename to superset-frontend/spec/fixtures/mockDatasource.ts
index 29f525fa8c..941d95ff13 100644
--- a/superset-frontend/spec/fixtures/mockDatasource.js
+++ b/superset-frontend/spec/fixtures/mockDatasource.ts
@@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { DatasourceType } from '@superset-ui/core';
+
export const id = 7;
export const datasourceId = `${id}__table`;
@@ -40,125 +42,135 @@ export default {
},
metrics: [
{
+ id: 1,
+ uuid: 'metric-1-uuid',
expression: 'SUM(birth_names.num)',
- warning_text: null,
verbose_name: 'sum__num',
metric_name: 'sum__num',
- description: null,
+ metric_type: 'sum',
+ certified_by: 'someone',
+ certification_details: 'foo',
+ warning_markdown: 'bar',
extra:
'{"certification":{"details":"foo", "certified_by":"someone"},"warning_markdown":"bar"}',
},
{
+ id: 2,
+ uuid: 'metric-2-uuid',
expression: 'AVG(birth_names.num)',
- warning_text: null,
verbose_name: 'avg__num',
metric_name: 'avg__num',
- description: null,
+ metric_type: 'avg',
},
{
+ id: 3,
+ uuid: 'metric-3-uuid',
expression: 'SUM(birth_names.num_boys)',
- warning_text: null,
verbose_name: 'sum__num_boys',
metric_name: 'sum__num_boys',
- description: null,
+ metric_type: 'sum',
},
{
+ id: 4,
+ uuid: 'metric-4-uuid',
expression: 'AVG(birth_names.num_boys)',
- warning_text: null,
verbose_name: 'avg__num_boys',
metric_name: 'avg__num_boys',
- description: null,
+ metric_type: 'avg',
},
{
+ id: 5,
+ uuid: 'metric-5-uuid',
expression: 'SUM(birth_names.num_girls)',
- warning_text: null,
verbose_name: 'sum__num_girls',
metric_name: 'sum__num_girls',
- description: null,
+ metric_type: 'sum',
},
{
+ id: 6,
+ uuid: 'metric-6-uuid',
expression: 'AVG(birth_names.num_girls)',
- warning_text: null,
verbose_name: 'avg__num_girls',
metric_name: 'avg__num_girls',
- description: null,
+ metric_type: 'avg',
},
{
+ id: 7,
+ uuid: 'metric-7-uuid',
expression: 'COUNT(*)',
- warning_text: null,
verbose_name: 'COUNT(*)',
metric_name: 'count',
- description: null,
+ metric_type: 'count',
},
],
column_formats: {},
columns: [
{
+ id: 1,
type: 'DATETIME',
- description: null,
filterable: false,
- verbose_name: null,
is_dttm: true,
+ is_active: true,
expression: '',
groupby: false,
column_name: 'ds',
},
{
+ id: 2,
type: 'VARCHAR(16)',
- description: null,
filterable: true,
- verbose_name: null,
is_dttm: false,
+ is_active: true,
expression: '',
groupby: true,
column_name: 'gender',
},
{
+ id: 3,
type: 'VARCHAR(255)',
- description: null,
filterable: true,
- verbose_name: null,
is_dttm: false,
+ is_active: true,
expression: '',
groupby: true,
column_name: 'name',
},
{
+ id: 4,
type: 'BIGINT',
- description: null,
filterable: false,
- verbose_name: null,
is_dttm: false,
+ is_active: true,
expression: '',
groupby: false,
column_name: 'num',
},
{
+ id: 5,
type: 'VARCHAR(10)',
- description: null,
filterable: true,
- verbose_name: null,
is_dttm: false,
+ is_active: true,
expression: '',
groupby: true,
column_name: 'state',
},
{
+ id: 6,
type: 'BIGINT',
- description: null,
filterable: false,
- verbose_name: null,
is_dttm: false,
+ is_active: true,
expression: '',
groupby: false,
column_name: 'num_boys',
},
{
+ id: 7,
type: 'BIGINT',
- description: null,
filterable: false,
- verbose_name: null,
is_dttm: false,
+ is_active: true,
expression: '',
groupby: false,
column_name: 'num_girls',
@@ -169,7 +181,9 @@ export default {
granularity_sqla: [['ds', 'ds']],
main_dttm_col: 'ds',
name: 'birth_names',
- owners: [{ first_name: 'joe', last_name: 'man', id: 1 }],
+ owners: [
+ { first_name: 'joe', last_name: 'man', id: 1, username: 'joeman' },
+ ],
database: {
name: 'main',
backend: 'sqlite',
@@ -198,6 +212,11 @@ export default {
['["num_girls", true]', 'num_girls [asc]'],
['["num_girls", false]', 'num_girls [desc]'],
],
- type: 'table',
+ type: DatasourceType.Table,
+ description: null,
+ is_managed_externally: false,
+ normalize_columns: false,
+ always_filter_main_dttm: false,
+ datasource_name: null,
},
};
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.jsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
similarity index 78%
rename from superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.jsx
rename to superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
index be1497f43b..3853830ef2 100644
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.jsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
@@ -25,7 +25,8 @@ import {
cleanup,
} from 'spec/helpers/testing-library';
import mockDatasource from 'spec/fixtures/mockDatasource';
-import { isFeatureEnabled } from '@superset-ui/core';
+import { DatasourceType, isFeatureEnabled } from '@superset-ui/core';
+import type { DatasetObject } from 'src/features/datasets/types';
import DatasourceEditor from '..';
/* eslint-disable jest/no-export */
@@ -34,8 +35,17 @@ jest.mock('@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 = {
+export const props: DatasourceEditorProps = {
datasource: mockDatasource['7__table'],
addSuccessToast: () => {},
addDangerToast: () => {},
@@ -47,16 +57,19 @@ export const props = {
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 = props =>
+
+export const asyncRender = (renderProps: DatasourceEditorProps) =>
waitFor(() =>
- render(<DatasourceEditor {...props} {...routeProps} />, {
+ render(<DatasourceEditor {...renderProps} {...routeProps} />, {
useRedux: true,
initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
useRouter: true,
@@ -87,16 +100,16 @@ describe('DatasourceEditor', () => {
test('can sync columns from source', async () => {
const columnsTab = screen.getByTestId('collection-tab-Columns');
- userEvent.click(columnsTab);
+ 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(resolve => {
+ const fetchPromise = new Promise<string>(resolve => {
fetchMock.get(
DATASOURCE_ENDPOINT,
- url => {
+ (url: string) => {
resolve(url);
return [];
},
@@ -104,7 +117,7 @@ describe('DatasourceEditor', () => {
);
});
- userEvent.click(syncButton);
+ await userEvent.click(syncButton);
// Wait for the fetch to be called
const url = await fetchPromise;
@@ -114,12 +127,12 @@ describe('DatasourceEditor', () => {
// to add, remove and modify columns accordingly
test('can modify columns', async () => {
const columnsTab = screen.getByTestId('collection-tab-Columns');
- userEvent.click(columnsTab);
+ await userEvent.click(columnsTab);
const getToggles = screen.getAllByRole('button', {
name: /expand row/i,
});
- userEvent.click(getToggles[0]);
+ await userEvent.click(getToggles[0]);
const getTextboxes = await screen.findAllByRole('textbox');
expect(getTextboxes.length).toBeGreaterThanOrEqual(5);
@@ -132,22 +145,39 @@ describe('DatasourceEditor', () => {
'Certification details',
);
- userEvent.type(inputLabel, 'test_label');
- userEvent.type(inputDescription, 'test');
- userEvent.type(inputDtmFormat, 'test');
- userEvent.type(inputCertifiedBy, 'test');
- userEvent.type(inputCertDetails, 'test');
+ // 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');
- userEvent.click(columnsTab);
+ await userEvent.click(columnsTab);
const getToggles = screen.getAllByRole('button', {
name: /expand row/i,
});
- userEvent.click(getToggles[0]);
+ await userEvent.click(getToggles[0]);
const deleteButtons = await screen.findAllByRole('button', {
name: /delete item/i,
@@ -155,7 +185,7 @@ describe('DatasourceEditor', () => {
const initialCount = deleteButtons.length;
expect(initialCount).toBeGreaterThan(0);
- userEvent.click(deleteButtons[0]);
+ await userEvent.click(deleteButtons[0]);
await waitFor(() => {
const countRows = screen.getAllByRole('button', { name: /delete item/i });
@@ -165,14 +195,14 @@ describe('DatasourceEditor', () => {
test('can add new columns', async () => {
const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
- userEvent.click(calcColsTab);
+ await userEvent.click(calcColsTab);
const addBtn = screen.getByRole('button', {
name: /add item/i,
});
expect(addBtn).toBeInTheDocument();
- userEvent.click(addBtn);
+ await userEvent.click(addBtn);
// newColumn (Column name) is the first textbox in the tab
await waitFor(() => {
@@ -185,7 +215,7 @@ describe('DatasourceEditor', () => {
const columnsTab = screen.getByRole('tab', {
name: /settings/i,
});
- userEvent.click(columnsTab);
+ await userEvent.click(columnsTab);
const extraField = screen.getAllByText(/extra/i);
expect(extraField.length).toBeGreaterThan(0);
@@ -199,7 +229,7 @@ describe('DatasourceEditor', () => {
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('DatasourceEditor Source Tab', () => {
beforeAll(() => {
- isFeatureEnabled.mockImplementation(() => false);
+ (isFeatureEnabled as jest.Mock).mockImplementation(() => false);
});
beforeEach(async () => {
@@ -215,12 +245,12 @@ describe('DatasourceEditor Source Tab', () => {
});
afterAll(() => {
- isFeatureEnabled.mockRestore();
+ (isFeatureEnabled as jest.Mock).mockRestore();
});
test('Source Tab: edit mode', async () => {
const getLockBtn = screen.getByRole('img', { name: /lock/i });
- userEvent.click(getLockBtn);
+ await userEvent.click(getLockBtn);
const physicalRadioBtn = screen.getByRole('radio', {
name: /physical \(table or view\)/i,
@@ -259,7 +289,7 @@ describe('DatasourceEditor Source Tab', () => {
datasource: {
...props.datasource,
table_name: 'Vehicle Sales +',
- datasourceType: 'virtual',
+ type: DatasourceType.Query,
sql: 'SELECT * FROM users',
},
});
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.jsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx
similarity index 80%
rename from superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.jsx
rename to superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx
index 3b918ac3a6..2e6d4380cb 100644
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.jsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx
@@ -19,13 +19,16 @@
import fetchMock from 'fetch-mock';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
+import type { DatasetObject } from 'src/features/datasets/types';
import DatasourceEditor from '..';
import { props, DATASOURCE_ENDPOINT } from './DatasourceEditor.test';
+type MetricType = DatasetObject['metrics'][number];
+
// Optimized render function that doesn't use waitFor initially
// This helps prevent one source of the timeout
-const fastRender = props =>
- render(<DatasourceEditor {...props} />, {
+const fastRender = (renderProps: typeof props) =>
+ render(<DatasourceEditor {...renderProps} />, {
useRedux: true,
initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
});
@@ -91,7 +94,7 @@ describe('DatasourceEditor Currency Tests', () => {
expect(positionSelector).toBeInTheDocument();
// Open the dropdown
- userEvent.click(positionSelector);
+ await userEvent.click(positionSelector);
// Wait for dropdown to open and find the suffix option
const suffixOption = await waitFor(
@@ -99,7 +102,7 @@ describe('DatasourceEditor Currency Tests', () => {
// Look for 'suffix' option in the dropdown
const options = document.querySelectorAll('.ant-select-item-option');
const suffixOpt = Array.from(options).find(opt =>
- opt.textContent.toLowerCase().includes('suffix'),
+ opt.textContent?.toLowerCase().includes('suffix'),
);
if (!suffixOpt) throw new Error('Suffix option not found');
@@ -112,7 +115,7 @@ describe('DatasourceEditor Currency Tests', () => {
propsWithCurrency.onChange.mockClear();
// Click the suffix option
- userEvent.click(suffixOption);
+ await userEvent.click(suffixOption);
// Check if onChange was called with the expected parameters
await waitFor(
@@ -123,15 +126,24 @@ describe('DatasourceEditor Currency Tests', () => {
// More robust check for the metrics array
const metrics = callArg.metrics || [];
const updatedMetric = metrics.find(
- m => m.currency && m.currency.symbolPosition === 'suffix',
+ (m: MetricType) =>
+ m.currency && m.currency.symbolPosition === 'suffix',
);
expect(updatedMetric).toBeDefined();
- expect(updatedMetric.currency.symbol).toBe('USD');
+ expect(updatedMetric?.currency?.symbol).toBe('USD');
},
{ timeout: 5000 },
);
+ // Verify the UI reflects the selection
+ await waitFor(() => {
+ const selectedValue = positionSelector.querySelector(
+ '.ant-select-selection-item',
+ );
+ expect(selectedValue?.textContent?.toLowerCase()).toContain('suffix');
+ });
+
// Now test changing the currency symbol
const currencySymbol = await screen.findByRole(
'combobox',
@@ -142,7 +154,7 @@ describe('DatasourceEditor Currency Tests', () => {
);
// Open the currency dropdown
- userEvent.click(currencySymbol);
+ await userEvent.click(currencySymbol);
// Wait for dropdown to open and find the GBP option
const gbpOption = await waitFor(
@@ -150,7 +162,7 @@ describe('DatasourceEditor Currency Tests', () => {
// Look for 'GBP' option in the dropdown
const options = document.querySelectorAll('.ant-select-item-option');
const gbpOpt = Array.from(options).find(opt =>
- opt.textContent.includes('GBP'),
+ opt.textContent?.includes('GBP'),
);
if (!gbpOpt) throw new Error('GBP option not found');
@@ -163,7 +175,7 @@ describe('DatasourceEditor Currency Tests', () => {
propsWithCurrency.onChange.mockClear();
// Click the GBP option
- userEvent.click(gbpOption);
+ await userEvent.click(gbpOption);
// Verify the onChange with GBP was called
await waitFor(
@@ -174,13 +186,21 @@ describe('DatasourceEditor Currency Tests', () => {
// More robust check
const metrics = callArg.metrics || [];
const updatedMetric = metrics.find(
- m => m.currency && m.currency.symbol === 'GBP',
+ (m: MetricType) => m.currency && m.currency.symbol === 'GBP',
);
expect(updatedMetric).toBeDefined();
- expect(updatedMetric.currency.symbolPosition).toBe('suffix');
+ expect(updatedMetric?.currency?.symbolPosition).toBe('suffix');
},
{ timeout: 5000 },
);
+
+ // Verify the UI reflects the GBP selection
+ await waitFor(() => {
+ const selectedValue = currencySymbol.querySelector(
+ '.ant-select-selection-item',
+ );
+ expect(selectedValue?.textContent).toContain('GBP');
+ });
}, 60000);
});
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.jsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.tsx
similarity index 94%
rename from superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.jsx
rename to superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.tsx
index 80639fa9e5..d581314d6e 100644
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.jsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.tsx
@@ -47,10 +47,10 @@ describe('DatasourceEditor RTL Metrics Tests', () => {
const certificationDetails = await screen.findByPlaceholderText(
/certification details/i,
);
- expect(certificationDetails.value).toEqual('foo');
+ expect(certificationDetails).toHaveValue('foo');
const warningMarkdown = await screen.findByPlaceholderText(/certified by/i);
- expect(warningMarkdown.value).toEqual('someone');
+ expect(warningMarkdown).toHaveValue('someone');
});
test('properly updates the metric information', async () => {
@@ -71,14 +71,14 @@ describe('DatasourceEditor RTL Metrics Tests', () => {
/certification details/i,
);
await waitFor(() => {
- expect(certifiedBy.value).toEqual('I am typing a new name');
+ expect(certifiedBy).toHaveValue('I am typing a new name');
});
await userEvent.clear(certificationDetails);
await userEvent.type(certificationDetails, 'I am typing something new');
await waitFor(() => {
- expect(certificationDetails.value).toEqual('I am typing something new');
+ expect(certificationDetails).toHaveValue('I am typing something new');
});
});
});