Skip to content

Conversation

@johnbillion
Copy link
Contributor

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.

@johnbillion johnbillion requested a review from a team as a code owner April 25, 2020 01:12
Comment on lines 192 to 198
Scenario: Testing WP-Cron
When I try `wp cron test`
Then STDERR should not contain:
"""
Error:
"""

Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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`

Copy link
Contributor Author

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.

@schlessera schlessera added this to the 2.1.0 milestone Jan 22, 2022
@schlessera schlessera merged commit bb9fd96 into wp-cli:master Jan 22, 2022
@johnbillion johnbillion deleted the fix/cron-test branch January 22, 2022 09:57
@johnbillion
Copy link
Contributor Author

Nice work on that test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-200 HTTP status code from the spawn test should result in an error

2 participants