chore(fe): migrate 4 Enzyme-based tests to RTL #31634

hainenber · 2024-12-27T16:14:27Z

chore(fe): migrate 4 Enzyme-based tests to RTL

SUMMARY

Part of ongoing effort to refactor away Enzyme-based unit test to RTL

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

The CI checks should all pass for this particular PR

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

Signed-off-by: hainenber <dotronghai96@gmail.com>

korbit-ai

I've completed my review and didn't find any issues... but I did find this rooster.

   \\
   (o>
\\_//)
 \_/_)
  _|_

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

Chat with Korbit on issues we post by tagging @korbit-ai in your reply.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

Check out our docs on how you can make Korbit work best for you and your team.

Customize Korbit for your organization through the Korbit Console.

Current Korbit Configuration

General Settings

Setting Value

Review Schedule Automatic excluding drafts

Max Issue Count 10

Automatic PR Descriptions ❌

Issue Categories

Category Enabled

Naming ✅

Database Operations ✅

Documentation ✅

Logging ✅

Error Handling ✅

Systems and Environment ✅

Objects and Data Structures ✅

Readability and Maintainability ✅

Asynchronous Processing ✅

Design Patterns ✅

Third-Party Libraries ✅

Performance ✅

Security ✅

Functionality ✅

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ support@korbit.ai

Note

Korbit Pro is free for open source projects 🎉

Looking to add Korbit to your team? Get started with a free 2 week trial here

rusackas · 2025-01-03T21:38:44Z

Running CI 🤞

hainenber · 2025-01-13T15:17:11Z

@rusackas I think the CI was failing due to Docker Hub token issue previously. Do you think it's worth another shot running? 👀

rusackas · 2025-01-13T19:37:46Z

I think if you rebase the PR now, it'll work.

hainenber · 2025-01-14T11:27:07Z

All green :D

Signed-off-by: hainenber <dotronghai96@gmail.com>

hainenber · 2025-01-29T10:12:37Z

re-bump :D

rusackas · 2025-02-11T15:02:57Z

Hey hey! I'd forgotten about this PR, sorry!

I'm working on the same effort (killing enzyme) and have this PR going:
#32207

In your PR you get a couple of tests that I also worked on, but you have a couple I haven't gotten to yet.

It looks like your PR is failing CI on a Cypress test (our next nemesis!) - I just started a re-run of it to see if it passes now. If not, would you rather fix and merge your PR before I merge mine, or would you rather close this and I'll copy over your work on the tests I haven't converted yet?

hainenber · 2025-02-11T15:10:41Z

funny thing is that the failed E2E test is due to non-Cypress issue 😆 . I'll be stand by to fix my changes if it's the case, btw

hainenber · 2025-02-11T15:20:11Z

Let me rebase the branch to be up-to-date with master

kgabryje · 2025-02-11T15:36:58Z

superset-frontend/packages/superset-ui-core/test/chart-composition/legend/WithLegend.test.tsx

 import { triggerResizeObserver } from 'resize-observer-polyfill';
 import { promiseTimeout, WithLegend } from '@superset-ui/core';
+import { render } from '@testing-library/react';


please import from spec/helpers/testing-library instead. Applies to all files - you won't need to import additional stuff like @testing-library/jest-dom

@hainenber can get that here, or (in theory) the linting rule on my PR should help sweep that up.

Gotchu. Will push new fixes in around upcoming 5s

superset-ui-core seems to be a tad different compared to others. I'm getting this

I also noticed that in Evan also employed direct testing-library import in the PR :D

superset-ui-core is built different, I guess.

Hmm can you try to add "../spec/**/*" to the include array in superset-ui-core/tsconfig.ts?

The path to spec might be different but you know what I mean 😄

Signed-off-by: hainenber <dotronghai96@gmail.com>

chore(fe): migrate 2 Enzyme-based tests to RTL

a90dc21

Signed-off-by: hainenber <dotronghai96@gmail.com>

pull-request-size bot added the size/M label Dec 27, 2024

korbit-ai bot reviewed Dec 27, 2024

View reviewed changes

github-actions bot added the packages label Dec 27, 2024

hainenber changed the title ~~chore(pkg/core): migrate 2 Enzyme-based tests in packages/superset-core to RTL~~ chore(fe/pkg/core): migrate 2 Enzyme-based tests in packages/superset-core to RTL Dec 27, 2024

rusackas requested a review from geido January 3, 2025 21:38

