Fix performance testing themes installation#49063
Merged
Merged
Conversation
WunderBart
added
[Type] Performance
|
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
|
WunderBart
force-pushed
the
fix/perf-themes-install
branch
from
935f1b1 to
fba3e5f
Compare
WunderBart
changed the base branch from
refactor/perf-test-results-path
to
trunk
WunderBart
requested review from
dmsnell,
kevin940726,
noahtallen and
youknowriad
WunderBart
force-pushed
the
fix/perf-themes-install
branch
from
fba3e5f to
0c0dc01
Compare
Contributor
|
Yes, this makes sense to me. cc @oandregal |
noahtallen
approved these changes
Mar 15, 2023
Member
noahtallen
left a comment
noahtallen
left a comment
There was a problem hiding this comment.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
One of the perf action steps is the installation of specific theme versions. Unfortunately, they're not installed for the correct
wp-envinstance, so they're never used in the tests, and the current default versions are used instead (e.g., v1.1 instead of v1.0 fortwentytwentythreeat the time of writing). The wp-env we use in CI for performance testing is configured via the.wp-env.performance.jsonconfig. 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
perfjob 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. 😄