-
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
Fix performance testing themes installation #49063
Conversation
Size Change: 0 B Total Size: 1.34 MB ℹ️ View Unchanged
|
Flaky tests detected in 935f1b1958fd69b330f81ed4fcee4d77969fc6fc. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4416181083
|
935f1b1
to
fba3e5f
Compare
fba3e5f
to
0c0dc01
Compare
Yes, this makes sense to me. cc @oandregal |
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.
Makes sense to me! The original change definitely wouldn't impact the performance environments, assuming that those commands run in a different directory than the final wp-env environments
What?
One of the perf action steps is the installation of specific theme versions. Unfortunately, they're not installed for the correct
wp-env
instance, so they're never used in the tests, and the current default versions are used instead (e.g., v1.1 instead of v1.0 fortwentytwentythree
at the time of writing). The wp-env we use in CI for performance testing is configured via the.wp-env.performance.json
config. According to the docs we can use that config to define theme source URLs that we want to be installed, so I moved it there. (Kudos to @noahtallen for pointing that out.)As a side note, I believe we should not update any wp-env params via the GH action step because it causes a discrepancy between running tests locally vs. in the CI.
How?
Ideally, the correct theme version should be installed and validated within the test itself, as only then we're confident that the testing env is consistent, even when running tests locally in isolation. I'm not sure how to achieve that at this point though, so I moved the installation to the wp-env config as mentioned above so that at least the CI uses the correct themes.
Testing Instructions
perf
job is currently broken, unless you're willing to apply this fix and the CLI one, and then run the job and verify the wp-env used the right versions of the themes. Or you can just trust me because I already did all of that. 😄