-
Notifications
You must be signed in to change notification settings - Fork 112
Add E2E tests setup for Auto Sizes #1988
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
base: feature/1511-incorporate-layout-constraints-from-ancestors
Are you sure you want to change the base?
Add E2E tests setup for Auto Sizes #1988
Conversation
…ors' into add/setup-e2e-tests
…ors' into add/setup-e2e-tests
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/1511-incorporate-layout-constraints-from-ancestors #1988 +/- ##
==============================================================================================
+ Coverage 69.60% 72.48% +2.87%
==============================================================================================
Files 86 85 -1
Lines 7018 7051 +33
==============================================================================================
+ Hits 4885 5111 +226
+ Misses 2133 1940 -193
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
In b4758e6, I tried adding E2E tests to verify the image When I run Does anyone know why this might be happening or what could be causing the difference in behavior between normal and debug modes? cc. @joemcgill @felixarntz |
@mukeshpanchal27 this is a tricky thing to try to test in a Playwright test. The I'm not entirely sure if the headless browser Playwright uses will process |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
wp-scripts already has a good global-setup
config. Why do we need a custom one here?
*/ | ||
import baseConfig from '@wordpress/scripts/config/playwright.config.js'; | ||
|
||
const config = defineConfig( { |
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 do we need a custom config over what's provided by wp-scripts? IMO the default one should suffice for our needs.
"test-e2e": "wp-scripts test-playwright --config plugins/auto-sizes/tests/e2e/playwright.config.ts", | ||
"test-e2e:debug": "wp-scripts test-playwright --config plugins/auto-sizes/tests/e2e/playwright.config.ts --ui", |
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.
What if we ever want to have e2e tests in other plugins too? It would be weird to copy these config files everywhere, so I think it would be better to move them to tools/
or so. But then again see my other comments about their purpose.
|
||
// Navigate to the post and wait for the image to load. | ||
await page.goto( `/?p=${ postId }` ); | ||
const imageElement = await page.waitForSelector( |
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.
It's typically better to use locators, for example getByAltText
, or alternatively CSS locators. But not waitFor*
.
const currentSrc = await updatedImageElement.evaluate( ( img ) => | ||
img instanceof HTMLImageElement ? img.currentSrc : null | ||
); | ||
currentSrc.endsWith( 'leaves-768x512.jpg' ); |
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 doesn't do anything, this isn't an assertion that fails the test.
Ideally we use something like await expect(locator).toHaveAttribute('src', /leaves-768x512\.jpeg$/ );
Summary
Part of #1511
Relevant technical choices
This PR adds the basic infrastructure for E2E testing for the Auto Sizes plugin.
More E2E test coverage will be added over time once this is merged into the feature branch.