You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/16 16:53:43 UTC
[GitHub] [incubator-superset] lilykuang opened a new pull request #10914: lily/refactor table selector
lilykuang opened a new pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#discussion_r494025642
##########
File path: superset-frontend/spec/javascripts/components/TableSelector_spec.jsx
##########
@@ -18,267 +18,275 @@
*/
import React from 'react';
import configureStore from 'redux-mock-store';
-import { shallow } from 'enzyme';
+import { mount } from 'enzyme';
+import { act } from 'react-dom/test-utils';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk';
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
+
+import DatabaseSelector from 'src/components/DatabaseSelector';
import TableSelector from 'src/components/TableSelector';
import { initialState, tables } from '../sqllab/fixtures';
-describe('TableSelector', () => {
- let mockedProps;
- const middlewares = [thunk];
- const mockStore = configureStore(middlewares);
- const store = mockStore(initialState);
- let wrapper;
- let inst;
+const mockStore = configureStore([thunk]);
+const store = mockStore(initialState);
- beforeEach(() => {
- mockedProps = {
- dbId: 1,
- schema: 'main',
- onSchemaChange: sinon.stub(),
- onDbChange: sinon.stub(),
- getDbList: sinon.stub(),
- onTableChange: sinon.stub(),
- onChange: sinon.stub(),
- tableNameSticky: true,
- tableName: '',
- database: { id: 1, database_name: 'main' },
- horizontal: false,
- sqlLabMode: true,
- clearable: false,
- handleError: sinon.stub(),
- };
- wrapper = shallow(<TableSelector {...mockedProps} />, {
- context: { store },
- });
- inst = wrapper.instance();
- });
-
- it('is valid', () => {
- expect(React.isValidElement(<TableSelector {...mockedProps} />)).toBe(true);
- });
-
- describe('onDatabaseChange', () => {
- it('should fetch schemas', () => {
- sinon.stub(inst, 'fetchSchemas');
- inst.onDatabaseChange({ id: 1 });
- expect(inst.fetchSchemas.getCall(0).args[0]).toBe(1);
- inst.fetchSchemas.restore();
- });
- it('should clear tableOptions', () => {
- inst.onDatabaseChange();
- expect(wrapper.state().tableOptions).toEqual([]);
- });
- });
+const FETCH_SCHEMAS_ENDPOINT = 'glob:*/api/v1/database/*/schemas/*';
+const GET_TABLE_ENDPOINT = 'glob:*/superset/tables/1/*/*';
+const GET_TABLE_NAMES_ENDPOINT = 'glob:*/superset/tables/1/main/*';
- describe('getTableNamesBySubStr', () => {
- const GET_TABLE_NAMES_GLOB = 'glob:*/superset/tables/1/main/*';
+const mockedProps = {
+ clearable: false,
+ database: { id: 1, database_name: 'main' },
+ dbId: 1,
+ formMode: false,
+ getDbList: sinon.stub(),
+ handleError: sinon.stub(),
+ horizontal: false,
+ onChange: sinon.stub(),
+ onDbChange: sinon.stub(),
+ onSchemaChange: sinon.stub(),
+ onTableChange: sinon.stub(),
+ sqlLabMode: true,
+ tableName: '',
+ tableNameSticky: true,
+};
- afterEach(fetchMock.resetHistory);
- afterAll(fetchMock.reset);
+const schemaOptions = {
+ result: ['main', 'erf', 'superset'],
+};
+const selectedSchema = { label: 'main', title: 'main', value: 'main' };
+const selectedTable = {
+ label: 'birth_names',
+ schema: 'main',
+ title: 'birth_names',
+ value: 'birth_names',
+ type: undefined,
+};
- it('should handle empty', () =>
- inst.getTableNamesBySubStr('').then(data => {
- expect(data).toEqual({ options: [] });
- return Promise.resolve();
- }));
+async function mountAndWait(props = mockedProps) {
+ const mounted = mount(<TableSelector {...props} />, {
+ context: { store },
+ wrappingComponent: ThemeProvider,
+ wrappingComponentProps: { theme: supersetTheme },
+ });
+ await waitForComponentToPaint(mounted);
- it('should handle table name', () => {
- fetchMock.get(GET_TABLE_NAMES_GLOB, tables, { overwriteRoutes: true });
+ return mounted;
+}
- return wrapper
- .instance()
- .getTableNamesBySubStr('my table')
- .then(data => {
- expect(fetchMock.calls(GET_TABLE_NAMES_GLOB)).toHaveLength(1);
- expect(data).toEqual({
- options: [
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
- ],
- });
- return Promise.resolve();
- });
- });
+describe('TableSelector', () => {
+ let wrapper;
- it('should escape schema and table names', () => {
- const GET_TABLE_GLOB = 'glob:*/superset/tables/1/*/*';
- wrapper.setProps({ schema: 'slashed/schema' });
- fetchMock.get(GET_TABLE_GLOB, tables, { overwriteRoutes: true });
+ beforeEach(async () => {
+ fetchMock.reset();
+ wrapper = await mountAndWait();
+ });
- return wrapper
- .instance()
- .getTableNamesBySubStr('slashed/table')
- .then(() => {
- expect(fetchMock.lastUrl(GET_TABLE_GLOB)).toContain(
- '/slashed%252Fschema/slashed%252Ftable',
- );
- return Promise.resolve();
- });
- });
+ it('renders', () => {
+ expect(wrapper.find(TableSelector)).toExist();
+ expect(wrapper.find(DatabaseSelector)).toExist();
});
- describe('fetchTables', () => {
- const FETCH_TABLES_GLOB = 'glob:*/superset/tables/1/main/*/*/';
+ describe('change database', () => {
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should clear table options', () => {
- inst.fetchTables(true);
- expect(wrapper.state().tableOptions).toEqual([]);
+ it('should fetch schemas', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
});
- it('should fetch table options', () => {
- fetchMock.get(FETCH_TABLES_GLOB, tables, { overwriteRoutes: true });
- return inst.fetchTables(true, 'birth_names').then(() => {
- expect(wrapper.state().tableOptions).toHaveLength(3);
- expect(wrapper.state().tableOptions).toEqual([
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
+ it('should fetch schema options', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
+
+ act(() => {
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().options,
+ ).toEqual([
+ { value: 'main', label: 'main', title: 'main' },
+ { value: 'erf', label: 'erf', title: 'erf' },
+ { value: 'superset', label: 'superset', title: 'superset' },
]);
- return Promise.resolve();
});
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- fetchMock.get(
- FETCH_TABLES_GLOB,
- { throws: 'error' },
- { overwriteRoutes: true },
- );
+ it('should clear table options', () => {
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
+ });
+ });
+ return Promise.resolve();
+ });
+ });
- wrapper
- .instance()
- .fetchTables(true, 'birth_names')
- .then(() => {
- expect(wrapper.state().tableOptions).toEqual([]);
- expect(wrapper.state().tableOptions).toHaveLength(0);
- expect(mockedProps.handleError.callCount).toBe(1);
- return Promise.resolve();
+ describe('change schema', () => {
+ beforeEach(async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
});
+ });
});
- });
- describe('fetchSchemas', () => {
- const FETCH_SCHEMAS_GLOB = 'glob:*/api/v1/database/*/schemas/?q=(force:!*)';
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should fetch schema options', () => {
- const schemaOptions = {
- result: ['main', 'erf', 'superset'],
- };
- fetchMock.get(FETCH_SCHEMAS_GLOB, schemaOptions, {
- overwriteRoutes: true,
+ it('should fetch table', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
});
-
- return wrapper
- .instance()
- .fetchSchemas(1)
- .then(() => {
- expect(fetchMock.calls(FETCH_SCHEMAS_GLOB)).toHaveLength(1);
- expect(wrapper.state().schemaOptions).toHaveLength(3);
- });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- const handleErrors = sinon.stub();
- expect(handleErrors.callCount).toBe(0);
- wrapper.setProps({ handleErrors });
- fetchMock.get(
- FETCH_SCHEMAS_GLOB,
- { throws: new Error('Bad kitty') },
- { overwriteRoutes: true },
- );
- wrapper
- .instance()
- .fetchSchemas(123)
- .then(() => {
- expect(wrapper.state().schemaOptions).toEqual([]);
- expect(handleErrors.callCount).toBe(1);
- });
- });
- });
+ it('should fetch table options', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().value[0],
+ ).toEqual(selectedSchema);
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
- describe('changeTable', () => {
- beforeEach(() => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ act(() => {
+ const { options } = wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props();
+ expect({ options }).toEqual(tables);
+ });
});
+ });
- afterEach(() => {
- wrapper.instance().fetchTables.restore();
+ describe('change table', () => {
+ beforeEach(async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
});
- it('test 1', () => {
- wrapper.instance().changeTable({
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
+ it('should change table value', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(wrapper.state().tableName).toBe('birth_names');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-table"]').first().props().value[0],
+ ).toEqual(selectedTable);
});
- it('should call onTableChange with schema from table object', () => {
- wrapper.setProps({ schema: null });
- wrapper.instance().changeTable({
- value: 'my_table',
- schema: 'other_schema',
- label: 'other_schema.my_table',
- title: 'other_schema.my_table',
+ it('should call onTableChange with schema from table object', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('my_table');
- expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('other_schema');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('birth_names');
+ expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('main');
});
});
- it('changeSchema', () => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ describe('getTableNamesBySubStr', () => {
+ beforeEach(() => {
+ wrapper.setProps({ schema: 'main' });
+ });
+ afterEach(fetchMock.resetHistory);
+ afterAll(fetchMock.reset);
- wrapper.instance().changeSchema({ label: 'main', value: 'main' });
- expect(wrapper.instance().fetchTables.callCount).toBe(1);
- expect(mockedProps.onChange.callCount).toBe(1);
- wrapper.instance().changeSchema();
- expect(wrapper.instance().fetchTables.callCount).toBe(2);
- expect(mockedProps.onChange.callCount).toBe(2);
+ it('should handle empty', () =>
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
Review comment:
since this is part of a promise, I'm not sure if the test is exiting before it gets to this line. Looking at [the docs](https://jestjs.io/docs/en/asynchronous) it appears we can return a promise and jest will wait for it to finish before finishing the test. Since this is wrapped in an `act`, I'm not sure how to do that. We may need to use `done`, or make this an async function and use a few awaits, or setup the test with `expect.assertions(1)`.
##########
File path: superset-frontend/spec/javascripts/components/TableSelector_spec.jsx
##########
@@ -18,267 +18,275 @@
*/
import React from 'react';
import configureStore from 'redux-mock-store';
-import { shallow } from 'enzyme';
+import { mount } from 'enzyme';
+import { act } from 'react-dom/test-utils';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk';
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
+
+import DatabaseSelector from 'src/components/DatabaseSelector';
import TableSelector from 'src/components/TableSelector';
import { initialState, tables } from '../sqllab/fixtures';
-describe('TableSelector', () => {
- let mockedProps;
- const middlewares = [thunk];
- const mockStore = configureStore(middlewares);
- const store = mockStore(initialState);
- let wrapper;
- let inst;
+const mockStore = configureStore([thunk]);
+const store = mockStore(initialState);
- beforeEach(() => {
- mockedProps = {
- dbId: 1,
- schema: 'main',
- onSchemaChange: sinon.stub(),
- onDbChange: sinon.stub(),
- getDbList: sinon.stub(),
- onTableChange: sinon.stub(),
- onChange: sinon.stub(),
- tableNameSticky: true,
- tableName: '',
- database: { id: 1, database_name: 'main' },
- horizontal: false,
- sqlLabMode: true,
- clearable: false,
- handleError: sinon.stub(),
- };
- wrapper = shallow(<TableSelector {...mockedProps} />, {
- context: { store },
- });
- inst = wrapper.instance();
- });
-
- it('is valid', () => {
- expect(React.isValidElement(<TableSelector {...mockedProps} />)).toBe(true);
- });
-
- describe('onDatabaseChange', () => {
- it('should fetch schemas', () => {
- sinon.stub(inst, 'fetchSchemas');
- inst.onDatabaseChange({ id: 1 });
- expect(inst.fetchSchemas.getCall(0).args[0]).toBe(1);
- inst.fetchSchemas.restore();
- });
- it('should clear tableOptions', () => {
- inst.onDatabaseChange();
- expect(wrapper.state().tableOptions).toEqual([]);
- });
- });
+const FETCH_SCHEMAS_ENDPOINT = 'glob:*/api/v1/database/*/schemas/*';
+const GET_TABLE_ENDPOINT = 'glob:*/superset/tables/1/*/*';
+const GET_TABLE_NAMES_ENDPOINT = 'glob:*/superset/tables/1/main/*';
- describe('getTableNamesBySubStr', () => {
- const GET_TABLE_NAMES_GLOB = 'glob:*/superset/tables/1/main/*';
+const mockedProps = {
+ clearable: false,
+ database: { id: 1, database_name: 'main' },
+ dbId: 1,
+ formMode: false,
+ getDbList: sinon.stub(),
+ handleError: sinon.stub(),
+ horizontal: false,
+ onChange: sinon.stub(),
+ onDbChange: sinon.stub(),
+ onSchemaChange: sinon.stub(),
+ onTableChange: sinon.stub(),
+ sqlLabMode: true,
+ tableName: '',
+ tableNameSticky: true,
+};
- afterEach(fetchMock.resetHistory);
- afterAll(fetchMock.reset);
+const schemaOptions = {
+ result: ['main', 'erf', 'superset'],
+};
+const selectedSchema = { label: 'main', title: 'main', value: 'main' };
+const selectedTable = {
+ label: 'birth_names',
+ schema: 'main',
+ title: 'birth_names',
+ value: 'birth_names',
+ type: undefined,
+};
- it('should handle empty', () =>
- inst.getTableNamesBySubStr('').then(data => {
- expect(data).toEqual({ options: [] });
- return Promise.resolve();
- }));
+async function mountAndWait(props = mockedProps) {
+ const mounted = mount(<TableSelector {...props} />, {
+ context: { store },
+ wrappingComponent: ThemeProvider,
+ wrappingComponentProps: { theme: supersetTheme },
+ });
+ await waitForComponentToPaint(mounted);
- it('should handle table name', () => {
- fetchMock.get(GET_TABLE_NAMES_GLOB, tables, { overwriteRoutes: true });
+ return mounted;
+}
- return wrapper
- .instance()
- .getTableNamesBySubStr('my table')
- .then(data => {
- expect(fetchMock.calls(GET_TABLE_NAMES_GLOB)).toHaveLength(1);
- expect(data).toEqual({
- options: [
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
- ],
- });
- return Promise.resolve();
- });
- });
+describe('TableSelector', () => {
+ let wrapper;
- it('should escape schema and table names', () => {
- const GET_TABLE_GLOB = 'glob:*/superset/tables/1/*/*';
- wrapper.setProps({ schema: 'slashed/schema' });
- fetchMock.get(GET_TABLE_GLOB, tables, { overwriteRoutes: true });
+ beforeEach(async () => {
+ fetchMock.reset();
+ wrapper = await mountAndWait();
+ });
- return wrapper
- .instance()
- .getTableNamesBySubStr('slashed/table')
- .then(() => {
- expect(fetchMock.lastUrl(GET_TABLE_GLOB)).toContain(
- '/slashed%252Fschema/slashed%252Ftable',
- );
- return Promise.resolve();
- });
- });
+ it('renders', () => {
+ expect(wrapper.find(TableSelector)).toExist();
+ expect(wrapper.find(DatabaseSelector)).toExist();
});
- describe('fetchTables', () => {
- const FETCH_TABLES_GLOB = 'glob:*/superset/tables/1/main/*/*/';
+ describe('change database', () => {
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should clear table options', () => {
- inst.fetchTables(true);
- expect(wrapper.state().tableOptions).toEqual([]);
+ it('should fetch schemas', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
});
- it('should fetch table options', () => {
- fetchMock.get(FETCH_TABLES_GLOB, tables, { overwriteRoutes: true });
- return inst.fetchTables(true, 'birth_names').then(() => {
- expect(wrapper.state().tableOptions).toHaveLength(3);
- expect(wrapper.state().tableOptions).toEqual([
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
+ it('should fetch schema options', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
+
+ act(() => {
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().options,
+ ).toEqual([
+ { value: 'main', label: 'main', title: 'main' },
+ { value: 'erf', label: 'erf', title: 'erf' },
+ { value: 'superset', label: 'superset', title: 'superset' },
]);
- return Promise.resolve();
});
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- fetchMock.get(
- FETCH_TABLES_GLOB,
- { throws: 'error' },
- { overwriteRoutes: true },
- );
+ it('should clear table options', () => {
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
+ });
+ });
+ return Promise.resolve();
+ });
+ });
- wrapper
- .instance()
- .fetchTables(true, 'birth_names')
- .then(() => {
- expect(wrapper.state().tableOptions).toEqual([]);
- expect(wrapper.state().tableOptions).toHaveLength(0);
- expect(mockedProps.handleError.callCount).toBe(1);
- return Promise.resolve();
+ describe('change schema', () => {
+ beforeEach(async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
});
+ });
});
- });
- describe('fetchSchemas', () => {
- const FETCH_SCHEMAS_GLOB = 'glob:*/api/v1/database/*/schemas/?q=(force:!*)';
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should fetch schema options', () => {
- const schemaOptions = {
- result: ['main', 'erf', 'superset'],
- };
- fetchMock.get(FETCH_SCHEMAS_GLOB, schemaOptions, {
- overwriteRoutes: true,
+ it('should fetch table', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
});
-
- return wrapper
- .instance()
- .fetchSchemas(1)
- .then(() => {
- expect(fetchMock.calls(FETCH_SCHEMAS_GLOB)).toHaveLength(1);
- expect(wrapper.state().schemaOptions).toHaveLength(3);
- });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- const handleErrors = sinon.stub();
- expect(handleErrors.callCount).toBe(0);
- wrapper.setProps({ handleErrors });
- fetchMock.get(
- FETCH_SCHEMAS_GLOB,
- { throws: new Error('Bad kitty') },
- { overwriteRoutes: true },
- );
- wrapper
- .instance()
- .fetchSchemas(123)
- .then(() => {
- expect(wrapper.state().schemaOptions).toEqual([]);
- expect(handleErrors.callCount).toBe(1);
- });
- });
- });
+ it('should fetch table options', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().value[0],
+ ).toEqual(selectedSchema);
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
- describe('changeTable', () => {
- beforeEach(() => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ act(() => {
+ const { options } = wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props();
+ expect({ options }).toEqual(tables);
+ });
});
+ });
- afterEach(() => {
- wrapper.instance().fetchTables.restore();
+ describe('change table', () => {
+ beforeEach(async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
});
- it('test 1', () => {
- wrapper.instance().changeTable({
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
+ it('should change table value', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(wrapper.state().tableName).toBe('birth_names');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-table"]').first().props().value[0],
+ ).toEqual(selectedTable);
});
- it('should call onTableChange with schema from table object', () => {
- wrapper.setProps({ schema: null });
- wrapper.instance().changeTable({
- value: 'my_table',
- schema: 'other_schema',
- label: 'other_schema.my_table',
- title: 'other_schema.my_table',
+ it('should call onTableChange with schema from table object', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('my_table');
- expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('other_schema');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('birth_names');
+ expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('main');
});
});
- it('changeSchema', () => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ describe('getTableNamesBySubStr', () => {
+ beforeEach(() => {
+ wrapper.setProps({ schema: 'main' });
+ });
+ afterEach(fetchMock.resetHistory);
+ afterAll(fetchMock.reset);
- wrapper.instance().changeSchema({ label: 'main', value: 'main' });
- expect(wrapper.instance().fetchTables.callCount).toBe(1);
- expect(mockedProps.onChange.callCount).toBe(1);
- wrapper.instance().changeSchema();
- expect(wrapper.instance().fetchTables.callCount).toBe(2);
- expect(mockedProps.onChange.callCount).toBe(2);
+ it('should handle empty', () =>
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
+ });
+ }));
- wrapper.instance().fetchTables.restore();
+ it('should handle table name', () => {
+ fetchMock.get(GET_TABLE_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(fetchMock.calls(GET_TABLE_ENDPOINT)).toHaveLength(1);
+ expect(data).toEqual(tables);
+ });
+ });
+ return Promise.resolve();
Review comment:
Same here, I think this test will always succeed since we're returning a resolved promise here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] lilykuang commented on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
lilykuang commented on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-700317631
@ktmud Nice catch. I will fix this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-696351607
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=h1) Report
> Merging [#10914](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.34%`.
> The diff coverage is `80.76%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10914 +/- ##
==========================================
- Coverage 65.88% 61.53% -4.35%
==========================================
Files 815 816 +1
Lines 38337 38406 +69
Branches 3601 3627 +26
==========================================
- Hits 25257 23633 -1624
- Misses 12978 14587 +1609
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.98% <80.76%> (+0.22%)` | :arrow_up: |
| #python | `61.27% <ø> (-0.21%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `64.22% <56.25%> (-8.84%)` | :arrow_down: |
| [superset-frontend/src/components/TableSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci50c3g=) | `83.18% <83.18%> (ø)` | |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `86.36% <86.36%> (ø)` | |
| [...nd/src/views/CRUD/data/dataset/AddDatasetModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXRNb2RhbC50c3g=) | `57.57% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [186 more](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=footer). Last update [141ef4a...8c668ee](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-696351607
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=h1) Report
> Merging [#10914](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6181994084067a32883bb01db55356cc7aa4712b?el=desc) will **increase** coverage by `5.91%`.
> The diff coverage is `82.42%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10914 +/- ##
==========================================
+ Coverage 59.95% 65.86% +5.91%
==========================================
Files 782 817 +35
Lines 37252 38478 +1226
Branches 3350 3638 +288
==========================================
+ Hits 22335 25345 +3010
+ Misses 14738 13027 -1711
+ Partials 179 106 -73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `57.25% <56.88%> (-0.05%)` | :arrow_down: |
| #javascript | `61.92% <79.91%> (?)` | |
| #python | `61.39% <ø> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `56.00% <ø> (+2.00%)` | :arrow_up: |
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `71.55% <55.88%> (+29.80%)` | :arrow_up: |
| [superset-frontend/src/components/TableSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci50c3g=) | `82.30% <82.30%> (ø)` | |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `92.30% <92.30%> (ø)` | |
| [...nd/src/views/CRUD/data/dataset/AddDatasetModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXRNb2RhbC50c3g=) | `57.57% <100.00%> (+45.57%)` | :arrow_up: |
| [superset-frontend/src/setup/setupPlugins.ts](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2lucy50cw==) | `22.22% <0.00%> (-77.78%)` | :arrow_down: |
| [superset-frontend/src/views/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2luZGV4LnRzeA==) | `25.00% <0.00%> (-75.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC50c3g=) | `25.00% <0.00%> (-75.00%)` | :arrow_down: |
| [superset-frontend/src/views/App.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0FwcC50c3g=) | `31.42% <0.00%> (-68.58%)` | :arrow_down: |
| [...rset-frontend/src/setup/setupErrorMessagesExtra.ts](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlc0V4dHJhLnRz) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| ... and [275 more](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=footer). Last update [6181994...2dfb192](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-696351607
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=h1) Report
> Merging [#10914](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6181994084067a32883bb01db55356cc7aa4712b?el=desc) will **increase** coverage by `5.91%`.
> The diff coverage is `82.42%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10914 +/- ##
==========================================
+ Coverage 59.95% 65.86% +5.91%
==========================================
Files 782 817 +35
Lines 37252 38478 +1226
Branches 3350 3638 +288
==========================================
+ Hits 22335 25345 +3010
+ Misses 14738 13027 -1711
+ Partials 179 106 -73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `57.25% <56.88%> (-0.05%)` | :arrow_down: |
| #javascript | `61.92% <79.91%> (?)` | |
| #python | `61.39% <ø> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIuanN4) | `56.00% <ø> (+2.00%)` | :arrow_up: |
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `71.55% <55.88%> (+29.80%)` | :arrow_up: |
| [superset-frontend/src/components/TableSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci50c3g=) | `82.30% <82.30%> (ø)` | |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `92.30% <92.30%> (ø)` | |
| [...nd/src/views/CRUD/data/dataset/AddDatasetModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXRNb2RhbC50c3g=) | `57.57% <100.00%> (+45.57%)` | :arrow_up: |
| [superset-frontend/src/setup/setupPlugins.ts](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2lucy50cw==) | `22.22% <0.00%> (-77.78%)` | :arrow_down: |
| [superset-frontend/src/views/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2luZGV4LnRzeA==) | `25.00% <0.00%> (-75.00%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/index.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC50c3g=) | `25.00% <0.00%> (-75.00%)` | :arrow_down: |
| [superset-frontend/src/views/App.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0FwcC50c3g=) | `31.42% <0.00%> (-68.58%)` | :arrow_down: |
| [...rset-frontend/src/setup/setupErrorMessagesExtra.ts](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlc0V4dHJhLnRz) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| ... and [275 more](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=footer). Last update [6181994...2dfb192](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-696351607
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=h1) Report
> Merging [#10914](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.34%`.
> The diff coverage is `80.76%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10914 +/- ##
==========================================
- Coverage 65.88% 61.53% -4.35%
==========================================
Files 815 816 +1
Lines 38337 38406 +69
Branches 3601 3627 +26
==========================================
- Hits 25257 23634 -1623
- Misses 12978 14586 +1608
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.98% <80.76%> (+0.22%)` | :arrow_up: |
| #python | `61.27% <ø> (-0.20%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `64.22% <56.25%> (-8.84%)` | :arrow_down: |
| [superset-frontend/src/components/TableSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci50c3g=) | `83.18% <83.18%> (ø)` | |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `86.36% <86.36%> (ø)` | |
| [...nd/src/views/CRUD/data/dataset/AddDatasetModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXRNb2RhbC50c3g=) | `57.57% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [185 more](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=footer). Last update [141ef4a...8c668ee](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai commented on a change in pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
nytai commented on a change in pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#discussion_r494025642
##########
File path: superset-frontend/spec/javascripts/components/TableSelector_spec.jsx
##########
@@ -18,267 +18,275 @@
*/
import React from 'react';
import configureStore from 'redux-mock-store';
-import { shallow } from 'enzyme';
+import { mount } from 'enzyme';
+import { act } from 'react-dom/test-utils';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk';
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
+
+import DatabaseSelector from 'src/components/DatabaseSelector';
import TableSelector from 'src/components/TableSelector';
import { initialState, tables } from '../sqllab/fixtures';
-describe('TableSelector', () => {
- let mockedProps;
- const middlewares = [thunk];
- const mockStore = configureStore(middlewares);
- const store = mockStore(initialState);
- let wrapper;
- let inst;
+const mockStore = configureStore([thunk]);
+const store = mockStore(initialState);
- beforeEach(() => {
- mockedProps = {
- dbId: 1,
- schema: 'main',
- onSchemaChange: sinon.stub(),
- onDbChange: sinon.stub(),
- getDbList: sinon.stub(),
- onTableChange: sinon.stub(),
- onChange: sinon.stub(),
- tableNameSticky: true,
- tableName: '',
- database: { id: 1, database_name: 'main' },
- horizontal: false,
- sqlLabMode: true,
- clearable: false,
- handleError: sinon.stub(),
- };
- wrapper = shallow(<TableSelector {...mockedProps} />, {
- context: { store },
- });
- inst = wrapper.instance();
- });
-
- it('is valid', () => {
- expect(React.isValidElement(<TableSelector {...mockedProps} />)).toBe(true);
- });
-
- describe('onDatabaseChange', () => {
- it('should fetch schemas', () => {
- sinon.stub(inst, 'fetchSchemas');
- inst.onDatabaseChange({ id: 1 });
- expect(inst.fetchSchemas.getCall(0).args[0]).toBe(1);
- inst.fetchSchemas.restore();
- });
- it('should clear tableOptions', () => {
- inst.onDatabaseChange();
- expect(wrapper.state().tableOptions).toEqual([]);
- });
- });
+const FETCH_SCHEMAS_ENDPOINT = 'glob:*/api/v1/database/*/schemas/*';
+const GET_TABLE_ENDPOINT = 'glob:*/superset/tables/1/*/*';
+const GET_TABLE_NAMES_ENDPOINT = 'glob:*/superset/tables/1/main/*';
- describe('getTableNamesBySubStr', () => {
- const GET_TABLE_NAMES_GLOB = 'glob:*/superset/tables/1/main/*';
+const mockedProps = {
+ clearable: false,
+ database: { id: 1, database_name: 'main' },
+ dbId: 1,
+ formMode: false,
+ getDbList: sinon.stub(),
+ handleError: sinon.stub(),
+ horizontal: false,
+ onChange: sinon.stub(),
+ onDbChange: sinon.stub(),
+ onSchemaChange: sinon.stub(),
+ onTableChange: sinon.stub(),
+ sqlLabMode: true,
+ tableName: '',
+ tableNameSticky: true,
+};
- afterEach(fetchMock.resetHistory);
- afterAll(fetchMock.reset);
+const schemaOptions = {
+ result: ['main', 'erf', 'superset'],
+};
+const selectedSchema = { label: 'main', title: 'main', value: 'main' };
+const selectedTable = {
+ label: 'birth_names',
+ schema: 'main',
+ title: 'birth_names',
+ value: 'birth_names',
+ type: undefined,
+};
- it('should handle empty', () =>
- inst.getTableNamesBySubStr('').then(data => {
- expect(data).toEqual({ options: [] });
- return Promise.resolve();
- }));
+async function mountAndWait(props = mockedProps) {
+ const mounted = mount(<TableSelector {...props} />, {
+ context: { store },
+ wrappingComponent: ThemeProvider,
+ wrappingComponentProps: { theme: supersetTheme },
+ });
+ await waitForComponentToPaint(mounted);
- it('should handle table name', () => {
- fetchMock.get(GET_TABLE_NAMES_GLOB, tables, { overwriteRoutes: true });
+ return mounted;
+}
- return wrapper
- .instance()
- .getTableNamesBySubStr('my table')
- .then(data => {
- expect(fetchMock.calls(GET_TABLE_NAMES_GLOB)).toHaveLength(1);
- expect(data).toEqual({
- options: [
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
- ],
- });
- return Promise.resolve();
- });
- });
+describe('TableSelector', () => {
+ let wrapper;
- it('should escape schema and table names', () => {
- const GET_TABLE_GLOB = 'glob:*/superset/tables/1/*/*';
- wrapper.setProps({ schema: 'slashed/schema' });
- fetchMock.get(GET_TABLE_GLOB, tables, { overwriteRoutes: true });
+ beforeEach(async () => {
+ fetchMock.reset();
+ wrapper = await mountAndWait();
+ });
- return wrapper
- .instance()
- .getTableNamesBySubStr('slashed/table')
- .then(() => {
- expect(fetchMock.lastUrl(GET_TABLE_GLOB)).toContain(
- '/slashed%252Fschema/slashed%252Ftable',
- );
- return Promise.resolve();
- });
- });
+ it('renders', () => {
+ expect(wrapper.find(TableSelector)).toExist();
+ expect(wrapper.find(DatabaseSelector)).toExist();
});
- describe('fetchTables', () => {
- const FETCH_TABLES_GLOB = 'glob:*/superset/tables/1/main/*/*/';
+ describe('change database', () => {
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should clear table options', () => {
- inst.fetchTables(true);
- expect(wrapper.state().tableOptions).toEqual([]);
+ it('should fetch schemas', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
});
- it('should fetch table options', () => {
- fetchMock.get(FETCH_TABLES_GLOB, tables, { overwriteRoutes: true });
- return inst.fetchTables(true, 'birth_names').then(() => {
- expect(wrapper.state().tableOptions).toHaveLength(3);
- expect(wrapper.state().tableOptions).toEqual([
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
+ it('should fetch schema options', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
+
+ act(() => {
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().options,
+ ).toEqual([
+ { value: 'main', label: 'main', title: 'main' },
+ { value: 'erf', label: 'erf', title: 'erf' },
+ { value: 'superset', label: 'superset', title: 'superset' },
]);
- return Promise.resolve();
});
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- fetchMock.get(
- FETCH_TABLES_GLOB,
- { throws: 'error' },
- { overwriteRoutes: true },
- );
+ it('should clear table options', () => {
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
+ });
+ });
+ return Promise.resolve();
+ });
+ });
- wrapper
- .instance()
- .fetchTables(true, 'birth_names')
- .then(() => {
- expect(wrapper.state().tableOptions).toEqual([]);
- expect(wrapper.state().tableOptions).toHaveLength(0);
- expect(mockedProps.handleError.callCount).toBe(1);
- return Promise.resolve();
+ describe('change schema', () => {
+ beforeEach(async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
});
+ });
});
- });
- describe('fetchSchemas', () => {
- const FETCH_SCHEMAS_GLOB = 'glob:*/api/v1/database/*/schemas/?q=(force:!*)';
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should fetch schema options', () => {
- const schemaOptions = {
- result: ['main', 'erf', 'superset'],
- };
- fetchMock.get(FETCH_SCHEMAS_GLOB, schemaOptions, {
- overwriteRoutes: true,
+ it('should fetch table', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
});
-
- return wrapper
- .instance()
- .fetchSchemas(1)
- .then(() => {
- expect(fetchMock.calls(FETCH_SCHEMAS_GLOB)).toHaveLength(1);
- expect(wrapper.state().schemaOptions).toHaveLength(3);
- });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- const handleErrors = sinon.stub();
- expect(handleErrors.callCount).toBe(0);
- wrapper.setProps({ handleErrors });
- fetchMock.get(
- FETCH_SCHEMAS_GLOB,
- { throws: new Error('Bad kitty') },
- { overwriteRoutes: true },
- );
- wrapper
- .instance()
- .fetchSchemas(123)
- .then(() => {
- expect(wrapper.state().schemaOptions).toEqual([]);
- expect(handleErrors.callCount).toBe(1);
- });
- });
- });
+ it('should fetch table options', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().value[0],
+ ).toEqual(selectedSchema);
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
- describe('changeTable', () => {
- beforeEach(() => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ act(() => {
+ const { options } = wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props();
+ expect({ options }).toEqual(tables);
+ });
});
+ });
- afterEach(() => {
- wrapper.instance().fetchTables.restore();
+ describe('change table', () => {
+ beforeEach(async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
});
- it('test 1', () => {
- wrapper.instance().changeTable({
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
+ it('should change table value', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(wrapper.state().tableName).toBe('birth_names');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-table"]').first().props().value[0],
+ ).toEqual(selectedTable);
});
- it('should call onTableChange with schema from table object', () => {
- wrapper.setProps({ schema: null });
- wrapper.instance().changeTable({
- value: 'my_table',
- schema: 'other_schema',
- label: 'other_schema.my_table',
- title: 'other_schema.my_table',
+ it('should call onTableChange with schema from table object', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('my_table');
- expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('other_schema');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('birth_names');
+ expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('main');
});
});
- it('changeSchema', () => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ describe('getTableNamesBySubStr', () => {
+ beforeEach(() => {
+ wrapper.setProps({ schema: 'main' });
+ });
+ afterEach(fetchMock.resetHistory);
+ afterAll(fetchMock.reset);
- wrapper.instance().changeSchema({ label: 'main', value: 'main' });
- expect(wrapper.instance().fetchTables.callCount).toBe(1);
- expect(mockedProps.onChange.callCount).toBe(1);
- wrapper.instance().changeSchema();
- expect(wrapper.instance().fetchTables.callCount).toBe(2);
- expect(mockedProps.onChange.callCount).toBe(2);
+ it('should handle empty', () =>
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
Review comment:
since this is part of a promise, I'm not sure if the test is exiting before it gets to this line. Looking at [the docs](https://jestjs.io/docs/en/asynchronous) it appears we can return a promise and jest will wait for it to finish before finishing the test. Since this is wrapped in an `act`, I'm not sure how to do that. We may need to use `done`, or make this an async function and use a few awaits, or setup the test with `expect.assertions(1)`.
##########
File path: superset-frontend/spec/javascripts/components/TableSelector_spec.jsx
##########
@@ -18,267 +18,275 @@
*/
import React from 'react';
import configureStore from 'redux-mock-store';
-import { shallow } from 'enzyme';
+import { mount } from 'enzyme';
+import { act } from 'react-dom/test-utils';
import sinon from 'sinon';
import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk';
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
+
+import DatabaseSelector from 'src/components/DatabaseSelector';
import TableSelector from 'src/components/TableSelector';
import { initialState, tables } from '../sqllab/fixtures';
-describe('TableSelector', () => {
- let mockedProps;
- const middlewares = [thunk];
- const mockStore = configureStore(middlewares);
- const store = mockStore(initialState);
- let wrapper;
- let inst;
+const mockStore = configureStore([thunk]);
+const store = mockStore(initialState);
- beforeEach(() => {
- mockedProps = {
- dbId: 1,
- schema: 'main',
- onSchemaChange: sinon.stub(),
- onDbChange: sinon.stub(),
- getDbList: sinon.stub(),
- onTableChange: sinon.stub(),
- onChange: sinon.stub(),
- tableNameSticky: true,
- tableName: '',
- database: { id: 1, database_name: 'main' },
- horizontal: false,
- sqlLabMode: true,
- clearable: false,
- handleError: sinon.stub(),
- };
- wrapper = shallow(<TableSelector {...mockedProps} />, {
- context: { store },
- });
- inst = wrapper.instance();
- });
-
- it('is valid', () => {
- expect(React.isValidElement(<TableSelector {...mockedProps} />)).toBe(true);
- });
-
- describe('onDatabaseChange', () => {
- it('should fetch schemas', () => {
- sinon.stub(inst, 'fetchSchemas');
- inst.onDatabaseChange({ id: 1 });
- expect(inst.fetchSchemas.getCall(0).args[0]).toBe(1);
- inst.fetchSchemas.restore();
- });
- it('should clear tableOptions', () => {
- inst.onDatabaseChange();
- expect(wrapper.state().tableOptions).toEqual([]);
- });
- });
+const FETCH_SCHEMAS_ENDPOINT = 'glob:*/api/v1/database/*/schemas/*';
+const GET_TABLE_ENDPOINT = 'glob:*/superset/tables/1/*/*';
+const GET_TABLE_NAMES_ENDPOINT = 'glob:*/superset/tables/1/main/*';
- describe('getTableNamesBySubStr', () => {
- const GET_TABLE_NAMES_GLOB = 'glob:*/superset/tables/1/main/*';
+const mockedProps = {
+ clearable: false,
+ database: { id: 1, database_name: 'main' },
+ dbId: 1,
+ formMode: false,
+ getDbList: sinon.stub(),
+ handleError: sinon.stub(),
+ horizontal: false,
+ onChange: sinon.stub(),
+ onDbChange: sinon.stub(),
+ onSchemaChange: sinon.stub(),
+ onTableChange: sinon.stub(),
+ sqlLabMode: true,
+ tableName: '',
+ tableNameSticky: true,
+};
- afterEach(fetchMock.resetHistory);
- afterAll(fetchMock.reset);
+const schemaOptions = {
+ result: ['main', 'erf', 'superset'],
+};
+const selectedSchema = { label: 'main', title: 'main', value: 'main' };
+const selectedTable = {
+ label: 'birth_names',
+ schema: 'main',
+ title: 'birth_names',
+ value: 'birth_names',
+ type: undefined,
+};
- it('should handle empty', () =>
- inst.getTableNamesBySubStr('').then(data => {
- expect(data).toEqual({ options: [] });
- return Promise.resolve();
- }));
+async function mountAndWait(props = mockedProps) {
+ const mounted = mount(<TableSelector {...props} />, {
+ context: { store },
+ wrappingComponent: ThemeProvider,
+ wrappingComponentProps: { theme: supersetTheme },
+ });
+ await waitForComponentToPaint(mounted);
- it('should handle table name', () => {
- fetchMock.get(GET_TABLE_NAMES_GLOB, tables, { overwriteRoutes: true });
+ return mounted;
+}
- return wrapper
- .instance()
- .getTableNamesBySubStr('my table')
- .then(data => {
- expect(fetchMock.calls(GET_TABLE_NAMES_GLOB)).toHaveLength(1);
- expect(data).toEqual({
- options: [
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
- ],
- });
- return Promise.resolve();
- });
- });
+describe('TableSelector', () => {
+ let wrapper;
- it('should escape schema and table names', () => {
- const GET_TABLE_GLOB = 'glob:*/superset/tables/1/*/*';
- wrapper.setProps({ schema: 'slashed/schema' });
- fetchMock.get(GET_TABLE_GLOB, tables, { overwriteRoutes: true });
+ beforeEach(async () => {
+ fetchMock.reset();
+ wrapper = await mountAndWait();
+ });
- return wrapper
- .instance()
- .getTableNamesBySubStr('slashed/table')
- .then(() => {
- expect(fetchMock.lastUrl(GET_TABLE_GLOB)).toContain(
- '/slashed%252Fschema/slashed%252Ftable',
- );
- return Promise.resolve();
- });
- });
+ it('renders', () => {
+ expect(wrapper.find(TableSelector)).toExist();
+ expect(wrapper.find(DatabaseSelector)).toExist();
});
- describe('fetchTables', () => {
- const FETCH_TABLES_GLOB = 'glob:*/superset/tables/1/main/*/*/';
+ describe('change database', () => {
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should clear table options', () => {
- inst.fetchTables(true);
- expect(wrapper.state().tableOptions).toEqual([]);
+ it('should fetch schemas', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
});
- it('should fetch table options', () => {
- fetchMock.get(FETCH_TABLES_GLOB, tables, { overwriteRoutes: true });
- return inst.fetchTables(true, 'birth_names').then(() => {
- expect(wrapper.state().tableOptions).toHaveLength(3);
- expect(wrapper.state().tableOptions).toEqual([
- {
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
- },
- {
- value: 'energy_usage',
- schema: 'main',
- label: 'energy_usage',
- title: 'energy_usage',
- },
- {
- value: 'wb_health_population',
- schema: 'main',
- label: 'wb_health_population',
- title: 'wb_health_population',
- },
+ it('should fetch schema options', async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
+ });
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(FETCH_SCHEMAS_ENDPOINT)).toHaveLength(1);
+
+ act(() => {
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().options,
+ ).toEqual([
+ { value: 'main', label: 'main', title: 'main' },
+ { value: 'erf', label: 'erf', title: 'erf' },
+ { value: 'superset', label: 'superset', title: 'superset' },
]);
- return Promise.resolve();
});
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- fetchMock.get(
- FETCH_TABLES_GLOB,
- { throws: 'error' },
- { overwriteRoutes: true },
- );
+ it('should clear table options', () => {
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
+ });
+ });
+ return Promise.resolve();
+ });
+ });
- wrapper
- .instance()
- .fetchTables(true, 'birth_names')
- .then(() => {
- expect(wrapper.state().tableOptions).toEqual([]);
- expect(wrapper.state().tableOptions).toHaveLength(0);
- expect(mockedProps.handleError.callCount).toBe(1);
- return Promise.resolve();
+ describe('change schema', () => {
+ beforeEach(async () => {
+ fetchMock.get(FETCH_SCHEMAS_ENDPOINT, schemaOptions, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper.find('[data-test="select-database"]').first().props().onChange({
+ id: 1,
+ database_name: 'main',
});
+ });
});
- });
- describe('fetchSchemas', () => {
- const FETCH_SCHEMAS_GLOB = 'glob:*/api/v1/database/*/schemas/?q=(force:!*)';
afterEach(fetchMock.resetHistory);
afterAll(fetchMock.reset);
- it('should fetch schema options', () => {
- const schemaOptions = {
- result: ['main', 'erf', 'superset'],
- };
- fetchMock.get(FETCH_SCHEMAS_GLOB, schemaOptions, {
- overwriteRoutes: true,
+ it('should fetch table', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, { overwriteRoutes: true });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
});
-
- return wrapper
- .instance()
- .fetchSchemas(1)
- .then(() => {
- expect(fetchMock.calls(FETCH_SCHEMAS_GLOB)).toHaveLength(1);
- expect(wrapper.state().schemaOptions).toHaveLength(3);
- });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
});
- // Test needs to be fixed: Github issue #7768
- it.skip('should dispatch a danger toast on error', () => {
- const handleErrors = sinon.stub();
- expect(handleErrors.callCount).toBe(0);
- wrapper.setProps({ handleErrors });
- fetchMock.get(
- FETCH_SCHEMAS_GLOB,
- { throws: new Error('Bad kitty') },
- { overwriteRoutes: true },
- );
- wrapper
- .instance()
- .fetchSchemas(123)
- .then(() => {
- expect(wrapper.state().schemaOptions).toEqual([]);
- expect(handleErrors.callCount).toBe(1);
- });
- });
- });
+ it('should fetch table options', async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-schema"]').first().props().value[0],
+ ).toEqual(selectedSchema);
+ expect(fetchMock.calls(GET_TABLE_NAMES_ENDPOINT)).toHaveLength(1);
- describe('changeTable', () => {
- beforeEach(() => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ act(() => {
+ const { options } = wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props();
+ expect({ options }).toEqual(tables);
+ });
});
+ });
- afterEach(() => {
- wrapper.instance().fetchTables.restore();
+ describe('change table', () => {
+ beforeEach(async () => {
+ fetchMock.get(GET_TABLE_NAMES_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="select-schema"]')
+ .first()
+ .props()
+ .onChange(selectedSchema);
+ });
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
});
- it('test 1', () => {
- wrapper.instance().changeTable({
- value: 'birth_names',
- schema: 'main',
- label: 'birth_names',
- title: 'birth_names',
+ it('should change table value', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(wrapper.state().tableName).toBe('birth_names');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(
+ wrapper.find('[name="select-table"]').first().props().value[0],
+ ).toEqual(selectedTable);
});
- it('should call onTableChange with schema from table object', () => {
- wrapper.setProps({ schema: null });
- wrapper.instance().changeTable({
- value: 'my_table',
- schema: 'other_schema',
- label: 'other_schema.my_table',
- title: 'other_schema.my_table',
+ it('should call onTableChange with schema from table object', async () => {
+ act(() => {
+ wrapper
+ .find('[name="select-table"]')
+ .first()
+ .props()
+ .onChange(selectedTable);
});
- expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('my_table');
- expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('other_schema');
+ await waitForComponentToPaint(wrapper);
+ wrapper.update();
+ expect(mockedProps.onTableChange.getCall(0).args[0]).toBe('birth_names');
+ expect(mockedProps.onTableChange.getCall(0).args[1]).toBe('main');
});
});
- it('changeSchema', () => {
- sinon.stub(wrapper.instance(), 'fetchTables');
+ describe('getTableNamesBySubStr', () => {
+ beforeEach(() => {
+ wrapper.setProps({ schema: 'main' });
+ });
+ afterEach(fetchMock.resetHistory);
+ afterAll(fetchMock.reset);
- wrapper.instance().changeSchema({ label: 'main', value: 'main' });
- expect(wrapper.instance().fetchTables.callCount).toBe(1);
- expect(mockedProps.onChange.callCount).toBe(1);
- wrapper.instance().changeSchema();
- expect(wrapper.instance().fetchTables.callCount).toBe(2);
- expect(mockedProps.onChange.callCount).toBe(2);
+ it('should handle empty', () =>
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(data).toEqual({ options: [] });
+ });
+ }));
- wrapper.instance().fetchTables.restore();
+ it('should handle table name', () => {
+ fetchMock.get(GET_TABLE_ENDPOINT, tables, {
+ overwriteRoutes: true,
+ });
+ act(() => {
+ wrapper
+ .find('[name="async-select-table"]')
+ .first()
+ .props()
+ .loadOptions()
+ .then(data => {
+ expect(fetchMock.calls(GET_TABLE_ENDPOINT)).toHaveLength(1);
+ expect(data).toEqual(tables);
+ });
+ });
+ return Promise.resolve();
Review comment:
Same here, I think this test will always succeed since we're returning a resolved promise here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-696351607
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=h1) Report
> Merging [#10914](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.34%`.
> The diff coverage is `80.76%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10914 +/- ##
==========================================
- Coverage 65.88% 61.53% -4.35%
==========================================
Files 815 816 +1
Lines 38337 38406 +69
Branches 3601 3627 +26
==========================================
- Hits 25257 23634 -1623
- Misses 12978 14586 +1608
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.98% <80.76%> (+0.22%)` | :arrow_up: |
| #python | `61.27% <ø> (-0.20%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `64.22% <56.25%> (-8.84%)` | :arrow_down: |
| [superset-frontend/src/components/TableSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci50c3g=) | `83.18% <83.18%> (ø)` | |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `86.36% <86.36%> (ø)` | |
| [...nd/src/views/CRUD/data/dataset/AddDatasetModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXRNb2RhbC50c3g=) | `57.57% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [185 more](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=footer). Last update [141ef4a...8c668ee](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-696351607
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=h1) Report
> Merging [#10914](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/141ef4a188698efb595cd5f5da402c148d9207bf?el=desc) will **decrease** coverage by `4.34%`.
> The diff coverage is `80.76%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10914 +/- ##
==========================================
- Coverage 65.88% 61.53% -4.35%
==========================================
Files 815 816 +1
Lines 38337 38406 +69
Branches 3601 3627 +26
==========================================
- Hits 25257 23633 -1624
- Misses 12978 14587 +1609
- Partials 102 186 +84
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `61.98% <80.76%> (+0.22%)` | :arrow_up: |
| #python | `61.27% <ø> (-0.21%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/datasource/DatasourceEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFzb3VyY2UvRGF0YXNvdXJjZUVkaXRvci5qc3g=) | `64.22% <56.25%> (-8.84%)` | :arrow_down: |
| [superset-frontend/src/components/TableSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci50c3g=) | `83.18% <83.18%> (ø)` | |
| [...erset-frontend/src/components/DatabaseSelector.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci50c3g=) | `86.36% <86.36%> (ø)` | |
| [...nd/src/views/CRUD/data/dataset/AddDatasetModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXRNb2RhbC50c3g=) | `57.57% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [186 more](https://codecov.io/gh/apache/incubator-superset/pull/10914/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=footer). Last update [141ef4a...8c668ee](https://codecov.io/gh/apache/incubator-superset/pull/10914?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-698667308
Oh one thought is that we may need for the virtual dataset name to be editable some place.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-698667308
Oh one thought is that we may need for the virtual dataset name to be editable some place.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914#issuecomment-700316033
I'm getting double scroll bars (overlaying on top of each other) when SQL Queries are long.
@lilykuang @nytai could you get this fixed?
I'd recommend using this CSS trick to let the SQL editor fill remaining vertical space:
https://stackoverflow.com/questions/25098042/fill-remaining-vertical-space-with-css-using-displayflex
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] nytai merged pull request #10914: refactor: table selector on dataset editor
Posted by GitBox <gi...@apache.org>.
nytai merged pull request #10914:
URL: https://github.com/apache/incubator-superset/pull/10914
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org