Subject: Re: bug#46668: [PATCH]: tests: do not hard code HTTP ports

From: Ludovic Courtès <[email-masked]>
Date: Tue, 02 Mar 2021 22:29:18 +0100

Hi,

Maxime Devos <[email-masked]> skribis:

> From 933cb85de0f50c54190e7c60420bef5245a3f2ed Mon Sep 17 00:00:00 2001
> From: Maxime Devos <[email-masked]>
> Date: Sat, 20 Feb 2021 22:04:59 +0100
> Subject: [PATCH] tests: do not hard code HTTP ports
>
> Previously, test cases could fail if some process was listening
> at a hard-coded port.  This patch eliminates most of these potential
> failures, by automatically assigning an unbound port.  This should
> allow for building multiple guix trees in parallel outside a build
> container, though this is currently untested.
>
> The test "home-page: Connection refused" in tests/lint.scm still
> hardcodes port 9999, however.
>
> * guix/tests/http.scm
>   (http-server-can-listen?): remove now unused procedure.
>   (%http-server-port): default to port 0, meaning the OS
>   will automatically choose a port.
>   (open-http-server-socket): remove the false statement claiming
>   this procedure is exported and also return the allocated port
>   number.
>   (%local-url): raise an error if the port is obviously unbound.
>   (call-with-http-server): set %http-server-port to the allocated
>   port while the thunk is called.
> * tests/derivations.scm: adjust test cases to use automatically
>   assign a port.  As there is no risk of a port conflict now,
>   do not make any tests conditional upon 'http-server-can-listen?'
>   anymore.
> * tests/elpa.scm: likewise.
> * tests/lint.scm: likewise, and add a TODO comment about a port
>   that is still hard-coded.
> * tests/texlive.scm: likewise.

Minor comment but nothing blocking:

> +  (let* ((text (random-text)))
> +    (with-http-server `((200 ,text))
> +      (let ((drv (derivation %store "world"
> +                             "builtin:download" '()
> +                             #:env-vars `(("url"
> +                                           . ,(object->string (%local-url))))
> +                             #:hash-algo 'sha256
> +                             #:hash (gcrypt:sha256 (string->utf8 text)))))
> +        (and drv (build-derivations %store (list drv))
> +             (with-http-server `((200 ,text))
> +               (build-derivations %store (list drv)
> +                                  (build-mode check)))
> +             (string=? (call-with-input-file (derivation->output-path drv)
> +                         get-string-all)
> +                       text))))))

It’s a tad confusing that the second ‘with-http-server’ is now nested;
it shouldn’t change the semantics though, so it’s okay.

Anyway, you’re welcome to push to ‘master’ if “make check” agrees.  :-)

Thanks again!

Ludo’.