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

Migrate iframed block test to Playwright #44164

Merged
merged 6 commits into from
Sep 30, 2022

Conversation

Rink9
Copy link
Contributor

@Rink9 Rink9 commented Sep 14, 2022

What?

Migrate iframed-block.test.js to its Playwright version.

Why?

See #38570 for its background and rationale.

How?

See MIGRATION.md for migration steps.

Testing Instructions

npm run test:e2e:playwright test/e2e/specs/editor/plugins/iframed-block.spec.js

Screencast

Screen.Recording.2022-09-14.at.11.45.56.PM.mov

@Rink9
Copy link
Contributor Author

Rink9 commented Sep 21, 2022

Hi @kevin940726 @talldan I have submitted another PR. Can you please take a look at it and please review?

Copy link
Member

@kevin940726 kevin940726 left a comment

Choose a reason for hiding this comment

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

Nice work! Sorry for the late reviews.

Just left some nitpicks but generally this looks pretty good!

test/e2e/specs/editor/plugins/iframed-block.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/plugins/iframed-block.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/plugins/iframed-block.spec.js Outdated Show resolved Hide resolved
@Rink9
Copy link
Contributor Author

Rink9 commented Sep 29, 2022

@kevin940726 Thanks for your review! I have addressed your feedbacks. Can you please take a look at it?

Copy link
Member

@kevin940726 kevin940726 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!

@Rink9
Copy link
Contributor Author

Rink9 commented Sep 30, 2022

@kevin940726 Thanks 😊 Do i have to add anything to get Merging this PR?

@kevin940726
Copy link
Member

Nothing! I'm just waiting for the CI to pass 😅.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants