This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 4ddc3f14ed refactor(frontend): convert DatasourceEditor tests to
TypeScript (#35606)
4ddc3f14ed is described below
commit 4ddc3f14ed32c99805d124814f5d3e76f1a9922d
Author: Joe Li <[email protected]>
AuthorDate: Wed Oct 15 16:42:17 2025 -0700
refactor(frontend): convert DatasourceEditor tests to TypeScript (#35606)
Co-authored-by: Claude <[email protected]>
---
.../{mockDatasource.js => mockDatasource.ts} | 79 +++++++++++++--------
...ceEditor.test.jsx => DatasourceEditor.test.tsx} | 80 +++++++++++++++-------
....test.jsx => DatasourceEditorCurrency.test.tsx} | 34 +++++----
...orRTL.test.jsx => DatasourceEditorRTL.test.tsx} | 15 ++--
4 files changed, 133 insertions(+), 75 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 83%
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..4648293a18 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'] } },
});
@@ -66,13 +69,15 @@ describe('DatasourceEditor Currency Tests', () => {
const metricButton = screen.getByTestId('collection-tab-Metrics');
await userEvent.click(metricButton);
- // Find and expand the first metric row
+ // Find and expand the metric row with currency
+ // Metrics are sorted by ID descending, so metric with id=1 (which has
currency)
+ // is at position 6 (last). Expand that one.
const expandToggles = await screen.findAllByLabelText(
/expand row/i,
{},
{ timeout: 5000 },
);
- await userEvent.click(expandToggles[0]);
+ await userEvent.click(expandToggles[6]);
// Check for currency section header
const currencyHeader = await screen.findByText(
@@ -91,7 +96,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 +104,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 +117,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,11 +128,12 @@ 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 },
);
@@ -142,7 +148,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 +156,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 +169,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,11 +180,11 @@ 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 },
);
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 88%
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..9bfa6b7818 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
@@ -42,15 +42,18 @@ describe('DatasourceEditor RTL Metrics Tests', () => {
await userEvent.click(metricButton);
const expandToggle = await screen.findAllByLabelText(/expand row/i);
- await userEvent.click(expandToggle[0]);
+ // 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,
);
- expect(certificationDetails.value).toEqual('foo');
+ const certifiedBy = await screen.findByPlaceholderText(/certified by/i);
- const warningMarkdown = await screen.findByPlaceholderText(/certified
by/i);
- expect(warningMarkdown.value).toEqual('someone');
+ expect(certificationDetails).toHaveValue('foo');
+ expect(certifiedBy).toHaveValue('someone');
});
test('properly updates the metric information', async () => {
@@ -71,14 +74,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');
});
});
});
