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

[jest] switch to jest-environment-jsdom #95125

Merged
merged 5 commits into from
Mar 24, 2021

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Mar 22, 2021

In #58095 we switched to the jest-environment-jsdom-thirteen package because at the time Jest was stuck on version 11 of JSDOM, they have since upgraded their version and we can now switch back over to the standard jest-environment-jsdom package which includes several other new features (like the "modern" timer mocks).

@spalger spalger added release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.13.0 v8.0.0 labels Mar 22, 2021
@spalger
Copy link
Contributor Author

spalger commented Mar 23, 2021

@elasticmachine merge upstream

@spalger
Copy link
Contributor Author

spalger commented Mar 23, 2021

@spalger spalger marked this pull request as ready for review March 23, 2021 17:59
@spalger spalger requested a review from a team March 23, 2021 17:59
@spalger spalger requested review from a team as code owners March 23, 2021 17:59
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@joshdover
Copy link
Contributor

These global_search failures seem like something, no? That test is explicitly testing which element is the active focus element and the results are now different when using the other jsdom. Seems potentially problematic for any a11y related tests in the future.

@spalger
Copy link
Contributor Author

spalger commented Mar 23, 2021

These global_search failures seem like something, no? That test is explicitly testing which element is the active focus element and the results are now different when using the other jsdom. Seems potentially problematic for any a11y related tests in the future.

I imagine it's possible that newer versions of JSDOM support a different amount of a11y interactions, maybe more or less, but I'm not sure and that's why those tests have been pinned to JSDOM 13. I think it's up to people who understand the test better to figure out if they can or should upgrade to a more modern version of JSDOM. I expect that JSDOM better emulates browsers in more modern versions, so maybe it's an actual issue with the component that the tests weren't able to test for before, or maybe there were special accommodations added for the way JSDOM 13 worked... Could be a lot of things I think.

Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for opening team-specific issues

@spalger spalger merged commit 0551472 into elastic:master Mar 24, 2021
@spalger spalger deleted the upgrade/jest-environment-jsdom branch March 24, 2021 15:47
@spalger spalger added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 24, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Mar 24, 2021
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #95326

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Mar 24, 2021
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Spencer <email@spalger.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants