[React18] use waitFor with assertion callbacks in place of waitForNextUpdate by eokoneyo · Pull Request #195087 · elastic/kibana

eokoneyo · 2024-10-04T14:30:50Z

Summary

This PR swaps usages of waitForNextUpdate exported from @testing-library/react-hooks for waitFor exported from @testing-library/react as part of the work being done to upgrade to react 18.

For context; on updating to react 18 there will be a need to update @testing-library/react and in turn the hook renderer, however the hook renderer does not expose the afore-mentioned method waitForNextUpdate hence why we are preemptively adopting this approach.

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(() => null) 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 mentioning that an eslint rule has been included to prevent further usages of waitForNextUpdate and destructuring or directly usages of waitFor from the renderHook function, pending the migration to the newer version of the renderHook to prevent any further work relating to this. Furthermore there were some modification to certain test suites to ensure the test is indeed testing the thing it claimed to test.

kibana-ci · 2024-10-04T15:15:59Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 46da91639803461ce4d6251d24d5e75e4b92b6eb
Interpreting CI Failures

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #7 / global query string useSyncGlobalQueryString deletes unregistered URL params
[job] [logs] Jest Tests #7 / global query string useSyncGlobalQueryString deletes unregistered URL params
[job] [logs] Jest Tests #17 / loadActions hooks useLoadActionsFn filters out disabled actions types
[job] [logs] Jest Tests #17 / loadActions hooks useLoadActionsFn filters out disabled actions types
[job] [logs] Jest Tests #17 / loadActions hooks useLoadActionsFn should throw error when getAction is rejected
[job] [logs] Jest Tests #17 / loadActions hooks useLoadActionsFn should throw error when getAction is rejected
[job] [logs] Jest Tests #18 / Sourcerer Hooks does not call addError if updateSourcererDataView receives an AbortError
[job] [logs] Jest Tests #18 / Sourcerer Hooks does not call addError if updateSourcererDataView receives an AbortError
[job] [logs] Jest Tests #2 / Use get all case configurations hook returns all available configurations
[job] [logs] Jest Tests #2 / Use get case configuration hook returns a configuration matching the owner
[job] [logs] Jest Tests #10 / use_field_value_autocomplete it uses full path name for nested fields to fetch suggestions
[job] [logs] Jest Tests #10 / use_field_value_autocomplete it uses full path name for nested fields to fetch suggestions
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns "isSuggestingValues" of false if field type is boolean
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns "isSuggestingValues" of false if field type is boolean
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns "isSuggestingValues" of false to note that autocomplete service is not in use if no autocomplete suggestions available
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns "isSuggestingValues" of false to note that autocomplete service is not in use if no autocomplete suggestions available
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns new suggestions on subsequent calls
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns new suggestions on subsequent calls
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns suggestions
[job] [logs] Jest Tests #10 / use_field_value_autocomplete returns suggestions
[job] [logs] Jest Tests #2 / useActionTypes should show a toast error message if failed to fetch
[job] [logs] Jest Tests #13 / useAgentSoftLimit should return shouldDisplayAgentSoftLimit:false if soft limit is enabled in config and there is less online agents than the limit
[job] [logs] Jest Tests #13 / useAgentSoftLimit should return shouldDisplayAgentSoftLimit:false if soft limit is enabled in config and there is less online agents than the limit
[job] [logs] Jest Tests #13 / useAgentSoftLimit should return shouldDisplayAgentSoftLimit:true if soft limit is enabled in config and there is more online agents than the limit
[job] [logs] Jest Tests #13 / useAgentSoftLimit should return shouldDisplayAgentSoftLimit:true if soft limit is enabled in config and there is more online agents than the limit
[job] [logs] Jest Tests #9 / useBulkGetMaintenanceWindows calls the api when invoked with the correct parameters
[job] [logs] Jest Tests #7 / useBulkGetUserProfiles hook returns an array of userProfiles
[job] [logs] Jest Tests #7 / useBulkGetUserProfiles hook returns an array of userProfiles
[job] [logs] Jest Tests #2 / useConnectors does not fetch connectors when the user does not has access to actions
[job] [logs] Jest Tests #2 / useConnectors shows a toast error when the API returns error
[job] [logs] Jest Tests #11 / useConversation should create a new conversation when called with valid conversationId and message
[job] [logs] Jest Tests #11 / useConversation should create a new conversation when called with valid conversationId and message
[job] [logs] Jest Tests #11 / useConversation should remove the last message from a conversation when called with valid conversationId
[job] [logs] Jest Tests #11 / useConversation should remove the last message from a conversation when called with valid conversationId
[job] [logs] Jest Tests #17 / useCriticalAlerts loads initial state
[job] [logs] Jest Tests #17 / useCriticalAlerts loads initial state
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should close popover then action executed
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should close popover then action executed
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should execute the action with correct context
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should execute the action with correct context
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should execute the action with correct page value
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should execute the action with correct page value
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should return array with actions for each columns
[job] [logs] Jest Tests #17 / useDataGridColumnsCellActions should return array with actions for each columns
[job] [logs] Jest Tests #11 / useDeleteKnowledgeBase should return delete response
[job] [logs] Jest Tests #11 / useDeleteKnowledgeBase should return delete response
[job] [logs] Jest Tests #14 / useExceptionLists initializes hook
[job] [logs] Jest Tests #14 / useExceptionLists initializes hook
[job] [logs] Jest Tests #3 / useExecutionEvents fetches data from the API
[job] [logs] Jest Tests #3 / useExecutionEvents fetches data from the API
[job] [logs] Jest Tests #3 / useExecutionEvents handles exceptions from the API
[job] [logs] Jest Tests #3 / useExecutionResults fetches data from the API
[job] [logs] Jest Tests #3 / useExecutionResults fetches data from the API
[job] [logs] Jest Tests #3 / useExecutionResults handles exceptions from the API
[job] [logs] Jest Tests #7 / useFetch APM tracking should end aborted
[job] [logs] Jest Tests #7 / useFetch APM tracking should end error
[job] [logs] Jest Tests #7 / useFetch APM tracking should end success
[job] [logs] Jest Tests #7 / useFetch APM tracking should track each request
[job] [logs] Jest Tests #7 / useFetch APM tracking should track each request
[job] [logs] Jest Tests #7 / useFetch APM tracking should track with request name
[job] [logs] Jest Tests #7 / useFetch should abort first request if fetch is called twice
[job] [logs] Jest Tests #13 / useFetchAgentsData should fetch agents and agent policies data
[job] [logs] Jest Tests #13 / useFetchAgentsData should fetch agents and agent policies data
[job] [logs] Jest Tests #13 / useFetchAgentsData sync querystring kuery with current search
[job] [logs] Jest Tests #13 / useFetchAgentsData sync querystring kuery with current search
[job] [logs] Jest Tests #1 / useFetchAlertData does not populate the results when the request is canceled
[job] [logs] Jest Tests #1 / useFetchAlertData does not populate the results when the request is canceled
[job] [logs] Jest Tests #1 / useFetchAlertData initially is not loading and does not have data
[job] [logs] Jest Tests #1 / useFetchAlertData initially is not loading and does not have data
[job] [logs] Jest Tests #1 / useFetchAlertDetail does not populate the results when the request is canceled
[job] [logs] Jest Tests #1 / useFetchAlertDetail does not populate the results when the request is canceled
[job] [logs] Jest Tests #2 / UseFindCaseUserActions returns proper state on findCaseUserActions
[job] [logs] Jest Tests #2 / UseFindCaseUserActions shows a toast error when the API returns an error
[job] [logs] Jest Tests #13 / useFleetServerUnhealthy should call notifications service if an error happen while fetching status
[job] [logs] Jest Tests #13 / useFleetServerUnhealthy should call notifications service if an error happen while fetching status
[job] [logs] Jest Tests #13 / useFleetServerUnhealthy should return isUnHealthy:false with an online fleet slerver
[job] [logs] Jest Tests #13 / useFleetServerUnhealthy should return isUnHealthy:false with an online fleet slerver
[job] [logs] Jest Tests #13 / useFleetServerUnhealthy should return isUnHealthy:true with only one offline fleet slerver
[job] [logs] Jest Tests #13 / useFleetServerUnhealthy should return isUnHealthy:true with only one offline fleet slerver
[job] [logs] Jest Tests #2 / useGetActionLicense unhappy path
[job] [logs] Jest Tests #2 / useGetCaseFiles shows an error toast when filesClient.list throws
[job] [logs] Jest Tests #2 / useGetCaseFileStats shows an error toast when filesClient.list throws
[job] [logs] Jest Tests #2 / useGetCaseMetrics shows an error toast when getSingleCaseMetrics throws
[job] [logs] Jest Tests #2 / useGetCasesMetrics shows a toast error when the api return an error
[job] [logs] Jest Tests #2 / useGetCaseUserActionsStats returns proper state on getCaseUserActionsStats
[job] [logs] Jest Tests #2 / useGetCaseUsers shows a toast error when the api return an error
[job] [logs] Jest Tests #2 / useGetCategories displays an error toast when an error occurs
[job] [logs] Jest Tests #7 / useGetCurrentUserProfile hook returns current user
[job] [logs] Jest Tests #7 / useGetCurrentUserProfile hook returns current user
[job] [logs] Jest Tests #2 / useGetTags displays and error toast when an error occurs
[job] [logs] Jest Tests #8 / useGrouping Renders child component with grouping table wrapper when group is selected
[job] [logs] Jest Tests #8 / useGrouping Renders child component with grouping table wrapper when group is selected
[job] [logs] Jest Tests #8 / useGrouping Renders child component without grouping table wrapper when no group is selected
[job] [logs] Jest Tests #8 / useGrouping Renders child component without grouping table wrapper when no group is selected
[job] [logs] Jest Tests #2 / UseInfiniteFindCaseUserActions fetches next page with correct params
[job] [logs] Jest Tests #2 / UseInfiniteFindCaseUserActions returns proper state on findCaseUserActions
[job] [logs] Jest Tests #2 / UseInfiniteFindCaseUserActions shows a toast error when the API returns an error
[job] [logs] Jest Tests #7 / useInstalledSecurityJobs when the user has permissions filters out non-security jobs
[job] [logs] Jest Tests #7 / useInstalledSecurityJobs when the user has permissions renders a toast error if the ML call fails
[job] [logs] Jest Tests #7 / useInstalledSecurityJobs when the user has permissions returns jobs and permissions
[job] [logs] Jest Tests #7 / useInstalledSecurityJobs when the user has permissions returns jobs and permissions
[job] [logs] Jest Tests #8 / useIntegrations fetches data from the API
[job] [logs] Jest Tests #8 / useIntegrations fetches data from the API
[job] [logs] Jest Tests #9 / useLoadRuleAggregations should call loadRuleAggregation API with params and handle result
[job] [logs] Jest Tests #9 / useLoadRuleAggregations should call loadRuleAggregations API and handle result
[job] [logs] Jest Tests #9 / useLoadRuleAggregations should call onError if API fails
[job] [logs] Jest Tests #9 / useLoadRules No data hasData should be false, if there is no Filter and no rules
[job] [logs] Jest Tests #9 / useLoadRules No data hasData should be false, if there is rule types filter and no rules with hasDefaultRuleTypesFiltersOn = true
[job] [logs] Jest Tests #9 / useLoadRules should call loadRules API and handle result
[job] [logs] Jest Tests #9 / useLoadRules should call loadRules API with params and handle result
[job] [logs] Jest Tests #9 / useLoadRules should call onError if API fails
[job] [logs] Jest Tests #9 / useLoadRules should reset the page if the data is fetched while paged
[job] [logs] Jest Tests #9 / useLoadTagsQuery should call loadRuleTags API and handle result
[job] [logs] Jest Tests #9 / useLoadTagsQuery should call onError if API fails
[job] [logs] Jest Tests #9 / useLoadTagsQuery should support pagination
[job] [logs] Jest Tests #9 / useLoadTagsQuery should support pagination when there are no tags
[job] [logs] Jest Tests #6 / useNotesInFlyout should close flyout correctly
[job] [logs] Jest Tests #6 / useNotesInFlyout should close the flyout when activeTab is changed
[job] [logs] Jest Tests #6 / useNotesInFlyout should return correct array of notes based on Events
[job] [logs] Jest Tests #6 / useNotesInFlyout should return correct instance of associate Note
[job] [logs] Jest Tests #6 / useNotesInFlyout should show flyout when eventId is not undefined
[job] [logs] Jest Tests #14 / usePersistExceptionItem "isSaved" is "true" when exception item saved successfully
[job] [logs] Jest Tests #14 / usePersistExceptionItem "isSaved" is "true" when exception item saved successfully
[job] [logs] Jest Tests #14 / usePersistExceptionItem "onError" callback is invoked and "isSaved" is "false" when api call fails
[job] [logs] Jest Tests #14 / usePersistExceptionItem "onError" callback is invoked and "isSaved" is "false" when api call fails
[job] [logs] Jest Tests #14 / usePersistExceptionItem it invokes "addExceptionListItem" when payload does not have "id"
[job] [logs] Jest Tests #14 / usePersistExceptionItem it invokes "addExceptionListItem" when payload does not have "id"
[job] [logs] Jest Tests #14 / usePersistExceptionItem it invokes "updateExceptionListItem" when payload has "id"
[job] [logs] Jest Tests #14 / usePersistExceptionItem it invokes "updateExceptionListItem" when payload has "id"
[job] [logs] Jest Tests #14 / usePersistExceptionList "isSaved" is "true" when exception item saved successfully
[job] [logs] Jest Tests #14 / usePersistExceptionList "isSaved" is "true" when exception item saved successfully
[job] [logs] Jest Tests #14 / usePersistExceptionList "onError" callback is invoked and "isSaved" is "false" when api call fails
[job] [logs] Jest Tests #14 / usePersistExceptionList "onError" callback is invoked and "isSaved" is "false" when api call fails
[job] [logs] Jest Tests #14 / usePersistExceptionList it invokes "updateExceptionList" when payload has "id"
[job] [logs] Jest Tests #14 / usePersistExceptionList it invokes "updateExceptionList" when payload has "id"
[job] [logs] Jest Tests #8 / useQueryAlerts fetch alert when query object changed
[job] [logs] Jest Tests #8 / useQueryAlerts fetch alert when query object changed
[job] [logs] Jest Tests #7 / useSecurityJobs when user has valid permissions combines multiple ML calls into an array of SecurityJobs
[job] [logs] Jest Tests #9 / useStats fetch rejects with an error it returns loading: false, because data loading reached a terminal state
[job] [logs] Jest Tests #9 / useStats fetch rejects with an error it returns null stats, because an error occurred
[job] [logs] Jest Tests #9 / useStats fetch rejects with an error it returns the expected error
[job] [logs] Jest Tests #9 / useStats query with date range when ILM is not available it calls the stats api with the expected params
[job] [logs] Jest Tests #9 / useStats successful response from the stats api it calls the stats api with the expected params
[job] [logs] Jest Tests #9 / useStats successful response from the stats api it returns a null error, because no errors occurred
[job] [logs] Jest Tests #9 / useStats successful response from the stats api it returns loading: false, because the data has loaded
[job] [logs] Jest Tests #9 / useStats successful response from the stats api it returns the expected stats
[job] [logs] Jest Tests #7 / useSuggestUsers hook returns an array of userProfiles
[job] [logs] Jest Tests #7 / useSuggestUsers hook returns an array of userProfiles
[job] [logs] Jest Tests #6 / useTimelineEvents Correlation pagination is calling search strategy when switching page
[job] [logs] Jest Tests #6 / useTimelineEvents init
[job] [logs] Jest Tests #7 / useTimelineLastEventTime should init
[job] [logs] Jest Tests #7 / useTimelineLastEventTime should init
[job] [logs] Jest Tests #8 / useUserInfo calls createSignalIndex if signal index template is outdated
[job] [logs] Jest Tests #8 / useUserInfo calls createSignalIndex if signal index template is outdated

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @eokoneyo

