Subject: Re: bug#53389: [PATCH 0/9] Replace some mocking with with-http-server*, avoid hardcoding ports,

From: Ludovic Courtès <[email-masked]>
Date: Sat, 22 Jan 2022 17:48:32 +0100

Hi Maxime,

Maxime Devos <[email-masked]> skribis:

> An incompatible change to with-http-server has been made: it now
> also exits when the thunk exits.  This change allows implementing
> with-http-server*.  It also keeps threads from lingering if the
> thunk doesn't access all of RESPONSES+DATA.
>
> Usually, this change is fine, but it does not interact nicely with
> monads in tests/challenge, so a variant with-http-server/lingering
> preserving the old behaviour has been defined.
>
> * guix/tests/http.scm
>   (call-with-http-server): Extract most functionality to ...
>   (call-with-http-server*): ... this new procedure.  Also stop the
>   server thread after 'thunk' returns instead of when the last response
>   has been sent unless requested not to.
>   (with-http-server/keep-lingering): New macro.
> * tests/challenge.scm (call-mismatch-test): Use the 'keep-lingering'
>   variant of 'with-http-server'.

[...]

>    #:export (with-http-server
> +            with-http-server/keep-lingering
> +            with-http-server*
>              call-with-http-server
> +            call-with-http-server*
>              %http-server-port
>              %local-url))

My first reaction was: have we gone overboard?  :-)

Since it’s an internal module and a test helper, I’m in favor of keeping
it as simple as possible.  Can we keep a single ‘with-http-server’ form
that would cover all cases?

We can update existing tests to include the expected URL path (or a
wildcard, if needed), instead of keeping several forms.  We don’t need
to worry about backward compatibility at all.

> +        (unless keep-lingering?
> +          ;; exit the server thread
> +          (system-async-mark (lambda () (throw 'quit)) server))

When do we need ‘keep-lingering?’?  So far, all uses of
‘with-http-server’ expected that the server would quit once the last
response has been sent.  It would be nice if we could keep it that way.

> +        (apply values results)))))
> +
> +
> +(define* (call-with-http-server responses+data thunk #:key (keep-lingering? #false))
> +  "Call THUNK with an HTTP server running and returning RESPONSES+DATA
> +on HTTP requests.  Each element of RESPONSES+DATA must be a tuple containing a
> +response and a string, or an HTTP response code and a string.
> +
> +The argument RESPONSES+DATA is thunked.  As such, RESPONSES+DATA can use
> +%http-server-port.  %http-server-port will be set to the port listened at.
> +It will be set for the dynamic extent of THUNK and RESPONSES+DATA.
> +
> +The server will exit after the last response.  When KEEP-LINGERING? is false,
> +the server will also exit after THUNK returns."

Within tests, it would be nice if we could avoid using the ‘thunk’ form
and instead always use the declarative form (list of URL path/response
code/response body).  That should make the tests more concise and
readable.

Or are there new uses where the declarative form is insufficiently
expressive?

Thanks,
Ludo’.