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

[Maxime Devos]

Ludovic Courtès schreef op za 22-01-2022 om 17:48 [+0100]:
> > +(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

We can't unthunk 'thunk', otherwise the code querying the server will
be run before the server starts, which isn't very useful.  It's the
same reasn why unthunking the 'thunk' argument of 'with-output-to-file'
is not useful -- unless you only want to make a file empty I suppose.

Did you mean 'responses+data'?  For some context, consider
tests/cpan.scm:

-  (with-http-server `((200 ,test-json)
-                      (200 ,test-source)
-                      (200 "{ \"distribution\" : \"Test-Script\" }"))
-    (parameterize ((%metacpan-base-url (%local-url))
-                   (current-http-proxy (%local-url)))
+  (with-http-server `(("/release/Foo-Bar" 200 ,(test-json))
+                      ("/Foo-Bar-0.1.tar.gz" 200 ,test-source)
+                      ("/module/Test::Script?fields=distribution"
+                       200 "{ \"distribution\" : \"Test-Script\" }"))
+    (parameterize ((%metacpan-base-url (%local-url* ""))
+                   (current-http-proxy #false))

(Side note: should parametrising current-http-proxy be moved into
'with-http-server', to avoid forgetting to do it in individual tests?)

This 'with-http-server' is ‘self-referrent’: the responses depend on
the port that the HTTP server bound to -- (test-json) refers to
http://localhost:THE-PORT/Foo-Bar-0.1.tar.gz.  As such, the
responses+data needs to be thunked, because this port is not known in
advance.

In principle, thunking can be avoided here by running two HTTP servers
by nesting two with-http-server forms, but why make things more
complicated by running multiple servers when a single one is
sufficient?  It can also be avoided by doing this proxy thing the
original code did, but why complicate things with proxies when
a regular server suffices?

Also, tests don't really see that thunking happens unless they actually
use the thunking to do make self-references, because all tests use
with-http-server (*) (which does thunking automatically, not unlike
'package' records).  So except for simplifying implementation details
of call-with-http-server, I don't see how unthunking would make things
nicer.

(*) Unless they use with-http-server*.

>  and instead always use the declarative form (list of URL
> path/response code/response body).

Thunking and declarative forms don't appear mutually exclusive to me;
the tests/cpan.scm example above uses a thunked declarative form.

It would be nice to always specify the paths in the declarative
form though, to make tests more precise, but there are a lot
of tests to convert.

>  That should make the tests more concise and readable.

The declarative form is still available and isn't going away.

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

tests/minetest.scm, see other mail.

with-http-server is still declarative, so maybe you meant the
functional with-http-server* instead?

Greetings,
Maxime

signature.asc
Description: This is a digitally signed message part