sabarasaba

kibana management changes lgtm 🚀

…library/react

…fix'

elasticmachine · 2024-11-11T23:53:07Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 03243d9
Storybooks Preview
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-195087-03243d959878

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #44 / EPM Endpoints Installs a package using stream-based approach security_detection_engine package should install security-rule assets from the package
[job] [logs] FTR Configs #44 / EPM Endpoints Installs a package using stream-based approach security_detection_engine package should install security-rule assets from the package

History

💔 Build #249914 failed b6887e931e5b583dd3c194ad0545e1342b370538
💛 Build #249544 was flaky 6979ee6a43e965527e6dabe310e8d85f106010b5
💔 Build #248936 failed 21c7e82472a97b4ee804aad9a3110ff44db710ff
💔 Build #248782 failed 6e436ae5bc8ea78a38426eb047d58797c84a9c07

cc @eokoneyo

eokoneyo · 2024-11-21T17:17:07Z

Hi all, thanks for all the feedback... given that initially I'd had to patch @testing-library/react to get over some issues that were brought that impacted dev experience, we thought it'd be valuable to just make the switch to upgrade the aforementioned library and not need the followup work that would have had to come after this particular PR got merged in.
That been said, considering the preliminary work that had been done, it was much easier to figure out the files that would get changes, this then informed the approach to adopt smaller PRs that specifically that contain changes specific to the teams that own the files changed, for this reason I'll be closing this PR in favour of much smaller team targeted PRs.

eokoneyo self-assigned this Oct 4, 2024

eokoneyo added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Platform AppEx-SharedUX (formerly Global Experience) t// labels Oct 4, 2024

This comment was marked as outdated.

Sign in to view

eokoneyo force-pushed the chore/switch-waitForNextUpdate-for-waitFor branch from dbc6b43 to eef0bd1 Compare October 8, 2024 15:37