Fixes #210

• Depends on Add hook to http_request() util wp-cli#6036 for mocking requests done by the WP-CLI http_request() util

@BrianHenryIE Would love to hear your thoughts on this especially :-)

Note:The original suggestion was to store the mocked responses in files and use those, but this can be achieved with this solution just as well.

Example:

  Background:
    When I run `cat my/mocked/response.json`
    Then save STDOUT as {HTTP_RESPONSE}

  Scenario: Mock HTTP request in WP-CLI
    Given an empty directory
    And an HTTP request to https://api.github.com/repos/wp-cli/wp-cli/releases?per_page=100 with this response:
    """
    HTTP/1.1 200
    Content-Type: application/json

    {HTTP_RESPONSE}
    """

I haven't tested that though, but I could add a test for it.

By the way this has already been successfully tested as part of wp-cli/wp-cli#6037, so I think it's good to go.

Appreciate a good review though :)

@wp-cli/committers This is ready for review :-)

It works quite well so far, as evidenced by wp-cli/wp-cli#6037. The API seems intuitive for the existing use cases, but could always be adapted in the future if needed.

Looks good to me if you think is ready to merge

Thanks!

The main concern @schlessera had about the API is the wording.

Usually a "Given ..." step performs a specific action, like installing WordPress. Here, "Given an HTTP request..." doesn't actually perform an HTTP request but merely sets up hooks for mocking requests later on.

So we could consider changing this to something like "Given an HTTP mock for requests to xyz with the following response", but not sure if that's better.

I almost did suggest changing the wording but I'm not sure if what I'd want even fits into the existing way tests are written.

What makes sense to me is something more like:

When any request is made to $url
Then the response will always be:

But that doesn't really fit into how "When" is used otherwise in our tests, so maybe it creates a different type of confusion.

But that doesn't really fit into how "When" is used otherwise in our tests, so maybe it creates a different type of confusion.

That is a good point.

Technically it would be feasible, as Behat doesn't actually differentiate between When or Then or Given when doing regex matching. There are semantic differences though:

The purpose of Given steps is to put the system in a known state before the user (or external system) starts interacting with the system (in the When steps). Avoid talking about user interaction in givens. If you have worked with use cases, givens are your preconditions.

The purpose of When steps is to describe the key action the user performs

The purpose of Then steps is to observe outcomes. The observations should be related to the business value/benefit in your feature description. The observations should inspect the output of the system (a report, user interface, message, command output) and not something deeply buried inside it (that has no business value and is instead part of the implementation).

• Verify that something related to the Given+When is (or is not) in the output
• Check that some external system has received the expected message (was an email with specific content successfully sent?)

That makes sense. Maybe something pretty similar to your original like

 Given any HTTP requests to https://api.github.com/repos/wp-cli/wp-cli/releases?per_page=100 will respond with:
    """
    HTTP/1.1 200
    Content-Type: application/json
    ...
    """

or

 Given HTTP requests to https://api.github.com/repos/wp-cli/wp-cli/releases?per_page=100 will always respond with:
    """
    HTTP/1.1 200
    Content-Type: application/json
    ...
    """

Which feels clear (to me) that it is about future requests and is not in that moment making a request. Using the phrase Mock in the rule text itself feels a bit more awkward and more of an implementation detail. If we really did want to use Mock I think putting it at the end is more clear:

 Given any HTTP requests to https://api.github.com/repos/wp-cli/wp-cli/releases?per_page=100 will respond with this mock data:
    """
    HTTP/1.1 200
    Content-Type: application/json
    ...
    """

How about something like this:

 Given that HTTP requests to https://api.github.com/repos/wp-cli/wp-cli/releases?per_page=100 will respond with:
    """
    HTTP/1.1 200
    Content-Type: application/json
    ...
    """

It reads more natural to me...

How about something like this:

 Given that HTTP requests to https://api.github.com/repos/wp-cli/wp-cli/releases?per_page=100 will respond with:
    """
    HTTP/1.1 200
    Content-Type: application/json
    ...
    """

It reads more natural to me...

Sounds good to me! I think using "that" and "will" in these make it clear that the request isn't happening as part of the test which was the initial concern.

Now I just need a review on #236 to fix those tests :)