Skip to content
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: Fix issue where docker & wp had different URLs #20228

Merged
merged 1 commit into from
Feb 14, 2020

Conversation

noahtallen
Copy link
Member

@noahtallen noahtallen commented Feb 14, 2020

Description

Fixes an issue where a custom port in .wp-env.json would not change the URL used to install WordPress. This resulted in the WordPress redirecting you to the default port. So if you would access docker at localhost:4013, it would send the request to WordPress, which thought the URL was localhost:8888, and you'd then be redirected to localhost:8888 which had nothing running on it.

How has this been tested?

I made sure that accessing the custom port in .wp-env.json correctly loaded WordPress on that port. I also added unit tests to verify that environment variables are validated and take precedent over config values.

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@noahtallen noahtallen added [Type] Bug An existing feature does not function as intended [Tool] Env /packages/env labels Feb 14, 2020
@noahtallen noahtallen self-assigned this Feb 14, 2020
packages/env/lib/config.js Outdated Show resolved Hide resolved
@noahtallen noahtallen force-pushed the fix/wp-env-install-p'rt branch 2 times, most recently from 9c3ac2f to b1e642b Compare February 14, 2020 03:40
@noahtallen noahtallen requested a review from epiqueras February 14, 2020 04:59
packages/env/lib/config.js Outdated Show resolved Hide resolved
@noahtallen noahtallen force-pushed the fix/wp-env-install-p'rt branch from 3bee527 to 985d30c Compare February 14, 2020 21:58
@noahtallen
Copy link
Member Author

Rebased; hopefully that will fix the failing e2e test

@noahtallen noahtallen changed the title wp-env: Use custom port for WP instance setup wp-env: Fix issue where docker & wp had different URLs Feb 14, 2020
@noahtallen noahtallen merged commit 3a86ce1 into master Feb 14, 2020
@noahtallen noahtallen deleted the fix/wp-env-install-p'rt branch February 14, 2020 22:21
@github-actions github-actions bot added this to the Gutenberg 7.6 milestone Feb 14, 2020
@jorgefilipecosta jorgefilipecosta added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 2, 2020
jorgefilipecosta pushed a commit that referenced this pull request Mar 2, 2020
- Fixed: docker and WP had different URLs
   Caused by WordPress install not using the
   .wp-env.json custom port values, so it
   would redirect you to the default port.

- Added: validation function for numeric env vars
- Added: tests to validate port override behavior
jorgefilipecosta pushed a commit that referenced this pull request Mar 2, 2020
- Fixed: docker and WP had different URLs
   Caused by WordPress install not using the
   .wp-env.json custom port values, so it
   would redirect you to the default port.

- Added: validation function for numeric env vars
- Added: tests to validate port override behavior
@jorgefilipecosta jorgefilipecosta removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] Env /packages/env [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants