This is an automated email from the ASF dual-hosted git repository. maximebeauchemin pushed a commit to branch knock_enzyme in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8dd17e2bab4aae80d53036c6f2d885a49b54a7ee Author: Maxime Beauchemin <[email protected]> AuthorDate: Wed Jan 29 00:08:11 2025 -0800 chore: move Dashboard.test.jsx from enzyme to react testing lib Done in about 10 minutes with a bit of back and forth with GPT o1 --- .../src/dashboard/components/Dashboard.test.jsx | 329 +++++++++------------ 1 file changed, 147 insertions(+), 182 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx index 5519c96bf2..da98bf2b5e 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx @@ -16,8 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { render } from '@testing-library/react'; -import '@testing-library/jest-dom/extend-expect'; + +import React from 'react'; +import { render, screen } from '@testing-library/react'; import Dashboard from 'src/dashboard/components/Dashboard'; import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; @@ -48,7 +49,9 @@ describe('Dashboard', () => { removeSliceFromDashboard() {}, triggerQuery() {}, logEvent() {}, + clearDataMaskState() {}, }, + dashboardId: 1, dashboardState, dashboardInfo, charts: chartQueries, @@ -66,16 +69,6 @@ describe('Dashboard', () => { const ChildrenComponent = () => <div>Test</div>; - function setup(overrideProps = {}) { - const { container } = render( - <Dashboard {...props} {...overrideProps}> - <ChildrenComponent /> - </Dashboard>, - ); - return container; - } - - // activeFilters map use id_column) as key const OVERRIDE_FILTERS = { '1_region': { values: [], scope: [1] }, '2_country_name': { values: ['USA'], scope: [1, 2] }, @@ -83,259 +76,231 @@ describe('Dashboard', () => { '3_country_name': { values: ['USA'], scope: [] }, }; - it('should render the children component', () => { - const container = setup(); - expect(container.querySelector('div')).toHaveTextContent('Test'); - }); + function renderDashboard(override = {}) { + // Helper to render the Dashboard and return the testing utils + return render( + <Dashboard {...props} {...override}> + <ChildrenComponent /> + </Dashboard>, + ); + } - describe('UNSAFE_componentWillReceiveProps', () => { + describe('UNSAFE_componentWillReceiveProps (simulated via re-render)', () => { const layoutWithExtraChart = { ...props.layout, 1001: newComponentFactory(CHART_TYPE, { chartId: 1001 }), }; it('should call addSliceToDashboard if a new slice is added to the layout', () => { - const container = setup(); - const addSliceToDashboardMock = jest.spyOn(props.actions, 'addSliceToDashboard'); - container.firstChild.UNSAFE_componentWillReceiveProps({ - ...props, - layout: layoutWithExtraChart, - }); - addSliceToDashboardMock.mockRestore(); + const addSliceToDashboardMock = jest.spyOn( + props.actions, + 'addSliceToDashboard', + ); + const { rerender } = renderDashboard(); + rerender( + <Dashboard {...props} layout={layoutWithExtraChart}> + <ChildrenComponent /> + </Dashboard>, + ); expect(addSliceToDashboardMock).toHaveBeenCalledTimes(1); + addSliceToDashboardMock.mockRestore(); }); it('should call removeSliceFromDashboard if a slice is removed from the layout', () => { - const container = setup({ layout: layoutWithExtraChart }); - const removeSliceFromDashboardMock = jest.spyOn(props.actions, 'removeSliceFromDashboard'); + const removeSliceFromDashboardMock = jest.spyOn( + props.actions, + 'removeSliceFromDashboard', + ); + // First, render with an extra slice + const { rerender } = renderDashboard({ layout: layoutWithExtraChart }); + // Then re-render with that slice removed const nextLayout = { ...layoutWithExtraChart }; delete nextLayout[1001]; - container.firstChild.UNSAFE_componentWillReceiveProps({ - ...props, - layout: nextLayout, - }); - removeSliceFromDashboardMock.mockRestore(); + rerender( + <Dashboard {...props} layout={nextLayout}> + <ChildrenComponent /> + </Dashboard>, + ); + expect(removeSliceFromDashboardMock).toHaveBeenCalledTimes(1); + removeSliceFromDashboardMock.mockRestore(); }); }); - describe('componentDidUpdate', () => { - let container; - let prevProps; + describe('componentDidUpdate (simulated via re-render)', () => { + // We'll spy on Dashboard.prototype to ensure refreshCharts gets called let refreshSpy; - beforeEach(() => { - container = setup({ activeFilters: OVERRIDE_FILTERS }); - container.firstChild.appliedFilters = OVERRIDE_FILTERS; - prevProps = container.firstChild.props; - refreshSpy = jest.spyOn(container.firstChild, 'refreshCharts'); + refreshSpy = jest.spyOn(Dashboard.prototype, 'refreshCharts'); }); - afterEach(() => { - refreshSpy.restore(); + refreshSpy.mockRestore(); jest.clearAllMocks(); }); - it('should not call refresh when is editMode', () => { - render(<Dashboard {...props} activeFilters={OVERRIDE_FILTERS} dashboardState={{ ...dashboardState, editMode: true }} />); - container.firstChild.componentDidUpdate(prevProps); + it('should not call refresh when in editMode', () => { + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); + rerender( + <Dashboard + {...props} + activeFilters={OVERRIDE_FILTERS} + dashboardState={{ ...dashboardState, editMode: true }} + > + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).not.toHaveBeenCalled(); }); - it('should not call refresh when there is no change', () => { - render(<Dashboard {...props} activeFilters={OVERRIDE_FILTERS} />); - container.firstChild.componentDidUpdate(prevProps); + it('should not call refresh when there is no change in activeFilters', () => { + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); + // Re-render with the exact same activeFilters + rerender( + <Dashboard {...props} activeFilters={OVERRIDE_FILTERS}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).not.toHaveBeenCalled(); - expect(container.firstChild.appliedFilters).toBe(OVERRIDE_FILTERS); }); - it('should call refresh when native filters changed', () => { + it('should call refresh when native filters changed (example: new filter added)', () => { getRelatedCharts.mockReturnValue([230]); - render(<Dashboard {...props} activeFilters={{ - dashboardState: { - ...dashboardState, - editMode: true, - }, - }); - container.firstChild.componentDidUpdate(prevProps); + const filtersWithNative = { + ...OVERRIDE_FILTERS, + ...getAllActiveFilters({ + dataMask: dataMaskWith1Filter, + nativeFilters: singleNativeFiltersState.filters, + allSliceIds: [227, 229, 230], + }), + }; + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); + rerender( + <Dashboard {...props} activeFilters={filtersWithNative}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); }); - it('should not call refresh when there is no change', () => { - render(<Dashboard {...props} activeFilters={{ - activeFilters: OVERRIDE_FILTERS, - }); - wrapper.instance().componentDidUpdate(prevProps); - expect(refreshSpy.callCount).toBe(0); - expect(container.firstChild.appliedFilters).toEqual({ - }); - - it('should call refresh when native filters changed', () => { - getRelatedCharts.mockReturnValue([230]); - render(<Dashboard {...props} activeFilters={newFilter} />); + it('should call refresh if a filter is added', () => { + getRelatedCharts.mockReturnValue([1]); + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); + const newFilter = { + gender: { values: ['boy', 'girl'], scope: [1] }, + }; + rerender( + <Dashboard {...props} activeFilters={newFilter}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); - expect(container.firstChild.appliedFilters).toEqual(newFilter); }); it('should call refresh if a filter is removed', () => { getRelatedCharts.mockReturnValue([]); - render(<Dashboard {...props} activeFilters={{}} />); + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); + rerender( + <Dashboard {...props} activeFilters={{}}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); - expect(container.firstChild.appliedFilters).toEqual({}); }); it('should call refresh if a filter is changed', () => { getRelatedCharts.mockReturnValue([1]); + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); + const newFilters = { ...OVERRIDE_FILTERS, '1_region': { values: ['Canada'], scope: [1] }, }; - render(<Dashboard {...props} activeFilters={newFilters} />); + rerender( + <Dashboard {...props} activeFilters={newFilters}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); - expect(container.firstChild.appliedFilters).toEqual(newFilters); expect(refreshSpy).toHaveBeenCalledWith([1]); }); it('should call refresh with multiple chart ids', () => { getRelatedCharts.mockReturnValue([1, 2]); + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); const newFilters = { ...OVERRIDE_FILTERS, '2_country_name': { values: ['New Country'], scope: [1, 2] }, }; - render(<Dashboard {...props} activeFilters={newFilters} />); + rerender( + <Dashboard {...props} activeFilters={newFilters}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); - expect(container.firstChild.appliedFilters).toEqual(newFilters); expect(refreshSpy).toHaveBeenCalledWith([1, 2]); }); it('should call refresh if a filter scope is changed', () => { + getRelatedCharts.mockReturnValue([2]); + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); const newFilters = { ...OVERRIDE_FILTERS, '3_country_name': { values: ['USA'], scope: [2] }, }; - - render(<Dashboard {...props} activeFilters={newFilters} />); + rerender( + <Dashboard {...props} activeFilters={newFilters}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); expect(refreshSpy).toHaveBeenCalledWith([2]); }); - it('should call refresh with empty [] if a filter is changed but scope is not applicable', () => { + it('should call refresh with empty array if a filter is changed but scope is not applicable', () => { getRelatedCharts.mockReturnValue([]); + const { rerender } = renderDashboard({ activeFilters: OVERRIDE_FILTERS }); const newFilters = { ...OVERRIDE_FILTERS, '3_country_name': { values: ['CHINA'], scope: [] }, }; - - render(<Dashboard {...props} activeFilters={newFilters} />); + rerender( + <Dashboard {...props} activeFilters={newFilters}> + <ChildrenComponent /> + </Dashboard>, + ); expect(refreshSpy).toHaveBeenCalledTimes(1); expect(refreshSpy).toHaveBeenCalledWith([]); }); - }); -}); - activeFilters: { - ...OVERRIDE_FILTERS, - ...getAllActiveFilters({ - dataMask: dataMaskWith1Filter, - nativeFilters: singleNativeFiltersState.filters, - allSliceIds: [227, 229, 230], - }), - }, - }); - wrapper.instance().componentDidUpdate(prevProps); - expect(refreshSpy).toHaveBeenCalledTimes(1); - expect(container.firstChild.appliedFilters).toEqual({ - ...OVERRIDE_FILTERS, - [NATIVE_FILTER_ID]: { - scope: [230], - values: extraFormData, - filterType: 'filter_select', - targets: [ - { - datasetId: 13, - column: { - name: 'ethnic_minority', - }, - }, - ], - }, - }); - }); - it('should call refresh if a filter is added', () => { - getRelatedCharts.mockReturnValue([1]); - const newFilter = { - gender: { values: ['boy', 'girl'], scope: [1] }, - }; - wrapper.setProps({ - activeFilters: newFilter, - }); - expect(refreshSpy.callCount).toBe(1); - expect(wrapper.instance().appliedFilters).toEqual(newFilter); - }); - - it('should call refresh if a filter is removed', () => { - getRelatedCharts.mockReturnValue([]); - wrapper.setProps({ - activeFilters: {}, - }); - expect(refreshSpy.callCount).toBe(1); - expect(wrapper.instance().appliedFilters).toEqual({}); - }); - - it('should call refresh if a filter is changed', () => { - getRelatedCharts.mockReturnValue([1]); - const newFilters = { - ...OVERRIDE_FILTERS, - '1_region': { values: ['Canada'], scope: [1] }, - }; - wrapper.setProps({ - activeFilters: newFilters, - }); - expect(refreshSpy.callCount).toBe(1); - expect(wrapper.instance().appliedFilters).toEqual(newFilters); - expect(refreshSpy.getCall(0).args[0]).toEqual([1]); - }); - - it('should call refresh with multiple chart ids', () => { - getRelatedCharts.mockReturnValue([1, 2]); - const newFilters = { - ...OVERRIDE_FILTERS, - '2_country_name': { values: ['New Country'], scope: [1, 2] }, - }; - wrapper.setProps({ - activeFilters: newFilters, - }); - expect(refreshSpy.callCount).toBe(1); - expect(wrapper.instance().appliedFilters).toEqual(newFilters); - expect(refreshSpy.getCall(0).args[0]).toEqual([1, 2]); - }); - - it('should call refresh if a filter scope is changed', () => { - const newFilters = { - ...OVERRIDE_FILTERS, - '3_country_name': { values: ['USA'], scope: [2] }, - }; - - wrapper.setProps({ - activeFilters: newFilters, - }); - expect(refreshSpy.callCount).toBe(1); - expect(refreshSpy.getCall(0).args[0]).toEqual([2]); - }); - - it('should call refresh with empty [] if a filter is changed but scope is not applicable', () => { - getRelatedCharts.mockReturnValue([]); - const newFilters = { + it('should call refresh when a native filter is added (more comprehensive case)', () => { + getRelatedCharts.mockReturnValue([230]); + const initialFilters = { ...OVERRIDE_FILTERS, - '3_country_name': { values: ['CHINA'], scope: [] }, }; - - wrapper.setProps({ - activeFilters: newFilters, - }); - expect(refreshSpy.callCount).toBe(1); - expect(refreshSpy.getCall(0).args[0]).toEqual([]); + const { rerender } = renderDashboard({ activeFilters: initialFilters }); + rerender( + <Dashboard + {...props} + activeFilters={{ + ...OVERRIDE_FILTERS, + [NATIVE_FILTER_ID]: { + scope: [230], + values: extraFormData, + filterType: 'filter_select', + targets: [ + { + datasetId: 13, + column: { name: 'ethnic_minority' }, + }, + ], + }, + }} + > + <ChildrenComponent /> + </Dashboard>, + ); + expect(refreshSpy).toHaveBeenCalledTimes(1); }); }); });
