-
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
WP-env test site uses a wrong theme's color palette making tests unreliable #48678
Comments
Color palette is disabled by this PR. #18761 |
@torounit thanks for pointing me to #18761. While I do understand the good intent behind that change, I'm not sure that's the right way to go.
I'd liek to hear more thoughts and feedback from other developers. To me, #18761 appears to introduce more troubles than benefits and I'd tend to think it should be reverted. Cc @youknowriad @epiqueras (not sure who else I can ping about this) 🙏 |
I disagree with this take personally, test sites should have test data and could be very different from dev sites. That said, today we have a better way of solving this. At the time that PR has been implemented, the tests used to rely on the default them of WordPress which was evolving. This is not the case anymore as we switched to a "fixed theme" explicitly when running the tests. So I think we can remove that test plugin and rely on the colors defined by the theme itself. We also have a dedicated test theme "emptytheme" that we can leverage to test some specific cases of palettes. |
Thanks @youknowriad for your feedback. Removing the
Yeah, let me rephrase it: they should be identical as in: identical configuration. Or, at least, the two sites configuration should be as similar as possible. Any difference should be clearly documented, otherwise contributors will likely spend hours trying to understand what's going on. Of course, the actual data used on the test site may be crafted for specific use in the tests. In addition to the above, I'd love to see some more consideration for the following points:
|
Oh yeah, it's been some time no one actually worked on making sure we can run these tests properly on WordPress without the plugin. I did that at some point in the past but this effort kind of stopped because we never enabled it by default in Core tests. If it's mandatory in Core tests, people will pay more attention to the tests and flows and keep things running smoothly. But since it adds friction to core commits and PRs and due to some test instability at the time, I didn't make it mandatory. |
I removed the env label, just because that's normally for development of the wp-env tool itself, but this behavior is unique to the Gutenberg plugin :) |
Description
Noticed while working on #42187
When using wp-env:
Some tests use Twenty Twenty-One and set some colors on the content by using various color pickers. Turns out the color palette on the test site is different from the color palette used on the development site. Thus, the tests are unreliable:
Not to mention that contributors may not use wp-env. In that case, correctly updating the test snapshots is impossible because developers will get the correct color palette while the test site uses the wrong color palette.
For example, I use a standard
wordpress-develop
cloned repo with Gutenberg cloned within theplugins
directory. I start my environment by using the WP local-env:npm run env:start
. Everything works as expected. With this setup, Twenty-Twenty-One uses the correct color palette defined in the theme'sfunctions.php
file. Updating the test snapshots will update the colors used in the snapshots with the ones from the correct color palette.These colors don't match the ones used on CI and the tests on CI will always fail.
When using wp-env, the dev site uses the correct color palette defined in the theme's
functions.php
file. However, turns out the test site uses the (wrong) default color palette defined insrc/wp-includes/theme.json
which makes very hard to build a reliable test and correctly update the snapshots.Please consider that this unexpected difference between the dev and test environments has been a blocker for #42187 for several weeks, as it was hard to figure out the reason for the test failure was actually something in the test environment.
Step-by-step reproduction instructions
functions.php
:npm run test:e2e -- --puppeteer-interactive --testPathPattern packages/e2e-tests/specs/editor/various/rich-text.test.js
should preserve internal formatting
.src/wp-includes/theme.json
:Screenshots, screen recording, code snippet
No response
Environment info
No response
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
The text was updated successfully, but these errors were encountered: