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

I don't see what it matters that this module is only available from
a git checkout (or extracted tarball) and that it is only used by
tests.

Keeping things simple is good, but making it simpler in such a way
that it becomes unusable to some tests (tests/minetest.scm) somewhat
defeats the purpose of the test helper.

>   Can we keep a single ‘with-http-server’ form
> that would cover all cases?

We have a single form that covers all cases: with-http-server*.
However, the full power of the functinal with-http-server*, accepting
an arbitrary mapping from requests to responses, often isn't necessary.
For those cases, we have the declarative with-http-server, which is
quite a bit simpler to use, but much less flexible.

We could remove 'with-http-server' and keep a single macro
'with-http-server*' but I don't think that's what you were going for.

This seems a bit like trivial-build-system/copy-build-system.
trivial-build-system is rather complicated to use, but can in theory do
anything. copy-build-system is rather limited in what it can do, but
when it is suitable to the problem, it is very easy to use.
There is no attempt to somehow move everything trivial-build-system
can do into copy-build-system.

There's also the option of letting 'call-with-http-server' test
if the (responses+data) is a procedure or a list, and in the first
case behave like the old 'with-http-server*' and in the second
case like 'with-http-server'.  This overloading doesn't seem
great though, I would rather have two separate procedures for
two separate APIs -- albeit with one implemented using the other.

> 
> 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.

Always including the URL path in the declarative forms
(with-http-server) seems good to me, but it would be a lot of work
-- actually, on second though, I ran
"git grep -F with-http-server | wc -l" and there were 51 hits, which
seems doable. Let's do that in the v2.

However, the declarative form is too limiting and not sufficiently
expressive for some tests (tests/minetest.scm), tests/minetest.scm
doesn't have any use for wildcards, so with-http-server remains
unsuitable for some tests.

Greetings,
Maxime.