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

feat(build): uplift Jest to v29 #29118

Merged
merged 29 commits into from
Jun 25, 2024
Merged

feat(build): uplift Jest to v29 #29118

merged 29 commits into from
Jun 25, 2024

Conversation

hainenber
Copy link
Contributor

feat(build): uplift Jest to v29

SUMMARY

This PR uplifted Jest in all sub-packages to latest version atm, v29. 99% of changes are within test files and no functionality has been modified.

There's one change for null safety here. I decide to drag it into this PR as v27 uplift failed to it

Acceptance threshold is CI checks all green

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

All CI steps are green

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added the javascript Dependabot - Pull requests that update Javascript code label Jun 8, 2024
@hainenber
Copy link
Contributor Author

There's 1 unfortunate commit tainted by me after doing a rebase. I've reverted the change in another commit. Lmk if this is A-OK or I'll need to do a proper rebase without tainting anything :D

@github-actions github-actions bot added the github_actions Pull requests that update GitHub Actions code label Jun 9, 2024
rusackas
rusackas reviewed Jun 10, 2024
View reviewed changes
superset-embedded-sdk/src/guestTokenRefresh.test.ts Outdated
@@ -26,7 +26,7 @@ import {

describe("guest token refresh", () => {
beforeAll(() => {
jest.useFakeTimers("modern"); // "modern" allows us to fake the system time
jest.useFakeTimers(); // "modern" allows us to fake the system time
Copy link
Member

Choose a reason for hiding this comment

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

might as well remove the comment too?

Copy link
Member

Choose a reason for hiding this comment

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

I assume that was was "modern" is now just... "normal"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment removed :D

mistercrunch
mistercrunch approved these changes Jun 17, 2024
View reviewed changes
Copy link
Member

@mistercrunch mistercrunch 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 doing this!

@hainenber
Copy link
Contributor Author

hi folks, is this merge-able? I've been keeping the PR up-to-date with latest changes from master branch and luckily so far, there aren't any significant Jest v29 incompatibilities.

@villebro
Copy link
Member

villebro commented Jun 25, 2024 •
edited
Loading

@hainenber FYI I was working on the same thing a while back: #26182 IIRC I came pretty close to getting it all working, but then got sidetracked :( With any luck you could reuse some of that work in this PR..

Never mind, I just saw you had already seen that PR. Hoping to see this merged soon!

@hainenber
Copy link
Contributor Author

@villebro I only discovered your PR after finishing my draft :D

A bit like calculus story of Newton and Leibniz 😄

Thanks for the blessing. I'll consult your PR if meeting any rough corner!

justinpark
justinpark approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@justinpark justinpark left a comment

Choose a reason for hiding this comment

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

Thanks for all updates! LGTM

@hainenber
Copy link
Contributor Author

Looks like we have to extend .eslintrc.js with .testSkipped.tsx pattern in order to get CI green. @justinpark are you OK with this? I think it's quicker to leave PropertiesModal.test.tsx to be just that :D

@justinpark
Copy link
Member

are you OK with this? I think it's quicker to leave PropertiesModal.test.tsx to be just that :D

Sounds good to me

@github-actions github-actions bot removed the github_actions Pull requests that update GitHub Actions code label Jun 25, 2024
@hainenber
Copy link
Contributor Author

Aww shucks, running Jest with all workers in CI isn't safe as well so I decide to play safe and to pin the worker to be running at 50% of total core, same as local.

@hainenber
Copy link
Contributor Author

CI is now passed :D

@villebro
Copy link
Member

@villebro I only discovered your PR after finishing my draft :D

A bit like calculus story of Newton and Leibniz 😄

Haha, that made my day 😆

Thanks for the blessing. I'll consult your PR if meeting any rough corner!

Just really happy to see jest finally being bumped! ❤️

@rusackas rusackas merged commit dffad48 into apache:master Jun 25, 2024
36 checks passed
@hainenber hainenber deleted the uplift-jest-to-v29 branch June 25, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rusackas rusackas rusackas approved these changes

@mistercrunch mistercrunch mistercrunch approved these changes

@justinpark justinpark justinpark approved these changes

@villebro villebro Awaiting requested review from villebro

@geido geido Awaiting requested review from geido

@eschutho eschutho Awaiting requested review from eschutho

@betodealmeida betodealmeida Awaiting requested review from betodealmeida

@nytai nytai Awaiting requested review from nytai

@craig-rueda craig-rueda Awaiting requested review from craig-rueda

@john-bodley john-bodley Awaiting requested review from john-bodley

@kgabryje kgabryje Awaiting requested review from kgabryje

@dpgaspar dpgaspar Awaiting requested review from dpgaspar

@michael-s-molina michael-s-molina Awaiting requested review from michael-s-molina

Assignees
No one assigned
Labels
dependencies:npm embedded javascript Dependabot - Pull requests that update Javascript code packages size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@hainenber @villebro @justinpark @mistercrunch @rusackas @john-bodley