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.

Sign up for GitHub

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

Jump to bottom

[NEEDS ACTION] Fix remaining unit tests for React@18 #206952

Closed
5 tasks done
Dosant opened this issue Jan 16, 2025 · 2 comments
Closed
5 tasks done

[NEEDS ACTION] Fix remaining unit tests for React@18 #206952

Dosant opened this issue Jan 16, 2025 · 2 comments
Assignees
@Dosant
Labels
Meta React@18 Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@Dosant
Copy link
Contributor

Dosant commented Jan 16, 2025 •
edited
Loading

Hi Teams,

@elastic/appex-sharedux is currently working on the React@18 upgrade, and we're almost ready for the package update. However, there are still a few failing unit tests that need attention.

You can find the ongoing test runs with the React@18 upgrade here. We're addressing the simpler test failures ourselves in this PR: #206917.

For the remaining test failures, we need assistance from the respective owner teams. These issues may either indicate actual bugs or require specific knowledge of the business logic, which makes them harder for us to resolve.

We kindly ask the relevant teams to review the failing tests, verify if they're not actual runtime bugs, and implement the necessary test fixes. Ideally, the fix should be backward compatible with React@17 so it can be merged into the main branch in a separate PR. If this isn't possible, please contribute directly to the general PR here.

You can run the tests with React@18 from the main branch using the following environment flag:

REACT_18=true yarn test:jest src/platform/plugins/private/kibana_overview/public/components/overview/overview.test.tsx

You can also run the app with React@18 using:

REACT_18=true yarn kbn bootstrap
REACT_18=true yarn start

Alternatively, you can use the PR with React@18 packages upgraded: #206411.

Failing Tests

Needs action from the respective owner teams:
@Dosant Dosant added Meta React@18 Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels Jan 16, 2025
@cnasikas cnasikas mentioned this issue Jan 16, 2025
Closed
@cnasikas
Copy link
Member

Issue for RO #206954.

@Dosant Dosant mentioned this issue Jan 20, 2025
Merged
3 tasks
@Dosant Dosant self-assigned this Jan 23, 2025
@Dosant Dosant closed this as completed Jan 27, 2025
@Dosant
Copy link
Contributor Author

Dosant commented Jan 27, 2025 •
edited
Loading

Couple more are failing that I didn't check before because they are under "integration" tests. Looking at them now...

  • [job] [logs] Jest Integration Tests / request flyout renders _meta field
  • [job] [logs] Jest Integration Tests / When on the host isolation exceptions page should search using expected exception item fields

@Dosant Dosant reopened this Jan 27, 2025
@Dosant Dosant mentioned this issue Jan 27, 2025
Merged
Dosant added a commit that referenced this issue Jan 27, 2025
@Dosant
[React@18] Fix index management unit tests that are failing with Reac…
9e8ab66 
…t@18 (#208387)

## Summary

Part of #206952

Fixes when run with React@18
#208339 (comment):


[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4701-478b-93ef-9013f7210234)
Jest Integration Tests / request flyout renders _meta field

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4702-4357-a67e-d59c6bb5f12b)
Jest Integration Tests / request flyout renders a json with default
policy name when only policy name is missing

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4701-4d18-8c34-1f4965486aa1)
Jest Integration Tests / request flyout renders an error callout if
policy form is invalid

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4701-4533-9e12-e7056e8277aa)
Jest Integration Tests / request flyout renders the correct json and
name for a new policy



To run with React@18 use: 

```
REACT_18=true yarn test:jest_integration x-pack/platform/plugins/private/index_lifecycle_management/integration_tests/edit_policy/features/request_flyout.test.ts
```

I tested that the component works in the browser, so the issue is in the
test. I think that because of some internal React@18 changes the order
in which the validation timeout inside our form lib is called conflcts
with fake timers (the timeout just stucks). So instead of trying to add
additional `jest.advanceTimersByTime(0);` in some places I decided to
simplify and just use the real timeouts that works well for both
react@17 an react@18
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Jan 27, 2025
@Dosant
[React@18] Fix index management unit tests that are failing with Reac…
e5a9da4 
…t@18 (elastic#208387)

## Summary

Part of elastic#206952

Fixes when run with React@18
elastic#208339 (comment):

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4701-478b-93ef-9013f7210234)
Jest Integration Tests / request flyout renders _meta field

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4702-4357-a67e-d59c6bb5f12b)
Jest Integration Tests / request flyout renders a json with default
policy name when only policy name is missing

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4701-4d18-8c34-1f4965486aa1)
Jest Integration Tests / request flyout renders an error callout if
policy form is invalid

[[job]](https://buildkite.com/elastic/kibana-pull-request/builds/270417#0194a78a-2649-4cd6-bac8-5f186c246055)
[[logs]](https://buildkite.com/organizations/elastic/pipelines/kibana-pull-request/builds/270417/jobs/0194a78a-2649-4cd6-bac8-5f186c246055/artifacts/0194a7ae-4701-4533-9e12-e7056e8277aa)
Jest Integration Tests / request flyout renders the correct json and
name for a new policy

To run with React@18 use:

```
REACT_18=true yarn test:jest_integration x-pack/platform/plugins/private/index_lifecycle_management/integration_tests/edit_policy/features/request_flyout.test.ts
```

I tested that the component works in the browser, so the issue is in the
test. I think that because of some internal React@18 changes the order
in which the validation timeout inside our form lib is called conflcts
with fake timers (the timeout just stucks). So instead of trying to add
additional `jest.advanceTimersByTime(0);` in some places I decided to
simplify and just use the real timeouts that works well for both
react@17 an react@18

(cherry picked from commit 9e8ab66)
@Dosant Dosant closed this as completed Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Dosant Dosant

Labels
Meta React@18 Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Dosant @cnasikas