Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Aug 6, 2024

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

@walterra walterra self-assigned this Aug 6, 2024
@walterra 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 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 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 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 added a commit that referenced this pull request Aug 28, 2024
## Summary

During `yarn kbn bootstrap` we get the following message:

> @testing-library/[email protected]" 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)
@walterra 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
Copy link
Contributor Author

walterra commented Sep 6, 2024

@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.

Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @walterra for the follow up updates.

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

// 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 });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome. Thank you

@@ -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'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@jaredburgettelastic jaredburgettelastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entity Analytics changes LGTM!

@walterra
Copy link
Contributor Author

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
Copy link
Collaborator

kibana-ci commented Sep 10, 2024

💛 Build succeeded, but was flaky

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

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

cc @walterra

@delanni delanni merged commit 6a270cf into elastic:main Sep 10, 2024
54 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 10, 2024
@walterra walterra deleted the update-testing-library-user-event-14-4-3 branch September 10, 2024 12:50
delanni added a commit that referenced this pull request Sep 11, 2024
## 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 <[email protected]>
ashokaditya added a commit that referenced this pull request Oct 23, 2024
## Summary

This PR attempts to fix tests skipped in /issues/193092
and /issues/193554.
The tests seem to be flaky right after an [upgrade to
`user-event`](#189949) dependency
on Sep 10th.

### 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
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 23, 2024
## Summary

This PR attempts to fix tests skipped in elastic/issues/193092
and elastic/issues/193554.
The tests seem to be flaky right after an [upgrade to
`user-event`](elastic#189949) dependency
on Sep 10th.

### 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

(cherry picked from commit e70b533)
kibanamachine added a commit that referenced this pull request Oct 23, 2024
…#197380)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[SecuritySolution][Endpoint] Re-enabled skipped tests
(#197220)](#197220)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Ash","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-23T08:33:37Z","message":"[SecuritySolution][Endpoint]
Re-enabled skipped tests (#197220)\n\n## Summary\r\n\r\nThis PR attempts
to fix tests skipped in /issues/193092\r\nand
/issues/193554.\r\nThe tests seem to be flaky right after
an [upgrade
to\r\n`user-event`](#189949)
dependency\r\non Sep 10th.\r\n\r\n### Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"e70b533bdb443a4ebc8229237583160eeeaad412","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Defend
Workflows","backport:version","v8.17.0"],"title":"[SecuritySolution][Endpoint]
Re-enabled skipped
tests","number":197220,"url":"https://github.com/elastic/kibana/pull/197220","mergeCommit":{"message":"[SecuritySolution][Endpoint]
Re-enabled skipped tests (#197220)\n\n## Summary\r\n\r\nThis PR attempts
to fix tests skipped in /issues/193092\r\nand
/issues/193554.\r\nThe tests seem to be flaky right after
an [upgrade
to\r\n`user-event`](#189949)
dependency\r\non Sep 10th.\r\n\r\n### Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"e70b533bdb443a4ebc8229237583160eeeaad412"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197220","number":197220,"mergeCommit":{"message":"[SecuritySolution][Endpoint]
Re-enabled skipped tests (#197220)\n\n## Summary\r\n\r\nThis PR attempts
to fix tests skipped in /issues/193092\r\nand
/issues/193554.\r\nThe tests seem to be flaky right after
an [upgrade
to\r\n`user-event`](#189949)
dependency\r\non Sep 10th.\r\n\r\n### Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"e70b533bdb443a4ebc8229237583160eeeaad412"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Ash <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team Team:Obs AI Assistant Observability AI Assistant Team:obs-ux-management Observability Management User Experience Team v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.