sadpandajoe added review:checkpoint Last PR reviewed during the daily review standup and removed review:checkpoint Last PR reviewed during the daily review standup labels Jan 7, 2025

Merge branch 'master' into chores/migrate-enzyme-tests-to-rtl

f417d27

hainenber added 2 commits January 24, 2025 08:20

Merge branch 'master' into chores/migrate-enzyme-tests-to-rtl

e53fb40

refactor(fe): migrate EmptyState and ExecutionLogList tests to RTL

12ef9ee

Signed-off-by: hainenber <dotronghai96@gmail.com>

pull-request-size bot added size/L and removed size/M labels Jan 24, 2025

hainenber changed the title ~~chore(fe/pkg/core): migrate 2 Enzyme-based tests in packages/superset-core to RTL~~ chore(fe/pkg/core): migrate 4 Enzyme-based tests in packages/superset-core to RTL Jan 24, 2025

hainenber changed the title ~~chore(fe/pkg/core): migrate 4 Enzyme-based tests in packages/superset-core to RTL~~ chore(fe): migrate 4 Enzyme-based tests to RTL Jan 24, 2025

rusackas requested a review from justinpark January 28, 2025 00:11

hainenber mentioned this pull request Feb 8, 2025

chore: remove all enzyme tests and references #32191

Closed

Merge branch 'master' into chores/migrate-enzyme-tests-to-rtl

955f0a1

rusackas approved these changes Feb 11, 2025

View reviewed changes

kgabryje reviewed Feb 11, 2025

View reviewed changes

rusackas merged commit db70c79 into apache:master Feb 11, 2025
48 checks passed

hainenber deleted the chores/migrate-enzyme-tests-to-rtl branch February 11, 2025 16:06

sfirke pushed a commit to sfirke/superset that referenced this pull request Feb 12, 2025

chore(fe): migrate 4 Enzyme-based tests to RTL (apache#31634)

ba3ee67

Signed-off-by: hainenber <dotronghai96@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(fe): migrate 4 Enzyme-based tests to RTL #31634

chore(fe): migrate 4 Enzyme-based tests to RTL #31634

hainenber commented Dec 27, 2024 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

rusackas commented Jan 3, 2025

hainenber commented Jan 13, 2025

rusackas commented Jan 13, 2025

hainenber commented Jan 14, 2025

hainenber commented Jan 29, 2025

rusackas commented Feb 11, 2025

hainenber commented Feb 11, 2025

hainenber commented Feb 11, 2025

kgabryje Feb 11, 2025

rusackas Feb 11, 2025

hainenber Feb 11, 2025

hainenber Feb 11, 2025

hainenber Feb 11, 2025

kgabryje Feb 11, 2025

kgabryje Feb 11, 2025

Setting	Value
Review Schedule	Automatic excluding drafts
Max Issue Count	10
Automatic PR Descriptions	❌

Category	Enabled
Naming	✅
Database Operations	✅
Documentation	✅
Logging	✅
Error Handling	✅
Systems and Environment	✅
Objects and Data Structures	✅
Readability and Maintainability	✅
Asynchronous Processing	✅
Design Patterns	✅
Third-Party Libraries	✅
Performance	✅
Security	✅
Functionality	✅

chore(fe): migrate 4 Enzyme-based tests to RTL #31634

chore(fe): migrate 4 Enzyme-based tests to RTL #31634

Conversation

hainenber commented Dec 27, 2024 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot left a comment • edited Loading

Choose a reason for hiding this comment

Interacting with Korbit

Customizing Korbit

Current Korbit Configuration

Feedback and Support

Looking to add Korbit to your team? Get started with a free 2 week trial here

rusackas commented Jan 3, 2025

hainenber commented Jan 13, 2025

rusackas commented Jan 13, 2025

hainenber commented Jan 14, 2025

hainenber commented Jan 29, 2025

rusackas commented Feb 11, 2025

hainenber commented Feb 11, 2025

hainenber commented Feb 11, 2025

kgabryje Feb 11, 2025

Choose a reason for hiding this comment

rusackas Feb 11, 2025

Choose a reason for hiding this comment

hainenber Feb 11, 2025

Choose a reason for hiding this comment

hainenber Feb 11, 2025

Choose a reason for hiding this comment

hainenber Feb 11, 2025

Choose a reason for hiding this comment

kgabryje Feb 11, 2025

Choose a reason for hiding this comment

kgabryje Feb 11, 2025

Choose a reason for hiding this comment

hainenber commented Dec 27, 2024 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading