-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ML] Migrate tests using enzyme
to use @testing-library/react
.
#153288
Labels
Meta
:ml
refactoring
technical debt
Improvement of the software architecture and operational architecture
Comments
Pinging @elastic/ml-ui (:ml) |
Merged
2 tasks
walterra
added
the
technical debt
Improvement of the software architecture and operational architecture
label
Mar 22, 2023
7 tasks
14 tasks
walterra
added a commit
that referenced
this issue
Aug 29, 2024
## Summary Fixes #153477. Fixes #153476. Part of #187772 (technical debt). Part of #153288 (migrate enzyme tests to react-testing-lib). Removes dependency cache. The major culprit making this PR large and not easy to split is that `getHttp()` from the dependency cache was used throughout the code base for services like `mlJobService` and `ml/mlApiServices` which then themselves were directly imported and not part of React component lifecycles. - For functional components this means mostly migrating to hooks that allow accessing services. - We still have a bit of a mix of usage of `withKibana` and `context` for class based React components. This was not consolidated in this PR, I took what's there and adjusted how services get used. These components access services via `this.props.kibana.services.*` or `this.context.services.*`. - Functions no longer access the global services provided via dependency cache but were updated to receive services via arguments. - Stateful services like `mlJobService` are exposed now via a factory that makes sure the service gets instantiated only once. - Some tests where the mocks needed quite some refactoring were ported to `react-testing-lib`. They no longer make use of snapshots or call component methods which should be considered implementation details. - We have a mix of usage of the plain `toasts` via `useMlKibana` and our own `toastNotificationServices` that wraps `toasts`. I didn't consolidate this in this PR but used what's available for the given code. - For class based components, service initializations were moved from `componentDidMount()` to `constructor()` where I spotted it. - We have a bit of a mix of naming: `ml`, `mlApiServices`, `useMlApiContext()` for the same thing. I didn't consolidate the naming in this PR, to avoid making this PR even larger. This can be done in a follow up, once this PR is in this should be more straightforward and less risky. - Turns out `explorer_chart_config_builder.js` is no longer used anywhere so I deleted it. - `jobs/jobs_list/components/utils.d.ts` was missing some definitions, tried to fix them. - Moved `stashJobForCloning` to be a method of `mlJobService`. - The `MetricSelector` component was an exact copy besides the i18n label, consolidated that so anomaly detection wizards use the same component. ### 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 - [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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Meta
:ml
refactoring
technical debt
Improvement of the software architecture and operational architecture
At the moment it looks like to be able to migrate to React 18, we need to migrate tests using
enzyme
to@testing-library/react
, since Enzyme officially supporting React 18 looks still unlikely (see e.g. enzymejs/enzyme#2524, https://dev.to/wojtekmaj/enzyme-is-dead-now-what-ekl). When porting these tests, we should also try to avoid the use of existing snapshots again and refactor to more useful tests that assert rendered content and act on the components so we rely less on implementation details. And since we write new tests with@testing-library/react
only anyway, we should aim to migrate these tests so we rely on one framework only eventually.plugins/ml
x-pack/plugins/ml/public/application/components/controls/select_interval/select_interval.test.tsx
[ML] Migrate SelectInterval/SelectSeverity unit tests from
enzyme
toreact-testing-lib
#153321x-pack/plugins/ml/public/application/components/controls/select_severity/select_severity.test.tsx
[ML] Migrate SelectInterval/SelectSeverity unit tests from
enzyme
toreact-testing-lib
#153321x-pack/plugins/ml/public/application/components/job_selector/timerange_bar/timerange_bar.test.js
x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/components/outlier_exploration/outlier_exploration.test.tsx
x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/create_analytics_button/create_analytics_button.test.tsx
x-pack/plugins/ml/public/application/explorer/components/explorer_no_influencers_found/explorer_no_influencers_found.test.js
x-pack/plugins/ml/public/application/explorer/components/explorer_no_results_found/explorer_no_results_found.test.js
x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container.test.js
x-pack/plugins/ml/public/application/explorer/explorer_charts/components/explorer_chart_label/explorer_chart_label_badge.test.js
x-pack/plugins/ml/public/application/explorer/explorer_charts/components/explorer_chart_label/explorer_chart_label.test.js
x-pack/plugins/ml/public/application/jobs/components/custom_url_editor/editor.test.tsx
react-testing-lib
that don't assert implementation details.x-pack/plugins/ml/public/application/jobs/components/custom_url_editor/list.test.tsx
x-pack/plugins/ml/public/application/settings/filter_lists/components/filter_list_usage_popover/filter_list_usage_popover.test.js
x-pack/plugins/ml/public/application/components/rule_editor/select_rule_action/rule_action_panel.test.js
[ML] Remove dependency cache. #189729x-pack/plugins/ml/public/application/settings/calendars/edit/new_calendar.test.js
[ML] Remove dependency cache. #189729x-pack/plugins/ml/public/application/components/validate_job/validate_job_view.test.js
[ML] Remove dependency cache. #189729plugins/transform
x-pack/plugins/transform/public/app/sections/create_transform/components/aggregation_list/agg_label_form.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/aggregation_list/list_form.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/aggregation_list/list_summary.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/aggregation_list/popover_form.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/group_by_list/group_by_label_form.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/group_by_list/group_by_label_summary.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/group_by_list/list_form.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/group_by_list/list_summary.test.tsx
x-pack/plugins/transform/public/app/sections/create_transform/components/group_by_list/popover_form.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/transform_management_section.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/action_delete/delete_action_name.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/action_start/start_action_name.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/action_stop/stop_action_name.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/create_transform_button/create_transform_button.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/expanded_row_details_pane.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/expanded_row_json_pane.test.tsx
x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/expanded_row_json_pane.test.tsx
The text was updated successfully, but these errors were encountered: