-
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
E2E Tests: Tweak Comments block tests after migrating to Playwright #42406
E2E Tests: Tweak Comments block tests after migrating to Playwright #42406
Conversation
Size Change: +5.17 kB (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
Hey @kevin940726 👋 I noticed that @ockham migrated |
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.
Nice work! 👍
it's being used by at least 3 test suites in
e2e-tests
, so I'm wondering if this util is actually a temporary solution
I still think the implementation is still a temporary solution, because it's going to the actual options.php
page to set them, which is slow and against the best practices. If we can implement it using rest
then it belongs in e2e-test-utils-playwright
, but until then, putting it there now means we'll inevitably have to introduce breaking changes in the future for that util.
packages/e2e-test-utils-playwright/src/request-utils/comments.ts
Outdated
Show resolved
Hide resolved
Thanks, @kevin940726! I followed your latest suggestions and I think it's ready to review again. 😊 |
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! This is really well done! Just some nitpicks left then I think it's good to go!
packages/e2e-test-utils-playwright/src/request-utils/comments.ts
Outdated
Show resolved
Hide resolved
packages/e2e-test-utils-playwright/src/request-utils/comments.ts
Outdated
Show resolved
Hide resolved
packages/e2e-test-utils-playwright/src/request-utils/comments.ts
Outdated
Show resolved
Hide resolved
packages/e2e-test-utils-playwright/src/request-utils/comments.ts
Outdated
Show resolved
Hide resolved
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.
Looks awesome! Thanks a lot!
Thanks @DAreRodz and @kevin940726! ❤️ |
What?
Follow up of #39826. This applies some suggestions shared by @kevin940726 in that PR.
Why?
To follow Playwright's best practices (and also to get familiar with that library 😄).
How?
await
usage withpage.locator
.toHaveCount
with.toBeVisible
,.toBeHidden
)afterEach
instead ofafterAll
Response
object fromrequestUtils.createComment
requestUtils.createComment
requestUtils.getCurrentUser
utilityadmin.setOption
utilityTesting Instructions
Simply running