Skip to content

Migrate Child Block Test to Playwright #55199

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
Merged
merged 9 commits into from
Nov 7, 2023
Merged

Migrate Child Block Test to Playwright #55199

merged 9 commits into from
Nov 7, 2023

Conversation

pooja-muchandikar
Copy link
Contributor

What?

Part of #38851.
Migrate child-blocks.test.js to its Playwright version.

Why?

Part of #38851.

How?

See MIGRATION.md for migration steps.

Testing Instructions

Run npm run test:e2e:playwright test/e2e/specs/editor/plugins/child-blocks.spec.js

@pooja-muchandikar
Copy link
Contributor Author

Hi @kevin940726 @Mamaduka,

I hope you are doing great!

Could you help review this PR?

Thanks!

@Mamaduka Mamaduka added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Package] E2E Tests /packages/e2e-tests labels Oct 12, 2023
Mamaduka
Mamaduka reviewed Oct 12, 2023
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @pooja-muchandikar!

I think some of the locators can be improved. Especially ones that use classes instead of role-based selectors.

Recently, I did a similar migration (#51677); you can check the inner-blocks-allowed-blocks.spec.js file and use it as a blueprint.

@pooja-muchandikar
Copy link
Contributor Author

Hi @Mamaduka,

Thanks for sharing the feedbacks! All the feedbacks are addressed please review.

Thanks!

@Mamaduka
Copy link
Member

Thanks, @pooja-muchandikar!

While changes look good, the two tests now check a different thing. They should test a block appender based on the original tests. Let's ensure that migrate tests are still checking for the same thing.

Affected tests

  • shows up in a parent block
  • display in a parent block with allowedItems.

Block appender

CleanShot 2023-10-13 at 13 25 30

@talldan talldan mentioned this pull request Nov 2, 2023
Closed
@Mamaduka
Copy link
Member

Mamaduka commented Nov 7, 2023

@pooja-muchandikar, let me know if you have any additional questions.

@pooja-muchandikar
Copy link
Contributor Author

Hi @Mamaduka,

I have addressed the feedback and pushed the changes. Please review and let me know if there are any further suggestions.

Thanks

Mamaduka
Mamaduka reviewed Nov 7, 2023
View reviewed changes
Mamaduka
Mamaduka reviewed Nov 7, 2023
View reviewed changes
Mamaduka
Mamaduka reviewed Nov 7, 2023
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

I just ran the original test in headed mode and adjusted my suggestions. The tests should be passing now.

@pooja-muchandikar
Copy link
Contributor Author

Hi @Mamaduka

Feedbacks addressed.

Mamaduka
Mamaduka approved these changes Nov 7, 2023
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @pooja-muchandikar! One last nitpick and I think we're good to merge once CI checks are green.

@Mamaduka Mamaduka merged commit 38726ab into WordPress:trunk Nov 7, 2023
@github-actions github-actions bot added this to the Gutenberg 17.1 milestone Nov 7, 2023
@pooja-muchandikar pooja-muchandikar deleted the migrate/child-block-test branch November 7, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mamaduka Mamaduka Mamaduka approved these changes

@kevin940726 kevin940726 Awaiting requested review from kevin940726

@ntwb ntwb Awaiting requested review from ntwb ntwb is a code owner

@nerrad nerrad Awaiting requested review from nerrad nerrad is a code owner

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees
No one assigned
Labels
[Package] E2E Tests /packages/e2e-tests [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Milestone
Gutenberg 17.1
Development

Successfully merging this pull request may close these issues.
2 participants
@pooja-muchandikar @Mamaduka