Expanded wp-env Lifecycle Scripts
#50570
Merged
Conversation
This file contains 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
|
Size Change: 0 B Total Size: 1.4 MB ℹ️ View Unchanged
|
noahtallen
reviewed
May 17, 2023
packages/env/lib/config/test/__snapshots__/config-integration.js.snap
Outdated
Show resolved
Hide resolved
ObliviousHarmony
force-pushed
the
enhance/wp-env-lifecycle-scripts
branch
from
b4511a5
to
318b5cf
Compare
|
This PR is now failing because of the config option validation. It isn't built to handle arguments with nested properties that are expected to be present. It's not too difficult a fix, but, I'd like to settle on whether or not we're going to ship this at all before moving forward with the fix. |
This is just an internal change aimed at supporting a shift to more lifecycle scripts. It does not change the external config file or the way that the scripts execute yet.
Instead of having a dedicated "afterSetup" execution path, we now have a generic execution path for lifecycle scripts. This will make it easy to execute other scripts on lifecycle events.
This removes `afterSetup` in favor of an `afterStart` and an `afterClean` script. These will provide a more granular way of handling things. We have also added a `beforeDestroy` script that will help.
This keeps the spinner from freezing and also has the benefit of making the script output more consumable.
ObliviousHarmony
force-pushed
the
enhance/wp-env-lifecycle-scripts
branch
from
318b5cf
to
97ca1be
Compare
noahtallen
reviewed
May 23, 2023
|
Oh, another thing I just noticed is that output gets swallowed by the spinner. Is that expected? The "test start" example goes away after the command completes. |
I went with this approach so that it would be consistent with all of the other actions |
Hm, I guess that makes sense. I was comparing it more in my mind to |
noahtallen
approved these changes
May 23, 2023
|
Flaky tests detected in 7b1b898. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5060945367
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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?
In #50196 I added an
afterSetupscript that is executed afterwp-env startandwp-env clean. This pull request expands on that work and provides more granular lifecycle scripts in its place.Why?
Once I began adopting the
afterSetupfeature I found that it wasn't granular enough and that there were cases where it would also be useful to have a script execute.How?
We're going to remove the
afterSetupoption in favor of a top-levellifecycleScriptsoption with entries for each lifecycle script that the user wants to implement. These scripts can all be overridden usingWP_ENV_LIFECYCLE_SCRIPT_{ EVENT_NAME }environment variables. For now this includesafterStart,afterClean, andbeforeDestroy. I've also made the execution of the script asynchronous because it was blocking the spinner.Testing Instructions
start,clean, anddestroyall work the same without any scripts set..wp-env.override.jsonwith content:start,clean, anddestroyand watch the output.--debugand it will persist the command output.export WP_ENV_LIFECYCLE_SCRIPT_AFTER_START="echo 'Test Override' && sleep 10"and runstart. Note that it is overridden.export WP_ENV_LIFECYCLE_SCRIPT_AFTER_START="echo "FAIL" 1>&2 && false"and runstart. It should give the error and indicate the failure.