csernazs / pytest-httpserver

Http server for pytest to test http clients
MIT License
214 stars 28 forks source link

Documentation: a small correction and adding a note for request handler #269

Closed matez0 closed 7 months ago

matez0 commented 10 months ago

Hi Zsolt,

  1. In the example of Running httpserver in blocking mode, the first server.clear() has to be removed.

  2. What about adding to the documentation (maybe at several places) the red :warning: note that the response for the RequestHandler has to be set before the expected request arrives, otherwise pytest_httpserver.httpserver.NoHandlerError: Matching request handler found but no response defined will be raised:

    
    request_handler = httpserver.expect_request("/foobar")

When an expected request arrives before setting the response, NoHandlerError will be raised:

requests.get(httpserver.url_for("/foobar"))

It is too late to set the response after the expected request has arrived:

request_handler.respond_with_data("OK foobar")


Sometime not the test code will explicitly launch the request, but another process and in that case the trigger in the test code may not wait for the response. In that situation, it is not so intuitive that setting the response has to be before the trigger.

Thanks for this great project. :blush:
csernazs commented 9 months ago

Hi Zoli,

The 1st point is trivial to implement, made #270 for this.

For the 2nd point, I'm thinking about where I should put the :warning: , but I haven't found any good place. Do you have any suggestion for this?

I was thinking about:

  1. adding to the tutorial would make it complicated
  2. adding to the howto: this is about positive cases (how to do ...) instead of (how not to do ...).

I can add it to the API reference or the background docs, but these are more hidden for the eyes, probably this would be the better choice but I wanted to ask you first.

Zsolt

matez0 commented 9 months ago

In my opinion, in section Writing your first test it may fit (inside a pale rusty colored warning box that can also be found in other places of the document). There is already an Important note on server port number as well.

My other vote for the place is in the API: expect_request. Since this is the detailed description to that all the expect_ functions refer. This was that I read first when I did not get why after an asynchronous request I got 500 and a response which did not contain the info about NoHandlerError.

Since there are several notes and warnings in the Howto, one more about the request handler after discussing the respond_with method would not harm, but I think the two places mentioned before are more important.

About the content: Something like "the response for the RequestHandler has to be set before the expected request is sent" should be in the text. It could be also mentioned that if the response is needed to be set after the incoming request, then one may want to use the non-blocking version of the HTTP server.

csernazs commented 9 months ago

Thanks!

I made some changes in #271 . Could you please look at it?

It adds two notes, one for tutorial and one for the API docs as you suggested. I chose to use the Note box which is not as hard as the warning as I think it looks better, we should keep warnings for more serious things.

What do you think?

matez0 commented 9 months ago

Thanks.

I think the Note is also fine.

I suggested some little adjustments.

csernazs commented 7 months ago

271 has been merged, closing this issue