-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Post Visibility Test to Playwright #44488
Migrate Post Visibility Test to Playwright #44488
Conversation
Hi @kevin940726, I hope you are going great!.. I worked on this PR and the CI is failing multiple times.. I tried fixing the issue and in local the test cases seems to be executing successfully, but CI is failing.. Could you suggest here please? |
Hi @kevin940726, Are there any further suggestions/feedbacks on this PR? |
Hi @kevin940726, I hope you are doing great! I wanted to check with you if there are any further feedbacks on this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience! Left some minor feedback.
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); | ||
|
||
test.describe( 'Post visibility', () => { | ||
const wp = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kevin940726,
We are using this variable below to get the status of the posts. So it's just declaration of that variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we can just call window.wp
instead. Will that work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking on that..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done
Hi @kevin940726, I have addressed all the feedbacks, please check and let me know if there are any further feedbacks. Thanks! |
Hi @kevin940726, All feedbacks are addressed, but the CI is failing and its not related to the test case I worked upon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks!
What?
Part of #38851.
Migrate post-visibility.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/various/post-visibility.spec.js