Copilot commented on code in PR #35993:
URL: https://github.com/apache/superset/pull/35993#discussion_r2496852545
##########
superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx:
##########
@@ -28,22 +28,43 @@ import {
waitFor,
} from 'spec/helpers/testing-library';
import { fallbackExploreInitialData } from 'src/explore/fixtures';
+import type { DatasetObject } from 'src/features/datasets/types';
import DatasourceControl from '.';
const SupersetClientGet = jest.spyOn(SupersetClient, 'get');
+let originalLocation: Location;
+
+beforeEach(() => {
+ originalLocation = window.location;
+});
+
afterEach(() => {
+ window.location = originalLocation;
fetchMock.reset();
fetchMock.restore();
+ jest.clearAllMocks(); // Clears mock history but keeps spy in place
});
-const mockDatasource = {
+type TestDatasource = Omit<
+ Partial<DatasetObject>,
+ 'columns' | 'main_dttm_col'
+> & {
+ name: string;
+ database: { name: string };
+ columns?: any[];
Review Comment:
Using `any[]` for columns defeats the purpose of TypeScript's type safety.
Consider using `ColumnObject[]` from the imported `DatasetObject` type or
`Partial<ColumnObject>[]` to maintain type safety in test data.
##########
superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx:
##########
@@ -28,22 +28,43 @@ import {
waitFor,
} from 'spec/helpers/testing-library';
import { fallbackExploreInitialData } from 'src/explore/fixtures';
+import type { DatasetObject } from 'src/features/datasets/types';
import DatasourceControl from '.';
const SupersetClientGet = jest.spyOn(SupersetClient, 'get');
+let originalLocation: Location;
+
+beforeEach(() => {
+ originalLocation = window.location;
+});
+
afterEach(() => {
+ window.location = originalLocation;
fetchMock.reset();
fetchMock.restore();
+ jest.clearAllMocks(); // Clears mock history but keeps spy in place
});
-const mockDatasource = {
+type TestDatasource = Omit<
+ Partial<DatasetObject>,
+ 'columns' | 'main_dttm_col'
+> & {
+ name: string;
+ database: { name: string };
+ columns?: any[];
+ type?: DatasourceType;
+ main_dttm_col?: string | null;
Review Comment:
The `TestDatasource` type is overly complex. Since `DatasetObject` already
has `columns`, `type`, and `main_dttm_col` properties, omitting them from
`Partial<DatasetObject>` and redefining them creates unnecessary complexity.
Consider simplifying to use `Partial<DatasetObject>` directly or extend it with
only the fields that need different definitions (e.g., make `database`
required).
```suggestion
type TestDatasource = Partial<DatasetObject> & {
database: { name: string };
```
--
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]
