-
Notifications
You must be signed in to change notification settings - Fork 30
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
NEW e2e environment using playground cli #810
base: develop
Are you sure you want to change the base?
NEW e2e environment using playground cli #810
Conversation
(which is a bit weird, because the playwright.config.ts setting is ignored and the folder needs to be re-named to 'tests' to make tests be found at all)
TRICKY: Enabled mapping leads to 404 for build blocks, Disabled mapping helps gatherpress blocks, but make the simple paragraph test fail (copied for debugging from gutenberg)
…oss one Github actions workflow attempt.
…iled attempts (suggested at GatherPress#776 (comment) by @swissspidy)
.locator('a') | ||
.filter({ hasText: 'Attend' }) | ||
.click({ force: true }); | ||
await page.getByText('Close').click({ force: true }); |
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.
Forcing clicks should be used only in specific edge cases. Do we know if it's necessary to use it here? I'm asking because it bypasses the auto-waiting mechanism which can lead to flaky results.
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.
Hello again @WunderBart, thanks for looking at this!
Yes, I'm aware of this bad behavior, as you can see from some of the inline comments and my commit messages like TRYOUT .click({ force: true }) which is ugly, but works :(.
I experienced, that using force:true
helped avoiding the before mentioned timeouts (error 500) to a minimum or even to not happen at all. With the commits from yesterday, I was a least able to reduce the failing tests to one, followed by an error 500. But it was only one 500 anymore, not 15-20 like in the workflow runs before.
But yes - all in all - I would like to remove all forcing too, if I (or maybe we) find a way around the fatal errors.
Wow! Given that we never ever had any contact and that I was working just yesterday evening on this stuck PR, I do call this a great community moment! I absolutely appreciate you flying by and taking the time, thank you very much! |
Happy to help, @carstingaxion! I did spend a lot of time with Playwright so please don't hesitate to ping me anytime! 🙌 😄 BTW, I noticed a lot of increased timeouts for the actions (mostly clicks) in this PR. Is it because the app is responding slowly? Maybe we could come up with some pre-test solution for that, or configure (action) timeouts globally. |
…iting), including visibility and enabled state
…assertion so that it's properly awaited
Preview changes with PlaygroundYou can preview the recent changes for PR#810 with the following PHP versions: PHP Version 8.3
PHP Version 7.4
Download Made with 💙 from GatherPress & a little bit of WordPress Playground. Changes will not persist between sessions. |
Also looping in @swissspidy who might find this work interesting |
This PR allows GatherPress to run automated & manual end-to-end tests, while sharing the same,
wp-playground/cli
powered, setup. The started playground imports theGatherPress/demo-data
, that can be used instead of mocks or fixtures (for now).NEEDEDnice-to-have, before mergeeditor.canvas
easily.(Feature/e2e tests workflow #776 (comment))
Closes #693
How to test the Change
npm run test:e2e
for exampleChangelog Entry
Credits
Props @carstingaxion, @WunderBart
Checklist:
/docs/contributor/e2e-tests