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

[Maxime Devos]

Hi,

Ludovic Courtès schreef op za 22-01-2022 om 17:48 [+0100]:
> > +        (unless keep-lingering?
> > +          ;; exit the server thread
> > +          (system-async-mark (lambda () (throw 'quit)) server))
> 
> When do we need ‘keep-lingering?’?

In tests/challenge.scm (call-mismatch-test), due to how the store monad
work, the thunk technically returns (*) before we are done with the
querying the server.  Perhaps this can be resolved with sufficient
monadology, but I don't quite see how.

(*) a monadic value.

> So far, all uses of ‘with-http-server’ expected that the server would
> quit once the last response has been sent.

AFAIK, they don't expect that the server quits. But they don't expect
that the server does not quit either.  Rather, they need the server
to keep running as long as needed -- i.e., after the last request
has been answered.

The only reason that the server quits, is to perform a form of garbage
collection, to avoid accumulating threads and server ports.

There appear to be two criteria for deciding when to exit the server:

 (a) Exit when the thunk returns. 

     This is similar to 'call-with-port' and 'call-with-output-file'
     automatically closing the port when the procedure returns.
     There don't seem to be any drawbacks with this criterium.

 (b) Exit when there are no responses left.

     This is problematic when there is no list of reponses but rather
     some function mapping requests to responses without any
     limitations on how often a resource is requested, or when there
     are multiple resources available and the ‘client’ is allowed to
     query a proper subset ...

     E.g., the way the tests in tests/minetest.scm are written, the
     tests don't care in which order resources are accessed and whether
     a resource is accessed multiple time.  Furthermore, the procedure
     for creating a testing model of ContentDB (call-with-packages) has
     no idea what parts of the model will be accessed.

     That is, the same model can be used for searches (either
     sorted by score or download), for requesting a description of a
     ContentDB ‘package’ and for requesting a specific release of a
     package.

     Furthermore, the space of resources is even infinite (due to the
     search API).

     In principle, that procedure could be modified to accept a few
     arguments specifying what things will be asked and with
     which parameters, but doing that and figuring out the arguments
     for each test would be rather tedious.

     Aside from verifying that no more traffic than strictly necessary
     happens (which would be a nice property but not really required),
     I don't see the point of verifying which resources exactly are
     queried.

     Furthermore, which resources precisely are queried and how often,
     seems more of an implementation detail to me.  I would rather
     focus on the end result: verifying that the imported package
     definition is what is expected.

     Most tests in tests/minetest.scm are like that: they tell
     call-with-packages to create a ContentDB model with some data
     (using make-package-json etc.) ask (guix import minetest) to
     import some package from the model and compare the result with a
     prediction (make-package-sexp) (can this called integration
     testing?).

     These tests do not care how (guix import minetest) works -- they
     don't care about which resources are queried.  Instead, they
     simply test that the end result (the package definition) is as
     predicted.

While criterium (b) might suffice for various unit tests
(e.g. in tests/lint.scm), it is rather impractical and limiting
for the kind of tests that tests/minetest.scm does.

OTOH, criterium (a) not only suffices for tests/lint.scm-style tests,
but also for tests/minetest.scm-style tests.  It seems to work
everywhere, except for the single exception tests/challenge.scm. 

> It would be nice if we could keep it that way.

Compare (a) and (b), then I think it will be easy to infer what I would
prefer 😉.

Greetings,
Maxime.

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