From 252c64b3979f15789f3f56c96ce896fa6cff8164 Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Mon, 24 May 2021 17:42:09 -0400 Subject: [PATCH] fix: Additional ResultSet tests (#14741) * fixing tests * added testing * Update superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * removed decribe Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- .../javascripts/sqllab/ResultSet_spec.jsx | 273 +++++++++--------- .../spec/javascripts/sqllab/fixtures.ts | 61 ++++ .../src/SqlLab/components/ResultSet.tsx | 14 +- 3 files changed, 211 insertions(+), 137 deletions(-) diff --git a/superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx b/superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx index 69e87f97e6b2b..0c8d0f74e13c7 100644 --- a/superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx +++ b/superset-frontend/spec/javascripts/sqllab/ResultSet_spec.jsx @@ -19,6 +19,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { styledMount } from 'spec/helpers/theming'; +import { render, screen } from 'spec/helpers/testing-library'; import { Provider } from 'react-redux'; import sinon from 'sinon'; import Alert from 'src/components/Alert'; @@ -38,147 +39,161 @@ import { stoppedQuery, initialState, user, + queryWithNoQueryLimit, } from './fixtures'; const mockStore = configureStore([thunk]); const store = mockStore(initialState); - -describe('ResultSet', () => { - const clearQuerySpy = sinon.spy(); - const fetchQuerySpy = sinon.spy(); - const reRunQuerySpy = sinon.spy(); - const mockedProps = { - actions: { - clearQueryResults: clearQuerySpy, - fetchQueryResults: fetchQuerySpy, - reRunQuery: reRunQuerySpy, +const clearQuerySpy = sinon.spy(); +const fetchQuerySpy = sinon.spy(); +const reRunQuerySpy = sinon.spy(); +const mockedProps = { + actions: { + clearQueryResults: clearQuerySpy, + fetchQueryResults: fetchQuerySpy, + reRunQuery: reRunQuerySpy, + }, + cache: true, + query: queries[0], + height: 140, + database: { allows_virtual_table_explore: true }, + user, + defaultQueryLimit: 1000, +}; +const stoppedQueryProps = { ...mockedProps, query: stoppedQuery }; +const runningQueryProps = { ...mockedProps, query: runningQuery }; +const cachedQueryProps = { ...mockedProps, query: cachedQuery }; +const failedQueryWithErrorMessageProps = { + ...mockedProps, + query: failedQueryWithErrorMessage, +}; +const failedQueryWithErrorsProps = { + ...mockedProps, + query: failedQueryWithErrors, +}; +const newProps = { + query: { + cached: false, + resultsKey: 'new key', + results: { + data: [{ a: 1 }], }, - cache: true, - query: queries[0], - height: 140, - database: { allows_virtual_table_explore: true }, - user, - defaultQueryLimit: 1000, - }; - const stoppedQueryProps = { ...mockedProps, query: stoppedQuery }; - const runningQueryProps = { ...mockedProps, query: runningQuery }; - const cachedQueryProps = { ...mockedProps, query: cachedQuery }; - const failedQueryWithErrorMessageProps = { - ...mockedProps, - query: failedQueryWithErrorMessage, - }; - const failedQueryWithErrorsProps = { + }, +}; + +test('is valid', () => { + expect(React.isValidElement()).toBe(true); +}); + +test('renders a Table', () => { + const wrapper = shallow(); + expect(wrapper.find(FilterableTable)).toExist(); +}); + +describe('componentDidMount', () => { + const propsWithError = { ...mockedProps, - query: failedQueryWithErrors, - }; - const newProps = { - query: { - cached: false, - resultsKey: 'new key', - results: { - data: [{ a: 1 }], - }, - }, + query: { ...queries[0], errorMessage: 'Your session timed out' }, }; - - it('is valid', () => { - expect(React.isValidElement()).toBe(true); + let spy; + beforeEach(() => { + reRunQuerySpy.resetHistory(); + spy = sinon.spy(ResultSet.prototype, 'componentDidMount'); + }); + afterEach(() => { + spy.restore(); }); - it('renders a Table', () => { - const wrapper = shallow(); - expect(wrapper.find(FilterableTable)).toExist(); + it('should call reRunQuery if timed out', () => { + shallow(); + expect(reRunQuerySpy.callCount).toBe(1); }); - describe('componentDidMount', () => { - const propsWithError = { - ...mockedProps, - query: { ...queries[0], errorMessage: 'Your session timed out' }, - }; - let spy; - beforeEach(() => { - reRunQuerySpy.resetHistory(); - spy = sinon.spy(ResultSet.prototype, 'componentDidMount'); - }); - afterEach(() => { - spy.restore(); - }); - it('should call reRunQuery if timed out', () => { - shallow(); - expect(reRunQuerySpy.callCount).toBe(1); - }); - it('should not call reRunQuery if no error', () => { - shallow(); - expect(reRunQuerySpy.callCount).toBe(0); - }); + it('should not call reRunQuery if no error', () => { + shallow(); + expect(reRunQuerySpy.callCount).toBe(0); }); - describe('UNSAFE_componentWillReceiveProps', () => { - const wrapper = shallow(); - let spy; - beforeEach(() => { - clearQuerySpy.resetHistory(); - fetchQuerySpy.resetHistory(); - spy = sinon.spy(ResultSet.prototype, 'UNSAFE_componentWillReceiveProps'); - }); - afterEach(() => { - spy.restore(); - }); - it('should update cached data', () => { - wrapper.setProps(newProps); +}); - expect(wrapper.state().data).toEqual(newProps.query.results.data); - expect(clearQuerySpy.callCount).toBe(1); - expect(clearQuerySpy.getCall(0).args[0]).toEqual(newProps.query); - expect(fetchQuerySpy.callCount).toBe(1); - expect(fetchQuerySpy.getCall(0).args[0]).toEqual(newProps.query); - }); +describe('UNSAFE_componentWillReceiveProps', () => { + const wrapper = shallow(); + let spy; + beforeEach(() => { + clearQuerySpy.resetHistory(); + fetchQuerySpy.resetHistory(); + spy = sinon.spy(ResultSet.prototype, 'UNSAFE_componentWillReceiveProps'); + }); + afterEach(() => { + spy.restore(); }); - describe('render', () => { - it('should render success query', () => { - const wrapper = shallow(); - const filterableTable = wrapper.find(FilterableTable); - expect(filterableTable.props().data).toBe(mockedProps.query.results.data); - expect(wrapper.find(ExploreResultsButton)).toExist(); - }); - it('should render empty results', () => { - const props = { - ...mockedProps, - query: { ...mockedProps.query, results: { data: [] } }, - }; - const wrapper = styledMount( - - - , - ); - expect(wrapper.find(FilterableTable)).not.toExist(); - expect(wrapper.find(Alert)).toExist(); - expect(wrapper.find(Alert).render().text()).toBe( - 'The query returned no data', - ); - }); - it('should render cached query', () => { - const wrapper = shallow(); - const cachedData = [{ col1: 'a', col2: 'b' }]; - wrapper.setState({ data: cachedData }); - const filterableTable = wrapper.find(FilterableTable); - expect(filterableTable.props().data).toBe(cachedData); - }); - it('should render stopped query', () => { - const wrapper = shallow(); - expect(wrapper.find(Alert)).toExist(); - }); - it('should render running/pending/fetching query', () => { - const wrapper = shallow(); - expect(wrapper.find(ProgressBar)).toExist(); - }); - it('should render a failed query with an error message', () => { - const wrapper = shallow( - , - ); - expect(wrapper.find(ErrorMessageWithStackTrace)).toExist(); - }); - it('should render a failed query with an errors object', () => { - const wrapper = shallow(); - expect(wrapper.find(ErrorMessageWithStackTrace)).toExist(); - }); + it('should update cached data', () => { + wrapper.setProps(newProps); + + expect(wrapper.state().data).toEqual(newProps.query.results.data); + expect(clearQuerySpy.callCount).toBe(1); + expect(clearQuerySpy.getCall(0).args[0]).toEqual(newProps.query); + expect(fetchQuerySpy.callCount).toBe(1); + expect(fetchQuerySpy.getCall(0).args[0]).toEqual(newProps.query); }); }); + +test('should render success query', () => { + const wrapper = shallow(); + const filterableTable = wrapper.find(FilterableTable); + expect(filterableTable.props().data).toBe(mockedProps.query.results.data); + expect(wrapper.find(ExploreResultsButton)).toExist(); +}); +test('should render empty results', () => { + const props = { + ...mockedProps, + query: { ...mockedProps.query, results: { data: [] } }, + }; + const wrapper = styledMount( + + + , + ); + expect(wrapper.find(FilterableTable)).not.toExist(); + expect(wrapper.find(Alert)).toExist(); + expect(wrapper.find(Alert).render().text()).toBe( + 'The query returned no data', + ); +}); + +test('should render cached query', () => { + const wrapper = shallow(); + const cachedData = [{ col1: 'a', col2: 'b' }]; + wrapper.setState({ data: cachedData }); + const filterableTable = wrapper.find(FilterableTable); + expect(filterableTable.props().data).toBe(cachedData); +}); + +test('should render stopped query', () => { + const wrapper = shallow(); + expect(wrapper.find(Alert)).toExist(); +}); + +test('should render running/pending/fetching query', () => { + const wrapper = shallow(); + expect(wrapper.find(ProgressBar)).toExist(); +}); + +test('should render a failed query with an error message', () => { + const wrapper = shallow(); + expect(wrapper.find(ErrorMessageWithStackTrace)).toExist(); +}); + +test('should render a failed query with an errors object', () => { + const wrapper = shallow(); + expect(wrapper.find(ErrorMessageWithStackTrace)).toExist(); +}); + +test('renders if there is no limit in query.results but has queryLimit', () => { + render(, { useRedux: true }); + expect(screen.getByRole('grid')).toBeInTheDocument(); +}); + +test('renders if there is a limit in query.results but not queryLimit', () => { + const props = { ...mockedProps, query: queryWithNoQueryLimit }; + render(, { useRedux: true }); + expect(screen.getByRole('grid')).toBeInTheDocument(); +}); diff --git a/superset-frontend/spec/javascripts/sqllab/fixtures.ts b/superset-frontend/spec/javascripts/sqllab/fixtures.ts index 911a89d5fed7e..b4b68454fa309 100644 --- a/superset-frontend/spec/javascripts/sqllab/fixtures.ts +++ b/superset-frontend/spec/javascripts/sqllab/fixtures.ts @@ -283,6 +283,67 @@ export const queries = [ results: null, }, ]; +export const queryWithNoQueryLimit = { + dbId: 1, + sql: 'SELECT * FROM superset.slices', + sqlEditorId: 'SJ8YO72R', + tab: 'Demo', + runAsync: false, + ctas: false, + cached: false, + id: 'BkA1CLrJg', + progress: 100, + startDttm: 1476910566092.96, + state: 'success', + changedOn: 1476910566000, + tempTable: null, + userId: 1, + executedSql: null, + changed_on: '2016-10-19T20:56:06', + rows: 42, + endDttm: 1476910566798, + limit_reached: false, + schema: 'test_schema', + errorMessage: null, + db: 'main', + user: 'admin', + limit: 1000, + serverId: 141, + resultsKey: null, + results: { + columns: [ + { + is_date: true, + name: 'ds', + type: 'STRING', + }, + { + is_date: false, + name: 'gender', + type: 'STRING', + }, + ], + selected_columns: [ + { + is_date: true, + name: 'ds', + type: 'STRING', + }, + { + is_date: false, + name: 'gender', + type: 'STRING', + }, + ], + data: [ + { col1: 0, col2: 1 }, + { col1: 2, col2: 3 }, + ], + query: { + limit: 100, + }, + }, +}; export const queryWithBadColumns = { ...queries[0], results: { diff --git a/superset-frontend/src/SqlLab/components/ResultSet.tsx b/superset-frontend/src/SqlLab/components/ResultSet.tsx index 398e2f2caa51e..a8fe019c7e6a0 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet.tsx @@ -526,34 +526,32 @@ export default class ResultSet extends React.PureComponent< const { results, rows, queryLimit, limitingFactor } = this.props.query; let limitMessage; const limitReached = results?.displayLimitReached; - const isAdmin = !!this.props.user?.roles.Admin; const limit = queryLimit || results.query.limit; + const isAdmin = !!this.props.user?.roles.Admin; const displayMaxRowsReachedMessage = { withAdmin: t( `The number of results displayed is limited to %(rows)d by the configuration DISPLAY_MAX_ROWS. `, { rows }, ).concat( t( - `Please add additional limits/filters or download to csv to see more rows up to the`, + `Please add additional limits/filters or download to csv to see more rows up to `, ), - t(`the %(limit)d limit.`, { - limit, - }), + t(`the %(limit)d limit.`, { limit }), ), withoutAdmin: t( `The number of results displayed is limited to %(rows)d. `, { rows }, ).concat( t( - `Please add additional limits/filters, download to csv, or contact an admin`, + `Please add additional limits/filters, download to csv, or contact an admin `, ), - t(`to see more rows up to the the %(limit)d limit.`, { + t(`to see more rows up to the %(limit)d limit.`, { limit, }), ), }; const shouldUseDefaultDropdownAlert = - queryLimit === this.props.defaultQueryLimit && + limit === this.props.defaultQueryLimit && limitingFactor === LIMITING_FACTOR.DROPDOWN; if (limitingFactor === LIMITING_FACTOR.QUERY && this.props.csv) {