From abccc4e4ccd2a38d554b2bd543239a6e53df6096 Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Wed, 20 Apr 2022 15:51:09 -0400 Subject: [PATCH] fix: remove & reimplement the tests for AlertReportCronScheduler component (#19288) --- .../AlertReportCronScheduler.test.tsx | 164 +++++++++++++----- .../components/AlertReportCronScheduler.tsx | 50 ++++-- 2 files changed, 154 insertions(+), 60 deletions(-) diff --git a/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.test.tsx b/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.test.tsx index 822b129c56de..5d36c2994dca 100644 --- a/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.test.tsx +++ b/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.test.tsx @@ -16,58 +16,138 @@ * specific language governing permissions and limitations * under the License. */ - import React from 'react'; -import { ReactWrapper } from 'enzyme'; -import { styledMount as mount } from 'spec/helpers/theming'; -import { CronPicker } from 'src/components/CronPicker'; -import { Input } from 'src/components/Input'; -import { AlertReportCronScheduler } from './AlertReportCronScheduler'; +import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { act } from 'react-dom/test-utils'; + +import { + AlertReportCronScheduler, + AlertReportCronSchedulerProps, +} from './AlertReportCronScheduler'; + +const createProps = (props: Partial = {}) => ({ + onChange: jest.fn(), + value: '* * * * *', + ...props, +}); + +test('should render', () => { + const props = createProps(); + render(); + + // Text found in the first radio option + expect(screen.getByText('Every')).toBeInTheDocument(); + // Text found in the second radio option + expect(screen.getByText('CRON Schedule')).toBeInTheDocument(); +}); + +test('only one radio option should be enabled at a time', () => { + const props = createProps(); + const { container } = render(); + + expect(screen.getByTestId('picker')).toBeChecked(); + expect(screen.getByTestId('input')).not.toBeChecked(); + + const pickerContainer = container.querySelector( + '.react-js-cron-select', + ) as HTMLElement; + const inputContainer = screen.getByTestId('input-content'); + + expect(within(pickerContainer).getAllByRole('combobox')[0]).toBeEnabled(); + expect(inputContainer.querySelector('input[name="crontab"]')).toBeDisabled(); + + userEvent.click(screen.getByTestId('input')); + + expect(within(pickerContainer).getAllByRole('combobox')[0]).toBeDisabled(); + expect(inputContainer.querySelector('input[name="crontab"]')).toBeEnabled(); + + userEvent.click(screen.getByTestId('picker')); + + expect(within(pickerContainer).getAllByRole('combobox')[0]).toBeEnabled(); + expect(inputContainer.querySelector('input[name="crontab"]')).toBeDisabled(); +}); + +test('picker mode updates correctly', async () => { + const onChangeCallback = jest.fn(); + const props = createProps({ + onChange: onChangeCallback, + }); + + const { container } = render(); -describe('AlertReportCronScheduler', () => { - let wrapper: ReactWrapper; + expect(screen.getByTestId('picker')).toBeChecked(); - it('calls onChnage when value chnages', () => { - const onChangeMock = jest.fn(); - wrapper = mount( - , - ); + const pickerContainer = container.querySelector( + '.react-js-cron-select', + ) as HTMLElement; - const changeValue = '1,7 * * * *'; + const firstSelect = within(pickerContainer).getAllByRole('combobox')[0]; + act(() => { + userEvent.click(firstSelect); + }); - wrapper.find(CronPicker).props().setValue(changeValue); - expect(onChangeMock).toHaveBeenLastCalledWith(changeValue); + expect(await within(pickerContainer).findByText('day')).toBeInTheDocument(); + act(() => { + userEvent.click(within(pickerContainer).getByText('day')); }); - it.skip('sets input value when cron picker changes', () => { - const onChangeMock = jest.fn(); - wrapper = mount( - , - ); + expect(onChangeCallback).toHaveBeenLastCalledWith('* * * * *'); + + const secondSelect = container.querySelector( + '.react-js-cron-hours .ant-select-selector', + ) as HTMLElement; + await waitFor(() => { + expect(secondSelect).toBeInTheDocument(); + }); + + act(() => { + userEvent.click(secondSelect); + }); - const changeValue = '1,7 * * * *'; + expect(await screen.findByText('9')).toBeInTheDocument(); + act(() => { + userEvent.click(screen.getByText('9')); + }); - wrapper.find(CronPicker).props().setValue(changeValue); - // TODO fix this class-style assertion that doesn't work on function components - // @ts-ignore - expect(wrapper.find(Input).state().value).toEqual(changeValue); + await waitFor(() => { + expect(onChangeCallback).toHaveBeenLastCalledWith('* 9 * * *'); }); +}); - it('calls onChange when input value changes', () => { - const onChangeMock = jest.fn(); - wrapper = mount( - , - ); - - const changeValue = '1,7 * * * *'; - const event = { - target: { value: changeValue }, - } as React.FocusEvent; - - const inputProps = wrapper.find(Input).props(); - if (inputProps.onBlur) { - inputProps.onBlur(event); - } - expect(onChangeMock).toHaveBeenLastCalledWith(changeValue); +test('input mode updates correctly', async () => { + const onChangeCallback = jest.fn(); + const props = createProps({ + onChange: onChangeCallback, }); + + render(); + + const inputContainer = screen.getByTestId('input-content'); + userEvent.click(screen.getByTestId('input')); + + const input = inputContainer.querySelector( + 'input[name="crontab"]', + ) as HTMLElement; + await waitFor(() => { + expect(input).toBeEnabled(); + }); + + userEvent.clear(input); + expect(input).toHaveValue(''); + + const value = '* 10 2 * *'; + await act(async () => { + await userEvent.type(input, value, { delay: 1 }); + }); + + await waitFor(() => { + expect(input).toHaveValue(value); + }); + + act(() => { + userEvent.click(inputContainer); + }); + + expect(onChangeCallback).toHaveBeenLastCalledWith(value); }); diff --git a/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.tsx b/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.tsx index dd1659f8ba9d..5418842aeaaa 100644 --- a/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.tsx +++ b/superset-frontend/src/views/CRUD/alert/components/AlertReportCronScheduler.tsx @@ -16,27 +16,33 @@ * specific language governing permissions and limitations * under the License. */ -import React, { FunctionComponent, useCallback, useRef, useState } from 'react'; +import React, { useState, useCallback, useRef, FocusEvent } from 'react'; import { t, useTheme } from '@superset-ui/core'; -import { AntdInput } from 'src/components'; +import { AntdInput, RadioChangeEvent } from 'src/components'; import { Input } from 'src/components/Input'; import { Radio } from 'src/components/Radio'; -import { CronError, CronPicker } from 'src/components/CronPicker'; +import { CronPicker, CronError } from 'src/components/CronPicker'; import { StyledInputContainer } from 'src/views/CRUD/alert/AlertReportModal'; -interface AlertReportCronSchedulerProps { +export interface AlertReportCronSchedulerProps { value: string; onChange: (change: string) => any; } -export const AlertReportCronScheduler: FunctionComponent = +export const AlertReportCronScheduler: React.FC = ({ value, onChange }) => { const theme = useTheme(); const inputRef = useRef(null); const [scheduleFormat, setScheduleFormat] = useState<'picker' | 'input'>( 'picker', ); + + const handleRadioButtonChange = useCallback( + (e: RadioChangeEvent) => setScheduleFormat(e.target.value), + [], + ); + const customSetValue = useCallback( (newValue: string) => { onChange(newValue); @@ -44,16 +50,25 @@ export const AlertReportCronScheduler: FunctionComponent) => { + onChange(event.target.value); + }, + [onChange], + ); + + const handlePressEnter = useCallback(() => { + onChange(inputRef.current?.input.value || ''); + }, [onChange]); + const [error, onError] = useState(); return ( <> - setScheduleFormat(e.target.value)} - value={scheduleFormat} - > +
- +
- + CRON Schedule - +
{ - onChange(event.target.value); - }} - onPressEnter={() => { - onChange(inputRef.current?.input.value || ''); - }} + onBlur={handleBlur} + onPressEnter={handlePressEnter} />