-
Notifications
You must be signed in to change notification settings - Fork 384
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
Fix test:php commands #7589
Fix test:php commands #7589
Conversation
Plugin builds for 5ada676 are ready 🛎️!
|
@@ -262,7 +262,9 @@ public function test_converter( $source, $expected = null, $args = [], $expected | |||
} | |||
|
|||
// Normalize across different testing environments where WP_TESTS_DOMAIN varies. | |||
$current_origin = '//' . WP_TESTS_DOMAIN; | |||
$tests_domain = WP_TESTS_DOMAIN; | |||
$tests_domain = strtok( $tests_domain, ':' ); // In wp-env, the WP_TESTS_DOMAIN constant can erroneously include the port: 'localhost:8889'. |
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.
Relatedly, I previously reported WordPress/gutenberg#41038 and fixed via WordPress/gutenberg#41039 an issue where the URL scheme was included in the WP_TESTS_DOMAIN
(e.g. http://localhost
). Apparently the port number is now erroneously being added.
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.
AMP_Form_Sanitizer_Test
tests were passing for some time in wp-env. Seems like this has been resurfaced again.
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.
LGTM!
This fixes an issue I experienced when running
npm run test:php
:It also simplifies the
test:php:*
commands and removestest:php:help
which is not needed.It also fixes PHPUnit issues specifically encountered when running in wp-env. Fixes #7590.