From 5af36e48ba1d7270311812363b5dff85d255f2cb Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Thu, 13 Oct 2022 08:56:36 +0100 Subject: [PATCH] convert ThresholdsEditor to RTL (#56791) --- .betterer.results | 7 +- .../ThresholdsEditor.test.tsx | 311 ++++++++---------- .../ThresholdsEditor/ThresholdsEditor.tsx | 10 +- 3 files changed, 152 insertions(+), 176 deletions(-) diff --git a/.betterer.results b/.betterer.results index 662f3bb90d2..5350fdb80f7 100644 --- a/.betterer.results +++ b/.betterer.results @@ -40,9 +40,6 @@ exports[`no enzyme tests`] = { ], "packages/jaeger-ui-components/src/TraceTimelineViewer/VirtualizedTraceView.test.js:551014442": [ [13, 26, 13, "RegExp match", "2409514259"] - ], - "public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.test.tsx:145048794": [ - [0, 17, 13, "RegExp match", "2409514259"] ] }` }; @@ -4114,9 +4111,7 @@ exports[`better eslint`] = { "public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"] + [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], "public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] diff --git a/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.test.tsx b/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.test.tsx index 49fc77ebd96..8b5c640ed26 100644 --- a/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.test.tsx +++ b/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.test.tsx @@ -1,32 +1,24 @@ -import { mount } from 'enzyme'; -import React, { ChangeEvent } from 'react'; +import { render, screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; import { createTheme, ThresholdsMode } from '@grafana/data'; -import { mockThemeContext, colors } from '@grafana/ui'; +import { mockThemeContext } from '@grafana/ui'; -import { ThresholdsEditor, Props, thresholdsWithoutKey } from './ThresholdsEditor'; +import { ThresholdsEditor, Props } from './ThresholdsEditor'; + +let props: Props; const setup = (propOverrides?: Partial) => { - const props: Props = { + props = { onChange: jest.fn(), thresholds: { mode: ThresholdsMode.Absolute, steps: [] }, }; - Object.assign(props, propOverrides); - const wrapper = mount(); - const instance = wrapper.instance() as ThresholdsEditor; - - return { - instance, - wrapper, - }; + render(); }; -function getCurrentThresholds(editor: ThresholdsEditor) { - return thresholdsWithoutKey(editor.props.thresholds, editor.state.steps); -} - describe('ThresholdsEditor', () => { let restoreThemeContext: () => void; @@ -38,181 +30,164 @@ describe('ThresholdsEditor', () => { restoreThemeContext(); }); - it('should render with base threshold', () => { - const { wrapper } = setup(); - expect(wrapper.find('input').length).toBe(3); + it('should render with an uneditable base threshold', () => { + setup(); + const baseThreshold = screen.getByRole('textbox', { name: 'Threshold 1' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); }); - describe('Initialization', () => { - it('should add a base threshold if missing', () => { - const { instance } = setup(); - expect(getCurrentThresholds(instance).steps).toEqual([{ value: -Infinity, color: 'green' }]); - }); + it('should have an "Add threshold" button', () => { + setup(); + const button = screen.getByRole('button', { name: 'Add threshold' }); + expect(button).toBeInTheDocument(); }); - describe('Add threshold', () => { - it('should add threshold', () => { - const { instance } = setup(); + it('can add thresholds', async () => { + setup(); - instance.onAddThreshold(); + expect(screen.queryAllByRole('spinbutton')).toHaveLength(0); - expect(getCurrentThresholds(instance).steps).toEqual([ - { value: -Infinity, color: 'green' }, // 0 - { value: 0, color: colors[1] }, // 1 - ]); - }); + let baseThreshold = screen.getByRole('textbox', { name: 'Threshold 1' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); - it('should add another threshold above last', () => { - const { instance } = setup({ - thresholds: { - mode: ThresholdsMode.Absolute, - steps: [ - { value: -Infinity, color: colors[0] }, // 0 - { value: 50, color: colors[2] }, // 1 - ], - }, - }); + await userEvent.click(screen.getByRole('button', { name: 'Add threshold' })); - instance.onAddThreshold(); + expect(screen.getAllByRole('spinbutton')).toHaveLength(1); - expect(getCurrentThresholds(instance).steps).toEqual([ - { value: -Infinity, color: colors[0] }, // 0 - { value: 50, color: colors[2] }, // 1 - { value: 60, color: colors[3] }, // 2 - ]); - }); + let customThreshold = screen.getByRole('spinbutton', { name: 'Threshold 1' }); + expect(customThreshold).toBeInTheDocument(); + expect(customThreshold).not.toBeDisabled(); + expect(customThreshold).toHaveValue(0); + + baseThreshold = screen.getByRole('textbox', { name: 'Threshold 2' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); + + await userEvent.click(screen.getByRole('button', { name: 'Add threshold' })); + + expect(screen.getAllByRole('spinbutton')).toHaveLength(2); + + let customThreshold2 = screen.getByRole('spinbutton', { name: 'Threshold 1' }); + expect(customThreshold2).toBeInTheDocument(); + expect(customThreshold2).not.toBeDisabled(); + expect(customThreshold2).toHaveValue(10); + + customThreshold = screen.getByRole('spinbutton', { name: 'Threshold 2' }); + expect(customThreshold).toBeInTheDocument(); + expect(customThreshold).not.toBeDisabled(); + expect(customThreshold).toHaveValue(0); + + baseThreshold = screen.getByRole('textbox', { name: 'Threshold 3' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); }); - describe('Remove threshold', () => { - it('should not remove threshold at index 0', () => { - const thresholds = { - mode: ThresholdsMode.Absolute, - steps: [ - { value: -Infinity, color: '#7EB26D' }, - { value: 50, color: '#EAB839' }, - { value: 75, color: '#6ED0E0' }, - ], - }; - const { instance } = setup({ thresholds }); - - instance.onRemoveThreshold(instance.state.steps[0]); - - expect(getCurrentThresholds(instance)).toEqual(thresholds); - }); - - it('should remove threshold', () => { - const thresholds = { - mode: ThresholdsMode.Absolute, - steps: [ - { value: -Infinity, color: '#7EB26D' }, - { value: 50, color: '#EAB839' }, - { value: 75, color: '#6ED0E0' }, - ], - }; - const { instance } = setup({ thresholds }); - - instance.onRemoveThreshold(instance.state.steps[1]); - - expect(getCurrentThresholds(instance).steps).toEqual([ + it('can remove thresholds', async () => { + const thresholds = { + mode: ThresholdsMode.Absolute, + steps: [ { value: -Infinity, color: '#7EB26D' }, + { value: 50, color: '#EAB839' }, { value: 75, color: '#6ED0E0' }, - ]); - }); + ], + }; + setup({ thresholds }); + + expect(screen.getAllByRole('spinbutton')).toHaveLength(2); + + let customThreshold2 = screen.getByRole('spinbutton', { name: 'Threshold 1' }); + expect(customThreshold2).toBeInTheDocument(); + expect(customThreshold2).not.toBeDisabled(); + expect(customThreshold2).toHaveValue(75); + + let customThreshold = screen.getByRole('spinbutton', { name: 'Threshold 2' }); + expect(customThreshold).toBeInTheDocument(); + expect(customThreshold).not.toBeDisabled(); + expect(customThreshold).toHaveValue(50); + + let baseThreshold = screen.getByRole('textbox', { name: 'Threshold 3' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); + + await userEvent.click(screen.getByRole('button', { name: 'Remove Threshold 1' })); + + expect(screen.getAllByRole('spinbutton')).toHaveLength(1); + + customThreshold = screen.getByRole('spinbutton', { name: 'Threshold 1' }); + expect(customThreshold).toBeInTheDocument(); + expect(customThreshold).not.toBeDisabled(); + expect(customThreshold).toHaveValue(50); + + baseThreshold = screen.getByRole('textbox', { name: 'Threshold 2' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); + + await userEvent.click(screen.getByRole('button', { name: 'Remove Threshold 1' })); + + expect(screen.queryAllByRole('spinbutton')).toHaveLength(0); + + baseThreshold = screen.getByRole('textbox', { name: 'Threshold 1' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); }); - describe('change threshold value', () => { - it('should not change threshold at index 0', () => { - const thresholds = { - mode: ThresholdsMode.Absolute, - steps: [ - { value: -Infinity, color: '#7EB26D' }, - { value: 50, color: '#EAB839' }, - { value: 75, color: '#6ED0E0' }, - ], - }; - const { instance } = setup({ thresholds }); + it('can not remove the base threshold', () => { + setup(); - const mockEvent = { target: { value: '12' } } as any as ChangeEvent; + const baseThreshold = screen.getByRole('textbox', { name: 'Threshold 1' }); + expect(within(baseThreshold).queryByRole('button', { name: /remove/i })).not.toBeInTheDocument(); + }); - instance.onChangeThresholdValue(mockEvent, instance.state.steps[0]); + it('sorts thresholds when values change', async () => { + const thresholds = { + mode: ThresholdsMode.Absolute, + steps: [ + { value: -Infinity, color: '#7EB26D', key: 1 }, + { value: 50, color: '#EAB839', key: 2 }, + { value: 75, color: '#6ED0E0', key: 3 }, + ], + }; + setup({ thresholds }); - expect(getCurrentThresholds(instance)).toEqual(thresholds); - }); + expect(screen.getByRole('spinbutton', { name: 'Threshold 1' })).toHaveValue(75); + expect(screen.getByRole('spinbutton', { name: 'Threshold 2' })).toHaveValue(50); - it('should update value', () => { - const { instance } = setup(); - const thresholds = { + await userEvent.clear(screen.getByRole('spinbutton', { name: 'Threshold 2' })); + await userEvent.type(screen.getByRole('spinbutton', { name: 'Threshold 2' }), '100'); + + expect(screen.getByRole('spinbutton', { name: 'Threshold 1' })).toHaveValue(100); + expect(screen.getByRole('spinbutton', { name: 'Threshold 2' })).toHaveValue(75); + }); + + it('should exclude invalid steps and render a proper list', () => { + setup({ + thresholds: { mode: ThresholdsMode.Absolute, steps: [ { value: -Infinity, color: '#7EB26D', key: 1 }, - { value: 50, color: '#EAB839', key: 2 }, - { value: 75, color: '#6ED0E0', key: 3 }, + { value: 75, color: '#6ED0E0', key: 2 }, + { color: '#7EB26D', key: 3 } as any, + { value: 78, color: '#EAB839', key: 4 }, + { value: null, color: '#7EB26D', key: 5 } as any, + { value: null, color: '#7EB26D', key: 6 } as any, ], - }; - - instance.state = { - steps: thresholds.steps, - }; - - const mockEvent = { target: { value: '78' } } as any as ChangeEvent; - - instance.onChangeThresholdValue(mockEvent, thresholds.steps[1]); - - expect(getCurrentThresholds(instance).steps).toEqual([ - { value: -Infinity, color: '#7EB26D' }, - { value: 75, color: '#6ED0E0' }, - { value: 78, color: '#EAB839' }, - ]); + }, }); - }); - describe('on blur threshold value', () => { - it('should resort rows and update indexes', () => { - const { instance } = setup(); - const thresholds = { - mode: ThresholdsMode.Absolute, - steps: [ - { value: -Infinity, color: '#7EB26D', key: 1 }, - { value: 78, color: '#EAB839', key: 2 }, - { value: 75, color: '#6ED0E0', key: 3 }, - ], - }; - - instance.setState({ - steps: thresholds.steps, - }); - - instance.onBlur(); - - expect(getCurrentThresholds(instance).steps).toEqual([ - { value: -Infinity, color: '#7EB26D' }, - { value: 75, color: '#6ED0E0' }, - { value: 78, color: '#EAB839' }, - ]); - }); - }); - - describe('on load with invalid steps', () => { - it('should exclude invalid steps and render a proper list', () => { - const { instance } = setup({ - thresholds: { - mode: ThresholdsMode.Absolute, - steps: [ - { value: -Infinity, color: '#7EB26D', key: 1 }, - { value: 75, color: '#6ED0E0', key: 2 }, - { color: '#7EB26D', key: 3 } as any, - { value: 78, color: '#EAB839', key: 4 }, - { value: null, color: '#7EB26D', key: 5 } as any, - { value: null, color: '#7EB26D', key: 6 } as any, - ], - }, - }); - - expect(getCurrentThresholds(instance).steps).toEqual([ - { value: -Infinity, color: '#7EB26D' }, - { value: 75, color: '#6ED0E0' }, - { value: 78, color: '#EAB839' }, - ]); - }); + expect(screen.getByRole('spinbutton', { name: 'Threshold 1' })).toHaveValue(78); + expect(screen.getByRole('spinbutton', { name: 'Threshold 2' })).toHaveValue(75); + const baseThreshold = screen.getByRole('textbox', { name: 'Threshold 3' }); + expect(baseThreshold).toBeInTheDocument(); + expect(baseThreshold).toBeDisabled(); + expect(baseThreshold).toHaveValue('Base'); }); }); diff --git a/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.tsx b/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.tsx index 2e1c44a78eb..f50b0dd8472 100644 --- a/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.tsx +++ b/public/app/features/dimensions/editors/ThresholdsEditor/ThresholdsEditor.tsx @@ -14,12 +14,12 @@ import { Input, colors, ColorPicker, - Icon, ThemeContext, Button, Label, RadioButtonGroup, stylesFactory, + IconButton, } from '@grafana/ui'; const modes: Array> = [ @@ -198,7 +198,12 @@ export class ThresholdsEditor extends PureComponent { } suffix={ - this.onRemoveThreshold(threshold)} /> + this.onRemoveThreshold(threshold)} + /> } /> ); @@ -326,6 +331,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme): ThresholdStyles => { trashIcon: css` color: ${theme.colors.textWeak}; cursor: pointer; + margin-right: 0; &:hover { color: ${theme.colors.text};