From cb23d6f6cf1636486a909d682a3846aed684a449 Mon Sep 17 00:00:00 2001 From: "Eyo O. Eyo" <7893459+eokoneyo@users.noreply.github.com> Date: Fri, 22 Nov 2024 13:35:52 +0100 Subject: [PATCH] [React18] Migrate test suites to account for testing library upgrades ml-ui (#201161) This PR migrates test suites that use `renderHook` from the library `@testing-library/react-hooks` to adopt the equivalent and replacement of `renderHook` from the export that is now available from `@testing-library/react`. This work is required for the planned migration to react18. ## Context In this PR, usages of `waitForNextUpdate` that previously could have been destructured from `renderHook` are now been replaced with `waitFor` exported from `@testing-library/react`, furthermore `waitFor` that would also have been destructured from the same renderHook result is now been replaced with `waitFor` from the export of `@testing-library/react`. ***Why is `waitFor` a sufficient enough replacement for `waitForNextUpdate`, and better for testing values subject to async computations?*** WaitFor will retry the provided callback if an error is returned, till the configured timeout elapses. By default the retry interval is `50ms` with a timeout value of `1000ms` that effectively translates to at least 20 retries for assertions placed within waitFor. See https://testing-library.com/docs/dom-testing-library/api-async/#waitfor for more information. This however means that for person's writing tests, said person has to be explicit about expectations that describe the internal state of the hook being tested. This implies checking for instance when a react query hook is being rendered, there's an assertion that said hook isn't loading anymore. In this PR you'd notice that this pattern has been adopted, with most existing assertions following an invocation of `waitForNextUpdate` being placed within a `waitFor` invocation. In some cases the replacement is simply a `waitFor(() => new Promise((resolve) => resolve(null)))` (many thanks to @kapral18, for point out exactly why this works), where this suffices the assertions that follow aren't placed within a waitFor so this PR doesn't get larger than it needs to be. It's also worth pointing out this PR might also contain changes to test and application code to improve said existing test. ### What to do next? 1. Review the changes in this PR. 2. If you think the changes are correct, approve the PR. ## Any questions? If you have any questions or need help with this PR, please leave comments in this PR. --- .../data_grid/hooks/use_column_chart.test.tsx | 3 +- .../src/hooks/use_timefilter.test.ts | 2 +- .../format_category.test.tsx | 2 +- .../use_copy_to_clipboard_action.test.tsx | 3 +- .../public/hooks/use_filters_query.test.tsx | 2 +- .../field_data_row/use_column_chart.test.tsx | 3 +- .../plugins/ml/common/types/storage.test.tsx | 34 ++++++------------- .../ml/ml_notifications_context.test.tsx | 2 +- .../hooks/use_as_observable.test.ts | 9 ++--- .../application/routing/use_resolver.test.ts | 6 ++-- .../public/app/hooks/use_index_data.test.tsx | 29 ++++++++-------- .../edit_transform_flyout_state.test.tsx | 3 +- .../transform_list/use_actions.test.tsx | 11 +++--- .../transform_list/use_columns.test.tsx | 6 ++-- 14 files changed, 50 insertions(+), 65 deletions(-) diff --git a/x-pack/packages/ml/data_grid/hooks/use_column_chart.test.tsx b/x-pack/packages/ml/data_grid/hooks/use_column_chart.test.tsx index 7a5493eedcfd7..4b8191de1d874 100644 --- a/x-pack/packages/ml/data_grid/hooks/use_column_chart.test.tsx +++ b/x-pack/packages/ml/data_grid/hooks/use_column_chart.test.tsx @@ -6,8 +6,7 @@ */ import React from 'react'; -import { render } from '@testing-library/react'; -import { renderHook } from '@testing-library/react-hooks'; +import { render, renderHook } from '@testing-library/react'; import { KBN_FIELD_TYPES } from '@kbn/field-types'; diff --git a/x-pack/packages/ml/date_picker/src/hooks/use_timefilter.test.ts b/x-pack/packages/ml/date_picker/src/hooks/use_timefilter.test.ts index 4b54b74d900f9..86a2c127939c0 100644 --- a/x-pack/packages/ml/date_picker/src/hooks/use_timefilter.test.ts +++ b/x-pack/packages/ml/date_picker/src/hooks/use_timefilter.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook } from '@testing-library/react'; import { useDatePickerContext } from './use_date_picker_context'; import { useTimefilter } from './use_timefilter'; diff --git a/x-pack/plugins/aiops/public/components/log_categorization/format_category.test.tsx b/x-pack/plugins/aiops/public/components/log_categorization/format_category.test.tsx index 9ed7de75999aa..c93a2de548620 100644 --- a/x-pack/plugins/aiops/public/components/log_categorization/format_category.test.tsx +++ b/x-pack/plugins/aiops/public/components/log_categorization/format_category.test.tsx @@ -7,7 +7,7 @@ import type { Category } from '@kbn/aiops-log-pattern-analysis/types'; import { useCreateFormattedExample } from './format_category'; -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook } from '@testing-library/react'; jest.mock('../../hooks/use_eui_theme', () => ({ useIsDarkTheme: () => false, diff --git a/x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_copy_to_clipboard_action.test.tsx b/x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_copy_to_clipboard_action.test.tsx index 506a92abce550..1d5dbd0dffb1e 100644 --- a/x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_copy_to_clipboard_action.test.tsx +++ b/x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_copy_to_clipboard_action.test.tsx @@ -7,8 +7,7 @@ import type { ReactElement } from 'react'; import userEvent from '@testing-library/user-event'; -import { render } from '@testing-library/react'; -import { renderHook } from '@testing-library/react-hooks'; +import { render, renderHook } from '@testing-library/react'; import type { SignificantItem } from '@kbn/ml-agg-utils'; diff --git a/x-pack/plugins/aiops/public/hooks/use_filters_query.test.tsx b/x-pack/plugins/aiops/public/hooks/use_filters_query.test.tsx index bfea21f9e8bbc..4ff9aaec9c271 100644 --- a/x-pack/plugins/aiops/public/hooks/use_filters_query.test.tsx +++ b/x-pack/plugins/aiops/public/hooks/use_filters_query.test.tsx @@ -6,7 +6,7 @@ */ import { FilterQueryContextProvider, useFilterQueryUpdates } from './use_filters_query'; -import { act, renderHook } from '@testing-library/react-hooks'; +import { renderHook, act } from '@testing-library/react'; import { dataPluginMock as mockDataPlugin } from '@kbn/data-plugin/public/mocks'; import type { TimefilterConfig } from '@kbn/data-plugin/public/query'; import { Timefilter } from '@kbn/data-plugin/public/query'; diff --git a/x-pack/plugins/data_visualizer/public/application/common/components/stats_table/components/field_data_row/use_column_chart.test.tsx b/x-pack/plugins/data_visualizer/public/application/common/components/stats_table/components/field_data_row/use_column_chart.test.tsx index 544ebe261f2e7..55346af2b49ca 100644 --- a/x-pack/plugins/data_visualizer/public/application/common/components/stats_table/components/field_data_row/use_column_chart.test.tsx +++ b/x-pack/plugins/data_visualizer/public/application/common/components/stats_table/components/field_data_row/use_column_chart.test.tsx @@ -6,8 +6,7 @@ */ import React from 'react'; -import { render } from '@testing-library/react'; -import { renderHook } from '@testing-library/react-hooks'; +import { render, renderHook } from '@testing-library/react'; import { KBN_FIELD_TYPES } from '@kbn/data-plugin/public'; diff --git a/x-pack/plugins/ml/common/types/storage.test.tsx b/x-pack/plugins/ml/common/types/storage.test.tsx index a1473b23c5dcd..5f3cc1222547b 100644 --- a/x-pack/plugins/ml/common/types/storage.test.tsx +++ b/x-pack/plugins/ml/common/types/storage.test.tsx @@ -7,7 +7,8 @@ import type { FC } from 'react'; import React from 'react'; -import { renderHook, act } from '@testing-library/react-hooks'; + +import { renderHook, act } from '@testing-library/react'; import type { Storage } from '@kbn/kibana-utils-plugin/public'; import { StorageContextProvider, useStorage } from '@kbn/ml-local-storage'; @@ -61,12 +62,9 @@ describe('useStorage', () => { }); test('updates the storage value', async () => { - const { result, waitForNextUpdate } = renderHook( - () => useStorage('ml.gettingStarted.isDismissed'), - { - wrapper: Provider, - } - ); + const { result } = renderHook(() => useStorage('ml.gettingStarted.isDismissed'), { + wrapper: Provider, + }); const [value, setValue] = result.current; @@ -74,7 +72,6 @@ describe('useStorage', () => { await act(async () => { setValue(false); - await waitForNextUpdate(); }); expect(result.current[0]).toBe(false); @@ -82,12 +79,9 @@ describe('useStorage', () => { }); test('removes the storage value', async () => { - const { result, waitForNextUpdate } = renderHook( - () => useStorage('ml.gettingStarted.isDismissed'), - { - wrapper: Provider, - } - ); + const { result } = renderHook(() => useStorage('ml.gettingStarted.isDismissed'), { + wrapper: Provider, + }); const [value, setValue] = result.current; @@ -95,7 +89,6 @@ describe('useStorage', () => { await act(async () => { setValue(undefined); - await waitForNextUpdate(); }); expect(result.current[0]).toBe(undefined); @@ -103,12 +96,9 @@ describe('useStorage', () => { }); test('updates the value on storage event', async () => { - const { result, waitForNextUpdate } = renderHook( - () => useStorage('ml.gettingStarted.isDismissed'), - { - wrapper: Provider, - } - ); + const { result } = renderHook(() => useStorage('ml.gettingStarted.isDismissed'), { + wrapper: Provider, + }); expect(result.current[0]).toBe(true); @@ -130,7 +120,6 @@ describe('useStorage', () => { newValue: null, }) ); - await waitForNextUpdate(); }); expect(result.current[0]).toBe(undefined); @@ -142,7 +131,6 @@ describe('useStorage', () => { newValue: 'false', }) ); - await waitForNextUpdate(); }); expect(result.current[0]).toBe(false); diff --git a/x-pack/plugins/ml/public/application/contexts/ml/ml_notifications_context.test.tsx b/x-pack/plugins/ml/public/application/contexts/ml/ml_notifications_context.test.tsx index a2623266712dc..b78164804f9c2 100644 --- a/x-pack/plugins/ml/public/application/contexts/ml/ml_notifications_context.test.tsx +++ b/x-pack/plugins/ml/public/application/contexts/ml/ml_notifications_context.test.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { renderHook, act } from '@testing-library/react-hooks'; +import { renderHook, act } from '@testing-library/react'; import { of, throwError } from 'rxjs'; import { useMlNotifications, MlNotificationsContextProvider } from './ml_notifications_context'; import { useStorage } from '@kbn/ml-local-storage'; diff --git a/x-pack/plugins/ml/public/application/hooks/use_as_observable.test.ts b/x-pack/plugins/ml/public/application/hooks/use_as_observable.test.ts index d8680f8393e6e..078300e3733ef 100644 --- a/x-pack/plugins/ml/public/application/hooks/use_as_observable.test.ts +++ b/x-pack/plugins/ml/public/application/hooks/use_as_observable.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { renderHook, act } from '@testing-library/react-hooks'; +import { act, renderHook } from '@testing-library/react'; import { useAsObservable } from './use_as_observable'; describe('useAsObservable', () => { @@ -16,6 +16,8 @@ describe('useAsObservable', () => { test('provides and observable preserving a reference', () => { const { result, rerender } = renderHook(useAsObservable, { initialProps: 1 }); + const initial = result.current; + let observableValue; const subscriptionMock = jest.fn((v) => (observableValue = v)); @@ -27,7 +29,7 @@ describe('useAsObservable', () => { act(() => rerender(1)); - expect(result.all[0]).toStrictEqual(result.all[1]); + expect(initial).toStrictEqual(result.current); expect(subscriptionMock).toHaveBeenCalledWith(1); expect(subscriptionMock).toHaveBeenCalledTimes(1); @@ -35,7 +37,7 @@ describe('useAsObservable', () => { }); test('updates the subject with a new value', async () => { - const { result, rerender, waitForNextUpdate } = renderHook(useAsObservable, { + const { result, rerender } = renderHook(useAsObservable, { initialProps: 'test', }); @@ -50,7 +52,6 @@ describe('useAsObservable', () => { await act(async () => { rerender('test update'); - await waitForNextUpdate(); }); expect(subscriptionMock).toHaveBeenCalledTimes(2); diff --git a/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts b/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts index b3b59c3f12a4b..164cbe1b2d955 100644 --- a/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts +++ b/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { renderHook } from '@testing-library/react-hooks'; +import { waitFor, renderHook } from '@testing-library/react'; import { useMlKibana, useMlLicenseInfo } from '../contexts/kibana'; import { usePermissionCheck } from '../capabilities/check_capabilities'; import { useRouteResolver } from './use_resolver'; @@ -63,8 +63,8 @@ describe('useResolver', () => { it.skip('redirects to the access denied page if some required capabilities are missing', async () => { (usePermissionCheck as jest.Mock).mockReturnValueOnce([false]); - const { waitForNextUpdate } = renderHook(() => useRouteResolver('full', ['canGetCalendars'])); - await waitForNextUpdate(); + renderHook(() => useRouteResolver('full', ['canGetCalendars'])); + await waitFor(() => new Promise((resolve) => resolve(null))); expect(useMlKibana().services.application.navigateToUrl).toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/transform/public/app/hooks/use_index_data.test.tsx b/x-pack/plugins/transform/public/app/hooks/use_index_data.test.tsx index 37e8096f355ed..7e9c1b312b996 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_index_data.test.tsx +++ b/x-pack/plugins/transform/public/app/hooks/use_index_data.test.tsx @@ -7,8 +7,7 @@ import React, { type FC, type PropsWithChildren } from 'react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { render, screen, waitFor } from '@testing-library/react'; -import { renderHook } from '@testing-library/react-hooks'; +import { render, screen, waitFor, renderHook } from '@testing-library/react'; import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; import type { CoreSetup } from '@kbn/core/public'; @@ -47,7 +46,7 @@ describe('Transform: useIndexData()', () => { ); - const { result, waitForNextUpdate } = renderHook( + const { result } = renderHook( () => useIndexData({ dataView: { @@ -62,13 +61,13 @@ describe('Transform: useIndexData()', () => { { wrapper } ); - const IndexObj: UseIndexDataReturnType = result.current; - - await waitForNextUpdate(); + await waitFor(() => { + const IndexObj: UseIndexDataReturnType = result.current; - expect(IndexObj.errorMessage).toBe(''); - expect(IndexObj.status).toBe(INDEX_STATUS.UNUSED); - expect(IndexObj.tableItems).toEqual([]); + expect(IndexObj.errorMessage).toBe(''); + expect(IndexObj.status).toBe(INDEX_STATUS.UNUSED); + expect(IndexObj.tableItems).toEqual([]); + }); }); test('dataView set triggers loading', async () => { @@ -78,7 +77,7 @@ describe('Transform: useIndexData()', () => { ); - const { result, waitForNextUpdate } = renderHook( + const { result } = renderHook( () => useIndexData({ dataView: { @@ -108,11 +107,11 @@ describe('Transform: useIndexData()', () => { const IndexObj: UseIndexDataReturnType = result.current; - await waitForNextUpdate(); - - expect(IndexObj.errorMessage).toBe(''); - expect(IndexObj.status).toBe(INDEX_STATUS.LOADING); - expect(IndexObj.tableItems).toEqual([]); + await waitFor(() => { + expect(IndexObj.errorMessage).toBe(''); + expect(IndexObj.status).toBe(INDEX_STATUS.LOADING); + expect(IndexObj.tableItems).toEqual([]); + }); }); }); diff --git a/x-pack/plugins/transform/public/app/sections/edit_transform/state_management/edit_transform_flyout_state.test.tsx b/x-pack/plugins/transform/public/app/sections/edit_transform/state_management/edit_transform_flyout_state.test.tsx index 7e5b38294b664..c70610017b7d7 100644 --- a/x-pack/plugins/transform/public/app/sections/edit_transform/state_management/edit_transform_flyout_state.test.tsx +++ b/x-pack/plugins/transform/public/app/sections/edit_transform/state_management/edit_transform_flyout_state.test.tsx @@ -6,7 +6,8 @@ */ import React, { type FC, type PropsWithChildren } from 'react'; -import { act, renderHook } from '@testing-library/react-hooks'; + +import { renderHook, act } from '@testing-library/react'; import { getTransformConfigMock } from './__mocks__/transform_config'; diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_actions.test.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_actions.test.tsx index 5007065113782..2805d2d0b3c6e 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_actions.test.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_actions.test.tsx @@ -7,7 +7,7 @@ import React, { type FC, type PropsWithChildren } from 'react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { renderHook } from '@testing-library/react-hooks'; +import { waitFor, renderHook } from '@testing-library/react'; jest.mock('../../../../app_dependencies'); @@ -19,12 +19,11 @@ describe('Transform: Transform List Actions', () => { const wrapper: FC> = ({ children }) => ( {children} ); - const { result, waitForNextUpdate } = renderHook( - () => useActions({ forceDisable: false, transformNodes: 1 }), - { wrapper } - ); + const { result } = renderHook(() => useActions({ forceDisable: false, transformNodes: 1 }), { + wrapper, + }); - await waitForNextUpdate(); + await waitFor(() => new Promise((resolve) => resolve(null))); const actions = result.current.actions; diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_columns.test.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_columns.test.tsx index dcab70822ecd9..04673a386d067 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_columns.test.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/use_columns.test.tsx @@ -7,7 +7,7 @@ import React, { type FC, type PropsWithChildren } from 'react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { renderHook } from '@testing-library/react-hooks'; +import { waitFor, renderHook } from '@testing-library/react'; import { useColumns } from './use_columns'; @@ -19,11 +19,11 @@ describe('Transform: Job List Columns', () => { const wrapper: FC> = ({ children }) => ( {children} ); - const { result, waitForNextUpdate } = renderHook(() => useColumns([], () => {}, 1, [], false), { + const { result } = renderHook(() => useColumns([], () => {}, 1, [], false), { wrapper, }); - await waitForNextUpdate(); + await waitFor(() => new Promise((resolve) => resolve(null))); const columns: ReturnType['columns'] = result.current.columns;