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 Button Block tests to Playwright #41494

Merged
merged 9 commits into from
Jun 20, 2022

Conversation

pooja-muchandikar
Copy link
Contributor

What?

Part of #38851.

Migrate button-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/blocks/button.spec.js

@pooja-muchandikar
Copy link
Contributor Author

Hi @kevin940726 👋

Please help me in reviewing this PR!!

Have a nice day ahead!

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.

Thank you for making this PR! Some suggestions about using role selectors here and there but overall it looks good 👍 !

test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
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.

Just some styling nitpicks left and then we can merge it! Thanks for the hard work!

test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
test/e2e/specs/editor/blocks/buttons.spec.js Outdated Show resolved Hide resolved
@kevin940726
Copy link
Member

Not sure why the e2e tests are failing. Seems unrelated though. Could you help rebase it to the latest trunk and try again?

@pooja-muchandikar
Copy link
Contributor Author

Hi @kevin940726,

I rebased the latest trunk and pushed the changes, but seems like the CI is failing. But not on the test case I worked on.

image


@kevin940726
Copy link
Member

Yeah, that's unrelated and is being tracked in #37976 👍

@pooja-muchandikar
Copy link
Contributor Author

Ok great!, any other changes required for this PR?

@kevin940726
Copy link
Member

Nope, looks perfect to me! Tests are failing but look unrelated to this PR so I think we can merge this 👍

@talldan, could you help merge it?

@talldan
Copy link
Contributor

talldan commented Jun 20, 2022

I re-ran the tests, and the navigation block tests are still failing. They're unrelated to this PR so I'll merge it, and I'll also look at migrating the navigation block tests to playwright this week. Maybe that'll improve stability.

@talldan talldan merged commit 08f0cea into WordPress:trunk Jun 20, 2022
@talldan talldan changed the title Migrate Button Block Test Case Migrate Button Block tests to Playwright Jun 20, 2022
@github-actions github-actions bot added this to the Gutenberg 13.6 milestone Jun 20, 2022
@pooja-muchandikar pooja-muchandikar deleted the add/playwright-button-test branch June 20, 2022 09:55
@pooja-muchandikar
Copy link
Contributor Author

Thank you @kevin940726 @talldan 🙇🏻‍♀️

@Mamaduka Mamaduka requested review from tellthemachines and removed request for tellthemachines September 5, 2022 19:15
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.

3 participants