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

refactor(frontend): migrate 6 Enzyme-based tests to RTL, part 2 #30281

Merged
merged 8 commits into from
Oct 1, 2024
Merged

refactor(frontend): migrate 6 Enzyme-based tests to RTL, part 2 #30281

merged 8 commits into from
Oct 1, 2024

Conversation

hainenber
Copy link
Contributor

@hainenber hainenber commented Sep 14, 2024 •
edited
Loading

refactor(frontend): migrate 6 Enzyme-based tests to RTL, part 2

SUMMARY

Remove an unused function and associated tests found when conducting Enzyme-to-RTL migration.

Also migrated a few Enzyme tests to RTL.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

GH checks with flying colors! :D

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 change:frontend Requires changing the frontend label Sep 14, 2024
@hainenber
refactor(pkg/ui-core/test/char/comp): migrate Enzyme-based createLoad…
7882223 
…ableRenderer.test.tsx to RTL

Signed-off-by: hainenber <[email protected]>
@hainenber
refactor(src/explore/comp/controls/filter-control): migrate Enzyme-ba…
8a19438 
…sed AdhocFilterEditPopoverSimpleTabContent.test.tsx to RTL

Signed-off-by: hainenber <[email protected]>
@hainenber hainenber changed the title chore(fe): remove unused withAsyncVerification and associated test feat(frontend): migrate Enzyme-based tests to RTL, part 2 Sep 14, 2024
@hainenber hainenber changed the title feat(frontend): migrate Enzyme-based tests to RTL, part 2 feat(frontend): migrate 3 Enzyme-based tests to RTL, part 2 Sep 14, 2024
@hainenber hainenber changed the title feat(frontend): migrate 3 Enzyme-based tests to RTL, part 2 feat(frontend): migrate 5 Enzyme-based tests to RTL, part 2 Sep 15, 2024
@hainenber hainenber changed the title feat(frontend): migrate 5 Enzyme-based tests to RTL, part 2 refactor(frontend): migrate 5 Enzyme-based tests to RTL, part 2 Sep 15, 2024
@hainenber
refactor(src/explore/comp/controls/datasource-controls): migrate Enzy…
e3b1474 
…me-based DatasourceControl.test.jsx to RTL

Signed-off-by: hainenber <[email protected]>
@hainenber hainenber changed the title refactor(frontend): migrate 5 Enzyme-based tests to RTL, part 2 refactor(frontend): migrate 6 Enzyme-based tests to RTL, part 2 Sep 15, 2024
justinpark
justinpark requested changes Sep 16, 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.

withAsyncVerification is being used as a custom override logic. Please exclude it from deletion

@hainenber
Copy link
Contributor Author

withAsyncVerification is being used as a custom override logic. Please exclude it from deletion

Got it. I've reverted its deletion.

mistercrunch
mistercrunch approved these changes Sep 19, 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!

@mistercrunch
Copy link
Member

@justinpark , mind lifting the hold around your requested changes?

@hainenber
Copy link
Contributor Author

hello fellas, anything I can do to have this merged? I'm about to have part 3 now :D

@rusackas
Copy link
Member

rusackas commented Oct 1, 2024

Pinging @justinpark on Slack... hopefully we can get this through :D

@rusackas rusackas merged commit da7a74e into apache:master Oct 1, 2024
36 checks passed
@hainenber hainenber deleted the remove-unused-async-verification-wrapper branch October 2, 2024 00:54
nyohasstium pushed a commit to Webgains/superset that referenced this pull request Jan 2, 2025
@hainenber @nyohasstium
refactor(frontend): migrate 6 Enzyme-based tests to RTL, part 2 (apac…
7a56c4e 
…he#30281)

Signed-off-by: hainenber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mistercrunch mistercrunch mistercrunch approved these changes

@justinpark justinpark justinpark approved these changes

@kgabryje kgabryje kgabryje approved these changes

@geido geido Awaiting requested review from geido

Assignees
No one assigned
Labels
change:frontend Requires changing the frontend packages size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hainenber @mistercrunch @rusackas @justinpark @kgabryje