Migrate Image Size to Playwright #40467
Conversation
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @juhi123! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
github-actions
bot
added
the
First-time Contributor
| * @this {import('./').PageUtils} | ||
| * @param {string} slug Plugin slug. | ||
| */ | ||
| export async function activatePlugin( slug ) { |
There was a problem hiding this comment.
Thanks for working on this.
There are activatePlugin and deactivatePlugin utils for playwright already in the codebase (as part of RequestUtils), so these PageUtils ones shouldn't be needed:
https://github.com/WordPress/gutenberg/blob/trunk/packages/e2e-test-utils-playwright/src/request/plugins.ts
edit: I think that also means the other new utils won't be needed.
talldan
added
the
[Type] Automated Testing
|
This looks good, I think the only thing (other than the comment above) is that other migration PRs have deleted the old puppeteer test file and any snapshots as part of the migration. Maybe the migration steps need to be updated to clarify that. |
|
Hey @talldan 👋 Thanks for reviewing the PR. Yeah, you are right, how did I miss that? I have removed those and used the existing ones. I am not sure about the other files you are talking about, I haven't deleted any. Can you please let me know, what do I need to do to fix that. Thanks! |
There was a problem hiding this comment.
Thanks for addressing the feedback.
Just to clarify that earlier comment - you can go ahead and delete the old test as part of this PR. This file can be removed:
https://github.com/WordPress/gutenberg/blob/trunk/packages/e2e-tests/specs/editor/plugins/image-size.test.js
| getCurrentUser = getCurrentUser; | ||
| switchUserToAdmin = switchUserToAdmin; | ||
| switchUserToTest = switchUserToTest; |
There was a problem hiding this comment.
It looks like these three utils are now unused since the activatePlugin and deactivatePlugin utils were removed.
What do you think about removing these too until they're needed?
There was a problem hiding this comment.
Yeah you are right, removed these
| if ( ( await this.getCurrentUser() ) === WP_ADMIN_USER.username ) { | ||
| return; | ||
| } | ||
| await this.loginUser( WP_ADMIN_USER.username, WP_ADMIN_USER.password ); |
There was a problem hiding this comment.
I couldn't see a loginUser utility for playwright.
There was a problem hiding this comment.
We don't need this anymore
|
@talldan |
|
Congrats on your first contribution 🎉 |
|
Thank you @talldan for quick reviews |
What?
Migrate image-size.test.js to its Playwright version.
Why?
See #38570 for its background and rationale.
How?
See MIGRATION.md for migration steps.
Testing Instructions
Run
npm run test-e2e:playwright test/e2e/specs/editor/plugins/image-size.spec.js