-
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
Env: Restrict WP_TESTS_DOMAIN
constant to just hostname and not home URL
#41039
Conversation
Size Change: 0 B Total Size: 1.26 MB ℹ️ View Unchanged
|
WP_TESTS_DOMAIN
constant to just hostname and not home URLWP_TESTS_DOMAIN
constant to just hostname and not home URL
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.
Thanks for the change! I think we weren't completely clear on the meaning of WP_TESTS_DOMAIN
when it was originally introduced. Additionally, we've had trouble when some of the URL-related config values don't including the port, so it was natural to apply the same treatment to this value.
@noahtallen Something I'm not sure about still here is the status it dumps in the console:
Notice the dev site says it started at |
I see. It's because it's using |
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.
Gotcha, the new change makes sense.
Could we also update the changelog before merging? That way it's easy for someone to see that the behavior slightly changed :)
This is good to merge from my perspective 👍 |
e2f23b2
to
86e3bf0
Compare
Resolved merge conflict, and I'm going to merge after all checks are green 👍 |
Co-authored-by: Noah Allen <noahtallen@gmail.com>
e6f71a7
to
a7821f0
Compare
🎉 Tests are finally passing ✅ |
Fixes #41038.
What?
I found that unit tests were being run with
WP_TESTS_DOMAIN
(and thus$_SERVER['HTTP_HOST']
) being set to a URLhttp://localhost:8889/
when it should be limited to just the hostname:localhost
.Why?
Constructing the current URL often looks like:
This is completely broken if the
HTTP_HOST
is not just the host name.How?
Don't attempt to populate a URL in
WP_TESTS_DOMAIN
.