Upgrade `@testing-library/user-event` to latest `^14.5.2` #189949

walterra · 2024-08-06T08:53:46Z

Summary

Upgrades @testing-library/user-event to ^14.5.2. See the release notes for v14 for breaking changes: https://github.com/testing-library/user-event/releases/tag/v14.0.0

I was facing an issue with v13.5.0 with userEvent.click() in a PR (#189729) and was able to verify that v14.4.3 onwards fixes it so I decided to update that package. What a rabbit hole 😅 !

In user-event v14 events return a promise, so this PR updates usage of the likes of userEvent.click with await userEvent.click. Regex to search for userEvent calls that miss await except .setup: (?<!await\s)userEvent\.(?!setup\b)
The way to handle pointer events needed changing from , undefined, { skipPointerEventsCheck: true }); to , { pointerEventsCheck: 0 });.
I tried a bit to do the refactor with codemods, but there were quite some edge cases so it ended up being done manually.
I looked into all failing tests and tried my best to update them, but for some of them I lacked the context to make them work again. If you're a code owner and find a skipped test in this PR please give it a try to fix and push in this PR or let me know if it's fine for you to fix in follow ups.

List of files where I had to skip tests (git diff main...HEAD -G'\.skip' --name-only):

`packages/kbn-dom-drag-drop`

packages/kbn-dom-drag-drop/src/droppable.test.tsx

`x-pack/plugins/cases`

x-pack/plugins/cases/public/components/templates/form.test.tsx
x-pack/plugins/cases/public/components/user_actions/user_actions_list.test.tsx

`x-pack/plugins/cloud_security_posture`

x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/policy_template_form.test.tsx

`x-pack/plugins/lens`

x-pack/plugins/lens/public/datasources/form_based/dimension_panel/format_selector.test.tsx

`x-pack/plugins/observability_solution`

x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/monitor_add_edit/fields/request_body_field.test.tsx

`x-pack/plugins/security_solution`

x-pack/plugins/security_solution/public/management/components/console/components/command_input/integration_tests/command_input.test.tsx
x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/kill_process_action.test.tsx
x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/release_action.test.tsx
x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/status_action.test.tsx
x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/upload_action.test.tsx
x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx
x-pack/plugins/security_solution/public/management/pages/event_filters/view/components/event_filters_flyout.test.tsx
x-pack/plugins/security_solution/public/management/pages/response_actions/view/response_actions_list_page.test.tsx

I plan to do a talk on Kibana Demo Days to walk through some of the breaking changes and learnings.

Checklist

Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
This was checked for breaking API changes and was labeled appropriately

## Summary During `yarn kbn bootstrap` we get the following message: > @testing-library/user-event@13.5.0" has unmet peer dependency "@testing-library/dom@>=7.21.4". This PR adds `@testing-library/dom` version `8.19.0` to `package.json` to match the version used by `@testing-library/react`. Future versions of `@testing-library/react` switched to requiring this dependency to be a peer dependency too. This PR is in preparation for #189949. ### Checklist - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

…_rule_editor/slo_selector

paul-tavares

Thank you for all of the changes to the x-pack/plugins/security_solution/public/management/** area.

I left some comments that I would like to hear from you on; mainly around: if we can avoid some changes to test mock utility function signatures (the pass in of user (UserEvent) and avoiding having to change tests to add { submitClick: true } to several calls of enterCommand().

(FYI: On our side, I have created issue security team issue no. 10470 for follow up on skipped tests in this PR)

x-pack/plugins/security_solution/public/management/components/console/mocks.tsx

.../endpoint_responder/command_render_components/integration_tests/kill_process_action.test.tsx

...nents/endpoint_responder/command_render_components/integration_tests/release_action.test.tsx

...onents/endpoint_responder/command_render_components/integration_tests/status_action.test.tsx

...nt/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx

paul-tavares · 2024-09-06T13:48:52Z

...olution/public/management/pages/endpoint_hosts/view/details/components/actions_menu.test.tsx

@@ -42,7 +42,7 @@ describe('When using the Endpoint Details Actions Menu', () => {
  let coreStart: AppContextTestRender['coreStart'];
  let renderResult: ReturnType<AppContextTestRender['render']>;
  let httpMocks: ReturnType<typeof endpointPageHttpMock>;
-  let middlewareSpy: AppContextTestRender['middlewareSpy'];
+  // let middlewareSpy: AppContextTestRender['middlewareSpy'];


I see you left a few commented out code on this one. Are you still following up? I did not see any tests skipped below.

I just tried again and I wasn't able to solve this unfortunately. With userEvent now being async there might now be a chicken/egg problem with the way this mock is set up with the promise to wait for the middleware event. I suspect it might already have triggered when we try to wait for it. With regular mocks we could do shouldHaveBeenCalled after it was called, but I wasn't able to get it to work with this custom approach. I added in some more comments that reference this PR in a803117. Maybe you can solve it in a follow up? In this case I didn't skip the whole test but just commented the middleware assertion. The rest of the test seems to pass.

Hmmm. if the tests are passing, then maybe we no longer need it.
Its been a long time, but I seem to remember creating this middleware "spy" to ensure that we did not do test assertion too soon (before the middleware was able to do what it needed to do). It address test flakiness mostly.

I'll note this in the internal issue I created so we can revisit... but maybe we no longer need it.

paul-tavares · 2024-09-06T13:50:31Z

...solution/public/management/pages/event_filters/view/components/event_filters_flyout.test.tsx

      expect(onCancelMock).toHaveBeenCalledTimes(0);
    });

-    it('should close when exception has been submitted successfully and close flyout', () => {
+    // TODO: Find out why this test passes when run via `it.only()` but fails when run with all tests.


I'll add this to an internal issue for follow up.
Based your comment, my guess is that there is some state sharing between tests (😱 )

…management/pages/endpoint_hosts/view/details/components/actions_menu.test.tsx

rylnd

Detection Engine changes (the addition of some missing awaits) LGTM !

walterra · 2024-09-06T16:30:56Z

@paul-tavares Thanks for the feedback, I addressed your individual comments. I don't think there's a way around not passing user into functions like enterConsoleCommand, tried to answer that in one of the comments too.

paul-tavares

Thanks @walterra for the follow up updates.

Approving on behave of the security-defend-workflows team 👍

x-pack/plugins/security_solution/public/management/components/console/mocks.tsx

paul-tavares · 2024-09-06T20:12:54Z

x-pack/plugins/security_solution/public/management/components/console/mocks.tsx

+    // so this uses fireEvent instead for the time being.
+    // See here for a related discussion: https://github.com/testing-library/user-event/discussions/1164
+    // await user.keyboard('[Enter]');
+    fireEvent.keyDown(keyCaptureInput, { key: 'enter', keyCode: 13 });


awesome. Thank you

paul-tavares · 2024-09-06T20:21:45Z

...olution/public/management/pages/endpoint_hosts/view/details/components/actions_menu.test.tsx

@@ -42,7 +42,7 @@ describe('When using the Endpoint Details Actions Menu', () => {
  let coreStart: AppContextTestRender['coreStart'];
  let renderResult: ReturnType<AppContextTestRender['render']>;
  let httpMocks: ReturnType<typeof endpointPageHttpMock>;
-  let middlewareSpy: AppContextTestRender['middlewareSpy'];
+  // let middlewareSpy: AppContextTestRender['middlewareSpy'];


Hmmm. if the tests are passing, then maybe we no longer need it.
Its been a long time, but I seem to remember creating this middleware "spy" to ensure that we did not do test assertion too soon (before the middleware was able to do what it needed to do). It address test flakiness mostly.

I'll note this in the internal issue I created so we can revisit... but maybe we no longer need it.

andrew-goldstein

elastic/security-generative-ai changes LGTM:

elastic/security-generative-ai

x-pack/packages/kbn-elastic-assistant/impl/assistant/context_pills/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/assistant/prompt_editor/selected_prompt_contexts/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/assistant/prompt_editor/system_prompt/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/assistant/prompt_editor/system_prompt/select_system_prompt/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/bulk_actions/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/context_editor/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/stats/allowed_stat/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/stats/anonymized_stat/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/data_anonymization_editor/stats/available_stat/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/new_chat/index.test.tsx
x-pack/packages/kbn-elastic-assistant/impl/new_chat_by_title/index.test.tsx
x-pack/plugins/security_solution/public/assistant/update_query_in_form/index.test.tsx
x-pack/plugins/security_solution/public/attack_discovery/tour/video_toast.test.tsx
x-pack/plugins/stack_connectors/public/connector_types/bedrock/connector.test.tsx
x-pack/plugins/stack_connectors/public/connector_types/gemini/connector.test.tsx
x-pack/plugins/stack_connectors/public/connector_types/openai/connector.test.tsx

jaredburgettelastic

Entity Analytics changes LGTM!

walterra · 2024-09-10T08:19:28Z

Hey @elastic/security-service-integrations @elastic/security-threat-hunting-investigations teams, would be great to get some code owner's reviews on this one please 👋

kibana-ci · 2024-09-10T09:31:53Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: a355d0a
Storybooks Preview
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-189949-a355d0a0c9ab

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	538	540	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	623	625	+2

History

💚 Build #232755 succeeded c36ce84
💛 Build #232542 was flaky 0d75919
💚 Build #232290 succeeded 8669579
💛 Build #232192 was flaky 5a9b526
💛 Build #232117 was flaky c2e7a95

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

cc @walterra

## Summary This test (#180931) was introduced right before the version bump of user-events (#189949). This PR updates the one async call to be awaited, hopefully fixing the test. Solves: #192475 Unskipped tests are passing now: https://buildkite.com/elastic/kibana-pull-request/builds/233177 --------- Co-authored-by: Antonio <antoniodcoelho@gmail.com>

…9949) ## Summary Upgrades `@testing-library/user-event` to `^14.5.2`. See the release notes for `v14` for breaking changes: https://github.com/testing-library/user-event/releases/tag/v14.0.0 I was facing an [issue](testing-library/user-event#662) with `v13.5.0` with `userEvent.click()` in a PR (elastic#189729) and was able to verify that `v14.4.3` onwards fixes it so I decided to update that package. What a rabbit hole 😅 ! - In `user-event` `v14` events return a promise, so this PR updates usage of the likes of `userEvent.click` with `await userEvent.click`. Regex to search for `userEvent` calls that miss `await` except `.setup`: `(?<!await\s)userEvent\.(?!setup\b)` - The way to handle pointer events needed changing from `, undefined, { skipPointerEventsCheck: true });` to `, { pointerEventsCheck: 0 });`. - I tried a bit to do the refactor with codemods, but there were quite some edge cases so it ended up being done manually. - I looked into all failing tests and tried my best to update them, but for some of them I lacked the context to make them work again. If you're a code owner and find a skipped test in this PR please give it a try to fix and push in this PR or let me know if it's fine for you to fix in follow ups. List of files where I had to skip tests (`git diff main...HEAD -G'\.skip' --name-only`): ### `packages/kbn-dom-drag-drop` - `packages/kbn-dom-drag-drop/src/droppable.test.tsx` ### `x-pack/plugins/cases` - `x-pack/plugins/cases/public/components/templates/form.test.tsx` - `x-pack/plugins/cases/public/components/user_actions/user_actions_list.test.tsx` ### `x-pack/plugins/cloud_security_posture` - `x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/policy_template_form.test.tsx` ### `x-pack/plugins/lens` - `x-pack/plugins/lens/public/datasources/form_based/dimension_panel/format_selector.test.tsx` ### `x-pack/plugins/observability_solution` - `x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/monitor_add_edit/fields/request_body_field.test.tsx` ### `x-pack/plugins/security_solution` - `x-pack/plugins/security_solution/public/management/components/console/components/command_input/integration_tests/command_input.test.tsx` - `x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/kill_process_action.test.tsx` - `x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/release_action.test.tsx` - `x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/status_action.test.tsx` - `x-pack/plugins/security_solution/public/management/components/endpoint_responder/command_render_components/integration_tests/upload_action.test.tsx` - `x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx` - `x-pack/plugins/security_solution/public/management/pages/event_filters/view/components/event_filters_flyout.test.tsx` - `x-pack/plugins/security_solution/public/management/pages/response_actions/view/response_actions_list_page.test.tsx` ---- I plan to do a talk on Kibana Demo Days to walk through some of the breaking changes and learnings. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

## Summary This test (elastic#180931) was introduced right before the version bump of user-events (elastic#189949). This PR updates the one async call to be awaited, hopefully fixing the test. Solves: elastic#192475 Unskipped tests are passing now: https://buildkite.com/elastic/kibana-pull-request/builds/233177 --------- Co-authored-by: Antonio <antoniodcoelho@gmail.com>

walterra self-assigned this Aug 6, 2024

walterra force-pushed the update-testing-library-user-event-14-4-3 branch 2 times, most recently from 9f4594f to d1c0eba Compare August 8, 2024 06:48

walterra force-pushed the update-testing-library-user-event-14-4-3 branch 5 times, most recently from a54e9da to 4b804a0 Compare August 27, 2024 16:10

walterra changed the title ~~Upgrade @testing-library/user-event to ^14.4.3~~ Upgrade @testing-library/user-event to ^14.5.2 Aug 27, 2024

walterra changed the title ~~Upgrade @testing-library/user-event to ^14.5.2~~ Upgrade @testing-library/user-event to latest ^14.5.2 Aug 27, 2024

walterra mentioned this pull request Aug 28, 2024

Adds missing @testing-library/dom peer dependency #191576

Merged

2 tasks

walterra force-pushed the update-testing-library-user-event-14-4-3 branch 8 times, most recently from 0e27a30 to f3973e7 Compare September 3, 2024 16:40

walterra added 10 commits September 4, 2024 07:20

updated @testing-library/user-event to ^14.4.3

8da200b

add await to userEvent.click

830fbc1

x-pack/plugins/observability_solution/slo/public/components/burn_rate…

130bb08

…_rule_editor/slo_selector

fix missing awaits

2646a2f

fix pointerEventsCheck

100e167

adds missing asyncs

a9bb202

eslint ignore fixes

dbaa3ed

fix async/await

e14dd25

fix async/await

e746371

fix async/await for plugins/lens

70c81a8

paul-tavares reviewed Sep 6, 2024

View reviewed changes

refactor with more comments: x-pack/plugins/security_solution/public/…

a803117

…management/pages/endpoint_hosts/view/details/components/actions_menu.test.tsx

rylnd approved these changes Sep 6, 2024

View reviewed changes

walterra added 5 commits September 6, 2024 17:40

adds missing fake timers

7d56410

fix timeouts for jest tests in plugins/security_solution

0ee0b56

user fireEvent instead of userEvent for enter key

9c2b18a

revert rest of submitClick

415bee5

remove unnecessary comment

0d75919

paul-tavares approved these changes Sep 6, 2024

View reviewed changes

Merge branch 'main' into update-testing-library-user-event-14-4-3

c36ce84

jloleysens approved these changes Sep 9, 2024

View reviewed changes

neptunian approved these changes Sep 9, 2024

View reviewed changes

pgayvallet approved these changes Sep 9, 2024

View reviewed changes

andrew-goldstein approved these changes Sep 9, 2024

View reviewed changes

TattdCodeMonkey approved these changes Sep 9, 2024

View reviewed changes

angorayc approved these changes Sep 9, 2024

View reviewed changes

jaredburgettelastic approved these changes Sep 9, 2024

View reviewed changes

Merge branch 'main' into update-testing-library-user-event-14-4-3

a355d0a

delanni merged commit 6a270cf into elastic:main Sep 10, 2024
54 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Sep 10, 2024

walterra deleted the update-testing-library-user-event-14-4-3 branch September 10, 2024 12:50

delanni mentioned this pull request Sep 10, 2024

Unskip failing unit test (#192475) #192488

Merged

szaffarano mentioned this pull request Sep 13, 2024

[Telemetry][Security Solution] Fix integration tests on branch 9.0 #192842

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade `@testing-library/user-event` to latest `^14.5.2` #189949

Upgrade `@testing-library/user-event` to latest `^14.5.2` #189949

walterra commented Aug 6, 2024 •

edited

Loading

paul-tavares left a comment

paul-tavares Sep 6, 2024

walterra Sep 6, 2024

paul-tavares Sep 6, 2024

paul-tavares Sep 6, 2024

rylnd left a comment

walterra commented Sep 6, 2024 •

edited

Loading

paul-tavares left a comment

paul-tavares Sep 6, 2024

paul-tavares Sep 6, 2024

andrew-goldstein left a comment

jaredburgettelastic left a comment

walterra commented Sep 10, 2024

kibana-ci commented Sep 10, 2024 •

edited

Loading

ESLint disabled line counts

Total ESLint disabled count

Upgrade @testing-library/user-event to latest ^14.5.2 #189949

Upgrade @testing-library/user-event to latest ^14.5.2 #189949

Conversation

walterra commented Aug 6, 2024 • edited Loading

Summary

packages/kbn-dom-drag-drop

x-pack/plugins/cases

x-pack/plugins/cloud_security_posture

x-pack/plugins/lens

x-pack/plugins/observability_solution

x-pack/plugins/security_solution

Checklist

paul-tavares left a comment

Choose a reason for hiding this comment

paul-tavares Sep 6, 2024

Choose a reason for hiding this comment

walterra Sep 6, 2024

Choose a reason for hiding this comment

paul-tavares Sep 6, 2024

Choose a reason for hiding this comment

paul-tavares Sep 6, 2024

Choose a reason for hiding this comment

rylnd left a comment

Choose a reason for hiding this comment

walterra commented Sep 6, 2024 • edited Loading

paul-tavares left a comment

Choose a reason for hiding this comment

paul-tavares Sep 6, 2024

Choose a reason for hiding this comment

paul-tavares Sep 6, 2024

Choose a reason for hiding this comment

andrew-goldstein left a comment

Choose a reason for hiding this comment

elastic/security-generative-ai

jaredburgettelastic left a comment

Choose a reason for hiding this comment

walterra commented Sep 10, 2024

kibana-ci commented Sep 10, 2024 • edited Loading

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

Upgrade `@testing-library/user-event` to latest `^14.5.2` #189949

Upgrade `@testing-library/user-event` to latest `^14.5.2` #189949

walterra commented Aug 6, 2024 •

edited

Loading

`packages/kbn-dom-drag-drop`

`x-pack/plugins/cases`

`x-pack/plugins/cloud_security_posture`

`x-pack/plugins/lens`

`x-pack/plugins/observability_solution`

`x-pack/plugins/security_solution`

walterra commented Sep 6, 2024 •

edited

Loading

kibana-ci commented Sep 10, 2024 •

edited

Loading