-
Notifications
You must be signed in to change notification settings - Fork 28
Promote a non-200 response from the cron spawn test to an error #66
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
Conversation
| Scenario: Testing WP-Cron | ||
| When I try `wp cron test` | ||
| Then STDERR should not contain: | ||
| """ | ||
| Error: | ||
| """ | ||
|
|
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.
Instead of deleting this test, I think it should stay for a valid setup check, and we need to add a second test after deleting the wp-cron.php to verify that it does indeed throw an error.
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.
As I mentioned in the issue description, this scenario performs a request to example.com over the internet (which returns a 404 because example.com doesn't use WordPress).
@schlessera Does WP-CLI contain any existing tests which perform an HTTP request to an actual local WordPress site?
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.
You could probably point to the proper WordPress instance using something like this as setup and populating the options accordingly:
Given a WP install
And I launch in the background `wp server --host=localhost --port=8080`
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.
Ah yeah nice, I'll look into it.
…r than a warning.
…quest to `example.com`
236e273 to
5191037
Compare
|
Nice work on that test |
Fixes #65
Note that the current feature test for testing the cron spawner sends an HTTP request to
example.com(which is the default domain configured for the tests), so here I'm removing it because it's useless and it's not possible to affect its response. Suggestions welcome for improving this.