wp-env: Fix errors which prevent it from working without internet #53547
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
noahtallen
added
[Tool] Env
|
Size Change: 0 B Total Size: 1.66 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 9b77af8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6016824682
|
noahtallen
force-pushed
the
wp-env-fix-offline-errors
branch
from
5f01737 to
8b12b51
Compare
|
Thank you very much for addressing this, @noahtallen! One question: My typical scenario is that I'd like to work on Gutenberg while offline. Its default Line 2 in 9bd56dc It seems that this isn't currently supported by this PR, correct? (Only empty or missing |
|
BTW -- unrelated to my other question -- do I need to destroy my existing wp-env environment to test the following?
I'm asking because if I change my (and so on). This is apparently unrelated to the PR as it seems to happen on |
That should work with this branch! As long as the environment was already created once, The check for the latest WordPress version only happens when "core" doesn't exist. So that function isn't called otherwise. But even after fixing that issue, I needed to prevent You can also override "core" with your own |
noahtallen
force-pushed
the
wp-env-fix-offline-errors
branch
from
8b12b51 to
9b77af8
Compare
|
cc @ockham if you have a chance to test this :) |
ockham
approved these changes
Oct 19, 2023
There was a problem hiding this comment.
Hey @noahtallen, apologies for not getting back to you sooner!
I just re-tested this offline, without changing my .wp-env.json, i.e. without removing the core field. This time around, it worked smoothely 🎉
LGMT then! 🚢 Thanks again for implementing this change!
noahtallen
force-pushed
the
wp-env-fix-offline-errors
branch
from
9b77af8 to
9159431
Compare
noahtallen
force-pushed
the
wp-env-fix-offline-errors
branch
from
9159431 to
f70dcbb
Compare
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?
Fixes #53433.
Fixes #52157.
Unfortunately, many
wp-envcommands don't work without internet due to a function which finds the latest WordPress version, which happens during config parsing. Beyond this function,wp-env startcould fail in many cases when it tries to do updates (like downloading sources or pulling docker images).Why?
Offline mode always has its uses!
How?
dns.resolve( 'WordPress.org' )to check for internet access. This seems to be a reasonable practice based on my search for how to do this with NodeJS. It's also very fast, which isn't the case when we wait for timeouts in other functions.shouldConfigureWplogic duringwp-env start, since most of that involves internet connections (pulling docker images, downloading sources, etc.) I think it's easier to do this than to add extra checks all over the place when we do something with the network.The side effect is that certain types of
wp-env.jsonupdates won't work if you're offline, but that seems like a reasonable compromise. For example, if you set thephpVersion, that involves downloading a different docker image. So a lot of the config options would be impacted anyways.Testing Instructions
corefrom Gutenberg's.wp-env.jsonfile to trigger the check for the latest core source.npx wp-env startnpx wp-env install-pathnpx wp-env run cli bashnpx wp-env run cli phpunit --versionAll should work.
Testing Instructions for Keyboard
Screenshots or screencast
You should see an offline message in the terminal as well: